No way to load a ModdedDex into battle simulator

[B0sh]

Hey, I'm trying to use this library to do showdown battle simulations that included modded data, and as far as I can tell there's no way to actually start a simulator battle from a modded dex. After a lot of code searching, I believe I've narrowed it down to this difference in @pkmn/dex vs @pkmn/sim (bottom of issue). In the sim version of the Dex.mod function, it does not commit the new dex into the dexes constant, so when a new Battle() is created, it cannot find the newly made dex, and I get various errors. I do have a workaround that seems to work that involves just overwriting everything after the battle is made, but I feel like it should be enough to just added new mods to the list, or maybe passing in a ModdedDex somewhere on creation.

export const MyModDex = Dex.mod('modded' as ID, MyModData as ModData);
const formatid = 'moddedformatid';

const newBattle = new BattleStreams.BattleStream();
this.streams = BattleStreams.getPlayerStreams(newBattle);

const battleSpec = {
    format: MyModDex.formats.get(formatid as ID),
    formatid: formatid as ID
};
this.streams.omniscient.write(`>start ${JSON.stringify(battleSpec)}`);
this.battle = newBattle.battle;
//@ts-ignore
this.battle.format = MyModDex.formats.get(formatid as ID);
this.battle.dex = MyModDex;

this.streams.omniscient.write(`>player p1 ${JSON.stringify(p1)}`);
this.streams.omniscient.write(`>player p2 ${JSON.stringify(p2)}`);

I'd submit a PR to fix but I'm pretty scared about the whole way this system is designed with the whole automatic code merging and all, so I'm hoping I just overlooked something and there's already a way to do this that I missed. Thank you for making this repo!

@pkmn/dex - Overwrites the dexes constant

ps/dex/index.ts

Lines 1243 to 1246 in ab700ad

	mod(modid: GenID | T.ID, modData?: ModData) {
	if (modid in dexes) return modData ? new ModdedDex(modid, modData) : dexes[modid];
	return (dexes[modid] = new ModdedDex(modid as T.ID, modData));
	}

@pkmn/sim - Does not overwrite the dexes constant

ps/sim/sim/dex.ts

Lines 219 to 227 in ab700ad

	mod(mod: string | undefined, modData?: DeepPartial<ModdedDex['data']>): ModdedDex {
	if (!mod) return dexes['base'];
	const modid = toID(mod);
	if (modData?.Types && !modData.TypeChart) modData.TypeChart = modData.Types;
	if (modData?.Species && !modData.Pokedex) modData.Pokedex = modData.Species;
	const dex = (modid in dexes) && !modData ? dexes[modid] : new ModdedDex(modid);
	dex.loadData(modData);
	return dex;
	}

[scheibo]

Hi! Your example is missing some includes - is Dex in that example from @pkmn/sim? And do I understand correctly that @pkmn/dex works fine with respect to modding and @pkmn/sim doesn't? Sorry, its been a while since I added support for the modding code and since I don't use mods myself its probably not as well tested as I'd like.

Going to label this a bug under the assumption it probably is one.

[B0sh]

Yea the example is all included from @pkmn/sim. I didn't try modding the @pkmn/dex version but that doesn't seem like it'd work. I do get new pokemon/items etc to load in though if I overwrite the dex as shown in my workaround, so I think it's just an issue of creating the battle

import { Battle, BattleStreams, PokemonSet, RandomPlayerAI, Streams, Pokemon, Effect, Dex, ID, ModData } from "@pkmn/sim";

[scheibo]

Cool. So is the problem here just that @pkmn/sim'smod function doesnt assign back to dexes[modid]? Maybe it should be:

const dex = (modid in dexes) && !modData ? dexes[modid] : new ModdedDex(modid);
dex.loadData(modData);
+if (!(modid in dexes)) dexes[modid] = dex;
return dex;

I think it can't be golfed any further because I think the code is deliberately trying to avoid overwriting the existing dexes[modid] if new modData is provided for whatever reason.

This change passes the existing test suite so I pushed it - could you maybe confirm that it fixes the issue for you? I agree the automatic code merging crap is kind of scary and don't blame you for not wanting to touch it, but sim/dex.ts thankfully is not automatically generated so you should be able to just run npm install && npm run build and then require from the local ./sim package to see if it fixes things for you. If so it will be included in the next release (which should be soon after PS finishes DLC support)

[B0sh]

Awesome to hear! Unfortunately I couldn't build it, seems like you've git ignored tsconfig.cjs.json and its not in the repo for me...

PS C:\Code\ps\sim> npm i

> @pkmn/sim@0.7.50 prepare
> npm run build


> @pkmn/sim@0.7.50 build
> node ../build

Running tsc -p tsconfig.cjs.json for @pkmn/sim...
<ref *1> Error: spawnSync npx ENOENT
    at Object.spawnSync (node:internal/child_process:1124:20)
    at spawnSync (node:child_process:873:24)
    at Object.execFileSync (node:child_process:916:15)
    at sh (C:\Code\ps\build:31:41)
    at C:\Code\ps\build:65:7
    at Object.<anonymous> (C:\Code\ps\build:174:3)
    at Module._compile (node:internal/modules/cjs/loader:1233:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1287:10)
    at Module.load (node:internal/modules/cjs/loader:1091:32)
    at Module._load (node:internal/modules/cjs/loader:938:12) {
  errno: -4058,
  code: 'ENOENT',
  syscall: 'spawnSync npx',
  path: 'npx',
  spawnargs: [ 'tsc', '-p', 'tsconfig.cjs.json' ],
  error: [Circular *1],
  status: null,
  signal: null,
  output: null,
  pid: 0,
  stdout: null,
  stderr: null
}
npm ERR! code 1
npm ERR! path C:\Code\ps\sim
npm ERR! command failed
npm ERR! command C:\Windows\system32\cmd.exe /d /s /c npm run build

[scheibo]

tsconfig.cjs.json is generated on the fly (huge hack, simultaneous ESM/CJS compat is a fucking nightmare), implying that the build script is broken for you. Presumably thats some sort of Windows only thing, I don't run the CI for windows so perhaps thats understandable. Oh well. Thanks for trying! Guess you can just test it when the package is release in a day or two :)

[scheibo]

Please try v0.7.51

[B0sh]

It's like half working, I pulled the repo on my macbook instead (my desktop is sad...) and wrote up some tests for this in the PR. The format is loading correctly when passed as a direct object, and it pulls the correct dex object for the battle, but when passing the "formatid" it doesn't work. This is an issue for the real code because when using BattleStreams it has to get turned into JSON first (lol but I understand why). We're adding custom hook functions to the format so that won't get converted of course. The workaround continues to work just fine after the changes too btw

I stopped short of trying to actually making a fix, but it seems like there's some caching for formats going in Dex.formats.get() so a new dex would have to retrigger an update there.

[scheibo]

OK yeah theres seems to be a bunch of things going on here, thanks for the tests and patience.

• forFormat calls this.formats.get(format).mod where this.formats the root global Dex object which has no idea about the mods you just made. As you suggested, you could maybe add code to Dex.mod which adds in new formats to the root Dex.formats.{formatsListCache,rulesetCache}, but... we don't actually have formats, we have rulesets. PS only defined formats on the singular config/formats.ts, when you mod the Dex you are providing alternative rulesets. I think we would maybe need to:
  1. change the signature of mod to like mod(mod: string | undefined, modData?: DeepPartial<ModdedDex['data']>, formats?: FormatList): ModdedDex
  2. add maybe an extend function to DexFormats which does something similar to DexFormats.load (builds up formats and mutates the existing formatsListCache & rulesetCache) that we could then call within Dex.mod if formats is provided.
• you might be better served not using the BattleStream abstraction at all, just new up a Battle directly (and we can add a param to Battle to let it work with the correct Dex as opposed to the global one). BattleStream is kind of a racy buggy mess that depending on your use case is possibly going to lead to pain.

I don't have a lot of bandwidth atm to be able to try to support either of these, but would be happy to merge a PR for one of them. Adding a param to Battle requires touching import because its not forked so theres some magic (read: hacks) involved so I'd probably handle that, but dex.ts and dex-formats.ts are forked and so just editing them normally should work if you wanted to try taking a stab at it.