search

Admiral-Fish / PokeFinder

Cross platform Pokémon RNG tool
GNU General Public License v3.0
300 stars 73 forks source link

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

Closed c-poole closed 1 month ago

c-poole commented 1 month ago

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 commented 1 month ago

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

c-poole commented 1 month ago

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 commented 1 month ago

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

c-poole commented 1 month ago

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.