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

Quest: Fixed the lack of quest's reputation spillover data #575

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

Conversation

JJBcku
Copy link
Contributor

@JJBcku JJBcku commented Aug 1, 2024

Current code apply reputation spillover to all sources of reputation while quest reputation spillover is supposed to be determined separately for each quest. To fix this behavior I have added the ReputationSpillover column to quest_template and changed the code to utilize this information.

Current changes are made to perfectly mimic previous behavior. The purpose of this commit is only to create basis for future changes of quest's reputation spillover.

I have added per quest reputation spillover data, and changed the code to utilize that data.

Current changes are made to perfectly mimic previous behavior.
The purpose of this commit is only to create basis for future changes of quest's reputation spillover.
@AnonXS
Copy link
Member

AnonXS commented Aug 1, 2024

Please name some sources for reference that this is needed.

@JJBcku JJBcku force-pushed the ReputationSpilloverFix branch from 6b13f68 to 0c9be8e Compare August 1, 2024 20:11
@JJBcku
Copy link
Contributor Author

JJBcku commented Aug 1, 2024

@killerwife
Copy link
Contributor

TrinityCore/TrinityCore@0f7b9c6 here is the actual needed implementation. Was told this was added to client in original cataclysm and this is actually how its meant to work. I wont integrate it myself this core sync cycle but at least now i know its intent. If you have time, pls look at changing the pr accordingly.

@JJBcku
Copy link
Contributor Author

JJBcku commented Nov 15, 2024

Hi, I have changed the code from using an on/off switch to a per reputation spillover mask. In your opinion is there something else I should include in this pull request?

@cpevors
Copy link

cpevors commented Nov 15, 2024

Have you tested this?

@JJBcku
Copy link
Contributor Author

JJBcku commented Nov 15, 2024

I have done some test, so far everything seems to work correctly. If you want me to fully test it I can do it but it will take a day or two.

@cpevors
Copy link

cpevors commented Nov 15, 2024

I might pull this into my repository and test it as well. Thanks for the legwork.

ratkosrb added a commit to vmangos/core that referenced this pull request Nov 16, 2024
@JJBcku
Copy link
Contributor Author

JJBcku commented Nov 17, 2024

Hi @cpevors
I have just finished fully testing the changes and everything worked as intended.
Did you have time to test it?

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.

4 participants