Boardgame.io: Suggestion to improve type definitions for PlayerAPI

Created on 7 Apr 2020  ·  10Comments  ·  Source: boardgameio/boardgame.io

Problem I'd like to address

Currently the PlayerAPI is defined like so

export interface PlayerAPI {
  state: Record<PlayerID, any>;
  get(): any;
  set(value: any): any;
  opponent?: {
    get(): any;
    set(value: any): any;
  };
}

And is by default a part of context as an optional property

export interface Ctx {
    ...
    player?: PlayerAPI
}

I understand the logic here - the plugin api isn't defined unless the user's implemented it. But can cause a few issues:

  • ctx is unnecessarily bloated with definitions that may not apply
  • If the API is implemented the implementor now has to perform typechecking to keep typescript happy. For example ctx.player.get() will throw a typescript error complaining that player is possibly undefined.
  • player state isn't strongly typed

Proposal for solution

I propose instead that player is not (by default) part of the GameConfig interface but can be added by developers that implement it on their own. Additionally, it would be neat if it took a type parameter for the player state.

Something like this:

export interface PlayerAPI<TPlayerState = any> {
  state: Record<PlayerID, TPlayerState >;
  get(): TPlayerState ;
  set(value: TPlayerState ): TPlayerState ;
  opponent?: {
    get(): TPlayerState ;
    set(value: TPlayerState ): TPlayerState ;
  };
}

Then a developer could do something like this:

interface MyPlayerState {
    teamID: string
}

interface MyGameConfig extends GameConfig {
    player: PlayerAPI<MyPlayerState>
}

const MyGame: MyGameConfig = {
    moves:  {
        drawCard: (G, ctx) => {
            const player = ctx.player.get();
            const teamID = player.teamID; // strongly typed!
        }
    }
}

All 10 comments

It would actually be pretty neat if GameConfig had a type parameter too for game state. Then a ton of stuff could be strongly typed. Same principle

export interface GameConfig<TGameState = any> {
    ...
    endIf?: (G: TGameState , ctx: Ctx) => any;
    onEnd?: (G: TGameState , ctx: Ctx) => any;
    playerView?: (G: TGameState , ctx: Ctx, playerID: PlayerID) => any;
}

@crhistianramirez These are good proposals. Linking an earlier discussion along these lines: https://github.com/nicolodavis/boardgame.io/pull/579#issuecomment-603736732

@crhistianramirez I really like the idea of passing in the type of the user's game state as a template parameter. Would you have time to work on a PR for this?

@nicolodavis yes I can take a crack at it tonight!

Just so we're all clear on the scope of this PR. Here is what I plan on doing (feel free to add or remove from this list as you see fit)

  • Refactor Game interface to receive an optional type parameter TGameState and pass it down to anything that consumes it.
  • Refactor Ctx so the plugins are not part of the interface (can be added later by developers)
  • Refactor PlayerAPI to take in an optional type parameter TPlayerState

@crhistianramirez That sounds good to me!

Some extra thoughts, if you have the time:

  • Perhaps move the default plugins (events and random) to a DefaultPlugins interface and allow Ctx to take a type parameter too, e.g.:

    type DefaultCtx = Ctx<DefaultPlugins>;
    
  • Game could also have a second optional type parameter for its context, so you could type plugins available in ctx (this is a little more work, and it can also be added later if it’s too much for this PR):

    type CtxWithPlugins = Ctx<DefaultPlugins & MyPlugin>;
    
    const game: Game<MyG, CtxWithPlugins> = {
      name: 'my-game',
      setup: ctx => {
        // will error and autocomplete for plugin APIs
      },
    };
    
  • In general the types have not used T or I prefixes, so the type parameters can probably be G and C, i.e.:

    interface Game<G extends any = any, C extends Ctx = Ctx> {}
    
    

    or PlayerState/State in the player plugin.

Perhaps move the default plugins (events and random) to a DefaultPlugins interface

Is it common to use all default plugins? Alternatively we could simply create each Plugin interface and they could be composed in a similar way to your example.

type MyCtx = Ctx<PlayerPlugin & EventsPlugin & MyPlugin>

Game could also have a second optional type parameter for its context

I like this, I think that would be very useful so I'll plan to include it.

In general the types have not used T or I prefixes,

Roger that

As I'm thinking about it more this is probably something that should be documented somewhere as well. I'm not the best copywriter but I'd be willing to take first pass at docs. Could also be a separate task. Thoughts?

Is it common to use all default plugins?

The Events & Random plugins are always included automatically in every game set-up, so these would be the default plugins. Player has to be explicitly added, so it should be separate.

Actually, given that Events & Random are always included, maybe it would be better if the Ctx exposed to programmers already includes their APIs so Ctx<PlayerPlugin> would include them and no need to remember the DefaultPlugins interface. What do you think?

this is probably something that should be documented somewhere as well

There should probably be a “Using with Typescript” guide at some point yes. It might be a little early as TS has only been supported as of a few weeks ago and things are likely to still change, but you could certainly make a start.

so Ctx would include them and no need to remember the DefaultPlugins interface

Yeah sounds good to me, simpler is definitely better. I'll plan on that.

things are likely to still change

That's a good point, I'll hold off for now then.

Thanks for the feedback! <3

Closing this as PR has been merged

@crhistianramirez Thanks again for your work on this!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

eseom picture eseom  ·  4Comments

amerine picture amerine  ·  5Comments

nicolodavis picture nicolodavis  ·  3Comments

alexwoodsy picture alexwoodsy  ·  7Comments

yangboz picture yangboz  ·  5Comments