No way to load a ModdedDex into battle simulator

[pkmn]

No way to load a ModdedDex into battle simulator

[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/[email protected] prepare
> npm run build


> @pkmn/[email protected] 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.