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

Player: Modified reputation gain calculations #553

Closed
wants to merge 6 commits into from

Conversation

JJBcku
Copy link
Contributor

@JJBcku JJBcku commented May 6, 2024

I have modified the calculation of level difference penalties for reputation gain to be more in line with the original World of Warcraft.

🍰 Pullrequest

Proof

Issues

  • fixes non vanilla wow-like reputation gains

JJBcku added 2 commits May 6, 2024 22:38
I have modified the calculation of level difference penalties for reputation gain to be more in line with the original World of Warcraft.
@JJBcku
Copy link
Contributor Author

JJBcku commented Jun 20, 2024

Could someone tell me why there is no discussion or even acknowledgment of my pull request?
Have I did something wrong?

@insunaa
Copy link
Contributor

insunaa commented Jun 20, 2024

Currently nobody on the dev team has the time to review pull requests

@killerwife
Copy link
Contributor

@ratkosrb

if (currentLevel > treshold)
percent *= std::max(minRate, (1.0f - (0.2f * (currentLevel - treshold))));
if (lastGreenLevel > creatureOrQuestLevel)
percent *= std::max(minRate, (1.0f - (0.2f * (lastGreenLevel - creatureOrQuestLevel))));
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand this is based on the forum post, but have you actually tested this on classic PTR or what specifically is this based on (I have read the links pointing it out)

Copy link
Contributor

Choose a reason for hiding this comment

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

After careful investigation, the function GetGrayLevel is deprecated and IsTrivialLevelDifference is the correct one. (trivial means grey in official naming)

I went ahead and a 48 npc (green) on lvl 60 gives full rep, and the GetGrayLevel function would not do that properly

// Pre-3.0.8: Declines with 20% for each level if 6 levels or more below the player down to a minimum (default: 20%)
const uint32 treshold = (creatureOrQuestLevel + 5);
// Old code seems to have been correct for computing level difference penalties for quests
if (source == REPUTATION_SOURCE_QUEST)
Copy link
Contributor

Choose a reason for hiding this comment

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

So here you are saying the reputation penalty isnt related for quest case with "grayness" in any way, but only related to the + 5 constant (cos gray level is dynamic based on level of source)

@killerwife
Copy link
Contributor

killerwife commented Jul 4, 2024

My proposed code based on client lua:

    if (source == REPUTATION_SOURCE_KILL)
    {
        if (MaNGOS::XP::IsTrivialLevelDifference(currentLevel, creatureOrQuestLevel))
        {
            const uint32 greenRange = MaNGOS::XP::GetQuestGreenRange(currentLevel);
            percent *= std::max(minRate, (1.0f - (0.2f * (currentLevel - greenRange - creatureOrQuestLevel))));
        }
    }
    else if (source == REPUTATION_SOURCE_QUEST)
    {
        // Pre-3.0.8: Declines with 20% for each level if 6 levels or more below the player down to a minimum (default: 20%)
        const uint32 treshold = (creatureOrQuestLevel + 5);

        if (currentLevel > treshold)
            percent *= std::max(minRate, (1.0f - (0.2f * (currentLevel - treshold))));
    }

Test case, grey mob in vanilla at 47 (grey diff in vanilla is 12 levels per client and lua) will get 80% rep. Quest case unchanged.

Bug fixed the GetGrayLevel function and added checking for trivial level difference to calculating kill reputation gain.
@JJBcku
Copy link
Contributor Author

JJBcku commented Jul 13, 2024

Hi,
I have added checking for the trivial level difference in the modified function.
I have not found a function named GetQuestGreenRange in Mangos::XP so instead I have modified the GetGrayLevel function to return values in line with the ones IsTrivialLevelDifference is using.

@killerwife
Copy link
Contributor

The function exists in lua client code. I extracted it over here: 9e95f5d take a look

@JJBcku
Copy link
Contributor Author

JJBcku commented Jul 13, 2024

Thank you for your help.
Could you tell me the email I should use to credit you as a co-author.

@killerwife
Copy link
Contributor

Just look at git history in commandline, its there. I prefer not to write it here cos it kinda leads to more spam email for me :D

Adds GetQuestGreenRange and uses it to calculate reputation gain on kill.

Co-Authored-By: killerwife <[email protected]>
@JJBcku
Copy link
Contributor Author

JJBcku commented Jul 13, 2024

I have added the changes you suggested.
I have kept in the fix for the GetGrayLevel function, is that okay?

@killerwife
Copy link
Contributor

I will review how all 3 cores use them and then I will merge it. We prefer to use official functions from client whenever named and available. The gray level one doesnt exist in client.

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