boardgameio / boardgame.io

State Management and Multiplayer Networking for Turn-Based Games
https://boardgame.io
MIT License
10.03k stars 709 forks source link

moves not changing G if in stage #486

Closed MichalPP closed 5 years ago

MichalPP commented 5 years ago

I have a very simple game: roll dice (1-6) then move by 1-6 positions.

I created two moves (rollDice and move) and two stages (stageDice and stageMove). works fine.

however, if I restrict stages to use only some moves, the moves don't work. so

turn: { stages: stageMove{ } } works fine (G is changing), but turn: { stages: stageMove{ moves: { move: G => G } } } does not (G remains unchanged).

any thoughts?

nicolodavis commented 5 years ago

It would be much easier to debug if you provide a https://codesandbox.io/ link.

delucis commented 5 years ago

@MichalPP Do you have some more detailed code? The following should work:

turn: {
  stages: {
    stageDice: {
      moves: {
        roll: (G, ctx) => {
          G.rollResult = ctx.random.D6()
        }
      }
    },
    stageMove: {
      moves: {
        move: G => {
          G.position = G.position + G.rollResult // or whatever
        }
      },
    },
  }
}

You do need to make sure the player is actually in one of the stages, otherwise there are no moves for them to make. The following would set the player to stageDice at the start of each turn:

turn {
  activePlayers: { player: 'stageDice' }
}
MichalPP commented 5 years ago

I had to keep the moves in moves section (I reuse them in different stages). I suspect the form of restricting the moves in stages is causing problems (@delucis format works):

moves: {
    moveRollDice: (G, ctx, id) => {
        G.dice=ctx.random.D6();
    },
turn: {
    stages: {
        rollDice: { moves: { moveRollDice: G => G }
   }
}

If you don't see any error in the above syntax, I will strip out a minimalistic demo of my game (currently it's bit longer).

delucis commented 5 years ago

If you need the moves for different stages, the best approach is to declare the function once and re-use it:

const moveRollDice = (G, ctx) => {
  G.dice = ctx.random.D6();
}

const game = {
  moves: { moveRollDice },
  turn: {
    stages: {
      rollDice: {
        moves: { moveRollDice },
      }
    }
  }
}

If you’re still having trouble, a minimal demo would be great.

MichalPP commented 5 years ago

@delucis thanks, it's working like charm.

I will create some PR for documentation.

nicolodavis commented 5 years ago

What specifically is missing in the documentation that you want to fix?

MichalPP commented 5 years ago

I was working with tic-tac-toe tutorial (which is excellent, thank you). there the moves are defined within the moves section of the game.

in the stage documentation, there is documentation on how to restrict moves within the stage. but there is no mention that the moves must be defined as global functions, not within moves section of the game (which should just link the function, not define it).

delucis'c snipped was all that I needed to understand it.

delucis commented 5 years ago

@MichalPP Good to understand that that’s not clear. I’m not sure your changes in #491 are the best way to go. I think we need to clarify that moves inside a stage are functions not references by name to the main moves section.

The moves inside a stage don’t have to be global, it’s just that if you’re using a move across different places, it’s convenient to declare the function once and re-use it (and for most moderately complex games, importing those functions from separate files helps keep the code more organised).

Both the following work and are equivalent:

// re-use a function
const moveRollDice = (G, ctx) => {
  G.dice = ctx.random.D6();
}

const game = {
  moves: { moveRollDice },
  turn: {
    stages: {
      rollDice: {
        moves: { moveRollDice },
      }
    }
  }
}
// declare a function each place it’s needed
const game = {
  moves: {
    moveRollDice: (G, ctx) => {
      G.dice = ctx.random.D6();
    },
  },
  turn: {
    stages: {
      rollDice: {
        moves: {
          moveRollDice: (G, ctx) => {
            G.dice = ctx.random.D6();
          },
        },
      }
    }
  }
}

Maybe the wording of the following would be a good place to clarify?

https://github.com/nicolodavis/boardgame.io/blob/e5fd40b16a961e026f451bf07fada6f2e1864ff8/docs/documentation/stages.md#L48

MichalPP commented 5 years ago

yes, clarify that stage-moves are functions not name references.

btw, i've spent some time trying to reference from stage-moves with links like this.game.props.moves.rollDice and similar. of course, it did not work :)

knowing that copy/pasting context of a function into two places is bad, i would just stress the first example in the documentation.

MichalPP commented 5 years ago

@nicolodavis maybe documentation like: if you have moves defined within stages, you can remove global moves definition.

the doc part saying This moves section completely overrides the global moves section for players in that stage (players are not allowed to make any moves from the global moves section while they are in that stage). implied to me, that each stage-move has to be a global move as well (and stage-moves just define which moves are allowed in given stage).