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

Fishing shake percentage seems off. #4105

Closed
andrewkm opened this issue Jan 16, 2020 · 10 comments
Closed

Fishing shake percentage seems off. #4105

andrewkm opened this issue Jan 16, 2020 · 10 comments

Comments

@andrewkm
Copy link
Contributor

Using 2.1.113 compiled on 406429f

At max rank, chance is 56%
https://i.imgur.com/zItFapn.png
Where is this 56% number even coming from... am I missing something?

Advanced.yml

    Fishing:

        ShakeChance:
            Rank_1: 15.0
            Rank_2: 15.0
            Rank_3: 25.0
            Rank_4: 35.0
            Rank_5: 45.0
            Rank_6: 55.0
            Rank_7: 65.0
            Rank_8: 75.0

Wiki states 75% as well:
https://mcmmo.fandom.com/wiki/Fishing

@andrewkm
Copy link
Contributor Author

In depth test:
Default paper, default mcMMO and not even a permissions plugin, straight up following mcmmo's plugin.yml

<fishing level>:<shake chance>
124:0%
125:0%
250:2%
350:6%
400:6%
800:30%
1000:56%
5000:56%
1000000:56%

https://i.imgur.com/JtoEc97.png
Conclusion: It's definitely an mcMMO bug.

@nossr50
Copy link
Member

nossr50 commented Mar 13, 2020

There's some unused settings in the config, with that said, I'll investigate and see what is going on.

@andrewkm
Copy link
Contributor Author

Any updates on this one @nossr50

@Lyther
Copy link
Contributor

Lyther commented Nov 8, 2021

Spotted this problem in the source code.

The fishing skill uses RandomChanceStatic to calculate the chances. However, this class has a mistaken definition with probabilityCap = XPos. After calculation in getChanceOfSuccess, the value probabilityCap is put into maxProbability. Leads to result formula chance = XPos * (XPos / 100.0D).

This explains why the bug gives chance definition ^ 2 / 100 as a result.

Fixed this problem in my pull request.

@andrewkm
Copy link
Contributor Author

@Lyther @nossr50
The above commit did not fix the issue.
Instead, fishing shake is now ALWAYS stuck to 30% regardless of your level.
It unlocks at level 250 and simply never increases, whether your fishing level 1000, level 10,000, etc.

The above PR broke it even more.

@Lyther
Copy link
Contributor

Lyther commented Nov 15, 2021

@Lyther @nossr50 The above commit did not fix the issue. Instead, fishing shake is now ALWAYS stuck to 30% regardless of your level. It unlocks at level 250 and simply never increases, whether your fishing level 1000, level 10,000, etc.

The above PR broke it even more.

The problem is in the new version of McMMO. With the percentage of shake comes to a fixed value of 30% and here's in the advanced.yml

        ShakeChance:
            Rank_1: 30

And here's in skillranks.yml

    Shake:
        Standard:
            Rank_1: 15
        RetroMode:
            Rank_1: 150

Thought the author has changed the definition of shake skill and makes it a fixed value (just like the IceFishing and other un-upgradable skills).

I haven't touched those files in my pull request.

@nossr50
Copy link
Member

nossr50 commented Nov 15, 2021

I'll adjust Shake to scale and gain ranks again, still getting over my cold

@andrewkm
Copy link
Contributor Author

Thanks,
Would be much appreciated!

@andrewkm
Copy link
Contributor Author

andrewkm commented Sep 26, 2023

@nossr50 unsure if this was forgotten, but it's still an issue on our server.
Any plans on this? Shake continues to be capped at 30%.

Still happening to this date on Paper 1.20.1 and latest mcMMO.

@andrewkm
Copy link
Contributor Author

andrewkm commented Oct 1, 2023

Fixed. :)

@andrewkm andrewkm closed this as completed Oct 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants