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

Feature: Added GetUnitFlags(), GetUnitFlagsTwo(), SetUnitFlags(flags), SetUnitFlagsTwo(flags), Added PlayerEvent OnApplyAura/OnRemoveAura #137

Merged
merged 14 commits into from
May 21, 2023

Conversation

New-HavenWotLK
Copy link

@New-HavenWotLK New-HavenWotLK commented May 16, 2023

Implemented the stated methods to the source... tested them... they worked fine! =D

#138

@New-HavenWotLK New-HavenWotLK changed the title Implement GetUnitFlags(), GetUnitFlagsTwo(), SetUnitFlags(flags), Set… …UnitFlagsTwo(flags) Feature: Added GetUnitFlags(), GetUnitFlagsTwo(), SetUnitFlags(flags), Set… …UnitFlagsTwo(flags) May 16, 2023
@New-HavenWotLK
Copy link
Author

@Helias if u got some time, could u check it out pls? =D

thx in advance! ^.^

@Helias Helias requested a review from r-o-b-o-t-o May 17, 2023 13:39
@New-HavenWotLK
Copy link
Author

added also now 2 new hooks which i have tested as well =)

they worked fine when checking the hooks via a print statement =D

@New-HavenWotLK New-HavenWotLK changed the title Feature: Added GetUnitFlags(), GetUnitFlagsTwo(), SetUnitFlags(flags), Set… …UnitFlagsTwo(flags) Feature: Added GetUnitFlags(), GetUnitFlagsTwo(), SetUnitFlags(flags), SetUnitFlagsTwo(flags), Added PlayerEvent OnApplyAura/OnRemoveAura May 20, 2023
@New-HavenWotLK New-HavenWotLK requested a review from Helias May 20, 2023 02:30
@Helias
Copy link
Member

Helias commented May 20, 2023

thanks for your PR!
If the pipeline will pass successfully I will merge this PR

@Helias
Copy link
Member

Helias commented May 20, 2023

error in the build

/home/runner/work/mod-eluna/mod-eluna/modules/mod-eluna/src/LuaEngine/PlayerHooks.cpp:694:13: fatal error: out-of-line definition of 'OnApplyAura' does not match any declaration in 'Eluna'
void Eluna::OnApplyAura(Player* player, Aura* aura, bool isNewAura)
            ^~~~~~~~~~~
1 error generated.
make[2]: *** [modules/CMakeFiles/modules.dir/build.make:328: modules/CMakeFiles/modules.dir/mod-eluna/src/LuaEngine/PlayerHooks.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/Makefile2:1199: modules/CMakeFiles/modules.dir/all] Error 2
make: *** [Makefile:146: all] Error 2

Copy link
Member

@r-o-b-o-t-o r-o-b-o-t-o left a comment

Choose a reason for hiding this comment

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

Hi there, thanks for the PR!

Looks like some parts are missing for your hooks, see this commit for an example on how to add a hook: 85714c1

Also, could you document your additions in README.md please? See 2a683a5 and the other commit I linked for examples.

src/LuaEngine/CreatureMethods.h Outdated Show resolved Hide resolved
src/LuaEngine/CreatureMethods.h Outdated Show resolved Hide resolved
src/LuaEngine/CreatureMethods.h Outdated Show resolved Hide resolved
src/LuaEngine/CreatureMethods.h Outdated Show resolved Hide resolved
src/LuaEngine/CreatureMethods.h Outdated Show resolved Hide resolved
@New-HavenWotLK
Copy link
Author

New-HavenWotLK commented May 20, 2023

ah uups... XD
seems i forgot to add some parts from my source on server to the forked repo ^^''

i will look to fix this when i am home =)

@New-HavenWotLK
Copy link
Author

sooo i think now there should be all properly added

shouldn't add stuff by hand when i already am half asleep ^^'

New-HavenWotLK and others added 5 commits May 20, 2023 19:21
Co-authored-by: Axel Cocat <ax.cocat@gmail.com>
Co-authored-by: Axel Cocat <ax.cocat@gmail.com>
Co-authored-by: Axel Cocat <ax.cocat@gmail.com>
Co-authored-by: Axel Cocat <ax.cocat@gmail.com>
Co-authored-by: Axel Cocat <ax.cocat@gmail.com>
@Helias
Copy link
Member

Helias commented May 21, 2023

@r-o-b-o-t-o what do you think?

Copy link
Member

@r-o-b-o-t-o r-o-b-o-t-o left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@r-o-b-o-t-o r-o-b-o-t-o merged commit 92818c6 into azerothcore:master May 21, 2023
@r-o-b-o-t-o
Copy link
Member

Just noticed this now while looking into #144, you're using hooks that don't exist for your aura events...
I didn't notice the missing overrides in ElunaLuaEngine_SC.cpp when reviewing.
Since the real aura hooks are in UnitScript and not PlayerScript I'm afraid I will have to revert this part of the PR, since we don't have Unit hooks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants