smogon / pokemon-showdown

Pokémon battle simulator.
https://pokemonshowdown.com
MIT License
4.77k stars 2.79k forks source link

Sim-protocol does not support dynamax moves + documentation missing dynamaxing #6425

Closed jzyrobert closed 4 years ago

jzyrobert commented 4 years ago

Hi,

The simulator protocol does not support dynamax move ids when the active pokemon is dynamaxed. Rather, you have to use the regular move name or index (e.g. 1-4).

Example:

Additionally, the documentation does not mention that /choose move [id/index] dynamax is the protocol for dynamaxing and using a certain move.

I'm rather busy at the moment, but if someone could point me towards where in the point this logic lies, I could have a go at submitting a PR soon.

Zarel commented 4 years ago

https://github.com/smogon/pokemon-showdown/blob/505836e27ec4cf39d9c3b9147761097aa710a8e2/sim/side.ts#L360-L373

I would actually really appreciate this.

Also if you try /move maxguard with an undynamaxed Pokémon, if it would auto-Dynamax, that would be even cooler.

jzyrobert commented 4 years ago

Does this look right?

I've tested it manually and it works as it should, could also add some proper tests for it.

Zarel commented 4 years ago

Looks good. I have suggestions but they're easier to do directly on the PR.

Zarel commented 4 years ago

(My thoughts are mainly: 1. this loop can be skipped by testing if targetType is truthy at this point, 2. if there are multiple same-type Max moves, the one with the higher base power should be selected, 3. if this could also be implemented for Z moves, that would be amazing.)

Zarel commented 4 years ago

Actually, probably number 1 most important because GitHub's suggestion feature can't cover it: getMoveRequestMoves()'s return value should be saved from line 347. It's a pretty intense function that shouldn't be called repeatedly.

Zarel commented 4 years ago

I haven't heard back from you in two weeks, so I guess I'm fixing this myself.