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

Fix chain fishing shiny rolls #4906

Merged
merged 4 commits into from
Jul 19, 2024

Conversation

cawtds
Copy link

@cawtds cawtds commented Jul 3, 2024

Description

According to https://bulbapedia.bulbagarden.net/wiki/Fishing#Generation_VI chain fishing adds 2 shiny rolls per Encounter. The current implementation adds 1 additional roll. Additionally, even with the feature disabled 1 roll is currently added to any fishing encounter.
I've also refactored a bit and removed (imo) redundant functions in a second commit. If these functions are supposed to stay feel free to revert the second commit.

Discord contact info

.cawt

pkmnsnfrn
pkmnsnfrn previously approved these changes Jul 5, 2024
Copy link
Collaborator

@pkmnsnfrn pkmnsnfrn left a comment

Choose a reason for hiding this comment

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

I did not test in game but looking at the changes, it looks like this PR fixes the problem called out in the description

still haven't properly gone through the big boy merge process and I'm sick, so today is not the day to try

src/pokemon.c Outdated
Comment on lines 1153 to 1154
if (I_FISHING_CHAIN && gIsFishingEncounter)
totalRerolls += (2 * gChainFishingDexNavStreak);

Choose a reason for hiding this comment

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

I feel these two changes still justify their functions, like adding I_FISHING_CHAIN to IsCurrentEncounterFishing and keeping CalculateChainFishingShinyRolls to let users add other multipliers for their streaks, like a charm or a Pokémon ability.

Copy link
Author

Choose a reason for hiding this comment

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

Imo including I_FISHING_CHAIN in IsCurrentEncounterFishing would be misleading with the current function name. From the name i would expect it to return true even if chain fishing is disabled.

…untering different species does not break the chain, differentiate between max shiny streak and max chain length
@tertu-m tertu-m merged commit 5ccf3e2 into rh-hideout:upcoming Jul 19, 2024
1 check passed
@cawtds cawtds deleted the fix-fishing-encounter branch August 12, 2024 16:01
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