architdate / PKHeX-Plugins

Plugins for PKHeX
MIT License
590 stars 132 forks source link

Not hyper training ATK or SPE when not specified 1 or 0 #119

Closed SteveCookTU closed 2 years ago

SteveCookTU commented 2 years ago

Describe the bug Using a showdown set, hyper training is not applied to ATK and SPE on egg encounters even when ATK and SPE are not specified as 0 or 1. First thought this was tied to nature but the provided set didn't have SPE hyper trained.

To Reproduce Steps to reproduce the behavior:

  1. Generate with showdown set.

Expected behavior Eggs without a naturally perfect ATK or SPE IV should be hyper trained if not specified 1 or 0 in the showdown set.

Additional context Was thinking of the logic behind this and I know the generation can't set the IVs to 0 or 1 itself but the wiki says it will only not hyper train them if they were originally specified 0 or 1. I'm wondering if the function checks the original set to see if 1 or 0 was passed in to then declare if it should be hyper trained or not.

Example showdown set:

Weavile @ TM36 Ability: Pressure Ball: Luxury Ball EVs: 252 Atk / 4 SpD / 252 Spe Jolly Nature - Throat Chop - Icicle Crash - Ice Shard - Fake Out

kwsch commented 2 years ago

https://github.com/architdate/PKHeX-Plugins/blob/e37523bf3a6009a25dccfaff1c5d850d879d02c9/PKHeX.Core.AutoMod/AutoMod/Legalization/SimpleEdits.cs#L421-L440

https://github.com/architdate/PKHeX-Plugins/blob/3dd826ca7792cf9aeb36e31e5692d66ed8b3684d/PKHeX.Core.AutoMod/AutoMod/APILegality.cs#L474-L504

I'm not sure why there are only 3 HT flags checked instead of all 6; @architdate would have to clarify.

SteveCookTU commented 2 years ago

Im waiting on confirmation to see if SPA is affected too. Only ATK and SPE have been noticed so far.

SteveCookTU commented 2 years ago

I believe those 3 are the only ones checked because of the common trend of having 0 or 1 in those stats for the sake of hidden power

SteveCookTU commented 2 years ago

Based on the order, by this point CurrentLevel is set to 100 already, the set has default IVs of 31 all around, and the pk's IVs have all been determined by PreSetPIDIVs.

The only thing that would then cause SPE to be off is the flag set by PKHeX. Because 5 of the IVs are max, no other flags are set and with a SPE IV of less than 17 the suggested flag will be false. This leads to the fix check to still come out false as well if the pk SPE IV is greater than 3. This would only explain the SPE though, the ATK shouldn't be affected based on this logic but still is in some instances.

What is the reason for the middle check here? pk.IVs[1] < 3 This pretty much makes it so it will only hyper train if the IVs generated are less than 3 when the set says they should be maxed.

SteveCookTU commented 2 years ago

Disregard attack, user didn't realize he set ATK IV to 0 in the set. If that just leaves speed then the previous comment is most likely the reason

architdate commented 2 years ago

The logic of only those 3 stats being checked is that when it comes to competitive pokemon, there is no reason ever to not have maxed out IVs on Def, SpDef and HP (except for the edge cases that I handle)

Regarding naturally generated 0/1 IVs not being hypertrained, I think the flaw lies in NeedsHyperTraining (it should be compared against IBattleTemplate instead of just the PKM incase the IV stat is <= 1) I'll push a fix for this soon

architdate commented 2 years ago

Okay correction to that: The issue isnt actually with NeedsHyperTraining since its never even used. The issue is that SetSuggestedHyperTrainingData within PKHeX core now has an updated logic for what HT data it applies. (It used to be more simplistic when I wrote my own hypertraining function). So I will adapt according to the change in that function

architdate commented 2 years ago

The issue stems from here: image

architdate commented 2 years ago

The latest commit should fix this issue. Do let me know if you find any edge cases my logic doesn't handle. Thanks :)