From 0ce3b420e6e50b5da814780e0a2aea2c4a97e557 Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Thu, 20 Aug 2026 18:44:39 +0200 Subject: [PATCH] fix: address final-review findings (TopBar hydration, snapshot test, armor doc, inventory refresh test) Fixes 4 Important findings from the final whole-branch review: - /inventory never called WorldStore.load(), leaving the TopBar stuck on "loading" and characterLevel() silently defaulting to 1 for any character above level 1. Mirrors the same guard already used in HuntPageComponent. - The combat/equipment snapshot-immutability integration test asserted only status/round, never the actual playerState snapshot the whole test claims to prove is untouched after a post-fight equip. - Documented (comment only, no behavior change) that the demo character's armor dropping from the old hardcoded 6 to 0 is an intentional, spec-sanctioned tradeoff (Slice 0.5 spec Section19), not a bug. - inventory.store.spec.ts's equip test used an identical inventory fixture before and after equip(), so a regression dropping the post-equip inventory re-fetch would still have passed. Now asserts the refetched fixture is actually reflected. Co-Authored-By: Claude Sonnet 5 --- .../combat-equipment-integration.spec.ts | 4 ++++ .../src/database/seeds/vertical-slice.seed.ts | 7 +++++++ .../inventory/inventory-page.component.spec.ts | 18 ++++++++++++++++-- .../inventory/inventory-page.component.ts | 3 +++ .../features/inventory/inventory.store.spec.ts | 14 ++++++++++++++ 5 files changed, 44 insertions(+), 2 deletions(-) diff --git a/apps/api/src/combat/combat-equipment-integration.spec.ts b/apps/api/src/combat/combat-equipment-integration.spec.ts index 5c35545..d74d9db 100644 --- a/apps/api/src/combat/combat-equipment-integration.spec.ts +++ b/apps/api/src/combat/combat-equipment-integration.spec.ts @@ -367,6 +367,10 @@ describe('equipping Räuberklinge increases combat damage (spec §45, §60)', () await equipmentService.equip(CHARACTER_ID, BANDIT_BLADE_ITEM_ID); + // The finished combat's playerState snapshot (written once at startCombat) + // must not be retroactively rewritten by equipping after the fight ends. + expect(state.combats[0].playerState).toEqual({ attack: 6, weaponDamage: 8, armor: 0 }); + const reloaded = await combatService.getCombat(CHARACTER_ID, combat.id); expect(reloaded.status).toBe(result.status); expect(reloaded.round).toBe(result.round); diff --git a/apps/api/src/database/seeds/vertical-slice.seed.ts b/apps/api/src/database/seeds/vertical-slice.seed.ts index febeb07..d8ac53e 100644 --- a/apps/api/src/database/seeds/vertical-slice.seed.ts +++ b/apps/api/src/database/seeds/vertical-slice.seed.ts @@ -217,6 +217,13 @@ export async function seedVisibleVerticalSlice( const characterItemRepository = dataSource.getRepository(CharacterItem); const characterEquipmentRepository = dataSource.getRepository(CharacterEquipment); + // Starting loadout is weapon-only -- no starter armor piece exists in + // content yet -- so the demo character's effective armor (sum of equipped + // bonusArmor) is 0 until the player loots and equips bandit-hood (+3 + // armor). This is a deliberate tradeoff, not a bug: Slice 0.5 spec §19 + // says to preserve the existing demo balance "as closely as the + // implemented content allows" and explicitly forbids fabricating a full + // starter gear set just to hit the old hardcoded TEMPORARY_ARMOR = 6. const existingStartingSword = await characterItemRepository.findOneBy({ characterId: DEMO_CHARACTER_ID, itemDefinitionId: ITEM_IDS['worn-short-sword'], diff --git a/apps/web/src/app/features/inventory/inventory-page.component.spec.ts b/apps/web/src/app/features/inventory/inventory-page.component.spec.ts index 47e2a2a..4fc928f 100644 --- a/apps/web/src/app/features/inventory/inventory-page.component.spec.ts +++ b/apps/web/src/app/features/inventory/inventory-page.component.spec.ts @@ -76,6 +76,7 @@ interface SetupOptions { inventoryData?: InventoryResponse; selectedItemId?: string | null; selectedItem?: InventoryItem | null; + character?: CharacterResponse | null; } async function setup(options: SetupOptions = {}) { @@ -91,7 +92,10 @@ async function setup(options: SetupOptions = {}) { selectedItem: vi.fn(() => options.selectedItem ?? null), equip: vi.fn(() => Promise.resolve()), }; - const worldStore = { character: signal(character) }; + const worldStore = { + character: signal(options.character === undefined ? character : options.character), + load: vi.fn(() => Promise.resolve()), + }; await TestBed.configureTestingModule({ imports: [InventoryPageComponent], @@ -103,7 +107,7 @@ async function setup(options: SetupOptions = {}) { const fixture = TestBed.createComponent(InventoryPageComponent); fixture.detectChanges(); - return { fixture, inventoryStore }; + return { fixture, inventoryStore, worldStore }; } describe('InventoryPageComponent', () => { @@ -112,6 +116,16 @@ describe('InventoryPageComponent', () => { expect(inventoryStore.load).toHaveBeenCalledOnce(); }); + it('loads the world state on init when no character has been loaded yet (direct navigation/hard refresh)', async () => { + const { worldStore } = await setup({ character: null }); + expect(worldStore.load).toHaveBeenCalledOnce(); + }); + + it('does not call world load again when a character is already present', async () => { + const { worldStore } = await setup(); + expect(worldStore.load).not.toHaveBeenCalled(); + }); + it('renders one tile per owned item', async () => { const { fixture } = await setup(); const tiles = (fixture.nativeElement as HTMLElement).querySelectorAll('.inventory-page__slot'); diff --git a/apps/web/src/app/features/inventory/inventory-page.component.ts b/apps/web/src/app/features/inventory/inventory-page.component.ts index e0b3eda..270c754 100644 --- a/apps/web/src/app/features/inventory/inventory-page.component.ts +++ b/apps/web/src/app/features/inventory/inventory-page.component.ts @@ -52,6 +52,9 @@ export class InventoryPageComponent implements OnInit { }); ngOnInit(): void { + if (this.worldStore.character() === null) { + void this.worldStore.load(); + } void this.inventoryStore.load(); } diff --git a/apps/web/src/app/features/inventory/inventory.store.spec.ts b/apps/web/src/app/features/inventory/inventory.store.spec.ts index 1cc1205..2deb123 100644 --- a/apps/web/src/app/features/inventory/inventory.store.spec.ts +++ b/apps/web/src/app/features/inventory/inventory.store.spec.ts @@ -65,6 +65,13 @@ const equippedAfter: EquipmentResponse = { stats: { maxHp: 100, attack: 7, weaponDamage: 11, armor: 0 }, }; +const inventoryAfterEquip: InventoryResponse = { + items: [ + { ...inventory.items[0], equipped: false }, + { ...inventory.items[1], equipped: true }, + ], +}; + describe('InventoryStore', () => { let api: { getInventory: ReturnType; @@ -109,12 +116,19 @@ describe('InventoryStore', () => { }); it('equips the selected item, refreshes inventory/equipment, and refreshes the character HUD', async () => { + api.getInventory + .mockReturnValueOnce(of(inventory)) // initial load() + .mockReturnValueOnce(of(inventoryAfterEquip)); // post-equip refetch + await store.load(); await store.equip('item-blade'); expect(api.equipItem).toHaveBeenCalledWith('item-blade'); expect(store.equipment()).toEqual(equippedAfter); + expect(store.inventory()).toEqual(inventoryAfterEquip); + expect(store.inventory()?.items.find((item) => item.id === 'item-sword')?.equipped).toBe(false); + expect(store.inventory()?.items.find((item) => item.id === 'item-blade')?.equipped).toBe(true); expect(worldStore.refreshCharacter).toHaveBeenCalledOnce(); });