From df7d1e12c2d4f3816f7b84caa96548b2b552d218 Mon Sep 17 00:00:00 2001 From: unanmed <1319491857@qq.com> Date: Fri, 11 Sep 2026 22:21:01 +0800 Subject: [PATCH] fix(03-17): await replay movement completion - Remove replay-safety decoration from command implementations - Await directional and pathfinding controller completion - Collapse directional commands into one parameterized class --- .../03-data-completion/03-REPLAY-CONTRACT.md | 10 +- .../data-state/src/replay/commands.test.ts | 197 ++++++++-------- .../data-state/src/replay/commands.ts | 215 ++++-------------- 3 files changed, 143 insertions(+), 279 deletions(-) diff --git a/.planning/phases/03-data-completion/03-REPLAY-CONTRACT.md b/.planning/phases/03-data-completion/03-REPLAY-CONTRACT.md index d69aa4e..f78c983 100644 --- a/.planning/phases/03-data-completion/03-REPLAY-CONTRACT.md +++ b/.planning/phases/03-data-completion/03-REPLAY-CONTRACT.md @@ -1,9 +1,9 @@ # Phase 3 Plan 03: Replay Command Contract -## User structural supersession +## User structural supersession (S-05) 本节晚于初始 replay checkpoint，优先于下方关于 command completion 和 decorator -placement 的旧记录。S-05 又 supersedes S-02 在 replay-step completion 边界上的结论： +placement 的旧记录。S-05 supersedes S-02 在 replay-step completion 边界上的结论： - replay command 不调用 `shouldReplay`；移动和寻路 command 必须等待各自 controller 完成后再返回，确保下一录像步不会与上一步并发。`shouldReplay` 的最终落点仍由用户放在真正改变最终状态的低层方法上。 - 既有 `ReplaySystem`、route 和 sandbox 只做使同步 command 正常运行所需的最小兼容调整，不重新设计录像系统。 @@ -52,8 +52,8 @@ their parameter count and primitive types before touching state: | `unequip` | one numeric slot | `CoreState.hero.equip.unequip(slot)` | Invalid parameter count/types, missing targets, an already-running action, or -a state API failure return `false`. A successful synchronous state API returns -silent success. +a state API failure return `false`. Item and equipment state APIs retain their +existing synchronous command boundary. ## Top-level registry ownership @@ -93,7 +93,7 @@ the singleton, IndexedDB, or a new options-bearing factory API. The existing remain the action targets; no new `ICoreState` member is required by this contract. -## Completion boundaries +## Completion boundaries under S-05 - Four-direction movement appends one direction to the hero mover, starts it, and awaits the returned mover controller's `onEnd`; it returns `true` only diff --git a/packages-user/data-state/src/replay/commands.test.ts b/packages-user/data-state/src/replay/commands.test.ts index 058f4df..771fd07 100644 --- a/packages-user/data-state/src/replay/commands.test.ts +++ b/packages-user/data-state/src/replay/commands.test.ts @@ -2,6 +2,7 @@ import { logger } from '@motajs/common'; import { FaceDirection, IReplayStepHandler, + IReplaySandbox, IMoverController, beginReplaySafetyCollection, endReplaySafetyCollection, @@ -40,6 +41,11 @@ function controller(onEnd: Promise): Readonly { }; } +interface IManualReplaySandbox extends IReplaySandbox { + playing: boolean; + pausing: boolean; +} + describe('replay commands', () => { // 验证默认 command item 只按稳定 enum 顺序提供八个实现 it('creates the approved command order without module-owned numbering', () => { @@ -49,10 +55,10 @@ describe('replay commands', () => { expect(items).toHaveLength(8); expect(items.map(item => item.command.execute)).toHaveLength(8); expect(items.map(item => item.command.constructor.name)).toEqual([ - 'ReplayUpCommand', - 'ReplayRightCommand', - 'ReplayDownCommand', - 'ReplayLeftCommand', + 'ReplayDirectionCommand', + 'ReplayDirectionCommand', + 'ReplayDirectionCommand', + 'ReplayDirectionCommand', 'ReplayAutoPathfindCommand', 'ReplayUseItemCommand', 'ReplayEquipCommand', @@ -133,41 +139,81 @@ describe('replay commands', () => { ); }); - // 验证四向移动不等待 controller.onEnd 即完成 command - it('completes directional movement without awaiting the controller', async () => { + // 验证四向移动等待 controller.onEnd 后才完成 command 并进入下一步 + it('awaits directional movement before the next replay step', async () => { const state = createCoreState(); const items = createReplayCommandItems(state); - const deferred = Promise.withResolvers(); + const first = Promise.withResolvers(); + const second = Promise.withResolvers(); const mover = state.hero.location.mover; const move = vi.spyOn(mover, 'step'); const start = vi .spyOn(mover, 'start') - .mockReturnValue(controller(deferred.promise)); - const result = items[ReplayCommandCode.Right].command.execute( - step(ReplayCommandCode.Right, []) - ); + .mockReturnValueOnce(controller(first.promise)) + .mockReturnValueOnce(controller(second.promise)); + const replay = new ReplaySystem(); + registerReplayCommandItems(replay, items); + replay.record(ReplayCommandCode.Right); + replay.record(ReplayCommandCode.Right); + const sandbox = replay.createReplaySandbox({ + route: replay.route, + reseter: { reset: () => {} } + }) as IManualReplaySandbox; + sandbox.playing = true; + sandbox.pausing = false; + const result = sandbox.step(); expect(move).toHaveBeenCalledWith(FaceDirection.Right); expect(start).toHaveBeenCalledTimes(1); + await Promise.resolve(); + expect(start).toHaveBeenCalledTimes(1); + first.resolve(); await expect(result).resolves.toBe(true); - deferred.resolve(); + const next = sandbox.step(); + await Promise.resolve(); + expect(start).toHaveBeenCalledTimes(2); + second.resolve(); + await expect(next).resolves.toBe(true); }); - // 验证自动寻路不等待 PathfindingSystem 返回的 controller - it('completes pathfinding without awaiting the controller', async () => { + // 验证自动寻路等待 PathfindingSystem 返回的 controller 后才进入下一步 + it('awaits pathfinding before the next replay step', async () => { const state = createCoreState(); const items = createReplayCommandItems(state); - const deferred = Promise.withResolvers(); - const moveTo = vi.spyOn(state.pathfinding, 'moveTo'); - moveTo.mockReturnValue({ - controller: controller(deferred.promise), - path: [] - }); - const result = items[ - ReplayCommandCode.AutoPathfindToPoint - ].command.execute(step(ReplayCommandCode.AutoPathfindToPoint, [2, 3])); + const first = Promise.withResolvers(); + const second = Promise.withResolvers(); + const moveTo = vi + .spyOn(state.pathfinding, 'moveTo') + .mockReturnValueOnce({ + controller: controller(first.promise), + path: [] + }) + .mockReturnValueOnce({ + controller: controller(second.promise), + path: [] + }); + const replay = new ReplaySystem(); + registerReplayCommandItems(replay, items); + replay.record(ReplayCommandCode.AutoPathfindToPoint, 2, 3); + replay.record(ReplayCommandCode.AutoPathfindToPoint, 4, 5); + const sandbox = replay.createReplaySandbox({ + route: replay.route, + reseter: { reset: () => {} } + }) as IManualReplaySandbox; + sandbox.playing = true; + sandbox.pausing = false; + const result = sandbox.step(); expect(moveTo).toHaveBeenCalledWith({ x: 2, y: 3 }); + expect(moveTo).toHaveBeenCalledTimes(1); + await Promise.resolve(); + expect(moveTo).toHaveBeenCalledTimes(1); + first.resolve(); await expect(result).resolves.toBe(true); - deferred.resolve(); + const next = sandbox.step(); + await Promise.resolve(); + expect(moveTo).toHaveBeenCalledWith({ x: 4, y: 5 }); + expect(moveTo).toHaveBeenCalledTimes(2); + second.resolve(); + await expect(next).resolves.toBe(true); moveTo.mockReturnValue(null); await expect( items[ReplayCommandCode.AutoPathfindToPoint].command.execute( @@ -245,80 +291,16 @@ describe('replay commands', () => { ]); }); - // 验证真实 registry 的生产移动入口在同步返回时恢复安全收集上下文 - it('restores registry movement safety context synchronously', async () => { + // 验证生产 command 不拥有 replay safety helper 且纯查询不制造安全记录 + it('keeps replay safety ownership below production commands', async () => { const state = createCoreState(); const replay = new ReplaySystem(); registerReplayCommandItems(replay, createReplayCommandItems(state)); - const deferred = Promise.withResolvers(); - const mover = state.hero.location.mover; - vi.spyOn(mover, 'start').mockReturnValue(controller(deferred.promise)); - const warning = vi.spyOn(logger, 'warn'); - let ended = false; - beginReplaySafetyCollection(replay); - try { - const action = replay - .getCommand(ReplayCommandCode.Right)! - .execute(step(ReplayCommandCode.Right, [])); - await expect(action).resolves.toBe(true); - deferred.resolve(); - endReplaySafetyCollection(); - ended = true; - - const detail = warning.mock.calls.find(call => call[0] === 161); - expect(detail).toBeDefined(); - expect(String(detail![2])).toContain('replay command: move hero'); - } finally { - if (!ended) endReplaySafetyCollection(); - warning.mockRestore(); - } - }); - - // 验证真实 registry 的道具和装备入口均经过生产 replay 安全边界 - it('decorates real registry item and equipment actions', async () => { - const state = createCoreState(); - const replay = new ReplaySystem(); - registerReplayCommandItems(replay, createReplayCommandItems(state)); - vi.spyOn(state.hero.items, 'useItem').mockReturnValue(true); - vi.spyOn(state.hero.equip, 'canEquipTo').mockReturnValue( - EquipStatus.CanEquip + const source = readFileSync( + new URL('./commands.ts', import.meta.url), + 'utf8' ); - vi.spyOn(state.hero.equip, 'getEquipped') - .mockReturnValueOnce(undefined) - .mockReturnValueOnce(99); - vi.spyOn(state.hero.equip, 'equip').mockImplementation(() => undefined); - const warning = vi.spyOn(logger, 'warn'); - let ended = false; - beginReplaySafetyCollection(replay); - try { - await expect( - replay - .getCommand(ReplayCommandCode.UseItem)! - .execute(step(ReplayCommandCode.UseItem, [12])) - ).resolves.toBe(true); - await expect( - replay - .getCommand(ReplayCommandCode.Equip)! - .execute(step(ReplayCommandCode.Equip, [99, 0])) - ).resolves.toBe(true); - endReplaySafetyCollection(); - ended = true; - - const detail = warning.mock.calls.find(call => call[0] === 161); - expect(detail).toBeDefined(); - expect(String(detail![2])).toContain('replay command: use item'); - expect(String(detail![2])).toContain('replay command: equip item'); - } finally { - if (!ended) endReplaySafetyCollection(); - warning.mockRestore(); - } - }); - - // 验证生产 command 的参数校验和寻路查询不会制造 replay 安全记录 - it('keeps pure path queries and validation outside the safety boundary', async () => { - const state = createCoreState(); - const replay = new ReplaySystem(); - registerReplayCommandItems(replay, createReplayCommandItems(state)); + expect(source).not.toContain('shouldReplay'); const getPath = vi .spyOn(state.pathfinding, 'getPath') .mockReturnValue([]); @@ -427,17 +409,14 @@ describe('replay safety decorators', () => { expect(source).not.toContain('shouldReplay'); }); - // 验证八个 command 类各自拥有 execute 且不存在共享入口或跨类调用 - it('keeps replay command ownership isolated in the command module', () => { + // 验证方向 command 共享一个参数化类且不存在旧的重复入口 + it('keeps directional command ownership parameterized', () => { const source = readFileSync( new URL('./commands.ts', import.meta.url), 'utf8' ); const classes = [ - 'ReplayUpCommand', - 'ReplayRightCommand', - 'ReplayDownCommand', - 'ReplayLeftCommand', + 'ReplayDirectionCommand', 'ReplayAutoPathfindCommand', 'ReplayUseItemCommand', 'ReplayEquipCommand', @@ -446,6 +425,24 @@ describe('replay safety decorators', () => { expect(source).not.toContain('ReplayCommandEntrances'); expect(source).not.toContain('createMoveCommand'); expect(source).not.toMatch(/\bentries\./); + expect(source).not.toMatch( + /class\s+Replay(?:Up|Right|Down|Left)Command\b/ + ); + expect( + (source.match(/new ReplayDirectionCommand\(state,/g) ?? []).length + ).toBe(4); + expect(source).toContain( + 'new ReplayDirectionCommand(state, FaceDirection.Up)' + ); + expect(source).toContain( + 'new ReplayDirectionCommand(state, FaceDirection.Right)' + ); + expect(source).toContain( + 'new ReplayDirectionCommand(state, FaceDirection.Down)' + ); + expect(source).toContain( + 'new ReplayDirectionCommand(state, FaceDirection.Left)' + ); for (const className of classes) { const body = source.match( new RegExp( diff --git a/packages-user/data-state/src/replay/commands.ts b/packages-user/data-state/src/replay/commands.ts index 65c8fa7..5758821 100644 --- a/packages-user/data-state/src/replay/commands.ts +++ b/packages-user/data-state/src/replay/commands.ts @@ -2,8 +2,7 @@ import { FaceDirection, IReplayStepHandler, IReplaySystem, - IReplayCommand, - shouldReplay + IReplayCommand } from '@user/data-common'; import { EquipStatus } from '@user/data-base'; import { @@ -41,139 +40,44 @@ function resolveSlot( return index < 0 ? null : index; } -class ReplayUpCommand implements IReplayCommand { - constructor(private readonly state: IReplayCommandState) { - this.moveHero = shouldReplay('replay command: move hero')( - this.moveHero, - { - name: 'moveHero' - } as ClassMethodDecoratorContext< - ReplayUpCommand, - (this: ReplayUpCommand) => boolean - > - ); - } +class ReplayDirectionCommand implements IReplayCommand { + constructor( + private readonly state: IReplayCommandState, + private readonly direction: FaceDirection + ) {} - private moveHero(): boolean { - const mover = this.state.hero.location.mover; - if (mover.moving) return false; - mover.step(FaceDirection.Up); - const controller = mover.start(); - if (!controller) return false; - return true; + private async moveHero(): Promise { + try { + const mover = this.state.hero.location.mover; + if (mover.moving) return false; + mover.step(this.direction); + const controller = mover.start(); + if (!controller) return false; + await controller.onEnd; + return true; + } catch { + return false; + } } execute(step: IReplayStepHandler): Promise { if (step.params.length !== 0) return Promise.resolve(false); - return Promise.resolve(this.moveHero()); - } -} - -class ReplayRightCommand implements IReplayCommand { - constructor(private readonly state: IReplayCommandState) { - this.moveHero = shouldReplay('replay command: move hero')( - this.moveHero, - { - name: 'moveHero' - } as ClassMethodDecoratorContext< - ReplayRightCommand, - (this: ReplayRightCommand) => boolean - > - ); - } - - private moveHero(): boolean { - const mover = this.state.hero.location.mover; - if (mover.moving) return false; - mover.step(FaceDirection.Right); - const controller = mover.start(); - if (!controller) return false; - return true; - } - - execute(step: IReplayStepHandler): Promise { - if (step.params.length !== 0) return Promise.resolve(false); - return Promise.resolve(this.moveHero()); - } -} - -class ReplayDownCommand implements IReplayCommand { - constructor(private readonly state: IReplayCommandState) { - this.moveHero = shouldReplay('replay command: move hero')( - this.moveHero, - { - name: 'moveHero' - } as ClassMethodDecoratorContext< - ReplayDownCommand, - (this: ReplayDownCommand) => boolean - > - ); - } - - private moveHero(): boolean { - const mover = this.state.hero.location.mover; - if (mover.moving) return false; - mover.step(FaceDirection.Down); - const controller = mover.start(); - if (!controller) return false; - return true; - } - - execute(step: IReplayStepHandler): Promise { - if (step.params.length !== 0) return Promise.resolve(false); - return Promise.resolve(this.moveHero()); - } -} - -class ReplayLeftCommand implements IReplayCommand { - constructor(private readonly state: IReplayCommandState) { - this.moveHero = shouldReplay('replay command: move hero')( - this.moveHero, - { - name: 'moveHero' - } as ClassMethodDecoratorContext< - ReplayLeftCommand, - (this: ReplayLeftCommand) => boolean - > - ); - } - - private moveHero(): boolean { - const mover = this.state.hero.location.mover; - if (mover.moving) return false; - mover.step(FaceDirection.Left); - const controller = mover.start(); - if (!controller) return false; - return true; - } - - execute(step: IReplayStepHandler): Promise { - if (step.params.length !== 0) return Promise.resolve(false); - return Promise.resolve(this.moveHero()); + return this.moveHero(); } } class ReplayAutoPathfindCommand implements IReplayCommand { - constructor(private readonly state: IReplayCommandState) { - this.moveToPoint = shouldReplay('replay command: pathfind hero')( - this.moveToPoint, - { - name: 'moveToPoint' - } as ClassMethodDecoratorContext< - ReplayAutoPathfindCommand, - ( - this: ReplayAutoPathfindCommand, - x: number, - y: number - ) => boolean - > - ); - } + constructor(private readonly state: IReplayCommandState) {} - private moveToPoint(x: number, y: number): boolean { - const result = this.state.pathfinding.moveTo({ x, y }); - if (!result) return false; - return true; + private async moveToPoint(x: number, y: number): Promise { + try { + const result = this.state.pathfinding.moveTo({ x, y }); + if (!result) return false; + await result.controller.onEnd; + return true; + } catch { + return false; + } } execute(step: IReplayStepHandler): Promise { @@ -181,19 +85,12 @@ class ReplayAutoPathfindCommand implements IReplayCommand { const x = step.params[0]; const y = step.params[1]; if (!isNumber(x) || !isNumber(y)) return Promise.resolve(false); - return Promise.resolve(this.moveToPoint(x, y)); + return this.moveToPoint(x, y); } } class ReplayUseItemCommand implements IReplayCommand { - constructor(private readonly state: IReplayCommandState) { - this.useItem = shouldReplay('replay command: use item')(this.useItem, { - name: 'useItem' - } as ClassMethodDecoratorContext< - ReplayUseItemCommand, - (this: ReplayUseItemCommand, item: number | string) => boolean - >); - } + constructor(private readonly state: IReplayCommandState) {} private useItem(item: number | string): boolean { return this.state.hero.items.useItem(item); @@ -208,19 +105,7 @@ class ReplayUseItemCommand implements IReplayCommand { } class ReplayEquipCommand implements IReplayCommand { - constructor(private readonly state: IReplayCommandState) { - this.equip = shouldReplay('replay command: equip item')(this.equip, { - name: 'equip' - } as ClassMethodDecoratorContext< - ReplayEquipCommand, - ( - this: ReplayEquipCommand, - uid: number, - slot: number | string, - autoUnload: boolean | undefined - ) => boolean - >); - } + constructor(private readonly state: IReplayCommandState) {} private equip( uid: number, @@ -258,17 +143,7 @@ class ReplayEquipCommand implements IReplayCommand { } class ReplayUnequipCommand implements IReplayCommand { - constructor(private readonly state: IReplayCommandState) { - this.unequip = shouldReplay('replay command: unequip item')( - this.unequip, - { - name: 'unequip' - } as ClassMethodDecoratorContext< - ReplayUnequipCommand, - (this: ReplayUnequipCommand, slot: number) => boolean - > - ); - } + constructor(private readonly state: IReplayCommandState) {} private unequip(slot: number): boolean { const equipment = this.state.hero.equip; @@ -294,43 +169,35 @@ export function createReplayCommandItems( return [ { code: ReplayCommandCode.Up, - // prettier-ignore - command: new (ReplayUpCommand)(state) + command: new ReplayDirectionCommand(state, FaceDirection.Up) }, { code: ReplayCommandCode.Right, - // prettier-ignore - command: new (ReplayRightCommand)(state) + command: new ReplayDirectionCommand(state, FaceDirection.Right) }, { code: ReplayCommandCode.Down, - // prettier-ignore - command: new (ReplayDownCommand)(state) + command: new ReplayDirectionCommand(state, FaceDirection.Down) }, { code: ReplayCommandCode.Left, - // prettier-ignore - command: new (ReplayLeftCommand)(state) + command: new ReplayDirectionCommand(state, FaceDirection.Left) }, { code: ReplayCommandCode.AutoPathfindToPoint, - // prettier-ignore - command: new (ReplayAutoPathfindCommand)(state) + command: new ReplayAutoPathfindCommand(state) }, { code: ReplayCommandCode.UseItem, - // prettier-ignore - command: new (ReplayUseItemCommand)(state) + command: new ReplayUseItemCommand(state) }, { code: ReplayCommandCode.Equip, - // prettier-ignore - command: new (ReplayEquipCommand)(state) + command: new ReplayEquipCommand(state) }, { code: ReplayCommandCode.Unequip, - // prettier-ignore - command: new (ReplayUnequipCommand)(state) + command: new ReplayUnequipCommand(state) } ]; }