Boardgame.io: "ctx.events.setPhase('MyPhase')" seems to have issues

Created on 9 Mar 2020  ·  17Comments  ·  Source: boardgameio/boardgame.io

I try to migrate my playing card game from boargame.io 0.32.1 to 0.37.2 (first my target was 0.33.0 but it behaves exactly the same).

So I followed the migration guide, but after a lot of debugging, I can't understand why it doesn't work now...

My game is split into several phases (Deal, Talk, PlayCard, CountPoint, etc.) and a lot of logic is done through onBegin phase hooks (Deal and CountPoint phases for instance, doesn't contain any move).

Here is a simple reproduction of my main issue:

{
  // ...
  phases: {
    Deal: {
      start: true,
      onBegin: (G, ctx) => {
        console.log('phase Deal');
        ctx.events.setPhase('Talk');
      },
    },
    Talk: {
      onBegin: (G, ctx) => {
        console.log('phase Talk');
        ctx.events.setPhase('PlayCards');
        console.log('TalkEnd');
      },
    },
    PlayCards: {
      onBegin: (G, ctx) => {
        console.log('phase PlayCards');
        ctx.events.setPhase('Talk');
        console.log('PlayCardsEnd');
      },
    },
  },
}

then my logs are:

phase Deal
phase Talk
TalkEnd

it just doesn't want to immediately go to PlayCards phase after the Talk one and I don't understand why... (it worked fine in version 0.32.1)

could you help?

(maybe this is relative to https://github.com/nicolodavis/boardgame.io/issues/526)

Most helpful comment

Glad you’ve got it working! For now, I think we should add some warning to avoid the setPhase and endPhase events in phase hooks. My earlier advice was bad because setPhase won’t work in an onEnd hook either (because at the point, the current phase is already ending, so setPhase will again break). Eventually, perhaps we should also revisit all the checks around processing these events.

All 17 comments

From the sample of code you shared, it looks like you’re ending PlayCards immediately and returning to the Talk phase. So something like this happens:

Deal —> Talk —> PlayCards —> Talk —> PlayCards —> Talk —> etc. forever

I need to double check the implementation, but because your code implements an infinite loop of setPhase calls, I’m guessing the framework is quitting early. Why exactly do you need to set several phases immediately one after another?

As I said, this is

a simple reproduction

because, even if I had an infinite loop somewhere, I'd expect to see at least the following logs:

phase Deal
phase Talk
TalkEnd
phase PlayCards
PlayCardsEnd

instead of just:

phase Deal
phase Talk
TalkEnd

If you're interested in the real code:

  • here is the current implementation which works well using boargame.io 0.32.1
  • here is what I tried to update my code for boardgame.io 0.37.2

Actually, you’re right in https://github.com/nicolodavis/boardgame.io/issues/526#issuecomment-596486568, that that check prevents you even getting to PlayCards, because it prevents any further phase events after the phase has ended/changed once. The same check is made to prevent more than one turn ending.

I’m not 100% how important those checks are, but I can see from your code why you have this kind of logic using phases, so maybe there’s a way to enable multiple events in the way you need.

That said, you might also be able to get it working by avoiding using phases only to apply specific state changes like you do in Deal and CountPoints. These phases always end themselves immediately and no player ever makes moves in these phases. Using a phase like this is more or less equivalent to calling a move function, e.g. Deal(G, ctx), which can also mutate state and trigger events, so it might simplify things to refactor them as internal “moves” that you can call at the points you’re currently calling setPhase.

I see, it sounds possible indeed 👌

Maybe I will have to keep something like a Setup phase or something, because I need a phase holding the start: true property... anyway, I will give it a try and tell you 🙇

You always go from Deal to Talk right? I would do Talk: { start: true } and either do the initial deal in your setup function, or call your Deal function during the Talk phase’s onBegin (maybe conditionally based on some value in G).

You always go from Deal to Talk right?

No, it also can go from Talk to Deal when all players say "skip" (meaning that no one want to take the risk to play the game using the cards they received). In this case, it deals new cards to each player.

I will try to call a Deal function at the beginning of the Talk phase

I tried to call a deal function (which mutates G such as G.availableCards = [...]) from the phases.Talk.endIf hook, but it fails with:

(node:34243) UnhandledPromiseRejectionWarning: TypeError: Cannot assign to read only property 'availableCards' of object '#<Object>'
    at deal (/Users/oliverthebault/projets_perso/coinche/client/src/shared/coinche/index.ts:1897:19)
    at Object.endIf (/Users/oliverthebault/projets_perso/coinche/client/src/shared/coinche/index.ts:1955:11)
    at ShouldEndPhase (/Users/oliverthebault/projets_perso/coinche/server/node_modules/boardgame.io/dist/server.js:2857:17)
    at Process (/Users/oliverthebault/projets_perso/coinche/server/node_modules/boardgame.io/dist/server.js:2620:28)
    at Object.ProcessMove (/Users/oliverthebault/projets_perso/coinche/server/node_modules/boardgame.io/dist/server.js:3195:12)
    at /Users/oliverthebault/projets_perso/coinche/server/node_modules/boardgame.io/dist/server.js:4602:29
    at Object.dispatch (/Users/oliverthebault/projets_perso/coinche/server/node_modules/boardgame.io/dist/server.js:4915:22)
    at Master.onUpdate (/Users/oliverthebault/projets_perso/coinche/server/node_modules/boardgame.io/dist/server.js:5248:13)
    at processTicksAndRejections (internal/process/task_queues.js:94:5)
    at Socket.<anonymous> (/Users/oliverthebault/projets_perso/coinche/server/node_modules/boardgame.io/dist/server.js:5480:15)
(node:34243) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)

no luck :/

note that calling this deal function works fine in phases.Talk.onBegin

@Oliboy50 Good work getting it working in the onBegin hook! It isn’t possible to mutate state in the endIf hook, because it is only supposed to check if the phase should end. That call to deal would have to be in onEnd or at the beginning of the next phase.

Try to make at least one of the phases to be ended by a move started by the player. Also end phases by providing and endIf condition, instead of using endPhase. I’m doing that and it’s working fine. Some phases end automatically and others by player’s moves.

`

PlayCards: {
  moves: {
    sayAnnounce,
    playCard,
  },
  endIf: (G, ctx) => {
    Return G.player1.playedCard && G.player2.playedCard; // state altered by move made by player
  },
  next: ‘Deal’ // setting which phase should start next
},

`

I can't use next: 'MyPhase' most of the time, because the next phase depends on the G state

however, it correctly goes to the defined static next phase, when I use it

but when I use ctx.events.setPhase('MyPhase') (without defining a next: 'MyPhase'),
it correctly ends the phase (I know it because it goes to the corresponding onEnd() hook),
but after the onEnd() hooks has been executed,
ctx.phase is equals to null

Note: don't know if it helps but I'm building a multiplayer (using the Server API) game 🤷‍♂
Note2: I've tried both to call ctx.events.setPhase('MyPhase') from endIf() and to call ctx.events.setPhase('MyPhase') from onEnd(), both ends with ctx.phase === null

I wonder if the framework should hold the responsibility of preventing the user from doing something which might end badly 🤔

In other words, I'm not sure if this check should even exist: https://github.com/nicolodavis/boardgame.io/blob/d05e7a7de5f019a7f57a5c3219100b1785f1654d/src/core/flow.ts#L221-L234

I'm just trying to understand why setPhase does not work as expected in my case...

I've finally managed to stop relying on ctx.events (especially ctx.events.setPhase which is currently a PITA):

before:

{
  SomePhase: {
      onBegin: (G, ctx) => {
        ctx.events.setPhase('OtherPhase');
      },
  }
}

after:

{
  SomePhase: {
      onBegin: (G) => {
        G.__forcedNextPhase = 'OtherPhase';
      },
      endIf: (G) => {
        return G.__forcedNextPhase ? { next: G.__forcedNextPhase } : false;
      },
      onEnd: (G) => {
        G.__forcedNextPhase = undefined;
      },
  }
}

using endIf: () => ({ next: 'OtherPhase' }) notation is fully working 🙆‍♂

so I was able to migrate from 0.32.1 to 0.37.2 🎉
it was painful, but I'm happy to benefit from the new features

Glad you’ve got it working! For now, I think we should add some warning to avoid the setPhase and endPhase events in phase hooks. My earlier advice was bad because setPhase won’t work in an onEnd hook either (because at the point, the current phase is already ending, so setPhase will again break). Eventually, perhaps we should also revisit all the checks around processing these events.

Hello, I kinda have the same problem

My game is cut into 4 phases, and I just want the phase to loop until action is needed (here in investigation phase). Why does this code don't work ?
When I try with the debugger, the endPhase/setPhase doesn't work.
If I click on endPhase it'll just go to the next phase without executing the onBegin. (for ex, clicking next phase on Investigation should trigger 3 phases then come back to investigation but it just stays at enemy phase.)

    phases: {
      mythos: {
        next: 'investigation',
        onBegin: (G, ctx) => {
          if (G.round === 1) {
            ctx.events.endPhase();
            return ;
          }
          G.currentAgenda.doom++;
          ctx.events.endPhase();
        },
        start: true,
      },
      investigation: {
        next: 'enemy',
      },
      enemy: {
        next: 'upkeep',
        onBegin: (G, ctx) => {
          ctx.events.endPhase();
        },
      },
      upkeep: {
        next: 'mythos',
        onBegin: (G, ctx) => {
          G.round++;
          ctx.events.endPhase();
        },
      },
    }

@GemN Looks like you’re using the mythos, enemy, and upkeep phases only to do some state set-up. (I’m guessing your actual code is more complex, but still.) I can see why you’d expect this to work, but phases are intended to be used to enable specific move and turn set-ups rather than just for modifying your state like this.

I would move all of the logic you’re using in those three stages to an internal function and run them all in the investigation phase’s hooks, e.g.:

phases: {
  investigation: {
    next: 'investigation',
    onBegin: (G, ctx) => {
      mythos(G, ctx);
    },
    onEnd(G, ctx) => {
      enemy(G, ctx);
      upkeep(G, ctx);
    },
  },
}

@delucis that's an early example, sometime these phases won't need any actions from the players, but sometimes they will need players to do something.
I think the conditional checking of infinity loop is a bit too restrictive and should be the responsibility of the developper because in this case it really feel like a work-around where I'll need to use stages to do things.

Hmm, you’re probably right. The workaround is to do conditional logic before ending the phase, but it can get tangled pretty quickly.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

nicolodavis picture nicolodavis  ·  4Comments

SaFrMo picture SaFrMo  ·  7Comments

nicolodavis picture nicolodavis  ·  3Comments

Pong420 picture Pong420  ·  9Comments

alexwoodsy picture alexwoodsy  ·  7Comments