Rochet2

The suggested fix may work, but it is a hack around the limitations of the send/receive AI event functionality. Instead of casting gameobject to a creature ReceiveAIEvent could be expanded to cover WorldObject as sender. @Gutterboy

I would suggest either trying to rework the send/receive AI event system to allow WorldObject as sender or similar, or then we can use this kind of approach:

diff --git a/scripts/eastern_kingdoms/scarlet_enclave/ebon_hold.cpp b/scripts/eastern_kingdoms/scarlet_enclave/ebon_hold.cpp
index 30b970c..f51a194 100644
--- a/scripts/eastern_kingdoms/scarlet_enclave/ebon_hold.cpp
+++ b/scripts/eastern_kingdoms/scarlet_enclave/ebon_hold.cpp
@@ -1001,8 +1001,9 @@ struct npc_unworthy_initiate_anchor : public CreatureScript
                 RegisterCloseInitiate(sender);
                 break;
             case AI_EVENT_CUSTOM_C: // notify me; @TODO inplement normal ObjectGuid transfer between scripts
+                if (const GameObjectData* godata = sObjectMgr.GetGOData(data))
                 {
-                ObjectGuid guid = ObjectGuid(HIGHGUID_GAMEOBJECT, 0, data);
+                ObjectGuid guid = ObjectGuid(HIGHGUID_GAMEOBJECT, godata->id, data);
                 if (GameObject *pGo = m_creature->GetMap()->GetGameObject(guid))
                     NotifyMe(invoker, pGo);
                 break;

@Necrovoice The locations are around
SD3\scripts\eastern_kingdoms\scarlet_enclave\ebon_hold.cpp(1003)
SD3\scripts\eastern_kingdoms\scarlet_enclave\ebon_hold.cpp(1245)

It should be noted that I did not test these changes in game.

https://github.com/mangosone/server/blob/13d333f2686efebd65130b611b53d52b69674e00/src/game/WorldHandlers/GridMap.cpp#L38
https://github.com/mangos/Extractor_projects/blob/c835b82e9a930d747a15b5db19af180119656d45/shared/ExtractorCommon.cpp#L304

Seems like a mismatch here with the versions of extractor and core magic number.

Probably same issue for all the maps (mmap, vmap, map) since all the versions were bumped in this commit:
https://github.com/mangos/Extractor_projects/commit/51086dafc9477557ebe21f59ad7076802d04d6dc

Looks like the change does not require changes in core code, so bumping the versions in core should, in my opinion, be enough to fix the issue. (see some of the latest commits at https://github.com/mangos/Extractor_projects/commits/master)

Alright. I will apply it as a fix to the source then. Thanks for testing.

btw. on my machine I tried with VS 2013 and VS 2017 and in debug and release and couldnt reproduce it.
The fault in the code does make sense though. The inconsistency is disturbing.

@khamarr3524 I may have found the issue.
Remove the earlier edits and try this: https://gist.github.com/Rochet2/f47056785ccf864e9be481605380d2d7
The issue may be that the hash function that was supposed to take 2 or more arguments is used instead of the functions that take one argument. The changes above should make the function require at least two arguments so it cannot be confused with the ones that take only one.

I couldnt reproduce the issue with
Windows 10, MS VS12 2013 (windows desktop update 5), MaNGOSTWO, Release Win32
which should be close match to your setup, so I cant test possible fixes myself.

@khamarr3524 Would it be possible for you to try debug and see what the types of the variables are and where they originate from? Also information about your environment would be nice. For example are you trying to compile 64 bit and so on.
Assuming you used windows since the report was posted at the end of windows installation guide. Tell us if this is not true.

This is a first issue about such since the code was added two years ago and I cannot directly see where it would come from.
Can you try making these changes and report back? https://gist.github.com/Rochet2/d2c4647df88aea3ebf3cb47b32a53baf

For reference, this is the line reported:
https://github.com/ElunaLuaEngine/Eluna/blob/c6b9d7364c811825465e82156c56656500ea9a64/BindingMap.h#L275

When you say that it works, does that mean that you also tried to compile the core?
According to cmake docs MSVC should be defined as true when the compiler is some version of VS.

If cmake running manually works, then probably the part that runs cmake needs to be tweaked in easybuild.
Seems that cmake cannot determine the compiler or determines it wrong or after the step that we are using it at somehow.

Probably need the easybuild source code for this one if it does not happen when running cmake on its own.

Here is the origin of the issue:

https://github.com/mangosthree/server/commit/43b74a38206ae117e74e041c960b46daa6c7d92d#diff-cbff7f9dcb78a3b83d4b09941070082eL573

 @H0zen

World.cpp 570 causes a lot of config options not to load (including maxlevel, causing it to be 0 and blocking level stat loading)
This is because
std::string forceLoadGridOnMaps = sConfig.GetStringDefault("LoadAllGridsOnMaps", "");
results in an empty string.

https://github.com/mangosthree/server/commit/e62ece1fdbd1cfd757e270ed3af39a6c8d878bf4

https://github.com/mangosthree/server/blob/e62ece1fdbd1cfd757e270ed3af39a6c8d878bf4/src/game/WorldHandlers/World.cpp#L570

https://github.com/mangosthree/server/blob/e62ece1fdbd1cfd757e270ed3af39a6c8d878bf4/src/game/WorldHandlers/World.cpp#L659-L662

I feel that the config options should not be inside the IF statement.
Someone probably make a mistake formatting the code.

1 hour ago, Talendrys said:

ACE is used for the complete network layer.

Ask yourself: Does vmap extractor connect to the internet? (and I do not mean that you should answer here)
Clearly it has other uses in addition to that.

1 hour ago, Talendrys said:

The "try, debug & fix" approach you're advising is kind of dangerous

You make my suggestion sound like the guy fixing this should just remove all code from the functions as that compiles. (This is a playful example, I do not mean that you said this, but that it comes out like this)

That is not what I meant at all. Obviously someone changing the implementation should have an idea about what they are doing. The point was that you asked for a guide, which doesnt exist, so I gave rough steps for what needs to be done.

1 hour ago, Talendrys said:

Replace entirely with C++ 11 ? Is there somewhere something explaining this ? What about performance ?

Although I think that ACE should go away at some point

I meant replacing it in the tools. So yes, entirely - in the tools.
This means that we do not remove ACE completely form the server. JUST the tools. (vmap extractor, mmap extractor, mapextractor ..)

We just implement the things we use from ACE, but in c++(11) ourselves. I dont think there is anything explaining this stuff, but its not like there needs to be. Simple steps: Remove ace dependency (linking) from tools and then fix the compile errors

Im pretty sure the performance is not relevant. Even if any implementation we make would be 100 times slower, it might not appear at all to the users. Note that we are changing only the tools, not the server, and its only a small part of what the tools do that uses ACE.
 

It might also be possible to remove the ACE dependency from the generators and then there is no need for the dll (except on the actual server).

Im relatively sure you can use C++11 to replace most of the stuff needed from ace.

Fixed on https://github.com/ElunaLuaEngine/Eluna/commit/90c5c02c694ead254ea3526ed16a51da7755c6c8
Please update submodules for Eluna and monitor if the issue is fixed.

Related: [url]https://github.com/TrinityCore/TrinityCore/issues/14851[/url]
Also a possible fix is described there.


Here is a quote incase the issue is lost:
[QUOTE]This bug may be impossible to reproduce on normal situation and it only affects riding.
In general you would need to get a riding skill and then any spell that has learn skill effect for any higher riding skill.

Learning [url]http://wotlk.openwow.com/spell=33388[/url] and then [url]http://wotlk.openwow.com/spell=52382[/url] causes a bug where your riding is at 75 and max riding is at 150. Cause of this you cant learn any riding above that. Also you can not unlearn [url]http://wotlk.openwow.com/spell=33391[/url] after getting bugged even by unlearning the whole skill - learning it again says you already have it.

.learn 33388
.cast 52382
The reason it buggs is that the learn skill effect uses character's pure skill value as the current skill value for SetSkill function.
However on LearnSpell functions there is a special step that uses SpellMgr::GetSpellLearnSkill to correct the skill value for riding to be 75*skillstep.

TC: 239f0b4
TDB 58
3.3.5, may affect other branches

[/QUOTE]

Fixed in [url]https://github.com/mangoszero/server/commit/8d7be47e39e7e1f0e075397e81be20c4fb66610e[/url]

Close once confirmed.

The issue was misplaced hooks.
However the crash was not noticed as an existing crash occurs before this one apparently when not using external ace ?

Fixed: [url]https://github.com/mangoszero/server/commit/8ade79aa394a8266178fd06e1dd7f5e933d2fae7[/url]

Could you paste us the line 56 from your local ElunaEventMgr.cpp? (the if check)
FYI the line should look like this:
[url]https://github.com/ElunaLuaEngine/Eluna/blob/master/ElunaEventMgr.cpp#L56[/url]

The crash is caused by mangos destroying Transports on MapManager singleton destructor, which acts at the very end of the program after Eluna is uninitialized and the transport destructors try to access eluna.
To fix the crash the way/place where transports are destroyed needs to change or we can ignore the actions they try to do on eluna side.
In my opinion leaving cleanup for the singleton destructor isnt probably the best thing, unless its stricly singleton related data, which transports are not. (Transport = gameobject)

On the latest version the access attempts should be ignored avoiding the crash and it doesnt matter at that point anymore as the program is closing and the data is deleted already anyways.
However seems you are still getting the crash..

The fix I provided was never tested by anyone else successfully. And it was never actually added to master.
Would you be able to test it, Xenithar?

Then we at least know the change fixes it and there are no other known issues with it.
The final solution will need some changes though.

Crash occurs when any pet (hunter, warlock, player pet or npc pet) is killed by an aura.
.go xyz 4919.25049 -393.118439 333.848633 1
.aura 25309

The crash is caused by
[QUOTE]Yes, seems its the unsummoning for pets on setdeathstate.
It calls CleanupsBeforeDelete, which removes all auras and timed events from the pet.
The deathstate is set when the creature dies.
The creature dies when the aura kills the creature, so when the aura is updated / ticks)
However, the ticks are made as one loop. All auras are looped through and they are processed.
But since the auras are deleted when the killing aura is processed, it will make the loop have null error.[/QUOTE]

Suggested fix stub based on what crashes. Disabling the unsummon on death state change and moving the final unsummon to the update loop for next time called:
diff --git a/src/game/Pet.cpp b/src/game/Pet.cpp
index 001f3cb..f140618 100644
--- a/src/game/Pet.cpp
+++ b/src/game/Pet.cpp
@@ -520,7 +520,7 @@ void Pet::SetDeathState(DeathState s) // overwrite virtual
{
// remove summoned pet (no corpse)
if (getPetType() == SUMMON_PET)
- { Unsummon(PET_SAVE_NOT_IN_SLOT); }
+ ;// { Unsummon(PET_SAVE_NOT_IN_SLOT); }
// other will despawn at corpse desppawning (Pet::Update code)
else
{
@@ -552,13 +552,8 @@ void Pet::Update(uint32 update_diff, uint32 diff)
{
case CORPSE:
{
- if (m_corpseRemoveTime - {
- MANGOS_ASSERT(getPetType() != SUMMON_PET); // Pet must be already removed
- Unsummon(PET_SAVE_NOT_IN_SLOT); // hunters' pets never get removed because of death, NEVER!
- return;
- }
- break;
+ Unsummon(PET_SAVE_NOT_IN_SLOT);
+ return;
}
case ALIVE:
{


Note. Do not directly add this fix to repo / master as it has for example empty if.
Unsure of using break or return in the case.
The fix has not been tested for other effects than fixing this issue.

[QUOTE]I am not that familiar with the system and not sure if it has effects .. it most likely affects hunter pets dying
you should be able to rezz them but this will likely make them disappear instantly
something to do with m_corpseRemoveTime probably[/QUOTE]

Need syncing from mangoszero:
[url]https://github.com/mangoszero/server/commit/d30be2513e64adf50712a036b505937dba826634[/url]
[url]https://github.com/mangoszero/server/commit/19dc35552c296f4d53f5834064bdd67de767c9bd[/url]

[quote=Xenithar]I never run ANY server process on Windows. I do have a 2008 R2 license somewhere, but I'd need hardware to plop it onto. so yes, this is Linux, but I cannot test Windows.[/quote]
But is it reproducable? How?
I tested on windows and could not reproduce.

Hmm.
The uninit order is correct.
I cant find a logical reason or a way to reproduce this. Eluna is not deleted at any point before that.

~Linux only?

I think the issue might be that transport unloads after eluna unloads, causing the crash.
[code] mangosd.exe!ElunaEventProcessor::~ElunaEventProcessor() Line 55 + 0x3 bytes C++
mangosd.exe!ElunaEventProcessor::`scalar deleting destructor'() + 0x16 bytes C++
mangosd.exe!WorldObject::~WorldObject() Line 937 + 0x1f bytes C++
mangosd.exe!GameObject::~GameObject() Line 87 + 0x57 bytes C++
mangosd.exe!Transport::~Transport() + 0x4e bytes C++
mangosd.exe!Transport::`scalar deleting destructor'() + 0x16 bytes C++
[B] mangosd.exe!MapManager::~MapManager() Line 53 + 0x29 bytes C++
mangosd.exe!MapManager::`scalar deleting destructor'() + 0x16 bytes C++
mangosd.exe!MaNGOS::OperatorNew::Destroy(MapManager * obj) Line 59 + 0x1c bytes C++[/B]
mangosd.exe!MaNGOS::Singleton,MaNGOS::OperatorNew,MaNGOS::ObjectLifeTime >::DestroySingleton() Line 143 + 0xb bytes C++
msvcr120d.dll!0fc3edf3()
[Frames below may be incorrect and/or missing, no symbols loaded for msvcr120d.dll]
msvcr120d.dll!0fc3eea0()
mangosd.exe!__tmainCRTStartup() Line 662 C
mangosd.exe!mainCRTStartup() Line 466 C
kernel32.dll!76b4338a()
ntdll.dll!77199f72()
ntdll.dll!77199f45() [/code]

Seems that transports are not unloaded on mapmgr unloadall. The transports unload when the MapMgr singleton gets destroyed when the program shuts down.
We could fix this on Eluna side by just checking if Eluna still exists at that point.
Or the fix could be that transports are unloaded when everything else is unloaded.
Not sure leaving any actual object data to be destroyed in the end of whole program should be done.
What do you think? : /

In addition to the transports being a problem, the BG data is also a problem as it also gets deleted only at the very end and not when for example maps unload

Ill make a PR that will remove the crash, but the unload handling should still be discussed.
[url]https://github.com/mangoszero/server/pull/238[/url]

The error is true, but you are using too new Eluna for your source.
The sources have fixed Eluna version hashes that should be followed.
The error doesnt happen on the designed version.

Thanks for the heads up.
Will fix it now.