Dex*: Consistent field init order in constructors

[larry-the-table-guy]

Spinoff of #10549

---

Rather simple change to get consistent field init order where possible.

I realized at the end that there's a very simple set of rules for making sure the ctors have the same behavior as they did with Object.assign():

1. Don't reference this.field before it's been initialized
   it doesn't exist.
2. Don't reference data.field after this.field has been initialized
   you'll miss out on the default values and other fix-up logic.
3. Copy extra fields at the end

And... that's it.

---

Results

Memory

Heap snapshots, analyzed w/ Chrome's devtools.

Going by 'Constructor' and 'Alloc. Size' columns.

All mods

Type	Before	After
Species	27059K	25986K
(system)	26360K	13444K
DataMove	17737K	16312K
Item	6220K	6085K
Ability	2893K	2606K
Format	242K	129K
Nature	155K	155K
TypeInfo	68K	68K

According to Chrome's UI, total memory went from 279 to 260 MB.

The point of consistent field ordering was to greatly reduce the number of hidden classes.
The 13M decrease in the (system) category indicates this worked.
Ta-da.

Just Dex

Type	Before	After
(system)	3490K	1836K
DataMove	1473K	731K
Species	1460K	705K
Item	382K	240K
Ability	326K	188K
Format	242K	129K
Nature	4200	4200
TypeInfo	2736	1824

According to Chrome's UI, total memory went from 53.6 to 50.0 MB.

Times

My test runs were taken on a local branch with #10616 merged in.

Action	Before	After	Unit
npm test	11961	11591	ticks
npm run full-test	140903	132434	ticks
Dex.loadData	246	248	ms
Dex.*.all()	120	67	ms

Where '*' is [moves, items, species, abilities, types, natures]
Dex also loads Formats as part of loadData.

About 8 seconds saved on npm run full-test.

Benchmark code

ticks for npm commands (./package.json)

mocha --v8-prof ...
# Note: enabling prof adds a few % overhead

milliseconds for Dex loading

const start0 = performance.now();
Dex.loadData();
const start1 = performance.now();
Dex.moves.all();
Dex.items.all();
Dex.species.all();
Dex.abilities.all();
Dex.types.all();
Dex.natures.all();
const end = performance.now();
console.log('loadData', start1 - start0);
console.log('.alls', end - start1);

Heap snapshots

const v8 = require('v8');
v8.writeHeapSnapshot(); // before
Dex.loadData();
Dex.moves.all();
Dex.items.all();
Dex.species.all();
Dex.abilities.all();
Dex.types.all();
Dex.natures.all();
v8.writeHeapSnapshot(); // after

---

Covers all relevant classes.

$ grep -Ern "extends BasicEffect" ./sim/ ./server/ ./tools/ ./test/ ./config/ ./data ./lib/
./sim/dex-items.ts:23:export class Item extends BasicEffect implements Readonly<BasicEffect> {
./sim/dex-abilities.ts:31:export class Ability extends BasicEffect implements Readonly<BasicEffect> {
./sim/dex-moves.ts:359:export class DataMove extends BasicEffect implements Readonly<BasicEffect & MoveData> {
./sim/dex-conditions.ts:613:export class Condition extends BasicEffect implements
./sim/dex-species.ts:87:export class Species extends BasicEffect implements Readonly<BasicEffect & SpeciesFormatsData> {
./sim/dex-formats.ts:388:export class Format extends BasicEffect implements Readonly<BasicEffect> {
./sim/dex-data.ts:150:export class Nature extends BasicEffect implements Readonly<BasicEffect & NatureData> {

---

old todo

TODO:

Simple testing indicates that Formats has a lot of extra fields.
Makes sense when you check the class definition - it has many optional properties.
In that scenario, one could try sorting the keys before iterating over them.
You'd want to measure whether sorting the keys meaningfully reduces the number of unique sequences.

I've tested sorting the extra keys and it did not improve any of the above metrics. So, I won't do that.

[urkerab]

It looks as if BasicEffect should be an abstract class, so that it will never have to worry about copying other fields itself.

Do you happen to know how many other fields there actually are?

[larry-the-table-guy]

(I will get back to you on this, I'm just working on making the test suite run faster)

[larry-the-table-guy]

Yeah, abstract class makes sense here and would simplify it.

Documented # of other fields in later comment in PR.

[larry-the-table-guy]

@urkerab Here's some stats on extra fields.
I'll look at what's loaded during npm run full-test next - I imagine that will exercise Formats and Conditions more/differently.

---

code

Inserted at assignNewFields

export const extra_fields = new Map();
export function assignNewFields(self: AnyObject, data: AnyObject) {
        for (const k in data) {
                // to behave like Object.assign, skip data's inherited properties
                if (!Object.prototype.hasOwnProperty.call(data, k)) continue;
                if (Object.prototype.hasOwnProperty.call(self, k)) continue;
                extra_fields.set(k, (extra_fields.get(k) || 0) + 1);
                self[k] = data[k];
        }
}

Inserted at the bottom of sim/dex.ts

Dex.includeMods();
Dex.formats.all();
console.log('start', Dex.formats.all().length, Data.extra_fields);
Data.extra_fields.clear();

let sum;

sum = 0;
for (const d in dexes) sum += dexes[d].species.all().length;
console.log('species', sum, Data.extra_fields);
Data.extra_fields.clear();

sum = 0;
for (const d in dexes) sum += dexes[d].moves.all().length;
console.log('moves', sum, Data.extra_fields);
Data.extra_fields.clear();

sum = 0;
for (const d in dexes) sum += dexes[d].items.all().length;
console.log('items', sum, Data.extra_fields);
Data.extra_fields.clear();

sum = 0;
for (const d in dexes) sum += dexes[d].abilities.all().length;
console.log('abilities', sum, Data.extra_fields);
Data.extra_fields.clear();

sum = 0;
for (const d in dexes) sum += dexes[d].natures.all().length;
console.log('natures', sum, Data.extra_fields);
Data.extra_fields.clear();

sum = 0;
for (const d in dexes) sum += dexes[d].types.all().length;
console.log('types', sum, Data.extra_fields);
Data.extra_fields.clear();

---

Format:

{label} {# objects} Map(# keys) { 
  'key': freq
}

Start (I think this is Dex.formats)

start 274 Map(40) {
  'team' => 43,
  'section' => 274,
  'column' => 274,
  'challengeShow' => 274,
  'searchShow' => 274,
  'tournamentShow' => 274,
  'bestOfDefault' => 274,
  'teraPreviewDefault' => 274,
  'battle' => 18,
  'onSwitchIn' => 13,
  'getEvoFamily' => 1,
  'validateSet' => 5,
  'onValidateTeam' => 7,
  'onSwitchOut' => 3,
  'onBeforeSwitchIn' => 6,
  'onFaint' => 2,
  'onValidateRule' => 5,
  'getSharedPower' => 1,
  'onSwitchInPriority' => 6,
  'checkCanLearn' => 1,
  'onModifySpecies' => 2,
  'onValidateSet' => 5,
  'onModifyMove' => 5,
  'onModifyMovePriority' => 3,
  'onModifyPriority' => 1,
  'onModifyTypePriority' => 1,
  'onModifyType' => 2,
  'onHitPriority' => 1,
  'onHit' => 1,
  'onAfterSubDamage' => 1,
  'onModifySecondaries' => 1,
  'onAfterMoveSecondaryPriority' => 1,
  'onAfterMoveSecondarySelf' => 1,
  'onBasePowerPriority' => 1,
  'onBasePower' => 1,
  'pokemon' => 2,
  'onAfterMega' => 2,
  'getSharedItems' => 1,
  'onPrepareHit' => 1,
  'actions' => 1
}

Species

species 55427 Map(93) {
  'evoCondition' => 2046,
  'evoItem' => 3420,
  'evoRegion' => 190,
  'mother' => 114,
  'requiredAbility' => 684,
  'requiredMove' => 114,
  'forceTeraType' => 418,
  'onModifyAccuracyPriority' => 3,
  'onModifyAccuracy' => 3,
  'onBasePowerPriority' => 11,
  'onBasePower' => 11,
  'onDamage' => 7,
  'onSetStatus' => 7,
  'onAnyTryMove' => 1,
  'onAnyDamage' => 1,
  'onTryBoost' => 10,
  'onTryAddVolatile' => 5,
  'onTryHit' => 14,
  'onDamagePriority' => 2,
  'onSourceModifyDamage' => 2,
  'onModifyMove' => 9,
  'onModifyMovePriority' => 3,
  'onModifyAtkPriority' => 3,
  'onModifyAtk' => 3,
  'onModifySpe' => 5,
  'onFlinch' => 1,
  'onUpdate' => 8,
  'onStart' => 15,
  'onDamagingHit' => 11,
  'onImmunity' => 7,
  'onModifyCritRatio' => 1,
  'onSourceDamagingHit' => 1,
  'onModifySTAB' => 1,
  'onSourceAfterFaint' => 1,
  'onAfterEachBoost' => 1,
  'onModifySpA' => 3,
  'onResidualOrder' => 2,
  'onResidualSubOrder' => 2,
  'onResidual' => 2,
  'onFoeTrapPokemon' => 2,
  'onFoeMaybeTrapPokemon' => 2,
  'onCheckShow' => 1,
  'onSwitchOut' => 2,
  'onModifyTypePriority' => 1,
  'onModifyType' => 1,
  'onModifyDamage' => 2,
  'onCriticalHit' => 2,
  'onAllyTryHitSide' => 2,
  'onTryHitPriority' => 2,
  'onSourceModifyAccuracyPriority' => 2,
  'onSourceModifyAccuracy' => 2,
  'onSourceModifyAtk' => 1,
  'onSourceModifySpA' => 1,
  'onSourceBasePowerPriority' => 3,
  'onSourceBasePower' => 3,
  'onSourceModifyAtkPriority' => 1,
  'onSourceModifySpAPriority' => 1,
  'onFoeRedirectTarget' => 1,
  'condition' => 2,
  'onAfterUseItem' => 1,
  'onTakeItem' => 2,
  'onEnd' => 6,
  'onWeather' => 5,
  'onSwitchIn' => 1,
  'onAfterSubDamage' => 1,
  'onHit' => 2,
  'onAnyInvulnerabilityPriority' => 1,
  'onAnyInvulnerability' => 1,
  'onAnyAccuracy' => 1,
  'onSourceTryHeal' => 1,
  'onAllySwitchIn' => 1,
  'onAllySetStatus' => 1,
  'onPreStart' => 2,
  'onDeductPP' => 1,
  'onModifySpAPriority' => 1,
  'onFoeTryMove' => 2,
  'onFractionalPriority' => 1,
  'onModifyBoost' => 1,
  'onAnyModifyBoost' => 1,
  'onAnyRedirectTarget' => 1,
  'onDamagingHitOrder' => 2,
  'onAfterSetStatus' => 1,
  'onFoeTryEatItem' => 1,
  'onEatItem' => 1,
  'onDragOutPriority' => 2,
  'onDragOut' => 2,
  'onModifySecondaries' => 1,
  'onModifyPriority' => 2,
  'onModifyWeightPriority' => 1,
  'onModifyWeight' => 1,
  'onAfterMoveSecondary' => 1,
  'onAllyTryAddVolatile' => 1,
  'inherit' => 3
}

Moves

moves 37924 Map(63) {
  'contestType' => 30876,
  'drain' => 496,
  'boosts' => 2744,
  'zMove' => 10732,
  'basePowerCallback' => 2059,
  'onHit' => 4097,
  'onPrepareHit' => 971,
  'condition' => 4708,
  'self' => 3392,
  'multihit' => 1192,
  'callsMove' => 266,
  'onTryImmunity' => 380,
  'onTry' => 1602,
  'onModifyType' => 503,
  'sideCondition' => 558,
  'onTryHit' => 1820,
  'hasCrashDamage' => 153,
  'onMoveFail' => 210,
  'stallingMove' => 445,
  'onBasePower' => 718,
  'priorityChargeCallback' => 115,
  'onAfterMove' => 438,
  'onModifyMove' => 1098,
  'onDisableMove' => 78,
  'beforeMoveCallback' => 77,
  'maxMove' => 1900,
  'onTryMove' => 910,
  'recoil' => 465,
  'onAfterHit' => 310,
  'onAfterSubDamage' => 421,
  'ignoreEvasion' => 114,
  'forceSwitch' => 153,
  'selfBoost' => 114,
  'damageCallback' => 419,
  'onModifyTarget' => 76,
  'beforeTurnCallback' => 126,
  'onHitField' => 231,
  'smartTarget' => 38,
  'terrain' => 153,
  'selfdestruct' => 267,
  'pseudoWeather' => 345,
  'onDamagePriority' => 77,
  'onDamage' => 77,
  'breaksProtect' => 202,
  'onAfterMoveSecondarySelf' => 118,
  'ohko' => 152,
  'willCrit' => 248,
  'onEffectiveness' => 119,
  'onHitSide' => 152,
  'onModifyPriority' => 46,
  'slotCondition' => 165,
  'heal' => 278,
  'realMove' => 608,
  'thawsTarget' => 190,
  'onUseMoveMessage' => 38,
  'mindBlownRecoil' => 76,
  'multiaccuracy' => 116,
  'sleepUsable' => 77,
  'tracksTarget' => 38,
  'stealsBoosts' => 41,
  'struggleRecoil' => 38,
  'noDamageVariance' => 12,
  'ignoreAccuracy' => 1
}

Items

items 20949 Map(79) {
  'spritenum' => 20412,
  'onSetAbility' => 38,
  'onTakeItem' => 5121,
  'onDamagingHit' => 342,
  'boosts' => 494,
  'onBasePowerPriority' => 2052,
  'onBasePower' => 2150,
  'forcedForme' => 2379,
  'onAfterBoost' => 76,
  'naturalGift' => 2919,
  'onUpdate' => 1368,
  'onTryEatItem' => 373,
  'onEat' => 2919,
  'onStart' => 456,
  'onAfterSubDamage' => 38,
  'onModifySpDPriority' => 129,
  'onModifySpD' => 136,
  'onDisableMove' => 38,
  'onSourceModifyDamage' => 684,
  'onTryHealPriority' => 38,
  'onTryHeal' => 38,
  'onResidualOrder' => 372,
  'onResidualSubOrder' => 296,
  'onResidual' => 410,
  'onSwitchIn' => 76,
  'onPrimal' => 76,
  'onModifyAccuracyPriority' => 76,
  'onModifyAccuracy' => 76,
  'onSourceTryPrimaryHit' => 684,
  'onModifyMove' => 266,
  'onModifyAtkPriority' => 177,
  'onModifyAtk' => 177,
  'isChoice' => 114,
  'onModifySpe' => 381,
  'onModifySpAPriority' => 192,
  'onModifySpA' => 193,
  'onTryBoost' => 38,
  'onModifySecondaries' => 38,
  'onFractionalPriorityPriority' => 76,
  'onFractionalPriority' => 152,
  'onAttractPriority' => 38,
  'onAttract' => 38,
  'onAfterMoveSecondaryPriority' => 38,
  'onAfterMoveSecondary' => 152,
  'onTerrainChange' => 152,
  'onHit' => 69,
  'onModifyDefPriority' => 76,
  'onModifyDef' => 77,
  'onModifyDamage' => 185,
  'onModifyWeight' => 38,
  'onDamagePriority' => 76,
  'onDamage' => 77,
  'onEffectiveness' => 38,
  'onModifyMovePriority' => 114,
  'onModifyCritRatio' => 190,
  'onAfterMoveSecondarySelf' => 116,
  'onAfterSetStatusPriority' => 38,
  'onAfterSetStatus' => 38,
  'condition' => 94,
  'onFoeAfterBoost' => 38,
  'onChargeMove' => 38,
  'onNegateImmunity' => 38,
  'onDamagingHitOrder' => 38,
  'onAnyPseudoWeatherChange' => 38,
  'onImmunity' => 38,
  'onTryHit' => 47,
  'onTrapPokemonPriority' => 38,
  'onTrapPokemon' => 38,
  'onAfterMoveSecondarySelfPriority' => 38,
  'onEnd' => 38,
  'onSourceModifyAccuracyPriority' => 76,
  'onSourceModifyAccuracy' => 76,
  'onModifyCritRatioPriority' => 12,
  'onAfterMove' => 27,
  'onBeforeTurn' => 9,
  'onCustap' => 9,
  'onModifyDamagePhase2' => 9,
  'inherit' => 1,
  'onFoeBasePower' => 1
}

Abilities

abilities 12375 Map(152) {
  'onModifySTAB' => 37,
  'onModifyTypePriority' => 232,
  'onModifyType' => 231,
  'onBasePowerPriority' => 801,
  'onBasePower' => 802,
  'onDamagingHitOrder' => 227,
  'onDamagingHit' => 1184,
  'onSwitchIn' => 386,
  'onStart' => 2502,
  'onEnd' => 723,
  'onHit' => 75,
  'onDamage' => 439,
  'onTryEatItem' => 114,
  'onAfterMoveSecondary' => 151,
  'onFoeTrapPokemon' => 114,
  'onFoeMaybeTrapPokemon' => 114,
  'onFoeTryMove' => 113,
  'onAllyTryAddVolatile' => 113,
  'onPreStart' => 188,
  'onFoeTryEatItem' => 113,
  'onSourceAfterFaint' => 267,
  'onAnyTryPrimaryHit' => 38,
  'onResidualOrder' => 495,
  'onResidualSubOrder' => 295,
  'onResidual' => 504,
  'onAllyBasePowerPriority' => 114,
  'onAllyBasePower' => 114,
  'onCriticalHit' => 151,
  'onAnyModifySpD' => 38,
  'onTryBoost' => 524,
  'onModifyAtkPriority' => 574,
  'onModifyAtk' => 611,
  'onModifySpAPriority' => 481,
  'onModifySpA' => 536,
  'onTryHit' => 833,
  'onEatItem' => 114,
  'onModifySpe' => 227,
  'onSetStatus' => 460,
  'onUpdate' => 567,
  'onAfterEachBoost' => 75,
  'onSourceModifyAccuracyPriority' => 77,
  'onSourceModifyAccuracy' => 77,
  'onChangeBoost' => 105,
  'condition' => 389,
  'onAnyTryMove' => 37,
  'onAnyDamage' => 37,
  'onAnyBasePowerPriority' => 77,
  'onAnyBasePower' => 77,
  'onAnySetWeather' => 117,
  'onDamagePriority' => 153,
  'onEffectiveness' => 115,
  'onSourceBasePowerPriority' => 67,
  'onSourceBasePower' => 70,
  'onWeather' => 156,
  'onEmergencyExit' => 76,
  'onSourceModifyDamage' => 349,
  'onWeatherChange' => 152,
  'onAllyModifyAtkPriority' => 38,
  'onAllyModifyAtk' => 38,
  'onAllyModifySpDPriority' => 38,
  'onAllyModifySpD' => 38,
  'onAllyTryBoost' => 38,
  'onAllySetStatus' => 113,
  'onAnyModifyDamage' => 38,
  'onModifyDefPriority' => 117,
  'onModifyDef' => 118,
  'onModifyPriority' => 117,
  'onBeforeMove' => 93,
  'onModifyMove' => 762,
  'onDisableMove' => 38,
  'onDragOutPriority' => 74,
  'onDragOut' => 74,
  'onSourceTryPrimaryHit' => 38,
  'onSourceModifyAtkPriority' => 145,
  'onSourceModifyAtk' => 145,
  'onSourceModifySpAPriority' => 145,
  'onSourceModifySpA' => 145,
  'onModifyWeightPriority' => 38,
  'onModifyWeight' => 76,
  'onImmunity' => 302,
  'onBeforeSwitchIn' => 38,
  'onFaint' => 39,
  'onTryAddVolatile' => 261,
  'onPrepareHit' => 116,
  'onAnyRedirectTarget' => 69,
  'onSourceTryHeal' => 37,
  'onTryHitPriority' => 152,
  'onAllyTryHitSide' => 151,
  'onAfterMoveSecondarySelf' => 39,
  'onModifyCritRatio' => 77,
  'onTerrainChange' => 76,
  'onModifyMovePriority' => 299,
  'onFractionalPriorityPriority' => 76,
  'onFractionalPriority' => 115,
  'onCheckShow' => 37,
  'onSwitchOut' => 116,
  'onModifyDamage' => 113,
  'onAnyInvulnerabilityPriority' => 39,
  'onAnyInvulnerability' => 39,
  'onAnyAccuracy' => 39,
  'onFoeAfterBoost' => 39,
  'onSourceModifySecondaries' => 39,
  'onAllySwitchIn' => 37,
  'onAnyAfterSetStatus' => 38,
  'onSourceDamagingHit' => 75,
  'onAllyFaint' => 76,
  'onDeductPP' => 37,
  'onAfterBoost' => 19,
  'onTryHeal' => 38,
  'onSourceModifyDamagePriority' => 38,
  'onTryEatItemPriority' => 38,
  'onModifyAccuracyPriority' => 149,
  'onModifyAccuracy' => 149,
  'onModifySecondaries' => 38,
  'onAnyFaintPriority' => 38,
  'onAnyFaint' => 38,
  'onFlinch' => 37,
  'onTakeItem' => 74,
  'onAllyAfterUseItem' => 38,
  'onAfterSetStatus' => 37,
  'onAnyModifyDef' => 39,
  'onAnyModifyAtk' => 39,
  'onAfterTerastallization' => 38,
  'onAnyAfterMove' => 38,
  'onBeforeMovePriority' => 55,
  'onAnyModifyBoost' => 40,
  'onAfterUseItem' => 37,
  'onAnyModifySpA' => 38,
  'onAnyModifyAccuracyPriority' => 38,
  'onAnyModifyAccuracy' => 38,
  'onAllySideConditionStart' => 76,
  'onFoeRedirectTarget' => 6,
  'onAfterSubDamage' => 8,
  'onModifyBoost' => 8,
  'onFoeBeforeMovePriority' => 1,
  'onFoeBeforeMove' => 3,
  'inherit' => 1,
  'onModifySpDPriority' => 3,
  'onModifySpD' => 4,
  'onAnyModifySpe' => 1,
  'onBeforeSwitchOutPriority' => 1,
  'onBeforeSwitchOut' => 1,
  'onSourceHit' => 1,
  'onAnyDeductPP' => 1,
  'onAnyModifyType' => 1,
  'onAfterMove' => 3,
  'onFoeBasePower' => 2,
  'onBeforeTurn' => 1,
  'onSourceAccuracy' => 1,
  'onAllyModifyMove' => 1,
  'onFoeDisableMove' => 1,
  'onResidualPriority' => 1
}

Natures

natures 975 Map(0) {}

Types

types 741 Map(0) {}

None of my dedup code is in, so those object counts should reflect* how many times the constructor was called from that type.

*Edit: Nope those are slightly inflated (by 1 mod's worth) because 'base' and 'gen9' are both in dexes.

I will look at the impact of sorting next
(Edit: conclusion is at the bottom of the top comment)

[larry-the-table-guy]

What's weird is that some of those keys don't appear in the corresponding ./data files for the type. Like onDeductPP happening during Species, yet it only appears in abilities data files.

I don't know why that happens.

[larry-the-table-guy]

Came up with a much better way to analyze the # of object shapes.

---

Old state (master)

3324 total shapes, 3030 of which could be merged (by re-ordering keys) into a set of 429. So, 2601 / 3324 shapes were avoidable.

# actual shapes per effectType Map(7) {
  'Format' => 80,
  'Pokemon' => 1265,
  'Move' => 1148,
  'Item' => 272,
  'Ability' => 551,
  'Type' => 6,
  'Nature' => 2
}
min possible # shapes per effectType Map(7) {
  'Format' => 24,
  'Pokemon' => 21,
  'Move' => 326,
  'Item' => 93,
  'Ability' => 257,
  'Type' => 1,
  'Nature' => 1
}

---

current PR state

Among the dex* objects, there are 937 object shapes, 379 of which could be reduced into a set of 165. So, 214 / 937 shapes are avoidable.

Breakdown by type:

# total object shapes: 937
# actual shapes per effectType Map(7) {
  'Format' => 38,
  'Pokemon' => 23,
  'Move' => 418,
  'Item' => 153,
  'Ability' => 303,
  'Type' => 1,
  'Nature' => 1
}
min possible # shapes per effectType Map(7) {
  'Format' => 24,
  'Pokemon' => 21,
  'Move' => 326,
  'Item' => 93,
  'Ability' => 257,
  'Type' => 1,
  'Nature' => 1
}

With sorting

741 shapes, 33 of which could be reduced into 15.

Those remaining extra shapes seem to come from the position of zMove and maxMove in Move, and onTypePriority & onType in Pokemon. Since sorting didn't remove those, there must be code paths in the constructors that don't init those fields in the same order as each other.

# actual shapes per effectType Map(7) {
  'Format' => 24,
  'Pokemon' => 22,
  'Move' => 343,
  'Item' => 93,
  'Ability' => 257,
  'Type' => 1,
  'Nature' => 1
}
min possible # shapes per effectType Map(7) {
  'Format' => 24,
  'Pokemon' => 21,
  'Move' => 326,
  'Item' => 93,
  'Ability' => 257,
  'Type' => 1,
  'Nature' => 1
}

---

Conclusion: will be put in a later comment

---

setup code

(bottom of dex.ts)

Dex.includeMods();
let obj_list = [...Dex.formats.all()];
for (const d in dexes) {
        if (d === 'base') continue;
        const m = dexes[d];
        // slow but w/e
        obj_list = obj_list.concat(
                m.species.all(),
                m.moves.all(),
                m.items.all(),
                m.abilities.all(),
                m.types.all(),
                m.natures.all()
        );
}
const key_seqs = new Map();
for (const obj of obj_list) {
        const sorted = obj.effectType + '|' + Object.keys(obj).sort().join(';');
        const keys = Object.keys(obj).join(';');
        let bucket = key_seqs.get(sorted);
        if (!bucket) key_seqs.set(sorted, bucket = new Map());
        bucket.set(keys, (bucket.get(keys) || 0) + 1);
}

let num_total_shapes = 0;
for (const v of key_seqs.values()) num_total_shapes += v.size;
console.log("# total object shapes:", num_total_shapes);

const num_act_shapes_per_effectType = new Map();
const num_min_shapes_per_effectType = new Map();
for (const [k, v] of key_seqs.entries()) {
        const key = k.split('|')[0];
        num_act_shapes_per_effectType.set(key, (num_act_shapes_per_effectType.get(key) || 0) + v.size);
        num_min_shapes_per_effectType.set(key, (num_min_shapes_per_effectType.get(key) || 0) + 1);
}
console.log("# actual shapes per effectType", num_act_shapes_per_effectType);
console.log("min possible # shapes per effectType", num_min_shapes_per_effectType);

for (const k of key_seqs.keys()) {
        if (key_seqs.get(k).size === 1) key_seqs.delete(k);
}
console.log(key_seqs);

console.log("# REDUNDANT SHAPES PER KEYSET");
let num_redundant_shapes = 0;
for (const [k, v] of key_seqs.entries()) {
        console.log(k, v.size);
        num_redundant_shapes += v.size;
}
console.log("# TOTAL REDUNDANT SHAPES, uniq vs actual:", key_seqs.size, ",", num_redundant_shapes);

[Zarel]

I'd prefer for this to be a method on BasicEffect, assuming there are no performance impacts there.

[larry-the-table-guy]

I don't think there's any perf problems with that, but I do think it would be counter-intuitive to have a method that can only be called during construction. I see this more as a utility method than something specific to BasicEffect.

[Zarel]

I guess we rarely export bare functions outside of lib/. My mind is saying either move to lib/utils or make it a static method on BasicEffect, either would make it feel more right, but yeah it's not really a big deal.

[larry-the-table-guy]

I'm happy to follow your lead here - I'm pretty much a blank slate on Typescript code style.

[Zarel]

Hm. On further reflection, this is probably fine this way.

[Slayer95]

What's weird is that some of those keys don't appear in the corresponding ./data files for the type. Like onDeductPP happening during Species, yet it only appears in abilities data files.

I don't know why that happens.

This is likely the doing of a deleted mod which hasn't been correspondingly deleted from the built files. Recreate the dist folder, probably?

[larry-the-table-guy]

What's weird is that some of those keys don't appear in the corresponding ./data files for the type. Like onDeductPP happening during Species, yet it only appears in abilities data files.
I don't know why that happens.

This is likely the doing of a deleted mod which hasn't been correspondingly deleted from the built files. Recreate the dist folder, probably?

So, the issue goes away when I group the extra fields by effectType (as opposed to relying on chronology), which led me to think that the loading order is not what I thought it was. And indeed it wasn't.

pokemon-showdown/sim/dex-species.ts

Line 566 in 5a23223

if (this.dex.gen === 3 && this.dex.abilities.get(species.abilities['1']).gen === 4) delete species.abilities['1'];

Well, that's good to know for the future.

Edit: This was only an issue for the (first version) benchmark code that was trying to count extra fields. This is orthogonal to the correctness of assignMissingFields

[larry-the-table-guy]

To conclude my yapping in #10608 (comment):

(Context)
For object shapes in JS, you want the common prefix to be as long as possible.

- Bad field order (completely unordered)
b,d,c,a
c,d,b

- Bad field order (naive sorting)
a,b,c,d
b,c,d

+ Good field order
d,c,b,a
d,c,b

+ Good field order
b,c,d,a
b,c,d

(How to further reduce # of obj shapes)
Sorting the extra keys isn't the best way to reduce the number of object shapes here. While it does reduce the number of 'final' shapes (937 -> 741), it can create many more 'intermediate' shapes, which eat up the savings bc V8 is storing the metadata for those, .

Most of the remaining shapes (700 ish) stem from several optional properties in Move and Ability. One idea is to turn those optional properties from name?: T to name: T | undefined. Of course, you'd want to identify the few properties where this is most worthwhile. But then you have inconsistency in the API on whether a field is absent vs present & undefined, which is a considerable downside.

All that said, I don't know how much further benefit there would be to reducing those last few hundred shapes. I guess I'd have to make a PoC to collect stats on that. But that belongs in a separate PR.

[Zarel]

It occurs to me that if you want the common prefix as long as possible, fullname and num should be below gen, to match TypeInfo.

[larry-the-table-guy]

I plan to eventually explore re-ordering fields. I suspect it will be a smaller effect than this PR, but I won't rule it out yet.
My gut says that it matters most within a class - previously, different instances of Species could have wildly different field orders. Same for other classes.
Outside of micro-benchmarks, it gets very hard to reason about what works and what doesn't, which is why I try to make sure I have measurements to justify code changes.

[Zarel]

Thanks!