Backwards-compatible BoxPokémon Refactor

[mrgriffin]

@AsparagusEduardo's #2372, but maintaining compatibility with existing saves. This means that the structures end up being a fair bit uglier than in Edu's PR, and also we can't ever reclaim bits that may be non-zero so things like the second isEgg, hasSpecies, etc have stayed.

Mostly the change is simple, just trim the size of existing members and make use of the known-zeros. It should go without saying that if any of the repurposed bits are not zero, things will go wrong.
The three complicated cases were:

• Shinyness and hidden natures which are implemented as a modifier which is xored onto the value computed from the personality. 0 ^ x = x so this means existing Pokémon keep their shinyness and have a hidden nature equal to their nature. Note that Pokémon made shiny this way do not have a shiny PID+OTID combination.
• Tera types which are implemented as 1 + type, with 0 being interpreted as choosing the Tera type based on the personality. This uses bit 2 instead of bit 1 so that Dustox/Beautify can have either Tera type.
• HP. We store the HP lost in BoxPokemon so that all existing boxed Pokémon have max HP, instead of zero HP.

There's 35 bits free, spread out all over the place. Because GetMonData/SetMonData exist, if we needed e.g. 10 bits for a single value we could use two of the 5s and some bitwise operations.

Note that this is not compatible with trading to vanilla, PKHeX, the GameCube games, etc. This is because those games will a) read new data in the upper bits of things like moves and PP, and b) write over the same upper bits. For the same reason, we aren't able to trade with earlier versions of a hack. It may be possible to get battles working.

• I haven't tested this yet.
• I'd like to write some tests to check that all the new behaviors work.
• Feel free to point out a Pokémon species that is biased by having Tera type come from bit 2 and I'll hash the personality or something.
• Any other feedback welcome.

I can only apologize if you've been pointed at this PR due to a merge conflict arising from you modifying the BoxPokemon struct. If you want to keep your players' saves then you'll need to find new locations for the changes in this PR, because you mustn't shift your own data.

[mrgriffin]

I'll rebase this down into a sensible history if/when it's accepted :)

Important changes:

• Prevent losing G-Max form through evolution
• Support Tera, Dynamax, Gigantamax Factor, and Shadow in Trainer Control
• Implement Nature MInts and Dynamax Candy
• Support Dynamax and Gigantamax Factor in battles
• Support Tera, Dynamax, Gigantamax Factor, and Shadow in battle tests
• Show Tera type on summary screen
• Some script commands for manipulating Hyper Training and Gigantamax Factor.

Not done:

• Show Gigantamax Factor on summary screen

[mrgriffin]

Ready for review!

Some notes for reviewers:

• isShadow/teraType are in TrainerMon, but maybe that should wait until the Shadow Pokémon/Terastallization PRs are merged?
• formCountdown is not used. A follow-up PR is needed to make it work (IDK what it actually does).
• The Terastallization type is chosen by PID bit 1 (& 0x1). The idea is that this is a random value that already exists, but please double-check that there's no species which has a biased bit 1 (e.g. Wurmple would if bit 1 controlled Silcoon vs Cascoon).
• We need to show the Gigantamax Factor on the summary screen (like we do for the Tera Type).
• I think we should probably have a config to choose between healing when Pokémon are placed in the box, and healing boxes only when the player uses a Pokémon Center. Edu implemented that side of things, so I'm not sure exactly where to start with that; I can probably figure it out, but I would appreciate it if somebody else could contribute that change.

[AsparagusEduardo]

• isShadow/teraType are in TrainerMon, but maybe that should wait until the Shadow Pokémon/Terastallization PRs are merged?

I feel so as well. No point confusing users with unused fields.

• formCountdown is not used. A follow-up PR is needed to make it work (IDK what it actually does).

Hoopa and Furfrou revert to their base forms after 3/7 days in the party. However, according to Serebii, Hoopa no longer reverts after 3 days. Couldn't find find this fact in Bulbapedia, so maybe we could remove this field.

• I think we should probably have a config to choose between healing when Pokémon are placed in the box, and healing boxes only when the player uses a Pokémon Center. Edu implemented that side of things, so I'm not sure exactly where to start with that; I can probably figure it out, but I would appreciate it if somebody else could contribute that change.

It should be simple enough to heal the mon upon placing it on the PC.

[mrgriffin]

Hoopa and Furfrou revert to their base forms after 3/7 days in the party. However, according to Serebii, Hoopa no longer reverts after 3 days. Couldn't find find this fact in Bulbapedia, so maybe we could remove this field.

Ah interesting! I've removed it for now until we decide what we want to do with it :)

[mrgriffin]

Rebased to clean up the history, implemented an OW_PC_HEAL config. Ready for review!

EDIT: Made a few other reverts, so I'll need to rebase again before this is merged.

[AsparagusEduardo]

Initial review.
Looking back at this with what I've learned, I think I would change all instances of bool8 isShiny to bool32 isShiny, so I'll likely make a PR to this PR.

[mrgriffin]

Ready for re-review. Rebased onto upcoming and conflicts fixed.

[kleeenexfeu]

We could get rid of the encryption entirely and keep save compatibility by just using one flag. It would also provide more freedom to reorganize the whole structure.

It would go like this :
-Upon loading a save, we would check if the flag is set. If it is, them load the save as usual using @AsparagusEduardo's #2372 new Get/Set(Box)MonData.
-If not, do a giant loop to decrypt the party and the whole PC (the game will clearly freeze for a few seconds, no doubt) using the old Get(Box)MonData, and the new Set(Box)MonData.

The downside if keeping the old functions in the ROM, but we'd save a lot of space in the RAM.

I know it works because I've done it in ASM many years ago, but I wanted to make the suggestion before making a PR in case it has already been discussed

[mrgriffin]

I think the big difficulty with migration is that we don't exist in isolation. If a developer has made their own changes to the structure then they need to edit the save migration code to replicate those changes or else all their additions will silently vanish and all affected saves will be unrecoverable.

Obviously with this PR it's still possible for people to lose their changes, but they'll at least be prompted to solve a merge conflict.

That said, I'm not opposed to trying to come up with a user-friendly way to migrate saves. This PR was just borne out of that approach stalling, and the desire to actually land these new features now. Edu's PR was parked for just over a year.

EDIT: Also if save migration ever comes in the future then we can clean up the structure. I don't think accepting this PR introduces any new difficulties for migration?

[mrgriffin]

See also #3081, its discussion on Discord, and Tustin's article that inspired it.

Having a single flag just for BoxPokemon definitely does avoid some of the pitfalls of being totally generic for the whole save.

[mrgriffin]

Oh actually, one last thing @AsparagusEduardo togglegigantamaxfactor should fail for Melmetal... but should it also fail for species that can't Gigantamax? Or can any Pokémon have the G-max factor, it's just that it doesn't do anything for most of them?

EDIT: Forgot to add a test for togglegigantamaxfactor, so I'd like to rebase before it's merged please :)

[mrgriffin]

Conflicts hopefully resolved, but needs testing in-game of all the scenes where a shiny Pokémon is shown. The species refactor conflicted in a load of places that Edu changed 😭

[Bassoonian]

There's some more conflicts. I'll try to test all the shiny possibilities this weekend and then we should merge this in :)

[mrgriffin]

Conflicts resolved (hopefully correctly!), this will conflict again if somebody fixes the bug in https://github.com/rh-hideout/pokeemerald-expansion/blob/upcoming/src/battle_tower.c#L2992-L2995 before this is merged.
EDIT: Hmm, src/data/text/item_descriptions.h is still considered conflicting 🤔

[GraionDilach]

Yeah, that conflict comes from the PR trying to edit the Serious Mint's description there, but upcoming dropped the file as part of the COMPOUND_STRING unification.

[mrgriffin]

Ah, I failed to fetch before merging, I see. I thought it would have been Lunos' PR, and was confused when nothing seemed to have changed.

[Bassoonian]

Conflicts resolved (hopefully correctly!), this will conflict again if somebody fixes the bug in https://github.com/rh-hideout/pokeemerald-expansion/blob/upcoming/src/battle_tower.c#L2992-L2995 before this is merged.

It was :) I've been checking in-game and all the shiny sprites seem to be fine from what I can gather, so if you fix the conflict I think we can happily merge this in!

[Bassoonian]

Oh, I found one! Catching a Pokémon, seeing its dex entry and then getting the nickname option turns it into a shiny:

Everything else seems fine though, contests included!

on vacation