Skip to content

Prioritize explicit turn endings - #1298

Draft
janmarsino98 wants to merge 1 commit into
boardgameio:mainfrom
janmarsino98:codex/924-endturn-next
Draft

Prioritize explicit turn endings#1298
janmarsino98 wants to merge 1 commit into
boardgameio:mainfrom
janmarsino98:codex/924-endturn-next

Conversation

@janmarsino98

@janmarsino98 janmarsino98 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Fixes #924

An explicit endTurn or pass queued by a move now takes precedence over the automatic maxMoves / legacy moveLimit turn end. This preserves arguments such as endTurn({ next: '2' }) instead of advancing through the default play order.

The automatic turn end previously ran before the queued event was flushed. By the time that event was processed, its captured turn no longer matched and its next argument was discarded.

Tests:

  • focused regression suites: 142 tests passed
  • pnpm run lint
  • pnpm test: 43 suites passed, 903 tests passed, 1 todo
  • pnpm run test:coverage
  • pnpm run ts
  • pnpm run build

@janmarsino98 janmarsino98 changed the title fix(flow): prioritize explicit turn endings Prioritize explicit turn endings Jul 31, 2026

@devill devill left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fixes #924's repro, but it also switches off turn.endIf.

Events.dispatch is never cleared, so isTurnEndPending() stays true after the queued endTurn
has been consumed — every later OnMove / UpdateStage / UpdateActivePlayers in that action
skips ShouldEndTurn, which gates endIf and not just maxMoves. No maxMoves here; passes on
main, Received: 2 on this branch:

const client = Client({
  game: {
    setup: () => ({}),
    turn: {
      onBegin: ({ ctx, events }) => {
        if (ctx.turn === 2) events.setActivePlayers({ all: Stage.NULL });
      },
      endIf: ({ ctx }) => ctx.turn === 2,
    },
    moves: { A: ({ events }) => { events.endTurn(); } },
  },
  numPlayers: 3,
});
client.moves.A();
expect(client.getState().ctx.turn).toBe(3);

Could you scope isTurnEndPending() to the entries update() hasn't reached yet, and add this
as a regression test?

Worth settling first: flow.ts:280 is the only _private use outside src/plugins/, and the
?? false fallback silently restores the old behaviour if that api is ever absent.

Moving it to draft, please reopen once comments are addressed.

@devill
devill marked this pull request as draft August 1, 2026 15:43
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.

moveLimit overrides the next parameter in endTurn

2 participants