Skip to content

feat(core): Expose ctx.playerID in onMove hook#1019

Merged
delucis merged 2 commits into
boardgameio:mainfrom
pardahlman:on-move-player-id
Oct 16, 2021
Merged

feat(core): Expose ctx.playerID in onMove hook#1019
delucis merged 2 commits into
boardgameio:mainfrom
pardahlman:on-move-player-id

Conversation

@pardahlman
Copy link
Copy Markdown
Contributor

This PR closes #1018 by adding playerID to the ctx used in the onMove hook.

When setting up my local environment, some tests failed on main - it looks like it is related to TypeScript, api.ts and possibly koa somehow 🤔 Attaching example output from a failed test

 FAIL  src/server/index.test.ts
  ● Test suite failed to run

    src/server/api.ts:151:28 - error TS2339: Property 'toLowerCase' does not exist on type 'string | string[]'.
      Property 'toLowerCase' does not exist on type 'string[]'.

In order to create this PR, I pushed the branch with --no-verify, hence the WIP prefix 😬

Checklist

  • [x ] Use a separate branch in your local repo (not main).
  • [ x] Test coverage is 100% (or you have a story for why it's ok).

This makes it possible to identify which player
carried out the move and possiblt act on it (e.g.
moving player to different stage).

Closes boardgameio#1018
Copy link
Copy Markdown
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

(That TS error is definitely not related to these changes. I’m guessing somehow you overrode the package-lock and installed a more recent version of @types/koa, which now types query.)

@delucis delucis changed the title WIP: Add playerID to onMove hook feat(core): Expose ctx.playerID in onMove hook Oct 16, 2021
@delucis delucis merged commit ff4c756 into boardgameio:main Oct 16, 2021
@pardahlman pardahlman deleted the on-move-player-id branch October 17, 2021 07:07
delucis added a commit that referenced this pull request Nov 5, 2021
Addresses a regression introduced in #1019, which passed a stale version of `ctx` when calling `onMove` hooks. This was particularly noticeable if the move triggered the end of a player’s stage, which the `onMove` hook would then not see.
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

Successfully merging this pull request may close these issues.

Missing playerID in ctx onMove callback

2 participants