Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strange behaviour for setStage and setActivePlayers for Stage.NULL #848

Closed
evandroabukamel opened this issue Nov 24, 2020 · 4 comments · Fixed by #849
Closed

Strange behaviour for setStage and setActivePlayers for Stage.NULL #848

evandroabukamel opened this issue Nov 24, 2020 · 4 comments · Fixed by #849
Labels

Comments

@evandroabukamel
Copy link
Contributor

evandroabukamel commented Nov 24, 2020

Hey guys, what's up? I have a question about using the Stage.NULL. In the docs it's said that players can become active but in no stage if we set them to Stage.NULL using setActivePlayers.

What I'm trying to achieve is to make all players to use a specific move from no stage but when they make some moves from specific stages they must exit that stage but still become active. So, I'm trying to keep all other players in the same state they are when players calls a move that should put themselves to Stage.NULL. Now, everybody receive a "player not active" error. So I try a few things:

1. Using setStage(Stage.NULL) at the end of a move: by doing this, the player was removed from activePlayers property, thus becoming inactive.

2. Using setActivePlayers([ctx.playerID]) at the end of a move: by doing this, only that player becomes active in Stage.NULL and all other players are removed from activePlayers property.

So, am I doing something wrong or is there a bug in one of those functions?
What you suggest me to do? I was thinking of merge the current ctx.activePlayers with setActivePlayers parameter, but I think this is very risky. Don't know.

Thanks.

@delucis
Copy link
Member

delucis commented Nov 24, 2020

setActivePlayers overwrites the entire state, so yes, one way to set just one player, would be to spread the existing state in:

function onMove(G, ctx) {
  ctx.events.setActivePlayers({
    value: {
      ...ctx.activePlayers,
      [ctx.playerID]: Stage.NULL,
    }
  });
}

That isn’t really risky, but setActivePlayers does reset move counts etc., which may be undesirable.

More importantly, setStage(Stage.NULL) should work. My guess would be there’s a check somewhere that does if (arg) instead of if (arg || arg === Stage.NULL), probably these two:

if (!arg && playerInStage) {
const conf = GetPhase(ctx);
const stage = conf.turn.stages[activePlayers[playerID]];
if (stage && stage.next) arg = stage.next;
}
if (next && arg) {
next.push({ fn: UpdateStage, arg, playerID });
}

Would you like to fix that and add a test for setStage(Stage.NULL)?

@delucis delucis added the bug label Nov 24, 2020
@evandroabukamel
Copy link
Contributor Author

evandroabukamel commented Nov 24, 2020

Sure, I'll do that. I've also tried that with a unit test and found that bug.

@evandroabukamel
Copy link
Contributor Author

evandroabukamel commented Nov 25, 2020

@delucis , can I solve #777 too? I believe I found it's code here and it's very simple to fix.

---- EDIT
I've tried that and many tests broke. Maybe I can work on that later.

@delucis
Copy link
Member

delucis commented Nov 25, 2020

can I solve #777 too?

I'm not sure I think #777 is so helpful either. We have to check that active players is empty quite often which would become something like Object.keys(ctx.activePlayers).length === 0, which is also not a great experience.

delucis added a commit that referenced this issue Nov 25, 2020
…loses #848) (#849)

* Fixing setStage to make players set on Stage.NULL active.

* Update src/core/turn-order.test.ts

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

* Update src/core/turn-order.test.ts

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

Co-authored-by: Evandro Abu Kamel <evandro.costa@maxmilhas.com.br>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
simlmx pushed a commit to simlmx/boardgame.io that referenced this issue Nov 28, 2020
…loses boardgameio#848) (boardgameio#849)

* Fixing setStage to make players set on Stage.NULL active.

* Update src/core/turn-order.test.ts

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

* Update src/core/turn-order.test.ts

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

Co-authored-by: Evandro Abu Kamel <evandro.costa@maxmilhas.com.br>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
simlmx pushed a commit to simlmx/boardgame.io that referenced this issue Nov 28, 2020
…loses boardgameio#848) (boardgameio#849)

* Fixing setStage to make players set on Stage.NULL active.

* Update src/core/turn-order.test.ts

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

* Update src/core/turn-order.test.ts

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

Co-authored-by: Evandro Abu Kamel <evandro.costa@maxmilhas.com.br>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
simlmx pushed a commit to simlmx/boardgame.io that referenced this issue Nov 28, 2020
…loses boardgameio#848) (boardgameio#849)

* Fixing setStage to make players set on Stage.NULL active.

* Update src/core/turn-order.test.ts

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

* Update src/core/turn-order.test.ts

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

Co-authored-by: Evandro Abu Kamel <evandro.costa@maxmilhas.com.br>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants