Earlier today, I tried to remove the hardcode for Smeargle without
realizing what it was for (Smeargle gets Spore as an event move, which
shouldn't override its ability to Sketch Spore).
It turns out, 7 years ago when I added the Smeargle hardcode, I wrote
a TODO to fix it which I never got around to:
2e1ab04eb6
Anyway, this commit fixes it properly.
`checkLearnset` has been renamed `checkCanLearn`.
`reconcileLearnset` has been replaced with `validateMoves`, which
bundles the actual `checkCanLearn` calls with the old
`reconcileLearnset`, making for a better name.
It turns out `sentLogPos` isn't incremented in unit tests (no actual
use-case was affected). This is the simplest fix, although at some point
we should revisit how battles send and receive data.
* Split up dex-data over individual files
This commit introduces:
- `dex-abilities.ts`
- `dex-conditions.ts`
- `dex-formats.ts`
- `dex-items.ts`
- `dex-moves.ts`
- `dex-species.ts`
These files centralize definitions from `dex-data` and `global-types`.
* Inherit ItemData from Item etc
Previously, Condition inherited from ConditionData. Now, ConditionData
inherits from Condition. The advantage of the new approach is that now,
Condition and DataCondition no longer need to be separate types, and
there should be much less duplication of type definitions in general.
This has also been done for
- ItemData/Item/DataItem
- AbilityData/Ability/DataAbility
- FormatData/Format/DataFormat
Species and DataSpecies was already merged, but this also reverses
their inheritance (saving a lot of duplicated definitions in the
process!)
The only one left is MoveData, which is just super complicated and
will need its own commit.
It turns out 001f98b4f2 was wrong.
When urkerab asked why it `peek` wasn't awaited:
e91c4c5260 (commitcomment-41364837)
The answer was because clearing the buffer after peeking needed to
happen synchronous: if the buffer is written to after peeking but
before the buffer is cleared, that write is lost forever.
This just goes to show, if you do something subtle enough to require
type assertions, you should probably add a comment about what's going
on.
Fixes#7605
This also removes `BattleStream#start()` which is completely useless
API complication. A better implementation would properly forward
crashes between streams (maybe `pipeTo` should do this) but as it
stands, it's not doing anything.
The sim no longer uses any global variables, so this can and should be
removed.
`server/global-variables` has also been cleaned up, since I'm working
on this.
This syncs data/text to exactly match the client's data/text, in
preparation for them to be merged. The server's data/text will be
canonical, moving forward:
Relevant changes:
- A new file `data/text/default` has been added, for battle messages
not associated with a move/item/ability.
- As One (Glastrier) and As One (Spectrier) should not have a start
message; only As One itself.
- Hidden Power's `realMove` property does not belong in data/text
and has been removed (it's still in data/moves where it belongs).
Specifically, if you target your ally and your ally uses Ally Switch
(causing you to target yourself), certain moves (including Thunder,
Hurricane, and SSA) crash. This has been fixed.
Running `./pokemon-showdown simulate-battle` no longer catches/rethrows
errors. This should make debugging with something like VS Code's
built-in debugger much easier.
* Lint arrow-body-style
* Lint prefer-object-spread
Object spread is faster _and_ more readable.
This also fixes a few unnecessary object clones.
* Enable no-parameter-properties
This isn't currently used, but this makes clear that it shouldn't be.
* Refactor more Promises to async/await
* Remove unnecessary code from getDataMoveHTML etc
* Lint prefer-string-starts-ends-with
* Stop using no-undef
According to the typescript-eslint FAQ, this is redundant with
TypeScript, and they're not wrong. This will save us from needing to
specify globals in two different places which will be nice.
We're skipping two major typescript-eslint versions, so there are a
bunch of changes here, including:
- it's catching a lot of things it didn't catch in the past, for
reasons unclear to me
- no-unused-vars has to be explicitly disabled in global-types now
- a lot of `ts-ignore`s were never necessary and have been fixed
- Crashlogger can now handle being thrown things that aren't errors.
This has never been a problem in the past, but to satisfy TypeScript
we might as well not die in a fire on the off chance someone tries to
`throw null` or something.
simulate-battle now has a new option: --spectate.
As a recap:
-D --debug: simulate in debug mode
-R --replay: output only the omniscient (debug) replay, rather than the
full stream
-S --spectate: output only the spectator replay, rather than the full
stream
We deployed a new suspect test system a few months ago, but we had to
back out of it when it didn't work as expected. This fixes the bug that
made it not work. In the future, setting `rated: "[Gen 8] Format Name"`
in `formats.ts` will cause ladder games in that format to actually
go in the specified format's ladder, instead of its own.
(The name FormatsData is inconsistent with how we name other things;
especially since it's supposed to be data for a single format. It was
probably originally a typo. -Zarel)
This adds some new `data/text/` files, which are the new home for
item/move/ability descriptions, as well as in-game messages, in one
place for ease of translation.
`this.dex.deepClone` still exists as an alias to `Utils.deepClone` for
use in `data/`. I'll need to spend more time figuring out the correct
solution there.
Regular ReadStreams still can't; I now believe they shouldn't have a
"default" read method, and you should explicitly choose whether you
want to read "by chunks as they become available", "by chunks of a
specific line" or "by a delimiter".
So you would specifically use `stream.byLine()` or
`stream.byChunk([size])`, which would return an
`ObjectReadStream<string>`.
Inspired by #7195
`BattlePokedex` is now `Pokedex`, `BattleItems` is now `Items`, etc.
I also renamed `Movedex` to `Moves` and `Statuses` to `Conditions`.
`TypeChart` isn't `Types` yet, because unlike the others, it's not
indexed by ID. That should probably be fixed one day.
Previously, ending a read stream was `stream.push(null)`, and ending a
write stream was `stream.end()`. This was often confusing, and so now,
these are consistently `stream.pushEnd()` and `stream.writeEnd()`.
This refactor already found a bug in which `stream.end()` was used
where `stream.push(null)` should have been.
Also in this refactor: By default, `pushError` ends the stream. You can
pass `true` as the second parameter if the error is recoverable (the
stream shouldn't end).
ede3aaec correctly added restorative berries to the
staleness/termination calculation as per the adjusted specification
but failed to update the win condition.
Inverse Mod needs to go first, to calculate the negated effectiveness.
Disguise goes second, to suppress effectiveness.
Delta Stream goes third, to weaken moves super-effective against Flying types.
Tar Shot goes last, to make its victim weak to Fire type moves.
This allows the existing test for Delta Stream with Tar Shot to pass.
Additionally a new test for Delta Stream with Inverse Mod now passes.
A test for Flying Press with Inverse Mod is also included.
It's a judgment call that doesn't belong among the other factual
information in `moves.ts`.
The information is still around; in the client's `battle-dex-search.ts`
in `BattleMoveSearch#moveIsNotUseless`. But now it's all in one place.
fb18721e82
This check exists for maxMove.basePower but not zMove. Thank you
SadisticMystic for confirming that while "it has an entry of 1 in
the data table, [it]'s a meaningless entry which is impossible
to access"
This mostly serves to provide cleaner and more consistent field
naming. maxMove currently doesn't have boosts or effects to group
together but who knows what will be thrown at us via DLC, and being
symmetrical with zMoves is a nice.
- `Modded[Effect]Data` are now correctly defined: they must either have
`inherit: true` and be partial, or not have `inherit: true` and be a
complete `[Effect]Data` entry
- `id` is no longer allowed; instead, it's calculated directly from
`toID(name)`. The one exception, Hidden Power, gets a `realMove`
property to track this (it's still used to set `.id`, though;
TODO: really fix it properly).
- `num` is still required in `data/pokedex.ts` (dex number),
`data/moves.ts` (move index number, for Metronome), and
`data/items.ts` (minisprite sprite-sheet location). It's still not
required for mod-only items and moves.
- `num` is no longer allowed for PureEffects (in `statuses.ts`) where
it's always been meaningless.
- `color` and `heightm`, being completely flavor, are still not
required for `pokedex.ts` in mods. They're still required in the base
pokedex.
The whole `checkBattleForme` system, introduced in 4410843e5a
and currently used in Gen 6 and 7, seems to have always been bugged.
As TI mentions in #5970, we already use `getPokemonPool` for alternate
forme handling.
I've replaced it with a roll for cosmetic formes.
As confirmed by SadisticMystic, base power cannot be above 255, so
this move data is clearly incorrect and results in downstream users
of the data files such as the Smogon or PS dex or the tooltips
displaying misleading information.
Apparently our old hack was to prevent Unown formes completely, because
locked users would sometimes use them to spell bad words.
I don't like that approach, so now only locked users can't use Unown
formes.
The server now uses the same approach as the client of treating
cosmetic formes as real formes, as documented in `FORMES.md`.
This eliminates the need for the `.forme` and `.speciesid` properties
of `Pokemon`.
`pokemon.id` has also been removed: useful, since it turns out half
of its uses were bugs that should have used `pokemon.species.id`.
In addition, they should not activate for a spread move that ends the game.
(This contrasts with Soul-Heart, which will activate until the last target.)
PS wasn't correctly detecting `disabled: 'hidden'` moves as disabled
for the purposes of detecting Struggle activation. This has been fixed.
Thanks to DaWoblefet for unit tests!
Fixes#6620
BasicEffect always had a `status` property to support the pattern of
testing it to see if it's a move that sets status directly.
This is just a situation that TypeScript is bad at.
Another possibility would be to set `status: undefined` on PureEffect,
Ability, Item, and Species, but I think that's also ugly. Casting to
Move is probably the best approach, so that's what we do now.
For historical reasons, move property definitions have been very blurry
across `EffectData`. Fortunately, recent refactors have made it
possible to put them all where they're supposed to be.
The data/.ts refactor left a few .js files in, which weren't deleted
between 13 and 3 days ago:
c85f5bccb0
Because of this, the corresponding `.ts` files were considered new,
rather than renamed, and changes to the `.js` files got deleted rather
than merged.
Fortunately, this only affected `moves.js`, which has now been manually
re-merged with the changes.
Previously, format bans/unbans were processed in order of "most specific
to least specific".
So `+Giratina-Origin` trumps `-Giratina` trumps `+Uber` trumps
`-All Pokemon`.
And nonstandard reasons (`Unobtainable`, `Past`, `CAP`, etc) were
slotted between `Uber` and `All Pokemon`.
This has the unfortunate effect that if a base ruleset allows a certain
normally-banned category of Pokémon, you can't use `-All Pokemon` to
whitelist a list of Pokémon.
Moving nonstandard reasons to lowest precedence once again allows
`-All Pokemon` to be used as intended.
Custom Game is intended to be a "choose your own rules" format. It's usually pretty easy to tell if a
rule is being followed or not (a level 500 Pokémon is rather conspicuous) but EVs are generally
considered a secret.
For this reason, I've added a message for a discrepancy in EV use: one player adhering to the 510 EV
limit, and the other player not adhering to it.
This message is specific to Debug Mode (which Custom Game is, by default), which already leaks some
minor information, anyway.
To avoid information leaks completely, use Custom Rules:
https://github.com/smogon/pokemon-showdown/blob/master/config/CUSTOM-RULES.md
Frankenstein checkJs-Typescript with globals derps out and lets this
kind of thing slide, but without `hit` (which is only assigned the
line below) you can't cast to ActiveMove.
Apparently the teambuilder still defaults nicknames to full species
names, instead of base names as you might expect. This will have to be
fixed later...
Renames:
- .status -> .effect
- .statusData -> .state
- .thing -> .effectHolder
`thing` was always a really weird "I don't know what to call this"
variable name, but it's been renamed `effectHolder`, which should be
much clearer. `status` -> `effect` is I think the last remnant of old
PS code which called all effects "statuses". `statusData` -> `state`,
on the other hand, is the very first step in an initiative to calling
less things "data".
This allows OMs like Cross Evolution to use nicknames of other species
in `onValidateSet`.
Enforcement also now generates an error message, instead of silently
changing the name.
I also made it such that unbans are no longer inherited. This makes it simpler to follow Smogon's system of tier transitivity, and suspect tests without suspect ladders.
Pokemon that are not available in-game are now consolidated into `isNonstandard`. 'Past' indicates that the Pokemon no longer works in the current generation. 'Unobtainable' indicates that the Pokemon works but can only be obtained through hacking.
Moves that are not available in-game are now consolidated into `isNonstandard`. 'Past' indicates that the item no longer works in the current generation. 'Unobtainable' indicates that the item works but can only be obtained through hacking.
Items that are not available in-game are now consolidated into `isNonstandard`. 'Past' indicates that the item no longer works in the current generation. 'Unobtainable' indicates that the item works but can only be obtained through hacking.
Credit to @lusamine for testing many of these items in Sw/Sh.
This removes the redundant 'SwitchOut' code, and continues to ensure
that switching and dragging share as much code as possible, which
should help avoid bugs where they previously were different for no
reason.
Regression caused by: 47b55f96bc
`Battle#dragIn` used to run the `SwitchOut` event among other functions
but were accidentally left off in the refactor. This commit adds them back.
The main bug caused by this is Natural Cure not curing status aliments when
phased out by Roar.
dragIn and switchIn being two separate functions is a weird historical
quirk that leads to inconsistencies in implementation.
The only reason they need to be separated is a Mold Breaker quirk:
1da65efb12
(This is now done with an `if` statement.)
This should fix a lot of Roar/Whirlwind mechanics issues from `dragIn`
being on outdated mechanics compared to `switchIn`.
`activeTurns` was previously a horrible hack, used for "first full turn
only" effects like Speed Boost as well as "first move action" effects
like Fake Out.
In addition to being a huge hazard for API users such as OMs, this also
means weird bugs such as Speed Boost not working if you get hit by
Sky Drop on your first turn.
This commit fixes them by splitting these counters into two - an
`activeTurns` counter for Speed Boost, and an `activeMoveActions`
counter for Fake Out.
Move targets are now intended to be +1 +2 +3 for foes. The old syntax
of using 1 2 3 is still supported, but is not recommended.
(The old syntax will still be used in the old client, but the Preact
client will support the new syntax going forward.)
This makes the difference between move number and move target
clearer.
This also fixes the Conversion 2 ambiguity (although for backwards
compatibility, we do still need to special-case it).
Four years ago, Slayer95 added an error for invalid data files.
c1e452ea59 (diff-498e7bc80cf67d246be8aa3f2bbbb653R88)
To bypass the try/catch, he returned the error, and then checked for
the result being `instanceof Error`.
Three years ago, I refactored `tryRequire` to `loadDataFile` and moved
the `MODULE_NOT_FOUND` check from outside the function to inside it.
2604780ec5
But I didn't notice the `return new Error` pattern, so I didn't update
that to the modern pattern, so error checking has been broken ever
since.
"TS4053: Return type of public method from exported class has or is
using name 'Nature' from external module but cannot be named."
PS misses out on these errors because its TSC is dump in checkJS
mode, but if you ever try to emit with TSC it will type check things
properly.
- add support to the RandomPlayerAI for Dynamaxing
- add support to ExhaustiveRunner for Gigantamax Pokemon
- simplify range logic in RandomPlayerAPI
- handle crash from toID (needed by data/scripts.js) and Config
(needed if a Battle takes long enough to potentially allow for
requesting ties) globals not being defined
- mark Gen 8 formats as runnable by the runners
