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

Make Magnet Pull and Static apply to correct encounter types in Gen 3 #399

Merged
merged 5 commits into from
Jun 18, 2024

Conversation

c-poole
Copy link
Contributor

@c-poole c-poole commented Jun 13, 2024

Magnet Pull should only be applied for Grass encounters.
Static should only be applied for Grass and Water encounters.

This can be seen from the following PokeEmerald code:
https://github.com/pret/pokeemerald/blob/18f84b78f2d1a8669753fa586836fca06036c790/src/wild_encounter.c#L422-L446
https://github.com/pret/pokeemerald/blob/18f84b78f2d1a8669753fa586836fca06036c790/src/wild_encounter.c#L458-L465

Current behavior is that they both apply to all wild encounter types. Fix involves modifying the wild generator and searcher classes to recognize when they should be skipped. Alternative solution would be to reset selected Lead to None if either of these were selected and to remove the option to select them when the encounter type is changed to an incompatible type.

@Admiral-Fish
Copy link
Owner

I think it makes more sense to handle this in the constructor

@c-poole
Copy link
Contributor Author

c-poole commented Jun 14, 2024

By this do you mean, scrubbing the lead passed into the constructor of the generator and searcher classes while still allowing the user to have leads that would be ignored selected or to actually modify the menu in response to the selected encounter type?

@Admiral-Fish
Copy link
Owner

Yeah scrub out the lead in the constructor if it wouldn't be allowed for the encounter type

@c-poole
Copy link
Contributor Author

c-poole commented Jun 14, 2024

I've pushed a version that fixes this through a UI-behavior change that modifies the visibility of the Lead options according to what is applicable to the encounter type selected whenever it is changed. I'm fine with doing the constructor based approach if you still prefer that to this, but I think this is most transparent both from a code readability (someone reading through generator/searcher without realizing there is lead scrubbing would be very confused) and an end-user perspective. This change also fixes what I think are two bugs in the ComboMenu class HideAction method which automatically un-selects an Action corresponding to the provided data even if the method was told to set (keep) its visibility to true. The other is that it would hide a Menu if any of the Actions in the Menu are invisible instead of, as the comment suggests is desired, when all Actions are invisible.

Copy link
Owner

@Admiral-Fish Admiral-Fish left a comment

Choose a reason for hiding this comment

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

Nice catch on the combo menu stuff. Totally agree there

Source/Form/Gen3/Wild3.cpp Outdated Show resolved Hide resolved
Source/Form/Gen3/Wild3.cpp Outdated Show resolved Hide resolved
@Admiral-Fish Admiral-Fish merged commit d142f17 into Admiral-Fish:master Jun 18, 2024
2 of 3 checks passed
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.

2 participants