search

smogon / pokemon-showdown-client

The client for Pokémon Showdown
http://pokemonshowdown.com
GNU Affero General Public License v3.0
562 stars 790 forks source link

fix redundant string slicing and explicit checks for protosynthesis and quarkdrive #2248

Closed Jerick120 closed 6 months ago

Jerick120 commented 6 months ago

PR: https://github.com/smogon/pokemon-showdown/pull/10300 needs to be merged along with this

Zarel commented 6 months ago

You say "fix" but this just changes the protocol.

I have two objections here:

Jerick120 commented 6 months ago

You say "fix" but this just changes the protocol.

I have two objections here:

  • When we change the protocol, the client needs to keep support for the old protocol. If we didn't, old replays would all be broken.
  • I don't see any reason to change the protocol. The old one's string slicing may be ugly but it works. Like, I understand, I like your syntax better, too, but I don't think it's so much better to be worth having a bunch of replays using one syntax and a bunch of replays using another.

Thats fair, however the logs are supposed to have a | delimiter for each value and every other stat changing ability does except these two and thats undocumented. The logs return protosynthesisdef while they should return protosynthesis|def.

Zarel commented 6 months ago

Hm. Yeah, I think that's a good point. Capitalization would also need to be fixed, and we'd need to update the volatile status display on the HP bar, but I think I could see that as being worth it.