Skip to content

fix: Consolidate duplicate unit/building definitions to JSON source o…#1171

Merged
braedonsaunders merged 2 commits into
mainfrom
claude/fix-duplicate-units-uUxrB
Jan 31, 2026
Merged

fix: Consolidate duplicate unit/building definitions to JSON source o…#1171
braedonsaunders merged 2 commits into
mainfrom
claude/fix-duplicate-units-uUxrB

Conversation

@braedonsaunders

Copy link
Copy Markdown
Owner

…f truth

This commit resolves the data drift between TypeScript and JSON definitions by making JSON the single source of truth for all game data.

Changes:

  • Updated JSON files with complete data from TypeScript definitions

    • units.json: All 20 units including naval with full properties
    • buildings.json: All 26 buildings including walls and naval
    • abilities.json: All 33 abilities including naval abilities
    • manifest.json: Added naval unit types and addon units
    • wall_upgrades.json: New file for wall upgrade definitions
  • Created JSON Schema files for validation

    • unit.schema.json
    • building.schema.json
    • ability.schema.json
    • faction-manifest.schema.json
  • Refactored TypeScript data files to proxy to DefinitionRegistry

    • src/data/units/dominion.ts -> Proxy to registry
    • src/data/buildings/dominion.ts -> Proxy to registry
    • src/data/research/dominion.ts -> Proxy to registry
    • src/data/buildings/walls.ts -> Proxy to registry (kept utility functions)
    • src/engine/components/Ability.ts -> DOMINION_ABILITIES proxies to registry
  • Updated bootstrap.ts to load from JSON by default

This architecture enables:

  • Non-programmers to modify game data via JSON
  • Hot-reload capability during development
  • Modding without recompiling
  • Type safety via JSON Schema validation
  • Backwards compatibility via Proxy objects

https://claude.ai/code/session_01DcpaMCwp5L8x7zcniyrMdS

…f truth

This commit resolves the data drift between TypeScript and JSON definitions
by making JSON the single source of truth for all game data.

Changes:
- Updated JSON files with complete data from TypeScript definitions
  - units.json: All 20 units including naval with full properties
  - buildings.json: All 26 buildings including walls and naval
  - abilities.json: All 33 abilities including naval abilities
  - manifest.json: Added naval unit types and addon units
  - wall_upgrades.json: New file for wall upgrade definitions

- Created JSON Schema files for validation
  - unit.schema.json
  - building.schema.json
  - ability.schema.json
  - faction-manifest.schema.json

- Refactored TypeScript data files to proxy to DefinitionRegistry
  - src/data/units/dominion.ts -> Proxy to registry
  - src/data/buildings/dominion.ts -> Proxy to registry
  - src/data/research/dominion.ts -> Proxy to registry
  - src/data/buildings/walls.ts -> Proxy to registry (kept utility functions)
  - src/engine/components/Ability.ts -> DOMINION_ABILITIES proxies to registry

- Updated bootstrap.ts to load from JSON by default

This architecture enables:
- Non-programmers to modify game data via JSON
- Hot-reload capability during development
- Modding without recompiling
- Type safety via JSON Schema validation
- Backwards compatibility via Proxy objects

https://claude.ai/code/session_01DcpaMCwp5L8x7zcniyrMdS
- Create tests/setup.ts that loads JSON data and registers with DefinitionRegistry
- Configure vitest to use the setup file via setupFiles option
- Add 'naval' to UnitCategory type to match manifest.json
- Update wall upgrade tests to match JSON source of truth values

This fixes test failures where proxy objects (UNIT_DEFINITIONS, BUILDING_DEFINITIONS,
etc.) were returning undefined because they were accessed before the DefinitionRegistry
was initialized.

https://claude.ai/code/session_01DcpaMCwp5L8x7zcniyrMdS

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 13908c6772

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +60 to +64
export const DOMINION_UNITS: UnitDefinition[] = new Proxy([] as UnitDefinition[], {
get(target, prop) {
if (prop === 'length') {
if (!DefinitionRegistry.isInitialized()) return 0;
return Object.keys(DefinitionRegistry.getAllUnits()).length;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bind DOMINION_UNITS array methods to real data

The array proxy only defines get, so built-in array methods (like filter/map) are called with this set to the proxy, which has no indexed properties and no has trap; as a result, those methods treat the array as empty and return [] even after initialization. This breaks call sites like BattleSimulatorPanel that rely on DOMINION_UNITS.filter(...) to populate the UI. Bind the returned methods to the real units array (e.g., units[prop].bind(units)) or add a has trap for numeric indices so array algorithms see the elements.

Useful? React with 👍 / 👎.

Comment on lines +58 to +63
export function bootstrapDefinitions(): void {
// Trigger async initialization but don't wait (for backwards compatibility with sync code)
if (!DefinitionRegistry.isInitialized()) {
debugInitialization.log('[Bootstrap] bootstrapDefinitions() called - starting async JSON load');
initializeDefinitions().catch((err) => {
console.error('[Bootstrap] Failed to load definitions from JSON:', err);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Await JSON definition load before systems run

bootstrapDefinitions() now fires initializeDefinitions() asynchronously and returns immediately. Callers like GameCore still invoke bootstrapDefinitions() synchronously, so any early access to UNIT_DEFINITIONS/BUILDING_DEFINITIONS (e.g., in SpawnSystem or ProductionSystem when initial spawns/queues happen) can observe an uninitialized registry and silently skip work. This is a regression from the prior synchronous bootstrap and will surface on slower loads. Either make bootstrap async and await it before system initialization, or explicitly wait on waitForDefinitions() before consuming definitions.

Useful? React with 👍 / 👎.

@braedonsaunders braedonsaunders merged commit 00e4c71 into main Jan 31, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants