endTurn() called twice when going through game log

[Peachball]

If ctx.events.endTurn() is called inside of a move, the log will display the game state differently.

An example of this using the tic-tac-toe example from the tutorial is here. If you make moves normally, the board looks correct, but if you view the game state through the log, it looks like only one player is playing.

The docs here imply that calling methods attached to ctx.events is supported.

[Stefan-Hanke]

In your example, going through the game inside the log always prints "0" inside of cells, even when player 1 made that move and there should be a "1" displayed.

The code's single change looks like being the addition of ctx.events.endTurn();.

Using boardgame.io latest, one can actually see the double calls to endTurn (your example uses 0.24 where these calls are not logged).

My hypothesis is that there is an unfortunate interaction with the movesPerTurn flow that is used for TicTacToe.

[Stefan-Hanke]

The endTurn event is not executed directly but stored away. The flow then actually triggers endTurn since every player is allowed one move, and that move has been made already. After that, the stored events are executed triggering the second call to endTurn.

Somehow, the automatic calls to events need to take calls eventually made by moves into account.

[Stefan-Hanke]

Well, thinking about this a bit more, the actual problem is the automatic call to events. This doesn't look like belonging to a core flow, but to a separate one (like a FlowWithDefinedMoveCount or something). If the automatic calls are no more, there will be no problem with doube calls since the game must call events on its own.

I'd say automatic event handling is a property of a flow, and not every flow should perform automatic event calls.

[nicolodavis]

The real problem here is how the GameLog rewinds its state. This is what is happening:

1. When you call clickCell, two events are recorded in the log (clickCell and endTurn, the endTurn being an automatic event).

2. When the log tries to replay these actions over the initial state, it tries both clickCell and endTurn at each step. This is incorrect, because the replayed clickCell also includes an embedded endTurn. What it should do instead is to just replay the clickCell events (i.e. only the actions that were initiated by the client, as opposed to the synthetic / automatic actions).

An easy fix for this would be to just tag automatic events so that the log knows not to replay them. This is good for rendering purposes as well so that users know which events were triggered manually / automatically.

[Stefan-Hanke]

You don't seem to have a problem with events that were called twice? That doesn't look consistent to me.

[nicolodavis]

If you mean do I see multiple 'endTurn` events in the game log, I don't (I only see one per turn).

Reference: https://imgur.com/a/fIQsx5P

[nicolodavis]

I've fixed the GameLog behavior in https://github.com/google/boardgame.io/commit/edd1df01c211e782df99f8665d3c2f840e6333d1. I think that should fix the entire issue here, but I'll leave this open until someone confirms that it is working correctly.

@Peachball @Stefan-Hanke Maybe one of you can try the Tic-Tac-Toe modification in the boardgame.io repository here (at examples/react/tic-tac-toe/game.js) and run npm run dev to verify behavior?

[Stefan-Hanke]

Fix confirmed.

I didn't mean the GameLog stuff, I meant the actual call to endTurnEvent. If this is done in a move, and the Game is configured with movesPerTurn in its flow, those events are called more than once.

[nicolodavis]

Oh I see. When movesPerTurn is specified we should probably just disable the endTurn event (and document this). The framework is handling the turns for you, so calling endTurn is meaningless.

I'm still in favor of having movesPerTurn in the core flow implementation rather than having another flow that has this ability (I want to see how much mileage we can get out of one very customizable flow that can handle many different game types).

This is a separate issue from the one in this thread though, correct? Looks like the OP hasn't used movesPerTurn in their example.

[Stefan-Hanke]

Oh my. OP used the TicTacToe example, dropped movesPerTurn and put a call to endTurn into it. Actually this should work fine. The stuff I previously wrote and debugged was my own making where I just put the call to endTurn into the TicTacToe example, but without removing movesPerTurn.

Otherwise, the statements should be valid.

Yeah we can create a separate issue for calling events twice when movesPerTurn is used.

[nicolodavis]

OK, let's close this and discuss on #242. Thanks for creating the issue.

[Peachball]

Thanks for the fast responses, and fixing it!