Release v0.39.5 started mismatching between activePlayers and currentPlayer

[vdfdev]

Hi, we have a game called secret codes implemented at FreeBoardGames.org. It has two phases, setup and play. Inside the play phase, it has two stages, one for "giving a clue" and another for "guessing": https://github.com/freeboardgames/FreeBoardGames.org/blob/7e26b4125fe3f9722b54d4479c11606fc52739f5/src/games/secretcodes/game.ts#L123

Whenever the user selects a wrong card, we call ctx.events.endTurn(); here: https://github.com/freeboardgames/FreeBoardGames.org/blob/7e26b4125fe3f9722b54d4479c11606fc52739f5/src/games/secretcodes/util.ts#L62

Until v0.39.4 of BGIO, when the first player guessed wrongly and ended the turn, that would mean that the currentPlayer would go from 0 to 1, and the activePlayers would have { "1": "giveClue" } for the first stage of the next turn as expected, video: https://drive.google.com/open?id=1dXCjA8Srg0JVF0QJdUxgNPpFjjV2aNGS

However, after the version v039.5 of BGIO, when the first player guessed wrongly and ended the turn, that would mean that the currentPlayer would go from 0 to 1, BUT the activePlayers would have { "0": "giveClue" } instead! This breaks the game as the currentplayer now has isActive as false... Video: https://drive.google.com/open?id=1IqFo2skN1cXUbJabUivzuoS52nxwCU-9

We detected this because it broke this unit test: https://github.com/freeboardgames/FreeBoardGames.org/blob/7e26b4125fe3f9722b54d4479c11606fc52739f5/src/games/secretcodes/game.test.ts#L22

If you want to debug with your own device, feel free to sync to the "upgradeBgio" branch of FreeBoardGames.org: https://github.com/freeboardgames/FreeBoardGames.org/tree/upgradeBgio/src

[delucis]

Hi @flamecoals — trying to track down the source of the bug. Can you confirm this happened specifically between v0.39.4 and v0.39.5? Or could it be anywhere between v0.39.4 and the latest release (v0.39.9)?

[delucis]

I see you’re doing ctx.events.setStage(Stages.giveClue) in the turn.onBegin hook. Could you try using turn.activePlayers instead?

turn: {
  activePlayers: { currentPlayer: Stages.giveClue },
}

I need to debug a little further, but I think this is related to #514 and may be a regression since rewriting the events plugin.

To summarise: the events plugin is initialised with a player ID at the start of processing a move/event (based on action.payload.playerID). If the turn ends, this ID is never updated on the plugin class, so is stale for events that depend on it like setStage or endStage where you’d probably expect the new player ID to be the new current player. In v0.39.0–0.39.4 there was a bug where the events plugin wasn’t passed any player ID at all, which meant that all events were triggered as if by the current player no matter what. v0.39.5 fixed that (in #598), but broke your code 😅 .

I’m pretty sure that both turn.activePlayers and the setActivePlayers event will run correctly, because they depend on ctx.currentPlayer rather than the action-provided playerID.

[vdfdev]

Perfect! Thank you so much for tracking this, I think we were depending on some unspecified behavior on the framework. Later today I will try your recommendations

[delucis]

Let me know how you get on!

I’m also seeing this from #523 again, which I forgot to act on:

setStage and endStage still rely on playerID, which is unavoidable. These should probably be disabled in onBegin and onEnd hooks for clarity’s sake.

Perhaps we should at least log a warning for these cases. I’ll leave this open as a reminder to do something like that — or at least to improve the documentation.

[vdfdev]

I had a look tonight, but while changing this fixed the local game mode, it broke multiplayer... I need to understand better what is going on, it will take me longer than I can spend today, I will get back to this tomorrow or so.

Thank you so much for the help. On a separate note, can you help me understand the semantic of this activePlayers API? What does it mean when activePlayers is { "0": null, "1": "clueGiven" } ? It seems to me that in that case both players 0 and 1 are active (even with null), so what is the difference putting the stage name there makes ? What does it mean ? When I set { currentPlayer: "clueGiven" } on the turn like you talked, does it mean that the currentPlayer will always be set as "clueGiven" for the framework, right? But if I do { currentPlayer: "clueGiven", others: Stage.NULL }, it still means that others are active too? I am a bit confused

[delucis]

Stage.NULL is a way to have active players even if you’re not using any Stages. In some simple games you might just want to have multiple players be active, without setting up special stages or moves. In this case, we use null as the stage “name”. The player is still active but not in a stage.

So in a 3-player game. { "0": null, "1": "clueGiven" } would mean that Player 0 is active but not in a custom stage, Player 1 is active in the clueGiven stage, and Player 2 is not active.

setActivePlayers always completely overwrites the active player state, so setActivePlayers({ currentPlayer: 'clueGiven' }) will result in activePlayers: { "1": "clueGiven" }) for example. setActivePlayers({ others: 'A' }) might result in activePlayers: { "0": "A", "2": "A" }). If you don’t want a player to be active, don’t include them in setActivePlayers.

There’s more details on the event in the stages docs: https://boardgame.io/documentation/#/stages?id=advanced

[vdfdev]

So stages are by player? I think that's what I was missing! This is the wrong abstraction for this game then, as every player are in the same stage at the same time. I will probably refactor

On Thu, Apr 23, 2020, 1:34 AM Chris Swithinbank notifications@github.com wrote:

Stage.NULL is a way to have active players even if you’re not using any Stages https://boardgame.io/documentation/#/stages. In some simple games you might just want to have multiple players be active, without setting up special stages or moves. In this case, we use null as the stage “name”. The player is still active but not in a stage.

So in a 3-player game. { "0": null, "1": "clueGiven" } would mean that Player 0 is active but not in a custom stage, Player 1 is active in the clueGiven stage, and Player 2 is not active.

setActivePlayers always completely overwrites the active player state, so setActivePlayers({ currentPlayer: 'clueGiven' }) will result in activePlayers: { "1": "clueGiven" }) for example. setActivePlayers({ others: 'A' }) might result in activePlayers: { "0": "A", "2": "A" }). If you don’t want a player to be active, don’t include them in setActivePlayers.

There’s more details on the event in the stages docs: https://boardgame.io/documentation/#/stages?id=advanced

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nicolodavis/boardgame.io/issues/638#issuecomment-618262230, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH4TBYHA3C4QL4G4JJB6TLRN74Q5ANCNFSM4MN5YDQQ .

[delucis]

Ah, OK. Yes exactly — sounds like phases might be a better fit. You can do setActivePlayers({ all: 'Stage' }), so it might work for you too! (Configurations with everyone in the same stage can be useful for scenarios where you might have multiple active players but you want them all to be able to take actions in any order.)

[vdfdev]

Thank so much for the help, I am closing this for now as it was a bug on our side