feat: Change move and hook signature

[delucis]

📣 Try the pre-release today 📣

You can start working with the new hook signature by installing the alpha release channel from npm:

npm i boardgame.io@alpha

If you run into any issues, let us know!

---

Closes #662.

This PR implements the change from (G, ctx) to ({ G, ctx, playerID, ...plugins }).

Game code that currently looks like this:

const move = (G, ctx, arg) => {
  const score = arg * ctx.random.D6();
  G.players[ctx.playerID] = score;
  if (score < 12) ctx.events.endTurn();
};

Would become:

const move = ({ G, random, playerID, events }, arg) => {
  const score = arg * random.D6();
  G.players[playerID] = score;
  if (score < 12) events.endTurn();
};

This applies across moves and hooks.

To-do

The following parts of game code haven’t been changed yet:

1. playerView is currently still (G, ctx, playerID) => G. Should this become ({ G, ctx, playerID }) => G?

2. I haven’t updated the bot interface at this point. Should the following be changed for consistency?

   • enumerate(G: any, ctx: Ctx, playerID: PlayerID)

   • Various other methods in the MCTS bot implementation

3. The undoable method of the long-form move syntax, which can be (G, ctx) => boolean. Should this become ({ G, ctx }) => boolean?

Notes

• In refactoring tests, I noticed that this change often results in (to my mind) clearer game code and will hopefully help to clear up ongoing confusion about ctx being different in moves and on the client. Types are also greatly improved.

• One thing this helped clarify for me, was that playerID (currently ctx.playerID) is only available in moves, not in any hooks. Given the slightly confusing nature of playerID in hooks, this may not be a bad thing. For example, it’s not necessarily easy to understand why playerID in turn.onBegin is the ID of the player whose action caused the previous turn to end, not the ID of the player whose turn is beginning. That said, if we add onEnd/onBegin events for stages as proposed in onBegin and onEnd for Stages #608, there will be a need to revisit this in the future.

• While updating the docs, I noticed it wasn’t very clear what to call the whole object that contains G, ctx etc. I think of this whole lot as “context” for lack of a better word, but that would be confusing as ctx is already in use. I wonder if in fact, this refactor might be an opportunity to rename ctx itself to something more descriptive. Not sure what that name would be, but flow comes to mind.

  (context: { G: any; flow: Flow; ... }) => {
    const { G, flow } = context;
    console.log(flow.turn, flow.currentPlayer);
    if (!flow.activePlayers) { /* ... */ }
  }

• I’m not sure when will be best to merge and release this as obviously it’s a significant breaking change for everyone, but I wanted to get the ball rolling and we can keep this PR up-to-date as other changes arrive on the main branch.

Migration Paths

This change is pretty impactful across game definitions. Here are some migration strategies.

1. Rewrite your functions

Ideally, you would rewrite your game to use the new syntax. In general this will result in simpler code. For example:

// Destructure G, ctx, and plugin APIs from the first argument
- function move(G, ctx, arg) {
+ function move({ G, random, playerID }, arg) {

// Access plugin APIs directly instead of from ctx
-   G.diceRoll = ctx.random.D6();
+   G.diceRoll = random.D6();

// Access playerID directly instead of from ctx
-   G.players[ctx.playerID].score += G.diceRoll;
+   G.players[playerID].score += G.diceRoll;

}

2. Use a stop-gap plugin for compatibility

If you don’t want to rewrite your moves, you can use a custom plugin to wrap those functions so they continue to receive the old style of arguments.

const CompatibilityPlugin = {
  name: 'compat-plugin',

  fnWrap: (fn) => ({ G, ctx, ...api }, ...args) => {
    const ctxWithAPI = { ...ctx, ...api };
    return fn(G, ctxWithAPI, ...args);
  },
};

const game = {
  plugins: [CompatibilityPlugin],

  moves: {
    oldStyle: (G, ctx) => {
      // Use ctx.random etc. as before
    },
  },
};

3. Gradual migration

Using a plugin is an all-or-nothing approach — it forces you to continue using the old-style of arguments. If you want to gradually migrate your code, you’ll need to wrap only functions you haven’t updated yet. To do this we could adapt the fnWrap method from above to selectively wrap certain moves.

// fnWrap adapted to account for long-form move objects, which
// boardgame.io does automatically when applying plugin wrappers
const moveAdapter = (moveFnOrObj) => {
  const isLongFormMove = typeof moveFnOrObj.move === 'function';
  const fn = isLongFormMove ? moveFnOrObj.move : moveFnOrObj;
  const move = ({ G, ctx, ...api }, ...args) => {
    const ctxWithAPI = { ...ctx, ...api };
    return fn(G, ctxWithAPI, ...args);
  };
  return isLongFormMove ? { ...moveFnOrObj, move } : move;
};

const game = {
  moves: {
    oldStyle: moveAdapter((G, ctx, arg) => {
      // Use ctx.random etc. as before
    }),
    newStyle: ({ G, ctx }, arg) => {},
  },
};

Depending on how your project is structured, you may be able to simplify applying this adapter rather than doing it manually for each function, for example if you store your moves something like this:

const moves = { rollDice, moveToken, capture, build };

You could apply the adapter programmatically:

for (const key in moves) {
  moves[key] = moveAdapter(moves[key]);
}

Then you could migrate by splitting your moves into two collections:

const oldstyleMoves = { rollDice, build };
const migratedMoves = { moveToken, capture };

for (const key in oldstyleMoves) {
  oldstyleMoves[key] = moveAdapter(oldstyleMoves[key]);
}

const moves = { ...oldstyleMoves, ...migratedMoves };

General Tips

Use Typescript to help find errors

If you’re already using TypeScript, make sure you use boardgame.io’s bundled types for your game to catch changes:

import type { Game, Move } from 'boardgame.io';

interface G {
  score: number;
}

const game: Game<G> = { /* ... */ };
const rollDice: Move<G> = ({ G, random }) => { /* ... */ };

If you’re writing plain JavaScript, you can often still use TypeScript types to get some in-editor hints by using Typescript’s JSDoc syntax (VS Code does this well for example).

/**
 * @typedef {{ score: number }} G
 */

/**
 * @type {import('boardgame.io').Game<G>}
 */
const game = { /* ... */ };

/**
 * @type {import('boardgame.io').Move<G>}
 */
const rollDice = ({ G, random }) => { /* ... */ };

[nicolodavis]

We can probably punt on bringing the bot code to be consistent with the new format, but I think having playerView and undoable adopt the new format right away would prevent bugs in user code when we release this.

[nicolodavis]

I do agree with the point that it probably makes sense to call the aggregate (G + ctx + ...) as Context, which begs the question about what to do to ctx.

Do you think ctx: GameContext is a suitable name combination? I'm less convinced by flow.

[delucis]

I’ll try to think further on what it might be named. I suppose I’m trying to avoid context.ctx as it seems rather confusing and vague to have a context inside a context. Some other options to brainstorm from (but something terser would be better): playState, flowState, engineState. I personally like how flow combines with the various members, e.g. flow.currentPlayer, flow.playOrder, flow.phase etc. while not being too verbose, but perhaps there’s something better.

That said, G + ctx is a pretty core concept for current users, so I could also definitely understand keeping it as is if we can’t find anything better.

[vdfdev]

I dont think this is a major problem, because the Context in this case is only used internally. Most of our docs were updated to use the object destructuring assignments, so it doesnt look confusing for users.

[YoavHortman]

Will this be merged?
This is a huge improvement over the current state of things.

[vdfdev]

I received feedback that people really want this and are already using the alpha. I vote for finishing reviewing/merging all other outstanding PRs and doing a minor release. After that, we merge this PR and do a major release. What do you think, @delucis ?

[delucis]

Major would mean [email protected]. Not totally opposed but I think we could also release this as a minor (0.50.0 was my target for merging this) — minors below v1 are treated as major semver by npm, so it’s OK to introduce breaking changes from minor to minor.

So if we have stuff we want to get out before this, that could be 0.50.0 and then this as 0.51.0 or 1.0.0 if we’re ready! (One thing I had always thought needed work prior to v1 was the bot/AI architecture, but improving that can be a v2 goal too if we want).

[vdfdev]

Oh ok, I don't mind either way, as long as we do a release before this PR goes in, because it might take a while to redactor all games to it. Tonight I will try to review your code, and then we can proceed!

[delucis]

Sounds good. Thanks @vdfdev! 🙌

[vdfdev]

Uh oh, I think I borked the merge, will revert and fix it.

[vdfdev]

Seems like aa99a9c conflicted with this PR. I was able to fix one test, but there is one that still fails. I will keep trying to fix it

[on3iro]

@delucis Hey, I have a pretty huge bgio codebase and after the update got over 500 typescript errors.
I wanted to use the stop-gap-plugin described above, but as far as I can tell this will only cover the issue at runtime. Typescript will still complain about incorrect signatures. Is there a way to fix this or am I stuck with either fully migrating or staying with a version <50?