Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Module/Eluna): Add Support for Multistate #21294

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

iThorgrim
Copy link
Contributor

@iThorgrim iThorgrim commented Jan 29, 2025

Note

Implementation based on this PR.

📝 Description

This pull request introduces modifications to AzerothCore to ensure compatibility with the Multistate system in mod-eluna. The changes include:

  • Adding MOD_ELUNA as a compilation flag in CMakeLists.txt.
  • Enhancing WorldObject, Map, and World classes to support the Eluna scripting engine.
  • Introducing GetEluna() methods to allow access to the Lua state.

These changes are a foundational step toward integrating advanced Multistate functionalities into mod-eluna, making future feature additions simpler and more maintainable.

🌟 Key Changes

1. CMake Configuration

Added -DMOD_ELUNA flag in modules/CMakeLists.txt to enable mod-eluna support at compile time when is detected.

2. Object and Map Integration

  • Introduced GetEluna() method in WorldObject and Map classes.
  • Ensured eluna is properly initialized when maps are loaded.
  • Implemented cleanup logic for eluna instances when maps are destroyed.

3. World Class Enhancements

  • Added a global Eluna* eluna instance to World.
  • Provided a GetEluna() accessor method.

@heyitsbench
Copy link
Contributor

Not a fan.

@iThorgrim
Copy link
Contributor Author

Not a fan.

Thanks for your "feedback", I'd still like to know why? The work isn't finished yet, but maybe you can give me some feedback (hopefully with more arguments) 🦾

@heyitsbench
Copy link
Contributor

Apologies for that blunt and unhelpful message, I'm tired. I do want Eluna to see multistate support, and I do applaud your work, but the part I am not a fan of is having compiler define checks for if Eluna is in use. Maybe it's just me, or maybe there's already code in AC like this, but I am of the opinion that a custom change/module like Eluna shouldn't be accommodated quite in this manner. Again, this is just my opinion, and I am not a significant vote on the matter. 😛

@iThorgrim
Copy link
Contributor Author

The thing is, in my opinion, it’s not possible to do otherwise—at least, I haven’t found a way. Moreover, in the emulator, specifically in the module folder, there are already conditions specific to mod-eluna. I’ve also seen files in the emulator that are only useful if mod-eluna is used. ElunaScript.cpp, Object.h

I’m not saying this is the best solution, but for now, I’m the only one who has looked into the "issue", and I’ve asked for help. Ever since Eluna implemented multistate to port it to AzerothCore, no one has ever responded.

I agree that it clutters AzerothCore with very specific instructions, but if someone has a way to do it differently, I’m all ears! 😃

@kissingers
Copy link
Contributor

In fact, there are same other modifications that match mod-Anticheat, which is good. However, mod-Anticheat is currently not well maintained and has many false positives. This is because the AZ side lacks sufficient similar matching for the module.
If someone can update the mod-Eluna to a more perfect state, but it requires a bit of cooperation from the AZ side, I personally think that would be great.

@zbhcn
Copy link

zbhcn commented Feb 1, 2025

windows log:
754f720c-dd35-492d-acef-2fdd5f222dc2

@zbhcn
Copy link

zbhcn commented Feb 2, 2025

Windows 日志: 754f720c-dd35-492d-acef-2fdd5f222dc2

Help me please , what's error is it?

@iThorgrim
Copy link
Contributor Author

iThorgrim commented Feb 2, 2025

Windows 日志: 754f720c-dd35-492d-acef-2fdd5f222dc2

Help me please , what's error is it?

Your screen gives no indication of a possible error, there's nothing to help you understand the error or to help you, there's no usable information. The only visible thing seems to be the fact that it's impossible to copy/paste a folder, but it's impossible to know why ..

@Foereaper
Copy link
Contributor

The define can be set through the cmake file of the module, no need to add it through the core cmake files. You'll definitely need the core file modifications though as shown, as well as for the core side storage (GetData/SetData) which you haven't added as part of this PR if you want that feature.

@sudlud
Copy link
Member

sudlud commented Feb 2, 2025

I must say that I'm also not a fan of adding code sections to the core surrounded by module specific compile flags. We should find a better solution for this

@Foereaper
Copy link
Contributor

There is no other solution to this. Eluna is not designed to be a module, it requires core functionality to work properly. The scope of a scripting engine is far beyond the current "module" script hook approach that it has been shoe-horned into the emulator as.

@zbhcn
Copy link

zbhcn commented Feb 3, 2025

The thread safe issue still hasn't been resolved.this is my crash log:
unknown_worldserver.exe_[3-2_9-22-13].txt
@iThorgrim

@@ -802,6 +811,10 @@ class Map : public GridRefMgr<NGridType>
std::unordered_set<Corpse*> _corpseBones;

std::unordered_set<Object*> _updateObjects;

#if defined(MOD_ELUNA)
Eluna* eluna;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::unique_ptr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CORE Related to the core file-cpp Used to trigger the matrix build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants