-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Added new hooks for repuation gaining to the player #21306
base: master
Are you sure you want to change the base?
Added new hooks for repuation gaining to the player #21306
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is no reason to create a new file for this. Keep it in Player.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changelog is an outdaterd - not used system. You can remove them
59fefdf
to
84a8f3c
Compare
added default case to switch Co-authored-by: Kitzunu <[email protected]>
/** | ||
* @brief This enum represent all known sources a character can get reputation | ||
*/ | ||
enum class ReputationSource : uint8 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enum class ReputationSource : uint8 { | |
enum class ReputationSource : uint8 | |
{ |
@@ -114,7 +115,9 @@ bool OutdoorPvPSI::HandleAreaTrigger(Player* player, uint32 trigger) | |||
// add 19 honor | |||
player->RewardHonor(nullptr, 1, 19); | |||
// add 20 cenarion circle repu | |||
player->GetReputationMgr().ModifyReputation(sFactionStore.LookupEntry(609), 20.f); | |||
float reputation = 20.f; | |||
ScriptMgr::instance()->OnBeforePlayerReputationChange(player, 609, reputation, ReputationSource::PvP); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sScriptMgr->
Is there any way for this to be condensed? So it's not called in 20 different places in the core? |
Maybe in ReputationMgr, but this will require to rework most functions, as they don't know about quests and units. And in my opinion ReputationMgr should not know about this information |
Changes Proposed:
This PR proposes changes to:
Issues Addressed:
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.