Vanilla trainers' Pokémon have wrong PIDs/Natures

[mrgriffin]

Description

#3114 changed all the vanilla trainers to TrainerMonCustomized, but TrainerMonCustomized generates its PIDs using a CRC32 of the party member being generated, whereas vanilla generates its PIDs from a hash of the trainer name combined with hashes of the species names that have previously been generated.

The PID affects the nature of the generated Pokémon. The gender and ability are unaffected because the hash is << 8.

Note also that because the species names are part of the hash, decapitalization would also affect the vanilla hash and alter the natures.

Because of the above, I think the best fix for this would be to explicitly list the natures in src/data/trainer_parties.h. We probably shouldn't just change back to a vanilla-compatible hash because that would change the natures of parties that were balanced around the current nature, although perhaps in practice that wouldn't be a big problem.

Version

1.6.2 (Default)

Upcoming Version

No response

Discord contact info

No response

[AlexOn1ine]

Does anyone know if there is a resource that lists all natures for each trainer's pokemon?

[mrgriffin]

There's probably a resource, but an alternative approach would be to build a vanilla Emerald, and use DebugPrintf to print out the missing information in CreateNPCTrainerParty—you could have a loop in AgbMain which simply calls CreateNPCTrainerParty for each trainer ID.

[AlexOn1ine]

There's probably a resource, but an alternative approach would be to build a vanilla Emerald, and use DebugPrintf to print out the missing information in CreateNPCTrainerParty—you could have a loop in AgbMain which simply calls CreateNPCTrainerParty for each trainer ID.

That's a good idea. I'll see what I can do.

[Bassoonian]

On second thought (and as discussed with some others on Discord):

• The fact that the names are used as hash in the first place means that PIDs/natures already vary from localisation to localisation, which trivialises the fact expansion deviates too
• Even without TrainerMonCustomized, PIDs and natures didn't match due to decapitalization
• Manually including the vanilla (English!) natures and genders etc would be a merge conflict hell for everyone who has touched Trainers for very little actual gain
• Pokémon already don't match their vanilla counterparts in terms of moves due to the update movesets
• Expansion already mentions several times that expansion as-is needs modification to be used as an actual game

In summary, I don't think this is as much of an issue in practice as it is made up to be and the hassle to "fix it" (when the vast majority of hacks will be changing Trainers anyway) isn't worth the merge conflict hassle for everyone downstream.

[mrgriffin]

That's fair.

This issue was motivated by #3545, which would set all the natures to a neutral one (Hardy as-written), because that's what it means to not specify a nature. Do we think that "not specified ⇒ neutral" is okay? I was keen to avoid implementing the hashing logic in the conversion script, which is why it only ports explicitly specified data; but I suppose an alternative option would be to generate a random nature (and gender) if one isn't explicitly specified.

If we think this isn't an issue then I'll probably resubmit #3545 for consideration when I'm back.

[AlexOn1ine]

I think setting the neutral nature as the default is reasonable and in my opinion preferable for people who don't want to mess with natures but still don't want a negative nature on a mon. Imo this could have been the default when trainer control was introduced.

[Bassoonian]

Fixed by #4172