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

Crafting with UseAllRes, instant GM #4774

Closed
juuso opened this issue May 2, 2020 · 2 comments
Closed

Crafting with UseAllRes, instant GM #4774

juuso opened this issue May 2, 2020 · 2 comments

Comments

@juuso
Copy link

juuso commented May 2, 2020

There are a few issues with crafting stacks of items. Most seriously, it's trivial to e.g. train cooking to GM by starting from < 10.0 skill and making 1000 fish steaks.

In the following code from SkillCheck.cs, the skill.Base < 10.0 check (for automatic gains) is done against skill.Base, which is not updated in the loop. Only gains and value are updated. Crafting 1000 items from skill 0 will give an automatic gain for each iteration.

for (int i = 0; i < amount; i++)
{
    double gc = GetGainChance(from, skill, (value - minSkill) / (maxSkill - minSkill), value) / 10;
    if (AllowGain(from, skill, new Point2D(from.Location.X / LocationSize, from.Location.Y / LocationSize)))
    {
        if (from.Alive && (skill.Base < 10.0 || Utility.RandomDouble() <= gc || CheckGGS(from, skill)))
        {
            gains++;
            value += 0.1;
        }
    }
}
@juuso
Copy link
Author

juuso commented May 2, 2020

Related to that, the GetGainChance call is also very suspect.

The actual method signature looks like this: private static double GetGainChance(Mobile from, Skill skill, double gains, double chance), with gains and chance expected as the last two parameters.

However, it is called with (value - minSkill) / (maxSkill - minSkill) and value as the last two parameters (basically "chance" and "the current skill value", respectively).

In addition, the actual implementation doesn't even use the chance method parameter.

@kevin-10
Copy link
Contributor

kevin-10 commented May 4, 2020

I see what you are saying. Thank you for bringing this to my attention. Also, I've noticed CheckGGS will bypass any checks (if its time for a ggs gain).

@kevin-10 kevin-10 closed this as completed May 8, 2020
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

No branches or pull requests

2 participants