diff --git a/src/common/rpg/components/effect.ts b/src/common/rpg/components/effect.ts index 0fc8bda..dd0029d 100644 --- a/src/common/rpg/components/effect.ts +++ b/src/common/rpg/components/effect.ts @@ -16,6 +16,7 @@ abstract class BaseEffect extends Component<{ condition: string | null; // keep effect while true; remove when it becomes false stacking: 'stack' | 'unique' | 'replace'; tag: string | null; // discriminator for stacking; null = no stacking enforcement + active: boolean; // true while the delta is currently applied to the target stat }> { constructor(opts: { target?: Class> | string, @@ -43,6 +44,7 @@ abstract class BaseEffect extends Component<{ condition: opts.condition ?? null, stacking: opts.stacking ?? 'stack', tag: opts.tag ?? null, + active: false, }); if (opts.gradual && opts.duration == null) { @@ -62,8 +64,9 @@ abstract class BaseEffect extends Component<{ @tag(ComponentTag.Equippable) @component export class Effect extends BaseEffect { - /** True while the effect's delta is applied to the target stat. */ - @variable('.') active: boolean = false; + /** True while the delta is applied. Backed by `state` so it survives serialize/clone. */ + @variable('.') get active(): boolean { return this.state.active; } + set active(v: boolean) { this.state.active = v; } override onAdd(): void { const { stacking, tag } = this.state; diff --git a/src/common/rpg/components/equipment.ts b/src/common/rpg/components/equipment.ts index f3743a2..486b60e 100644 --- a/src/common/rpg/components/equipment.ts +++ b/src/common/rpg/components/equipment.ts @@ -43,7 +43,8 @@ export type SlotInput = SlotDefinition | SlotDefinition[]; @component export class Equipment extends Component { - #cachedVars: RPGVariables | null = null; + // TS `private`, not `#private`: clone/deserialize rehydrate via Object.create(), which omits #members. + private cachedVars: RPGVariables | null = null; constructor(...slots: SlotInput[]) { const record: Record = {}; @@ -55,13 +56,13 @@ export class Equipment extends Component { super({ slots: record }); } - #slot(slotName: string): SlotRecord | undefined { + private slot(slotName: string): SlotRecord | undefined { return this.state.slots[slotName]; } /** ItemId in the named slot, or null if empty. */ getItemId(slotName: string): string | null { - return this.#slot(slotName)?.itemId ?? null; + return this.slot(slotName)?.itemId ?? null; } getItem(slotName: string): Entity | undefined { @@ -103,7 +104,7 @@ export class Equipment extends Component { */ @action equip({ slotName, itemId }: { slotName: string; itemId: string }): boolean { - const slot = this.#slot(slotName); + const slot = this.slot(slotName); if (!slot) return false; const itemEntity = this.entity.world.getEntity(itemId); @@ -117,7 +118,7 @@ export class Equipment extends Component { this.unequip(slotName); slot.itemId = itemId; - this.#cachedVars = null; + this.cachedVars = null; let id = 0; for (const [key, component] of itemEntity) { @@ -138,19 +139,19 @@ export class Equipment extends Component { */ @action unequip(slotName: string): boolean { - const slot = this.#slot(slotName); + const slot = this.slot(slotName); if (!slot || slot.itemId === null) return false; const itemId = slot.itemId; - this.#removeEffects(slot); + this.removeEffects(slot); slot.itemId = null; - this.#cachedVars = null; + this.cachedVars = null; this.emit('unequip', { slotName, itemId }); return true; } - #removeEffects(slot: SlotRecord): void { + private removeEffects(slot: SlotRecord): void { for (const key of slot.appliedEffectKeys) { this.entity.remove(key); } @@ -158,7 +159,7 @@ export class Equipment extends Component { } override getVariables(): RPGVariables { - if (this.#cachedVars) return this.#cachedVars; + if (this.cachedVars) return this.cachedVars; const result: RPGVariables = {}; for (const { slotName, itemId } of Object.values(this.state.slots)) { @@ -166,7 +167,7 @@ export class Equipment extends Component { result[slotName] = itemId; } } - this.#cachedVars = result; + this.cachedVars = result; return result; } diff --git a/src/common/rpg/components/experience.ts b/src/common/rpg/components/experience.ts index 2579c41..868aae3 100644 --- a/src/common/rpg/components/experience.ts +++ b/src/common/rpg/components/experience.ts @@ -57,7 +57,7 @@ export class Experience extends Component<{ /** Progress toward the next level as a 0–1 fraction. `1` at max level. */ @variable get progress(): number { const needed = xpForStep(this.state.spec, this.state.level); - if (needed === null) return 1; + if (needed === null || needed <= 0) return 1; // avoid /0 → NaN at a degenerate threshold return Math.min(this.xpInLevel / needed, 1); } @@ -68,6 +68,7 @@ export class Experience extends Component<{ while (true) { const needed = xpForStep(this.state.spec, this.state.level); if (needed === null) break; + if (needed <= 0) break; // a 0 threshold would never be reached → infinite loop if (this.xpInLevel < needed) break; this.state.xpAtLevel += needed; const prev = this.state.level++; diff --git a/src/common/rpg/components/inventory.ts b/src/common/rpg/components/inventory.ts index e064b97..9190fe8 100644 --- a/src/common/rpg/components/inventory.ts +++ b/src/common/rpg/components/inventory.ts @@ -37,7 +37,8 @@ function buildInventoryState(input?: number | InventorySlotInput[]): InventorySt @component export class Inventory extends Component { - #cachedVars: RPGVariables | null = null; + // TS `private`, not `#private`: clone/deserialize rehydrate via Object.create(), which omits #members. + private cachedVars: RPGVariables | null = null; /** Infinite inventory — grows on demand, no slot cap. */ constructor(); @@ -49,25 +50,25 @@ export class Inventory extends Component { super(buildInventoryState(input)); } - #slot(slotId: SlotId): SlotRecord | undefined { + private slot(slotId: SlotId): SlotRecord | undefined { return this.state.slots[slotId]; } - #capFor(slot: SlotRecord, itemId: string): number { + private capFor(slot: SlotRecord, itemId: string): number { const limitCap = slot.limit ?? Infinity; const stackable = this.entity.world.getEntity(itemId)?.get(Stackable); const stackCap = stackable ? stackable.maxStack : 1; return Math.min(limitCap, stackCap); } - #roomFor(slot: SlotRecord, itemId: string): number { + private roomFor(slot: SlotRecord, itemId: string): number { if (slot.contents !== null && slot.contents.itemId !== itemId) return 0; - return this.#capFor(slot, itemId) - (slot.contents?.amount ?? 0); + return this.capFor(slot, itemId) - (slot.contents?.amount ?? 0); } @action add(arg: { itemId: string; amount: number; slotId?: SlotId } | Entity): boolean { - this.#cachedVars = null; + this.cachedVars = null; const { itemId, amount = 1, slotId } = (arg instanceof Entity) ? { itemId: arg.id } : arg; if (amount < 0) return false; if (amount === 0) return true; @@ -79,9 +80,9 @@ export class Inventory extends Component { // ── Direct slot ─────────────────────────────────────────────────────── if (slotId !== undefined) { - const slot = this.#slot(slotId); + const slot = this.slot(slotId); if (!slot) return false; - if (this.#roomFor(slot, itemId) < amount) return false; + if (this.roomFor(slot, itemId) < amount) return false; slot.contents = { itemId, amount: (slot.contents?.amount ?? 0) + amount }; this.emit('add', { itemId, amount, slotIds: [slotId] }); return true; @@ -92,7 +93,7 @@ export class Inventory extends Component { let canFit = 0; for (const slot of Object.values(this.state.slots)) { if (slot.contents === null || slot.contents.itemId === itemId) - canFit += this.#roomFor(slot, itemId); + canFit += this.roomFor(slot, itemId); } if (canFit < amount) return false; @@ -100,7 +101,7 @@ export class Inventory extends Component { const slotIds: SlotId[] = []; for (const slot of Object.values(this.state.slots)) { if (slot.contents?.itemId === itemId && remaining > 0) { - const take = Math.min(this.#roomFor(slot, itemId), remaining); + const take = Math.min(this.roomFor(slot, itemId), remaining); slot.contents!.amount += take; remaining -= take; slotIds.push(slot.slotId); @@ -108,7 +109,7 @@ export class Inventory extends Component { } for (const slot of Object.values(this.state.slots)) { if (slot.contents === null && remaining > 0) { - const take = Math.min(this.#capFor(slot, itemId), remaining); + const take = Math.min(this.capFor(slot, itemId), remaining); slot.contents = { itemId, amount: take }; remaining -= take; slotIds.push(slot.slotId); @@ -124,7 +125,7 @@ export class Inventory extends Component { for (const slot of Object.values(this.state.slots)) { if (slot.contents?.itemId === itemId && remaining > 0) { - const take = Math.min(this.#roomFor(slot, itemId), remaining); + const take = Math.min(this.roomFor(slot, itemId), remaining); if (take > 0) { slot.contents!.amount += take; remaining -= take; @@ -134,7 +135,7 @@ export class Inventory extends Component { } for (const slot of Object.values(this.state.slots)) { if (slot.contents === null && remaining > 0) { - const take = Math.min(this.#capFor(slot, itemId), remaining); + const take = Math.min(this.capFor(slot, itemId), remaining); slot.contents = { itemId, amount: take }; remaining -= take; slotIds.push(slot.slotId); @@ -143,7 +144,7 @@ export class Inventory extends Component { while (remaining > 0) { const newSlot: SlotRecord = { slotId: this.state.nextSlotId++, limit: undefined, contents: null }; this.state.slots[newSlot.slotId] = newSlot; - const take = Math.min(this.#capFor(newSlot, itemId), remaining); + const take = Math.min(this.capFor(newSlot, itemId), remaining); newSlot.contents = { itemId, amount: take }; remaining -= take; slotIds.push(newSlot.slotId); @@ -155,12 +156,12 @@ export class Inventory extends Component { @action remove({ itemId, amount, slotId }: { itemId: string; amount: number; slotId?: SlotId }): boolean { - this.#cachedVars = null; + this.cachedVars = null; if (amount < 0) return false; if (amount === 0) return true; if (slotId !== undefined) { - const slot = this.#slot(slotId); + const slot = this.slot(slotId); if (!slot || slot.contents?.itemId !== itemId) return false; if (slot.contents.amount < amount) return false; slot.contents.amount -= amount; @@ -194,7 +195,7 @@ export class Inventory extends Component { */ @action equip(arg: string | { itemId?: string; slotId?: SlotId; slotName?: string } | Entity): boolean { - const resolved = this.#resolveItem(arg); + const resolved = this.resolveItem(arg); if (!resolved) return false; const { itemId, slotId } = resolved; const slotName = typeof arg === 'object' && 'slotName' in arg ? arg.slotName : undefined; @@ -234,7 +235,7 @@ export class Inventory extends Component { */ @action use(arg?: string | { itemId?: string; slotId?: SlotId }): boolean { - const resolved = this.#resolveItem(arg); + const resolved = this.resolveItem(arg); if (!resolved) return false; const { itemId, slotId } = resolved; @@ -262,7 +263,7 @@ export class Inventory extends Component { } /** Resolve an item reference, deriving `itemId` from slot contents if only `slotId` is given. */ - #resolveItem(arg?: string | { itemId?: string; slotId?: SlotId } | Entity): { itemId: string; slotId?: SlotId } | null { + private resolveItem(arg?: string | { itemId?: string; slotId?: SlotId } | Entity): { itemId: string; slotId?: SlotId } | null { if (!arg) return null; if (typeof arg === 'string') return { itemId: arg }; if (arg instanceof Entity) return { itemId: arg.id }; @@ -278,12 +279,12 @@ export class Inventory extends Component { } getSlotContents(slotId: SlotId): { itemId: string; amount: number } | null { - return this.#slot(slotId)?.contents ?? null; + return this.slot(slotId)?.contents ?? null; } getAmount(itemId: string, slotId?: SlotId): number { if (slotId !== undefined) { - const slot = this.#slot(slotId); + const slot = this.slot(slotId); return slot?.contents?.itemId === itemId ? slot.contents.amount : 0; } let total = 0; @@ -305,7 +306,7 @@ export class Inventory extends Component { } override getVariables(): RPGVariables { - if (this.#cachedVars) return this.#cachedVars; + if (this.cachedVars) return this.cachedVars; const result: RPGVariables = {}; for (const [itemId, amount] of this.getItems()) { result[itemId] = amount; @@ -316,7 +317,7 @@ export class Inventory extends Component { } } } - this.#cachedVars = result; + this.cachedVars = result; return result; } } diff --git a/src/common/rpg/components/questLog.ts b/src/common/rpg/components/questLog.ts index 36d3f5c..2f7acd4 100644 --- a/src/common/rpg/components/questLog.ts +++ b/src/common/rpg/components/questLog.ts @@ -22,7 +22,8 @@ interface QuestLogState { @component export class QuestLog extends Component { - #cachedVars: RPGVariables | null = null; + // TS `private`, not `#private`: clone/deserialize rehydrate via Object.create(), which omits #members. + private cachedVars: RPGVariables | null = null; constructor(quests: Quest[] = []) { const questsRecord: Record = {}; @@ -43,9 +44,9 @@ export class QuestLog extends Component { this.state.runtimeStates[quest.id] = { status: 'inactive', stageIndex: 0 }; } - #invalidate() { this.#cachedVars = null; } + private invalidate() { this.cachedVars = null; } - #transition(op: string, questId: string, from: QuestStatus, to: QuestStatus, event: string): boolean { + private transition(op: string, questId: string, from: QuestStatus, to: QuestStatus, event: string): boolean { const runtimeState = this.state.runtimeStates[questId]; if (!runtimeState) { console.warn(`[QuestLog] ${op}: quest '${questId}' is not registered`); @@ -57,25 +58,25 @@ export class QuestLog extends Component { } runtimeState.status = to; if (to === 'active' || to === 'inactive') runtimeState.stageIndex = 0; - this.#invalidate(); + this.invalidate(); this.emit(event, { questId }); return true; } start(questId: string): boolean { - return this.#transition('start', questId, 'inactive', 'active', 'started'); + return this.transition('start', questId, 'inactive', 'active', 'started'); } complete(questId: string): boolean { - return this.#transition('complete', questId, 'active', 'completed', 'completed'); + return this.transition('complete', questId, 'active', 'completed', 'completed'); } fail(questId: string): boolean { - return this.#transition('fail', questId, 'active', 'failed', 'failed'); + return this.transition('fail', questId, 'active', 'failed', 'failed'); } abandon(questId: string): boolean { - return this.#transition('abandon', questId, 'active', 'inactive', 'abandoned'); + return this.transition('abandon', questId, 'active', 'inactive', 'abandoned'); } getState(questId: string): QuestRuntimeState | undefined { @@ -125,7 +126,7 @@ export class QuestLog extends Component { const quest = this.state.quests[questId]; const runtimeState = this.state.runtimeStates[questId]; if (!quest || !runtimeState) return; - this.#invalidate(); + this.invalidate(); if (runtimeState.stageIndex + 1 < quest.stages.length) { runtimeState.stageIndex++; this.emit('stage', { questId, index: runtimeState.stageIndex, stage: quest.stages[runtimeState.stageIndex] }); @@ -150,13 +151,13 @@ export class QuestLog extends Component { } override getVariables(): RPGVariables { - if (this.#cachedVars) return this.#cachedVars; + if (this.cachedVars) return this.cachedVars; const result: RPGVariables = {}; for (const [questId, runtimeState] of Object.entries(this.state.runtimeStates)) { result[`${questId}.status`] = runtimeState.status; result[`${questId}.stage`] = runtimeState.stageIndex; } - this.#cachedVars = result; + this.cachedVars = result; return result; } } diff --git a/src/common/rpg/core/serialization.ts b/src/common/rpg/core/serialization.ts index 53162ab..a3737a7 100644 --- a/src/common/rpg/core/serialization.ts +++ b/src/common/rpg/core/serialization.ts @@ -70,13 +70,19 @@ function deserializeComponent(data: ComponentData): Component { } const savedVersion = data.version ?? 0; + if (savedVersion > meta.version) { + throw new Error( + `Component '${data.name}' was saved at version ${savedVersion} but this build only knows ` + + `version ${meta.version}. The save is from a newer version of the game and cannot be loaded.` + ); + } const state = savedVersion < meta.version ? migrateState(data.name, data.state as Record, savedVersion) : data.state; - // Bypass constructor: create a bare instance and restore state directly. - // Safe because constructors must only call super(state) — all initialization - // logic goes in onAdd(), which entity.add() calls after this. + // Bypass the constructor (state holds all persistent data). Components must NOT use ES #private + // members — Object.create() omits them, so any access throws; use TS `private`. Re-inserted via + // entity.restore() (not add()), so onAdd side effects are not re-applied on load. const instance = Object.create(meta.ctor.prototype) as Component; (instance as unknown as { state: unknown }).state = state; return instance; @@ -87,9 +93,9 @@ function deserializeEntity(data: EntityData, world: World): Entity { for (const componentData of data.components) { const component = deserializeComponent(componentData); if (componentData.key === null) { - entity.add(component); + entity.restore(component); } else { - entity.add(component, componentData.key); + entity.restore(component, componentData.key); } } return entity; diff --git a/src/common/rpg/core/world.ts b/src/common/rpg/core/world.ts index a2fa476..af38a04 100644 --- a/src/common/rpg/core/world.ts +++ b/src/common/rpg/core/world.ts @@ -56,6 +56,10 @@ export abstract class Component> { onAdd(): void { } onRemove(): void { } + /** Called instead of {@link onAdd} on clone/deserialize, when `state` is already consistent. + * Override to rebuild runtime-only fields; never re-apply state-mutating side effects here. */ + onRestore(): void { } + getVariables(): RPGVariables { const meta = (this.constructor as Function)[Symbol.metadata]; const keys = meta?.[VARIABLE_KEYS] as Map | undefined; @@ -128,6 +132,19 @@ export class Entity { return component; } + /** Re-insert a pre-built component on clone/deserialize: fires `onRestore()`, not `onAdd()`, + * so state-mutating side effects (e.g. an Effect's delta) are not re-applied. @internal */ + restore>(component: T, k?: string): T { + if (this.#destroyed) throw new Error('Entity has been destroyed'); + if (component == null) throw new Error(`Component must be an instance of Component`); + const key = k ?? Symbol(); + component.entity = this; + component.key = key; + this.#components.set(key, component); + component.onRestore(); + return component; + } + clone>(component: T, key: string): T { if (this.#destroyed) throw new Error('Entity has been destroyed'); const clone = Object.create(component.constructor.prototype) as T; @@ -224,12 +241,14 @@ export class Entity { removeAll>(ctor: Class, filter?: ComponentFilter): void { if (this.#destroyed) throw new Error('Entity has been destroyed'); + // Collect first, then remove, so onRemove() side effects can't disturb iteration. + const keys: (string | symbol)[] = []; for (const [k, c] of this.#components) { if (!(c instanceof ctor)) continue; if (typeof filter === 'function' && !filter(c)) continue; - - this.#removeByKey(k); return; + keys.push(k); } + for (const k of keys) this.#removeByKey(k); } #removeByKey(key: string | symbol): void { @@ -282,13 +301,17 @@ export class Entity { } export class World { + /** Set to true to log every event emit via console.debug. Off by default (it is a hot path). */ + static logEvents = false; + /** World-level variables, accessible in conditions via the $. prefix */ readonly globals: RPGVariables = {}; readonly #entities = new Map(); readonly #handlers = new Map>(); readonly #globalHandlers = new Map>(); - readonly #onceWrappers = new Map(); + /** Maps a handler-map key (`entityId\0event` or global `event`) → (original handler → once-wrapper). */ + readonly #onceWrappers = new Map>(); readonly #systems: System[] = []; #entityCounter = 0; @@ -325,8 +348,13 @@ export class World { destroyEntity(entity: Entity): void { entity._destroy(); this.#entities.delete(entity.id); + const prefix = `${entity.id}\0`; for (const key of this.#handlers.keys()) { - if (key.startsWith(`${entity.id}\0`)) this.#handlers.delete(key); + if (key.startsWith(prefix)) this.#handlers.delete(key); + } + // release pending once-wrappers for this entity (else they leak) + for (const key of this.#onceWrappers.keys()) { + if (key.startsWith(prefix)) this.#onceWrappers.delete(key); } } @@ -338,10 +366,13 @@ export class World { */ cloneEntity(source: Entity, newId?: string): Entity { const target = this.createEntity(newId); - for (const [key, component] of source) { + for (const [, component] of source) { + // COMPONENT_KEY (real key), not Component.key — the latter returns the class name for + // symbol keys, which would collapse anonymous components of the same type together. + const realKey = component[COMPONENT_KEY]; const clone = Object.create(component.constructor.prototype) as Component; (clone as unknown as { state: unknown }).state = structuredClone(component.state); - target.add(clone, key); + target.restore(clone, typeof realKey === 'symbol' ? undefined : realKey); } return target; } @@ -398,16 +429,20 @@ export class World { } emit(entityId: string, event: string, data?: unknown): void { - console.debug(`Emitting event ${event} for entity ${entityId}, data:`, data); + if (World.logEvents) console.debug(`Emitting event ${event} for entity ${entityId}, data:`, data); const entity = this.getEntity(entityId); if (!entity) return; - this.#handlers.get(`${entityId}\0${event}`)?.forEach(h => h({ target: entity, data })); - this.#globalHandlers.get(event)?.forEach(h => h({ target: entity, data })); + // Snapshot handler sets so mutation during dispatch (off/once/destroy) is safe. + const local = this.#handlers.get(`${entityId}\0${event}`); + if (local) for (const h of [...local]) h({ target: entity, data }); + const global = this.#globalHandlers.get(event); + if (global) for (const h of [...global]) h({ target: entity, data }); } emitGlobal(event: string, data?: unknown): void { - console.debug(`Emitting global event ${event}, data:`, data); - this.#globalHandlers.get(event)?.forEach(h => h({ target: this, data })); + if (World.logEvents) console.debug(`Emitting global event ${event}, data:`, data); + const global = this.#globalHandlers.get(event); + if (global) for (const h of [...global]) h({ target: this, data }); } on(event: string, handler: WorldEventHandler): () => void; @@ -423,13 +458,12 @@ export class World { off(entityId: string, event: string, handler: EntityEventHandler): void; off(arg1: string, arg2: WorldEventHandler | string, arg3?: EntityEventHandler): void { if (typeof arg2 === 'string') { - const handler = (this.#onceWrappers.get(arg3!) ?? arg3!) as EntityEventHandler; - this.#handlers.get(`${arg1}\0${arg2}`)?.delete(handler); - this.#onceWrappers.delete(arg3!); + const mapKey = `${arg1}\0${arg2}`; + const handler = (this.#takeOnceWrapper(mapKey, arg3!) ?? arg3!) as EntityEventHandler; + this.#handlers.get(mapKey)?.delete(handler); } else { - const handler = (this.#onceWrappers.get(arg2) ?? arg2) as WorldEventHandler; + const handler = (this.#takeOnceWrapper(arg1, arg2) ?? arg2) as WorldEventHandler; this.#globalHandlers.get(arg1)?.delete(handler); - this.#onceWrappers.delete(arg2); } } @@ -437,17 +471,42 @@ export class World { once(entityId: string, event: string, handler: EntityEventHandler): () => void; once(arg1: string, arg2: WorldEventHandler | string, arg3?: EntityEventHandler): () => void { if (typeof arg2 === 'string') { + const mapKey = `${arg1}\0${arg2}`; const original = arg3!; - const wrapped: EntityEventHandler = data => { this.#onceWrappers.delete(original); unsub(); original(data); }; - this.#onceWrappers.set(original, wrapped); + const wrapped: EntityEventHandler = data => { this.#deleteOnceWrapper(mapKey, original); unsub(); original(data); }; + this.#setOnceWrapper(mapKey, original, wrapped); const unsub = this.on(arg1, arg2, wrapped); - return () => { this.#onceWrappers.delete(original); unsub(); }; + return () => { this.#deleteOnceWrapper(mapKey, original); unsub(); }; } + const mapKey = arg1; const original = arg2; - const wrapped: WorldEventHandler = data => { this.#onceWrappers.delete(original); unsub(); original(data); }; - this.#onceWrappers.set(original, wrapped); + const wrapped: WorldEventHandler = data => { this.#deleteOnceWrapper(mapKey, original); unsub(); original(data); }; + this.#setOnceWrapper(mapKey, original, wrapped); const unsub = this.on(arg1, wrapped); - return () => { this.#onceWrappers.delete(original); unsub(); }; + return () => { this.#deleteOnceWrapper(mapKey, original); unsub(); }; + } + + // Scoped per handler-map key so the same handler can be once()'d on multiple events without colliding. + #setOnceWrapper(mapKey: string, original: Function, wrapped: Function): void { + let inner = this.#onceWrappers.get(mapKey); + if (!inner) { inner = new Map(); this.#onceWrappers.set(mapKey, inner); } + inner.set(original, wrapped); + } + #deleteOnceWrapper(mapKey: string, original: Function): void { + const inner = this.#onceWrappers.get(mapKey); + if (!inner) return; + inner.delete(original); + if (inner.size === 0) this.#onceWrappers.delete(mapKey); + } + #takeOnceWrapper(mapKey: string, original: Function): Function | undefined { + const inner = this.#onceWrappers.get(mapKey); + if (!inner) return undefined; + const wrapped = inner.get(original); + if (wrapped) { + inner.delete(original); + if (inner.size === 0) this.#onceWrappers.delete(mapKey); + } + return wrapped; } #addHandler | EntityEventHandler>( diff --git a/src/common/rpg/systems/combat.ts b/src/common/rpg/systems/combat.ts index 34e31f3..0c42987 100644 --- a/src/common/rpg/systems/combat.ts +++ b/src/common/rpg/systems/combat.ts @@ -16,6 +16,7 @@ export interface HitEvent { export class CombatSystem extends System { override update(world: World) { let random: Random | undefined; + const rng = () => (random ??= getWorldRandom(world)); for (const [target] of world.query(Attacked)) { const healths = target.getAll(Health).sort((a, b) => b.priority - a.priority); @@ -25,9 +26,7 @@ export class CombatSystem extends System { continue; } - let damageSum = 0; const hitEvents: HitEvent[] = []; - let lastHit: HitEvent | null = null; for (const attack of target.getAll(Attacked)) { const { attackerId, sourceId } = attack.state; @@ -45,51 +44,47 @@ export class CombatSystem extends System { continue; } - const damage = source.get(Damage); - if (!damage) { + const damages = source.getAll(Damage); + if (damages.length === 0) { console.warn(`[CombatSystem] No Damage on source ${source.id}`); continue; } - const { damageType, minDamage = 0, variance = 0 } = damage.state; - let damageAmount = damage.value; - - if (variance > 0) { - if (!random) { - random = getWorldRandom(world); - } - const variedDamage = random.use(r => r.nextInt(-variance, variance + 1)); - - if (variedDamage > 0) { - damageAmount += variedDamage; - } - } - + // Crit rolls once per attack; guard so a value < 1 can't reduce damage. + let critMult = 1; const crit = source.get(Crit); - if (crit) { - const { chance } = crit.state; - if (!random) { - random = getWorldRandom(world); + const roll = rng().use(r => r.nextFloat()); + if (roll < crit.state.chance) critMult = Math.max(1, crit.value); + } + + // Each Damage component contributes its own typed hit (multi-type weapons). + for (const damage of damages) { + const { damageType, minDamage = 0, variance = 0 } = damage.state; + let damageAmount = damage.value; + + // symmetric variance: [-variance, +variance] + if (variance > 0) { + damageAmount += rng().use(r => r.nextInt(-variance, variance + 1)); } - const roll = random.use(r => r.nextFloat()); - if (roll < chance) { - damageAmount *= crit.value; + + damageAmount *= critMult; + + // sum all matching-type + general defenses, so stacked armor accumulates + let defense = 0; + for (const d of target.getAll(Defense)) { + if (d.state.damageType === damageType || d.state.damageType == null) { + defense += d.value; + } } + damageAmount -= defense; + + damageAmount = Math.max(0, minDamage, damageAmount); + + hitEvents.push({ attackerId, sourceId, amount: damageAmount, damageType }); } - const typeDefense = target.get(Defense, (c) => c.state.damageType === damageType); - if (typeDefense) { - damageAmount -= typeDefense.value; - } - const generalDefense = target.get(Defense, (c) => c.state.damageType == null); - if (generalDefense) { - damageAmount -= generalDefense.value; - } - - damageAmount = Math.max(0, minDamage, damageAmount); - - // Apply on-hit effects from source onto target + // on-hit effects from source → target, once per attack for (const component of source.getAll(EffectTemplate)) { const s = component.state; target.add( @@ -108,19 +103,16 @@ export class CombatSystem extends System { `__hit_${source.id}_${component.key}_${hitEffectCounter++}`, ); } - - damageSum += damageAmount; - lastHit = { attackerId, sourceId, amount: damageAmount, damageType }; - hitEvents.push(lastHit); } - if (damageSum === 0) continue; + if (hitEvents.length === 0) continue; let totalBefore = 0; for (const pool of healths) totalBefore += pool.value; for (const hit of hitEvents) { let remaining = hit.amount; + let applied = 0; if (hit.damageType) { for (const pool of healths) { @@ -130,6 +122,7 @@ export class CombatSystem extends System { const take = Math.min(pool.value, remaining); pool.update(-take); remaining -= take; + applied += take; } } @@ -141,10 +134,15 @@ export class CombatSystem extends System { const take = Math.min(pool.value, remaining); pool.update(-take); remaining -= take; + applied += take; } } + + hit.amount = applied; // report damage actually dealt, not pre-mitigation } + const lastHit = hitEvents[hitEvents.length - 1] ?? null; + for (const info of hitEvents) { target.emit('Combat.hit', info); } diff --git a/src/common/rpg/utils/decorators.ts b/src/common/rpg/utils/decorators.ts index 5a338c1..87603be 100644 --- a/src/common/rpg/utils/decorators.ts +++ b/src/common/rpg/utils/decorators.ts @@ -135,6 +135,10 @@ export function migrateState( `[registry] No migration for '${name}' from version ${current} to ${meta.version}. ` + `Register one with registerMigration('${name}', ${current}, ...).` ); + if (entry.toVersion <= current) throw new Error( + `[registry] Migration for '${name}' from version ${current} does not advance the version ` + + `(toVersion=${entry.toVersion}); this would loop forever. Migrations must increase the version.` + ); s = entry.fn(s); current = entry.toVersion; } diff --git a/test/common/rpg/fixes.test.ts b/test/common/rpg/fixes.test.ts new file mode 100644 index 0000000..c206fe1 --- /dev/null +++ b/test/common/rpg/fixes.test.ts @@ -0,0 +1,268 @@ +// Regression tests for the engine fixes (rehydration, loop guards, event cleanup, combat). +// Each test below fails against the pre-fix engine. +import { describe, it, expect } from 'bun:test'; +import { World, Component, COMPONENT_KEY } from '@common/rpg/core/world'; +import { Serialization } from '@common/rpg/core/serialization'; +import { component, registerMigration, migrateState } from '@common/rpg/utils/decorators'; +import { resolveVariables } from '@common/rpg/utils/variables'; +import { Stat, Health } from '@common/rpg/components/stat'; +import { Effect } from '@common/rpg/components/effect'; +import { Inventory } from '@common/rpg/components/inventory'; +import { Equipment } from '@common/rpg/components/equipment'; +import { QuestLog } from '@common/rpg/components/questLog'; +import { Experience } from '@common/rpg/components/experience'; +import { Attacked, Damage, Defense, Crit } from '@common/rpg/components/combat'; +import { CombatSystem } from '@common/rpg/systems/combat'; +import { EffectSystem } from '@common/rpg/systems/effect'; + +const roundtrip = (w: World) => Serialization.deserialize(Serialization.serialize(w)) as World; + +function combatWorld() { + const w = new World(); + w.addSystem(new CombatSystem()); + w.addSystem(new EffectSystem()); + return w; +} + +describe('fix: rehydration (#private + onAdd double-apply)', () => { + it('deserialized Inventory/Equipment/QuestLog do not throw on use', () => { + const w = new World(); + w.createEntity('sword'); + const p = w.createEntity('player'); + p.add(new Inventory(5)); + p.add(new Equipment('weapon')); + p.add(new QuestLog()); + + const p2 = roundtrip(w).getEntity('player')!; + expect(() => p2.get(Inventory)!.add({ itemId: 'sword', amount: 1 })).not.toThrow(); + expect(() => p2.get(Inventory)!.getVariables()).not.toThrow(); + expect(() => p2.get(Equipment)!.getVariables()).not.toThrow(); + expect(() => p2.get(Equipment)!.getItemId('weapon')).not.toThrow(); + expect(() => p2.get(QuestLog)!.getVariables()).not.toThrow(); + expect(() => resolveVariables(p2)).not.toThrow(); + }); + + it('cloned Inventory does not throw on use', () => { + const w = new World(); + w.createEntity('potion'); + const a = w.createEntity('a'); + a.add(new Inventory(3)); + const b = w.cloneEntity(a, 'b'); + expect(() => b.get(Inventory)!.add({ itemId: 'potion', amount: 1 })).not.toThrow(); + }); + + it('deserialization does NOT double-apply an active Effect delta', () => { + const w = new World(); + const e = w.createEntity('e'); + e.add(new Stat({ value: 100 }), 'hp'); + e.add(new Effect({ targetKey: 'hp', delta: -10 })); // permanent modifier + expect(e.get(Stat, 'hp')!.value).toBe(90); + + const e2 = roundtrip(w).getEntity('e')!; + expect(e2.get(Stat, 'hp')!.value).toBe(90); // not 80 + expect(e2.getAll(Effect).length).toBe(1); + }); + + it('cloneEntity does NOT double-apply an active Effect delta', () => { + const w = new World(); + const src = w.createEntity('src'); + src.add(new Stat({ value: 100 }), 'hp'); + src.add(new Effect({ targetKey: 'hp', delta: -10 })); + const clone = w.cloneEntity(src, 'clone'); + expect(clone.get(Stat, 'hp')!.value).toBe(90); // not 80 + }); + + it('a deserialized active Effect still reverses its delta when removed', () => { + const w = new World(); + const e = w.createEntity('e'); + e.add(new Stat({ value: 100 }), 'hp'); + e.add(new Effect({ targetKey: 'hp', delta: -10 }), 'fx'); + + const e2 = roundtrip(w).getEntity('e')!; + e2.remove('fx'); + expect(e2.get(Stat, 'hp')!.value).toBe(100); // active survived in state → reversal works + }); +}); + +describe('fix: Entity.removeAll removes ALL matches', () => { + it('removes every matching component, not just the first', () => { + const w = new World(); + const e = w.createEntity(); + e.add(new Stat({ value: 1 })); + e.add(new Stat({ value: 2 })); + e.add(new Stat({ value: 3 })); + e.removeAll(Stat); + expect(e.getAll(Stat).length).toBe(0); + }); + + it('respects the filter and removes all that match it', () => { + const w = new World(); + const e = w.createEntity(); + e.add(new Stat({ value: 5 })); + e.add(new Stat({ value: 5 })); + e.add(new Stat({ value: 9 })); + e.removeAll(Stat, s => s.value === 5); + expect(e.getAll(Stat).map(s => s.value)).toEqual([9]); + }); +}); + +describe('fix: cloneEntity preserves anonymous (symbol) keys', () => { + it('does not collapse two anonymous components of the same type', () => { + const w = new World(); + const src = w.createEntity('src'); + src.add(new Stat({ value: 7 })); + src.add(new Stat({ value: 8 })); + const clone = w.cloneEntity(src, 'clone'); + const stats = clone.getAll(Stat); + expect(stats.length).toBe(2); + expect(stats.map(s => s.value).sort()).toEqual([7, 8]); + for (const [, c] of clone) expect(typeof c[COMPONENT_KEY]).toBe('symbol'); + }); + + it('preserves string keys on clone', () => { + const w = new World(); + const src = w.createEntity('src'); + src.add(new Stat({ value: 42 }), 'str'); + const clone = w.cloneEntity(src, 'clone'); + expect(clone.get(Stat, 'str')!.value).toBe(42); + }); +}); + +describe('fix: Experience never infinite-loops on degenerate thresholds', () => { + it('award terminates when a geometric threshold floors to 0', () => { + const w = new World(); + const xp = w.createEntity('p').add(new Experience({ base: 10, factor: 0.5 })); + xp.award(25); // would hang pre-fix + expect(xp.level).toBeGreaterThanOrEqual(1); + expect(Number.isNaN(xp.progress)).toBeFalse(); + expect(xp.progress).toBe(1); + }); +}); + +describe('fix: migration guards', () => { + @component({ name: 'MigTestFixes', version: 2 }) + class MigTestFixes extends Component<{ x: number }> { + constructor() { super({ x: 0 }); } + } + + it('throws (does not loop) on a non-advancing migration', () => { + registerMigration('MigTestFixes', 0, 0, s => s); // toVersion does not advance + expect(() => migrateState('MigTestFixes', { x: 0 }, 0)).toThrow(/advance|loop/i); + }); + + it('rejects a save from a newer engine version', () => { + const w = new World(); + w.createEntity('e').add(new Stat({ value: 5 })); // Stat is version 0 + const data = JSON.parse(Serialization.serialize(w)); + data.entities[0].components[0].version = 99; // pretend it came from the future + expect(() => Serialization.deserialize(JSON.stringify(data))).toThrow(/newer version/i); + }); +}); + +describe('fix: once() bookkeeping is scoped per event', () => { + it('off() on one event does not leave the other event armed/disarmed wrongly', () => { + const w = new World(); + const e = w.createEntity('e'); + const calls: unknown[] = []; + const handler = ({ data }: { data?: unknown }) => calls.push(data); + e.once('a', handler); + e.once('b', handler); // same fn, different event + e.off('a', handler); // must remove ONLY the 'a' registration + e.emit('a', 1); // disarmed → no call + e.emit('b', 2); // still armed → fires once + expect(calls).toEqual([2]); + }); + + it('the same handler fires once for each event it was registered on', () => { + const w = new World(); + const e = w.createEntity('e'); + const calls: unknown[] = []; + const handler = ({ data }: { data?: unknown }) => calls.push(data); + e.once('a', handler); + e.once('b', handler); + e.emit('a', 1); + e.emit('b', 2); + e.emit('a', 3); // already consumed + e.emit('b', 4); // already consumed + expect(calls).toEqual([1, 2]); + }); +}); + +describe('fix: combat correctness', () => { + it('stacks multiple defenses of the same type', () => { + const w = combatWorld(); + w.createEntity('sword').add(new Damage({ value: 20, damageType: 'physical' })); + w.createEntity('a'); + const t = w.createEntity('t'); + t.add(new Health({ value: 100, min: 0 })); + t.add(new Defense({ value: 5, damageType: 'physical' })); + t.add(new Defense({ value: 3, damageType: 'physical' })); + t.add(new Attacked({ attackerId: 'a', sourceId: 'sword' })); + w.update(1); + expect(t.get(Health)!.value).toBe(88); // 20 - (5 + 3) + }); + + it('applies every Damage component on a multi-type weapon', () => { + const w = combatWorld(); + const sword = w.createEntity('sword'); + sword.add(new Damage({ value: 10, damageType: 'physical' })); + sword.add(new Damage({ value: 5, damageType: 'fire' })); + w.createEntity('a'); + const t = w.createEntity('t'); + t.add(new Health({ value: 100, min: 0 })); + const types: (string | undefined)[] = []; + t.on('Combat.hit', ({ data }) => types.push((data as any).damageType)); + t.add(new Attacked({ attackerId: 'a', sourceId: 'sword' })); + w.update(1); + expect(t.get(Health)!.value).toBe(85); // 10 + 5 + expect(types.sort()).toEqual(['fire', 'physical']); + }); + + it("Combat.hit reports the damage actually applied, not the pre-mitigation amount", () => { + const w = combatWorld(); + w.createEntity('sword').add(new Damage({ value: 100, damageType: 'physical' })); + w.createEntity('a'); + const t = w.createEntity('t'); + t.add(new Health({ value: 30, min: 0 })); + let reported = -1; + t.on('Combat.hit', ({ data }) => { reported = (data as any).amount; }); + t.add(new Attacked({ attackerId: 'a', sourceId: 'sword' })); + w.update(1); + expect(reported).toBe(30); // not 100 + expect(t.get(Health)!.value).toBe(0); + }); + + it('a crit value below 1 never reduces damage', () => { + const w = combatWorld(); + const sword = w.createEntity('sword'); + sword.add(new Damage({ value: 20, damageType: 'physical' })); + sword.add(new Crit({ value: 0.5, chance: 1 })); // always "crit", but multiplier < 1 + w.createEntity('a'); + const t = w.createEntity('t'); + t.add(new Health({ value: 100, min: 0 })); + t.add(new Attacked({ attackerId: 'a', sourceId: 'sword' })); + w.update(1); + expect(t.get(Health)!.value).toBe(80); // 20, not 10 + }); + + it('damage variance can roll below the base value (symmetric)', () => { + const w = combatWorld(); + const sword = w.createEntity('sword'); + sword.add(new Damage({ value: 20, damageType: 'physical', variance: 5 })); + w.createEntity('a'); + const t = w.createEntity('t'); + t.add(new Health({ value: 1_000_000, min: 0 })); + let min = Infinity, max = -Infinity; + t.on('Combat.hit', ({ data }) => { + const a = (data as any).amount as number; + min = Math.min(min, a); max = Math.max(max, a); + }); + for (let i = 0; i < 300; i++) { + t.add(new Attacked({ attackerId: 'a', sourceId: 'sword' })); + w.update(1); + } + expect(min).toBeLessThan(20); // pre-fix: variance only ever added + expect(min).toBeGreaterThanOrEqual(15); // within [-variance, +variance] + expect(max).toBeLessThanOrEqual(25); + }); +});