Instead of needing to support old protocol messages, we now introduce
BattleTextParser.upgradeArgs, which automatically upgrades them to new
protocol messages.
Fixes#1199
BattleTextParser.parseLine -> BattleTextParser.parseArgs
This makes it so Battle depends on BattleTextParser, rather than
BattleTextParser depending on Battle.
This is probably better, because someone is way more likely to want
BattleTextParser standalone, than to want Battle standalone.
...which I'm sure matters to literally no one but me, but whatever. I
think it makes more sense this way.
Previously, we used the `eval` hack to "import" our code into test
files. The biggest problem with that approach is that we don't get
line numbers.
Now, we're assigning relevant variables to globals in Node.js for
tests. It's ugly, but it works. There's no simple way to import local
variables only if we're in Node.
Ideally, we'd build this in two different ways: A .mjs file for Node,
and a .js file for the browser. Or maybe use UMD. I'll figure it out
later, I guess.
This spec file is, incidentally, Jest-compatible. I benchmarked and
Jest is noticeably slower right now, so I'll stick with Mocha, but
if we ever need Jest features, it'll be useful to be able to easily
switch.
Out of 12 issues found:
3 bugs:
- duplicate property - caught a bug in Gen 1 Light Screen
- duplicate property - caught a bug in Gen 1 Reflect
- unused variable - caught a bug in type animations
7 harmless but good for code quality:
- unused variable - harmless but good for code quality
- unused variable - harmless but good for code quality
- unused variable - harmless but good for code quality
- unused variable - harmless but good for code quality
- duplicate case - harmless but important for code quality
- unused variable - harmless but good for code quality
- unused variable - harmless but important for code quality
2 not-bugs that had to be worked around:
- unused variable - used for an `eval` trick, had to use a workaround
- unused variable - used for readable destructuring
I think on balance, LGTM does more good than bad. Catching bugs early
is worth some amount of hassle.
(Also like half these problems are problems tslint could catch if I
actually bothered to set it up...)
Mostly, this involves removing `BattleLog.escapeHTML` from `battle.ts`.
All previous use-cases have been replaced with something like
`Tools.sanitizeName`.
Technically, the dependency remains for `|controlshtml|` and
`|fieldhtml|`, but these will be dropped for BattleRoom/GameRoom
separation, to be done in the Preact rewrite.
This splits battle-dex.ts up into:
- `battle-dex.ts`
- dex data access, misc tools
- `battle-log.ts`
- manipulating HTML, especially in battle logs
This turned out to be a pretty significant portion of what was
previously battle-dex.
I would prefer if the test would still test if a mega gets an item,
which is why Charizard-Mega-Y was picked. It's unlikely to be banned,
but is powerful enough to stay in OU.
A safer choice would be an Arceus, but there is no code yet that
automatically gives a correct item to formes (may be worth fixing).
