search

smogon / damage-calc

Pokemon games damage calculator
https://calc.pokemonshowdown.com
MIT License
396 stars 366 forks source link

correctHiddenPower can change hidden power type #650

Closed ctl2 closed 3 weeks ago

ctl2 commented 1 month ago

When assigning "default preset hidden power IVs", the correctHiddenPower function doesn't overwrite stats that aren't included in the HP ivs object. This can cause issues when these stats don't happen to have values of 31 in a set that's getting corrected.

I found the bug using the gen 3 set:

Salamence @ Leftovers
Ability: Intimidate
EVs: 180 Atk / 212 SpD / 116 Spe
Adamant Nature
IVs: 30 SpA / 30 SpD / 30 Spe
- Hidden Power [Flying]
- Rock Slide
- Earthquake
- Dragon Dance

Calling correctHiddenPower on the set results in IVs of 30 in all stats. The intended IVs are 30 in everything but speed, which should be 31, however the import's speed IV of 30 doesn't get overwritten because HP.Flying.ivs doesn't have a spe value. The next time correctHiddenPower is called, the function will set its hidden power type to Fighting.

This can be fixed by removing the pokemon.ivs || part from the following line: https://github.com/smogon/damage-calc/blob/611b95ebeac124df6c4dc3b499225d7d4833ea47/src/js/shared_controls.js#L979

Demo vid:

https://github.com/user-attachments/assets/259da711-13b9-4949-9b71-f417fa6cca43

Presumably the issue applies to DVs as well but I'm not familiar with pre gen 3 mechanics. Also I guess the issue only occurs in gens prior to 7 because of the condition at the start of the function. https://github.com/smogon/damage-calc/blob/611b95ebeac124df6c4dc3b499225d7d4833ea47/src/js/shared_controls.js#L956

thejetou commented 1 month ago

I don't understand how this is a bug. The import specifies 30 Speed IVs so why should it get overwritten to 31 IVs?

ctl2 commented 1 month ago

The code's written to overwrite IVs if a set's using Hidden Power. If it only partially overwrites then it can change Hidden Power's type.

I don't think that it should overwrite IVs at all, but the functionality must be there for a reason.

thejetou commented 4 weeks ago

The current method intent seems to 'optimize' the IVs, meaning if you have 26 Speed IVs but the Hidden Power allows 30 IVs it will change the Speed IVs to 30. I'm not sure why but it doesn't seem correct because some Pokemon will require minimal Speed IVs for Trick Room and the current implementation will always override it to 30 / 31 Speed IVs.

I think a better way to do would be:

maxed & matches -> OK maxed & !matches -> change IVs to PS default for the HP type !maxed & matches -> OK !maxed & !matches -> change HP type to match

Let me know if I'm missing something with the above please.

ctl2 commented 4 weeks ago

Yeah that's a good approach. I don't think you're missing anything

ctl2 commented 4 weeks ago

Btw there's a related bit of code that can max IVs & DVs on sets that don't have those values maxed. Figured I'd mention it since you pointed out that some sets want minimal IVs.

Reproduction steps:

  1. Go to the gen 3 damage calculator
  2. Select the Abra (LC Offensive Calm Mind) set
  3. Observe that the set has 0 attack IVs
  4. Select the Aerodactyl (OU Choice Band) set (or any set with a moveset including hidden power)
  5. Reselect the Abra set
  6. Observe that the attack IV has been changed to 31

edited to correct link to code

thejetou commented 3 weeks ago

Abra (LC Offensive Calm Mind) can't be overwritten by correctHiddenPower because it doesn't have a HP move so its IVs aren't modified there. It's probably a bug in some other area of the file. I'll try to track it down though, thanks.

ctl2 commented 3 weeks ago

apologies, gave the wrong code link. The issue's from https://github.com/smogon/damage-calc/blob/81f2e785da80b10d190c55cf6b3f94bcd0a4855e/src/js/shared_controls.js#L507