smogon / pokemon-showdown

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

use PRNG to roll Pokemon gender #4364

Closed yuzeh closed 6 years ago

yuzeh commented 6 years ago

Is there a reason that Math.random is being used on this line?

https://github.com/Zarel/Pokemon-Showdown/blob/99d682ca26fb184f10a788bf7c5406dad8e951af/sim/pokemon.js#L119

Zarel commented 6 years ago

No, this should definitely be changed to PRNG.

yuzeh commented 6 years ago

Cool! This is actually where the roll should happen, I believe:

https://github.com/Zarel/Pokemon-Showdown/blob/e8d8a6f/data/random-teams.js#L498

Zarel commented 6 years ago

We support random gender with customized teams. It's actually the default setting.

Zarel commented 6 years ago

image

See the "Random" setting?

yuzeh commented 6 years ago

Okay, sounds like this should happen at the first callsite then.

Zarel commented 6 years ago

It should be replaceable with this.battle.random()

yuzeh commented 6 years ago

While we're here, I think these calls should also be changed to use the PRNG:

https://github.com/Zarel/Pokemon-Showdown/blob/aae46ffd4669d47cbdf47798ab21120f21f6f98a/sim/side.js#L187

https://github.com/Zarel/Pokemon-Showdown/blob/aae46ffd4669d47cbdf47798ab21120f21f6f98a/sim/battle.js#L489

https://github.com/Zarel/Pokemon-Showdown/blob/aae46ffd4669d47cbdf47798ab21120f21f6f98a/sim/battle.js#L542

https://github.com/Zarel/Pokemon-Showdown/blob/aae46ffd4669d47cbdf47798ab21120f21f6f98a/sim/battle.js#L2417

https://github.com/Zarel/Pokemon-Showdown/blob/aae46ffd4669d47cbdf47798ab21120f21f6f98a/sim/battle.js#L2429

Does this sound right?

Zarel commented 6 years ago

Agreed, there should be no Math.random() in sim/.

yuzeh commented 6 years ago

Fixed in #4365