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 <noreply@anthropic.com>
This commit is contained in:
@@ -367,6 +367,10 @@ describe('equipping Räuberklinge increases combat damage (spec §45, §60)', ()
|
|||||||
|
|
||||||
await equipmentService.equip(CHARACTER_ID, BANDIT_BLADE_ITEM_ID);
|
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);
|
const reloaded = await combatService.getCombat(CHARACTER_ID, combat.id);
|
||||||
expect(reloaded.status).toBe(result.status);
|
expect(reloaded.status).toBe(result.status);
|
||||||
expect(reloaded.round).toBe(result.round);
|
expect(reloaded.round).toBe(result.round);
|
||||||
|
|||||||
@@ -217,6 +217,13 @@ export async function seedVisibleVerticalSlice(
|
|||||||
const characterItemRepository = dataSource.getRepository(CharacterItem);
|
const characterItemRepository = dataSource.getRepository(CharacterItem);
|
||||||
const characterEquipmentRepository = dataSource.getRepository(CharacterEquipment);
|
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({
|
const existingStartingSword = await characterItemRepository.findOneBy({
|
||||||
characterId: DEMO_CHARACTER_ID,
|
characterId: DEMO_CHARACTER_ID,
|
||||||
itemDefinitionId: ITEM_IDS['worn-short-sword'],
|
itemDefinitionId: ITEM_IDS['worn-short-sword'],
|
||||||
|
|||||||
@@ -76,6 +76,7 @@ interface SetupOptions {
|
|||||||
inventoryData?: InventoryResponse;
|
inventoryData?: InventoryResponse;
|
||||||
selectedItemId?: string | null;
|
selectedItemId?: string | null;
|
||||||
selectedItem?: InventoryItem | null;
|
selectedItem?: InventoryItem | null;
|
||||||
|
character?: CharacterResponse | null;
|
||||||
}
|
}
|
||||||
|
|
||||||
async function setup(options: SetupOptions = {}) {
|
async function setup(options: SetupOptions = {}) {
|
||||||
@@ -91,7 +92,10 @@ async function setup(options: SetupOptions = {}) {
|
|||||||
selectedItem: vi.fn(() => options.selectedItem ?? null),
|
selectedItem: vi.fn(() => options.selectedItem ?? null),
|
||||||
equip: vi.fn(() => Promise.resolve()),
|
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({
|
await TestBed.configureTestingModule({
|
||||||
imports: [InventoryPageComponent],
|
imports: [InventoryPageComponent],
|
||||||
@@ -103,7 +107,7 @@ async function setup(options: SetupOptions = {}) {
|
|||||||
|
|
||||||
const fixture = TestBed.createComponent(InventoryPageComponent);
|
const fixture = TestBed.createComponent(InventoryPageComponent);
|
||||||
fixture.detectChanges();
|
fixture.detectChanges();
|
||||||
return { fixture, inventoryStore };
|
return { fixture, inventoryStore, worldStore };
|
||||||
}
|
}
|
||||||
|
|
||||||
describe('InventoryPageComponent', () => {
|
describe('InventoryPageComponent', () => {
|
||||||
@@ -112,6 +116,16 @@ describe('InventoryPageComponent', () => {
|
|||||||
expect(inventoryStore.load).toHaveBeenCalledOnce();
|
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 () => {
|
it('renders one tile per owned item', async () => {
|
||||||
const { fixture } = await setup();
|
const { fixture } = await setup();
|
||||||
const tiles = (fixture.nativeElement as HTMLElement).querySelectorAll('.inventory-page__slot');
|
const tiles = (fixture.nativeElement as HTMLElement).querySelectorAll('.inventory-page__slot');
|
||||||
|
|||||||
@@ -52,6 +52,9 @@ export class InventoryPageComponent implements OnInit {
|
|||||||
});
|
});
|
||||||
|
|
||||||
ngOnInit(): void {
|
ngOnInit(): void {
|
||||||
|
if (this.worldStore.character() === null) {
|
||||||
|
void this.worldStore.load();
|
||||||
|
}
|
||||||
void this.inventoryStore.load();
|
void this.inventoryStore.load();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -65,6 +65,13 @@ const equippedAfter: EquipmentResponse = {
|
|||||||
stats: { maxHp: 100, attack: 7, weaponDamage: 11, armor: 0 },
|
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', () => {
|
describe('InventoryStore', () => {
|
||||||
let api: {
|
let api: {
|
||||||
getInventory: ReturnType<typeof vi.fn>;
|
getInventory: ReturnType<typeof vi.fn>;
|
||||||
@@ -109,12 +116,19 @@ describe('InventoryStore', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it('equips the selected item, refreshes inventory/equipment, and refreshes the character HUD', async () => {
|
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.load();
|
||||||
|
|
||||||
await store.equip('item-blade');
|
await store.equip('item-blade');
|
||||||
|
|
||||||
expect(api.equipItem).toHaveBeenCalledWith('item-blade');
|
expect(api.equipItem).toHaveBeenCalledWith('item-blade');
|
||||||
expect(store.equipment()).toEqual(equippedAfter);
|
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();
|
expect(worldStore.refreshCharacter).toHaveBeenCalledOnce();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user