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

Curious Flow: Second phase's onTurnBegin() is not calling #394

Closed
ybbarng opened this issue May 10, 2019 · 19 comments
Closed

Curious Flow: Second phase's onTurnBegin() is not calling #394

ybbarng opened this issue May 10, 2019 · 19 comments

Comments

@ybbarng
Copy link

ybbarng commented May 10, 2019

Hello, I'd like to use your impressive framework, but I met a strange situation.

Here's my simplified code:
https://github.com/ybbarng/boardgame-io-test/blob/master/src/App.js

In phase1, each turn is automatically ended because endTurn() is called in onTurnBegin().
Because the phase1's TurnOrder is ONCE, the phase will be moved to next phase2.
At this time, onTurnBegin() of phase2 is not calling.
I hope someone figure out my mistake or explain why this happens.

Context when it stuck,

The log of console:

phase1 onPhaseBegin
phase1 onTurnBegin 0
phase1 onTurnEnd 0
phase1 onTurnBegin 1
phase1 onTurnEnd 1
phase1 onPhaseEnd
phase2 onPhaseBegin

The debug pannel:

G: {}
ctx: {
  "numPlayers": 2,
  "turn": 2,
  "currentPlayer": "0",
  "actionPlayers": [
    "0"
  ],
  "currentPlayerMoves": 0,
  "playOrder": [
    "0",
    "1"
  ],
  "playOrderPos": 0,
  "stats": {
    "turn": {
      "numMoves": {},
      "allPlayed": false
    },
    "phase": {
      "numMoves": {},
      "allPlayed": false
    }
  },
  "allPlayed": false,
  "phase": "phase2",
  "prevPhase": "phase1",
  "allowedMoves": null
}
@nicolodavis
Copy link
Member

This is a bug that will be fixed as part of the Phases overhaul described here.

@nicolodavis
Copy link
Member

I'm working on this presently, so it should be fixed in the next couple of weeks or so.

@JosefKuchar
Copy link

Any updates on this or the phases overhaul?

@nicolodavis
Copy link
Member

nicolodavis commented Jul 15, 2019

Still about a couple of weeks out. Was traveling for a bit so this got dropped.

@gadamgrey
Copy link

Hi, I am running into a similar issue and was wondering if the new phases structure will be coming soon. Thanks in advance!

@nicolodavis
Copy link
Member

It's ready in the phases-overhaul branch of you'd like to test it. I need to work on the documentation and polish it a bit before I release it.

@gadamgrey
Copy link

I did try the branch out but I'm still experiencing the same problem.

Essentially I'm trying to have a setup phase that happens at the beginning of a 2 player game. Each player can make a setup "move" or pass. Then there is a main phase that alternates between players until the game is over. The problem is I'm never seeing ctx.currentPlayer to change to the second player and I can't really figure out when it is supposed to change. I've tried calling endPhase, endTurn, and the endPhaseIf/endTurnIf hooks.

Is this different enough that I should open a new issue? Or is there some quick guidance you could give that could clear up my understanding of when currentPlayer should change? Thanks!

@delucis
Copy link
Member

delucis commented Aug 27, 2019

@gadamgrey Do you have some code you could share?

Usually, ctx.currentPlayer should change when a turn ends (as long as your turn order isn’t set to something that prevents that).

With the phases-overhaul branch, the game you describe might look something like this (N.B. some of this API may change, see for example #445 and #446):

import { ActivePlayers } from 'boardgame.io/core'

const game = {
  phases: {
    setup: {
      start: true,                              // the game should start in this phase
      turn: {
        activePlayers: ActivePlayers.ALL_ONCE   // all players can play once
      },
      moves: {
        setupMove: (G, ctx) => {},
        pass: G => G
      },
      endIf: (G, ctx) => ctx.activePlayersDone, // end phase when each player has played
      next: 'main',                             // move to “main” stage on end
    },

    main: {
      turn: {
        endIf: (G, ctx) => ctx.numMoves >= 1    // end turn after one move
      },
      moves: {
        // ...
      },
    },
  },
}

The setup phase makes use of the new activePlayers API to allow all players to play (in any order). The ALL_ONCE preset removes each player from activePlayers as each plays, so we can check ctx.activePlayersDone in setup.endIf to decide when to end the phase.

During the main phase, we check if a move has been made in main.turn.endIf and end the turn if it has. Alternatively, you could also check for some specific state before ending the turn or call events.endTurn from a move to end the turn. Because main.turn.order isn’t set, boardgame.io will default to TurnOrder.DEFAULT, which will increment ctx.currentPlayer at the end of each turn.

Again, this API is still in development and might change, but hope this helps!

@gadamgrey
Copy link

@delucis that snippet is very helpful. I don't have code to share at the moment but if I still get stuck I will prepare an example. Thank you!

@gadamgrey
Copy link

@delucis I think one of things I'm having trouble with is actually referencing the branch outside of npm. Using npm -i git://github.com/nicolodavis/boardgame.io.git#phases-overhaul is giving me errors. So I cloned the repo and branch and was trying npm link to reference it. That seemed to work at first but I think it's still pointing to the registry version v0.32.1 because it cannot find ActivePlayers in boardgame.io/core.

Do you have a recommended way of referencing your in progress branch in another project?

@nicolodavis
Copy link
Member

@delucis You can use moveLimit: 1 for the "main" phase to make the code more idiomatic:

import { ActivePlayers } from 'boardgame.io/core'

const game = {
  phases: {
    setup: {
      start: true,                              // the game should start in this phase
      next: 'main',                             // move to “main" after this phase

      turn: {
        activePlayers: ActivePlayers.ALL_ONCE   // all players can play once
      },

      moves: {
        setupMove: (G, ctx) => {},
        pass: G => G
      },

      endIf: (G, ctx) => ctx.activePlayersDone, // end phase when all players have played

    },

    main: {
      turn: {
        moveLimit: 1    // end turn after one move
      },

      moves: {
        // ...
      },
    },
  },
}

@gadamgrey What errors are you seeing when you run npm -i git://..?

@delucis
Copy link
Member

delucis commented Aug 28, 2019

@gadamgrey The library needs building if you’re using a git clone:

git clone https://github.com/nicolodavis/boardgame.io.git
cd boardgame.io
npm i
npm run prepack    # builds and exposes bindings
npm link
cd /path/to/project
npm link boardgame.io

Might be wise to run npm un boardgame.io in your project before linking just in case, but I think the above should work.

@gadamgrey
Copy link

npm run prepack was what I was missing, thanks.

FWIW here is the error I was seeing trying to reference the git branch directly. This was after installing cross-env globally:

> cross-env BABEL_ENV=rollup rollup --config rollup.npm.js

events.js:186
      throw er; // Unhandled 'error' event
      ^

Error: spawn rollup ENOENT
    at Process.ChildProcess._handle.onexit (internal/child_process.js:264:19)
    at onErrorNT (internal/child_process.js:456:16)
    at processTicksAndRejections (internal/process/task_queues.js:77:11)
Emitted 'error' event on ChildProcess instance at:
    at Process.ChildProcess._handle.onexit (internal/child_process.js:270:12)
    at onErrorNT (internal/child_process.js:456:16)
    at processTicksAndRejections (internal/process/task_queues.js:77:11) {
  errno: 'ENOENT',
  code: 'ENOENT',
  syscall: 'spawn rollup',
  path: 'rollup',
  spawnargs: [ '--config', 'rollup.npm.js' ]
}
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! boardgame.io@0.32.1 build: `cross-env BABEL_ENV=rollup rollup --config rollup.npm.js`
npm ERR! Exit status 1

@delucis
Copy link
Member

delucis commented Aug 28, 2019

@gadamgrey I think the error is because rollup isn’t available globally. To avoid the global installs you could use npx.

@gadamgrey
Copy link

gadamgrey commented Aug 28, 2019

Expanding from my example earlier, let's say I wanted the main phase to be split into two. Player 0 begins main phase A. They do an action and move to main phase B. After they do an action and pass I want to return flow to main phase A as Player 1.

Right now when I return to main phase A, the current player is still 0.

And expanding that further let's say I want to implement a cleanup phase where if a player has too many cards in their hand they will have to discard. Otherwise they will automatically end their turn and return to main phase A with their opponent being the current player.

Any advice on how to accomplish that? I won't be able to share example code until tonight (US Eastern time).

@nicolodavis
Copy link
Member

If you ever want to subdivide a turn into multiple "phases", you should use stages. phases are for general game phases inside which multiple turns are played. stages are different parts of a single turn.

import { ActivePlayers } from 'boardgame.io/core'

const game = {
  turn: {
    activePlayers: {
      currentPlayer: 'A',  // put the player in Stage A at the beginning of the turn
    },

    stages: {
      A: {
        moves: {
          moveA: (G, ctx) => {
            ...

            // move the player to Stage B.
            ctx.events.setActivePlayers({ currentPlayer: 'B' });
          },
        },
      },

      B: {
        moves: { ... },
      },
    }
  }
}

@gadamgrey
Copy link

Are there hooks for onStageBegin and onStageEnd like there are for phases? For some stages I will need the game to do some automated activities that can move things along without a player making any moves (like drawing a hand, or applying effects that trigger at different points in the game).

@nicolodavis
Copy link
Member

nicolodavis commented Aug 29, 2019

I deliberately left these out because of the following reasons:

  1. turn.onBegin and phase.onBegin refer to single events that happen deterministically, and the game can only be in one phase or turn at a given time.

  2. When it comes to stages, multiple stages can be active at the same time, and at that point the order in which these hooks trigger can matter, which can confuse users.

We can explore doing something like always triggering them in increasing order of playerID or something like that, but in general I'd prefer to leave them out altogether if possible.

@nicolodavis
Copy link
Member

Closing this now that v0.33 is out. Feel free to reopen if you find that the issue you're dealing with still persists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants