feat(Core/Scripting): Add hook for OnPlayerCanLootFishingPool #22291
Conversation
github-actions
bot
added
CORE
|
Should probably be named OnPlayerCan... |
I fixed the naming. Thanks. |
Zerkenn
changed the title
| { | ||
| ok->Use(player); | ||
| } |
There was a problem hiding this comment.
There should be no braces here. See AC Codestyle guide for reference
Also I'm missing a parameter referring to the pool itself (go pointer for example) - seems useful for a fishing pool related hook imo
|
It somehow feels wrong to add another hook like 3 lines after an existing hook with exactly the same parameters. |
|
I would argue that you can already implement all your desired functionality with the existing hook. Just fetch the game object there and loot it / deactivate it or something similar, no need for a new hook here imo. |
I can modify this to just pass the player, skill, fishing_skill, zone_skill, pointer to the fishing pool object. I really don't want to have to fetch the fishing pool object every time when using OnPlayerUpdateFishingSkill and then check if a pool exist before rewarding the player. Then try to figure out how to stop the code a few lines down from doing its normal looting behavior on the fishing pool. I just wanted a simple hook where I can either add additional items/currency to fishing pools and return false to prevent looting of the fishing pool in certain circumstances. I figured t would be a nice hook to add for others to use too. If you still think this is unacceptable than feel free to close the PR. I am primarily using this on my own server. I can just add it to my own fork. I am a little new to C++. I don't want to waste anyone's time here. |
Changes Proposed:
I am implementing this script for a module I am creating so I can reward players additional items when they loot a fishing pool. You can return false to stop the looting of the fishing pool object.
This PR proposes changes to:
SOURCE:
The changes have been validated through:
Tests Performed:
This PR has been:
How to Test the Changes:
Known Issues and TODO List:
How to Test AzerothCore PRs
When a PR is ready to be tested, it will be marked as [WAITING TO BE TESTED].
You can help by testing PRs and writing your feedback here on the PR's page on GitHub. Follow the instructions here:
http://www.azerothcore.org/wiki/How-to-test-a-PR
REMEMBER: when testing a PR that changes something generic (i.e. a part of code that handles more than one specific thing), the tester should not only check that the PR does its job (e.g. fixing spell XXX) but especially check that the PR does not cause any regression (i.e. introducing new bugs).
For example: if a PR fixes spell X by changing a part of code that handles spells X, Y, and Z, we should not only test X, but we should test Y and Z as well.