pkmn / ps

Modular Pokémon Showdown
MIT License
103 stars 15 forks source link

Potential RandomPlayerAI bug in simulator #32

Closed cyi1341 closed 1 week ago

cyi1341 commented 1 week ago

Describe the bug:

Example in README of @pkmn/sim not working as expected when string omniscient is changed from p1 or p2 for the following:

void (async () => {
  for await (const chunk of streams.omniscient) {
    console.log(chunk);
  }
})();

Example:

p1:

import {Dex, BattleStreams, RandomPlayerAI, Teams} from '@pkmn/sim';
import {TeamGenerators} from '@pkmn/randoms';

Teams.setGeneratorFactory(TeamGenerators);

const streams = BattleStreams.getPlayerStreams(new BattleStreams.BattleStream());
const spec = {formatid: 'gen7customgame'};

const p1spec = {name: 'Bot 1', team: Teams.pack(Teams.generate('gen7randombattle'))};
const p2spec = {name: 'Bot 2', team: Teams.pack(Teams.generate('gen7randombattle'))};

const p1 = new RandomPlayerAI(streams.p1);
const p2 = new RandomPlayerAI(streams.p2);

void p1.start();
void p2.start();

void (async () => {
  for await (const chunk of streams.p1) {
    console.log(chunk);
  }
})();

void streams.omniscient.write(`>start ${JSON.stringify(spec)}
>player p1 ${JSON.stringify(p1spec)}
>player p2 ${JSON.stringify(p2spec)}`);

p2:

import {Dex, BattleStreams, RandomPlayerAI, Teams} from '@pkmn/sim';
import {TeamGenerators} from '@pkmn/randoms';

Teams.setGeneratorFactory(TeamGenerators);

const streams = BattleStreams.getPlayerStreams(new BattleStreams.BattleStream());
const spec = {formatid: 'gen7customgame'};

const p1spec = {name: 'Bot 1', team: Teams.pack(Teams.generate('gen7randombattle'))};
const p2spec = {name: 'Bot 2', team: Teams.pack(Teams.generate('gen7randombattle'))};

const p1 = new RandomPlayerAI(streams.p1);
const p2 = new RandomPlayerAI(streams.p2);

void p1.start();
void p2.start();

void (async () => {
  for await (const chunk of streams.p2) {
    console.log(chunk);
  }
})();

void streams.omniscient.write(`>start ${JSON.stringify(spec)}
>player p1 ${JSON.stringify(p1spec)}
>player p2 ${JSON.stringify(p2spec)}`);

With p1 it returns nothing, with p2 it returns the battle stream of p2, but does not include any |request| messages

Expected behavior:

Accurate and complete battle stream for both sides with |request| messages.

Additional context:

Most likely an issue originated from showdown, see https://github.com/smogon/pokemon-showdown/issues/10542

scheibo commented 1 week ago

You have a race because after your changes two places are trying to consume the same player stream at once (the respective RandomPlayerAI and your loop printing the stream). Print from within your player or create a wrapping stream that can tee it, eg. https://github.com/pkmn/ps/blob/05cac58a88c5259ab899090ddc0b1e04f07dbd17/integration/src/test/client.js#L148