boardgameio / boardgame.io

State Management and Multiplayer Networking for Turn-Based Games
https://boardgame.io
MIT License
10.02k stars 708 forks source link

Typescript error in custom turn config #617

Closed janKir closed 4 years ago

janKir commented 4 years ago

Upgrading to version 0.39.5 leads to the following typescript error:

Property 'next' is missing in type '{ first: (g: WizardState, ctx: Ctx) => number; }' but required in type 'TurnOrderConfig'.

I am using a custom turn configuration for a phase where the first player does not follow the normal turn order. The next function is not needed in my use case and it worked fine so far without it.

Is there a reason why the next field should always be set when using a custom turn configuration? (I think the same question applies to the first field). Or can we just make these fields optional?

delucis commented 4 years ago

Currently if turn.order is not set, it is set using the default order:

https://github.com/nicolodavis/boardgame.io/blob/f34f46b78a8018cce313f8cae698bf0ea403039f/src/core/flow.ts#L125-L127

But, if you only set first, as far as I can tell, there are no guards to prevent the game flow then trying to call next, which would be undefined, so I think the type is correct (even if in your game, next will never be called if I’ve understood correctly.)

Perhaps there’s an argument for having the default above set as:

conf.turn.order = {
  ...TurnOrder.DEFAULT,
  ...conf.turn.order,
}

to allow users to progressively modify the turn order.

janKir commented 4 years ago

But, if you only set first, as far as I can tell, there are no guards to prevent the game flow then trying to call next, which would be undefined, so I think the type is correct (even if in your game, next will never be called if I’ve understood correctly.)

Ah, indeed, in my case, the phase has only a single turn, so next will never be called. Hence, the types are correct and just revealed a potential source of error in my project.

Perhaps there’s an argument for having the default above set as

conf.turn.order = {
 ...TurnOrder.DEFAULT,
 ...conf.turn.order,
}

to allow users to progressively modify the turn order.

Yeah, this sounds like a good idea to me!