boardgame.io icon indicating copy to clipboard operation
boardgame.io copied to clipboard

Illegal moves API

Open Izhaki opened this issue 7 years ago • 23 comments

Between the problem and proposed solution below, the former is more important - someone else might come up with a better solution.

Background

Some moves are illegal. The logic behind an illegal move depends on any combination of these:

  • G
  • ctx
  • Additional arguments passed

Note that it may be desired to know if a move is legal before actually attempting to dispatch it. Examples:

  • A poker client disabling some buttons.
  • A player getting an immediate feedback that a stone cannot be place in a particular position in a game of GO.

Also note that in the last example some reason might be useful ("Stone will have no liberties"). This is to highlight that just returning a boolean may not suffice.

Currently

Client

Here's an example from the docs:

class TicTacToeBoard extends React.Component {
  onClick(id) {
    if (this.isActive(id)) {
      this.props.moves.clickCell(id);
      this.props.events.endTurn();
    }
  }

  isActive(id) {
    if (!this.props.isActive) return false;
    if (this.props.G.cells[id] !== null) return false;
    return true;
  }

  ...

}

But this doesn't prevent a hacker dispatching an illegal move to the server. So...

Server

A move function that returns undefined denotes an illegal move:

  moves: {
    clickCell(G, ctx, id) {
      if (G.cells[id] !== null) return undefined;
      ...
    },
  },

Note that you cannot invoke a move to check if it's illegal - if it is legal, it will be executed; so the check needs to live elsewhere.

In other words, the fact that one can return undefined from the move function fuses together two separate concerns:

  • The execution of the move.
  • The legal check.

The issue

There's potential repetition here. Clean coders will extract the related logic into a function; something like:

const isCellFree = (G, ctx, id) => G.cells[id] === null;

And will use this in both client (react) and game config.

This is a simple case. If such function needs to return a reason it may involve quite a few checks with some games.

Proposed solution

I believe that the logic of weather a move is illegal should live as close as possible to the move definition itself.

So how about:

  moves: {
    clickCell: {
      reduce: (G, ctx, id) => {
        const cells = [...G.cells];
        cells[id] = ctx.currentPlayer;
        return { ...G, cells };
      },
      isValid: (G, ctx, id) => G.cells[id] === null,
    },
  },

Notes:

  • Internally, the framework will call isValid (if provided) before calling reduce.
  • Perhaps allow both current and proposed versions (for cases where isValid is not needed).
  • Client API will be moves.clickCell(id) and moves.clickCell.isValid(id)

Revision

Consider:

const clickCell = (G, ctx, id) => {
  const cells = [ ...G.cells ];
  cells[id] = ctx.currentPlayer;
  return { ...G, cells };
};

clickCell.isValid = (G, ctx, id) => G.cells[id] === null;

const TicTacToe = Game({
  moves: {
    clickCell,
  },
});

Which doesn't change the current API, yet allow to plug in move validation.

Others may have better suggestions.

Izhaki avatar Oct 19 '18 22:10 Izhaki

In other words, the fact that one can return undefined from the move function fuses together two separate concerns

This was actually a deliberate decision. The logic to check if a move is valid and the logic to actually execute the move often overlap. Sometimes you may need to go deep into the move (pathfinding, for example) before you realize that there isn't a valid outcome. In these cases, it might also be expensive to both validate and run the move every time in the cases where the move is actually valid.

I agree with you that there isn't a good way to provide the UI with hints about invalid moves. The current recommended approach is to just put that logic inside your view layer.

Given that people can just factor out the IsValid logic into a clean function that can be reused in both view layer and game logic, we need to weigh the option of adding more complexity to the API as proposed here vs. merely documenting this pattern in the documentation. I'm not sure if inserting an isValid function into the API that you just call directly on the client via moves.clickCell.isValid is any better than just calling isValid directly.

nicolodavis avatar Oct 19 '18 23:10 nicolodavis

Separately, I'm not the biggest fan of returning undefined. Maybe we should throw an exception instead that can provide a more helpful message that can be surfaced on the client easily.

nicolodavis avatar Oct 19 '18 23:10 nicolodavis

Instead of thinking about validation to check whether a move is valid, I like to think about the set of valid moves. Given that set, validation becomes checking whether the move is contained, and be used by the UI to indicate valid moves. When the game logic rejects a move, there must be a bug somewhere.

Instead of throwing exceptions, I'm pretty much into using algebraic types. This could be tweaked in such way to have it cleanly integrated into e.g. TypeScript's type system. This could be hidden by some API; just like the redux action creators, for example.

Stefan-Hanke avatar Oct 22 '18 02:10 Stefan-Hanke

Maybe we should throw an exception instead...

The fundamental issue with this is that this will break the referential transparency of the function, making it impure.

Also, it is widely recommended not to substitute exceptions with return values.

There's a big debate around exceptions, which we can delve into, but the two points above will suffice for now.

The logic to check if a move is valid and the logic to actually execute the move often overlap.

Indeed. But there are also cases for checks only.

In these cases, it might also be expensive to both validate and run the move every time in the cases where the move is actually valid.

That's a good argument, but I'm out of my depths here - can you give an example? How can you be sure a move is valid without validating it (specifically in the case of authoritative server)?

Given that people can just factor out the IsValid logic into a clean function that can be reused in both view layer and game logic...

That OK, but things will get smelly in tests - if the validation and execution are not separated, your move test will always require to also handle an invalid move case. That again hints at violation of the SOC principle.

Whereas if there are two separate functions and it is the framework to call validation before execution that you do have two separate test suits for each function.

we need to weigh the option of adding more complexity to the API as proposed here vs. merely documenting this pattern in the documentation...

I agree. Inflating the API is never a good thing so there should be solid reason to do so.

Neither of the solutions I've proposed seem sufficiently elegant to me.


At any rate, I think exceptions are not the adequate solution here. If there will be no changes to the API this leaves us with:

  • The move function (a reducer really) will return undefined for invalid moves (note that this sentence makes little sense - a reducer that returns undefined).
  • The move function can call an isMoveXValid() function that will return a reason that can also be consumed by the view (the server won't be able to return reasons - it is the view code that will handle reasons by calling this function).
  • Move function tests will have to cover both valid and invalid moves, but invalid branch tests (should fail if A, should fail if B...) can simply be attached to the validation function so not to inflate the move function tests.

This may or may not be an acceptable conclusion to this issue.

Izhaki avatar Oct 22 '18 11:10 Izhaki

@Stefan-Hanke

Instead of thinking about validation to check whether a move is valid, I like to think about the set of valid moves.

That makes sense! But how do you derive such set without calling an isMoveValid of sorts behind the scenes?

Again, I feel it is important we keep (client-code/consumer) testing in mind here.

Given that set, validation becomes checking whether the move is contained, and be used by the UI to indicate valid moves. When the game logic rejects a move, there must be a bug somewhere.

That's all good, but makes an assumption that hackers will not dispatch to the server illegal moves.

In a client/server settings the server must have a way to assert a move is valid regardless of the ui. (I can easily see UIs simply disabling a control if a move is illegal without guarding the code; even if the latter is the case it is easy to hack around).

But I feel I'm just re-iterating what everyone agree with already - a proper game execution will have to involve a valid check before the move is executed, albeit the client responsibility (unless isValidMove or the set of valid moves is part of the framework in which case the framework will invoke it automatically before executing a move - the benefits of which is currently being debated).

Instead of throwing exceptions, I'm pretty much into using algebraic types.

This, in my view, is better than undefined.

It is also better than the following alternative:

return {
  error: undefined, // reason if illegal
  G
}

But only so long everyone is happy with the issues I've highlighted above related to SOC (interestingly, algebraic types is a typical hint for lack of SOC - that's what Maybes are for, but I doubt you want to go there).

Izhaki avatar Oct 22 '18 11:10 Izhaki

I don't know where you take from my comment that I didn't have testing in mind, or that the server shouldn't check whether moves are valid or not. I didn't write explicitly that my comment was about legit use cases, next time I'll be more precise.

If your last example would be typed, this would be a product type, so OK with me. But you raised an interesting viewpoint, I can't remember reading about ADTs hinting on violating SOC before. Got a link?

Stefan-Hanke avatar Oct 22 '18 15:10 Stefan-Hanke

PS Are we going off on a tangent here little relevant to this issue? Shall we move this to the chat?

@Stefan-Hanke maybe that statement is too generic, but I think it holds for our purpose.

See, for example, Clean Code by Uncle Bob, or this corresponding CheatSheet, where:

Method with Out/Ref Arguments - Prevent usage. Return complex object holding all values, split into several methods.

Also relevant:

Methods Should Do One Thing

On a more fundamental level, if a function may return more than one type, it follows that the function must be able to produce these two types based on some branch (implicit or explicit). Does it not follow that the function thus must do two things (hence two concerns)?

Looking at even an elementary function like this:

// Can you see the violation of SOC principle here?
const divide = (a,b) => {
  if (b === 0) return NaN; // or throw
  return a/b;
}

Functional purists will write (roughly):

// All functions: (a,b) → Maybe<number>
const safeDivide = pipe(ensureArgIsnNotZero(2), unsafeDivide);

(This is somewhat a misleading example: It marries return type with SOC principles. The main point is that where the first function does everything, the second example involves 3 functions each with separate concern: validation (ensureX), execution (unsadeDivide), composition (safeDevide).)


I don't know where you take from my comment that I didn't have testing in mind, or that the server shouldn't check whether moves are valid or not.

In short - I didn't. But good thing I wrote this because the point about whether or not valid moves are part of the API (validation must take place by the framework) or down to specific implementation (clients may or may not call it as part of the move function) is an important one.

Izhaki avatar Oct 22 '18 15:10 Izhaki

No we don't, and I'll stop digging into this here. I just wanted to know about your POV.

Stefan-Hanke avatar Oct 23 '18 04:10 Stefan-Hanke

So it seems that the current API is satisfactory and the mentioned proposals do not provide clear improvements. At any rate, I wouldn't mark this as a priority right now.

So Closing. Feel free to re-open.

Izhaki avatar Oct 25 '18 00:10 Izhaki

Let's keep this open until we find a good solution (or there isn't much activity on the thread for a while). This is a good discussion to have. There are quite a few themes / ideas on this thread, so let me try my best to filter it down to a few talking points:

Separation of move logic and validation logic

I think we all agree that these should live as close as possible in the source file. The current implementation keeps them as close as is theoretically possible (in the same function). This sacrifices separability, so the proposal is to do:

  moves: {
    clickCell: {
      reduce: (G, ctx, id) => {
        const cells = [...G.cells];
        cells[id] = ctx.currentPlayer;
        return { ...G, cells };
      },
      isValid: (G, ctx, id) => G.cells[id] === null,
    },
  },

The current implementation (for reference) would be:

  moves: {
    clickCell(G, ctx, id) {
      if (!isValid(G, ctx, id)) {
        return INVALID_MOVE;
      }

      const cells = [...G.cells];
      cells[id] = ctx.currentPlayer;
      return { ...G, cells };
    },
  },

I think the current implementation comes with a lower cognitive overhead (which is a very important design principle for this framework), so I'm a bit hesitant to do anything that makes the basic operation of implementing a move more complex. That said, as long as we keep the basic case simple, I'm OK adding complexity for advanced users.

Let's put aside the discussion of where the isValid logic should sit for a moment. I'm not actually sure if your proposal changes things semantically (unless I misunderstood your proposal). Note that it's trivial to change our current implementation to your proposal via the following transform:

move.isValid = (G, ctx, id) => {
  return move(G, ctx, id) === INVALID_MOVE;
}

Also, to clarify semantics, the current implementation already treats an invalid move as a no-op. Even if a hacker crafts a socket message with an invalid combination of arguments, the server will pretend that it never saw that message. So, the framework does validate moves and does not rely on the UI to gate invalid moves.

Functional Purity

Let me point out that functional purity of the individual move is actually not as important as one might think. What is important is that the overall game reducer stays pure. We already cheat in a few places (the Events API and the Randomness API, for example) but without sacrificing functional purity of the overall reducer.

Taking this one step further, we could in theory not even require moves to be reducers. We could add our own API for G that allows people to modify it in a non-immutable way (with the framework taking care of immutability behind the scenes).

That said, I'm not trying to champion exceptions here. Let's keep the current implementation of returning undefined until we find something better.

Testing / SOC

I think you have a valid point that if a move is to also handle the invalid cases, then the tests need to also take that into account. However, I'm not sure if that's necessarily a bad thing. Tests could be structured so that they describe things on a per-move basis (and handle everything related to that particular move). You don't need to have separate tests for validity checks in this structure.

Enumerating valid moves for the UI

This is something that also touches the AI implementation (it needs to enumerate valid moves in the game). IMO, this is not the same thing as a validity check. Adding an isValid to each move does nothing to help you enumerate moves (it's not possible to try all possible combinations of arguments in order to find valid ones).

nicolodavis avatar Oct 25 '18 09:10 nicolodavis

About Functional Purity: This exactly. Maybe the framework should encode moves in a way allowing different styles on top of it, and still yields the other benefits. The current approach of returning undefined feels like the null pointer problem.

By any chance, did you misspell UI for AI in your last paragraph? The UI needs the information what moves are possible also, which must be represented in the state either way.

Stefan-Hanke avatar Nov 27 '18 22:11 Stefan-Hanke

@Stefan-Hanke In the last paragraph, I wanted to highlight that enumerating valid moves is relevant to both UI and AI, and if we do any work in this direction, it must respect both use cases (I'd prefer not to have one way of enumerating moves for AI and another for UI).

nicolodavis avatar Dec 03 '18 03:12 nicolodavis

Please do not just return a bool. We should be able to get from this logic WHY something failed and return that to end user.

I could envision a use case something like the following based on my needs:

const logic from 'game\logic'

 moves: {
    movement: {
      reduce: (G, ctx, source, destination) => {
        logic.movement(G, ctx, source, destination);
      },
     isValid: (G, ctx, id) => {
       try { 
          logic.movement(G, ctx, source, destination); 
          return { result: true, msg: null };
        } catch(err) { 
         return { result: false, msg: err.message }; 
       }
     }
    },
  },

RGBKnights avatar Dec 17 '18 15:12 RGBKnights

Related: nicolodavis/boardgame.io#592

vdfdev avatar Mar 29 '20 08:03 vdfdev

@sparr posted this related feature request in #635:

I want to be able to annotate a move in some way, in my game definition, that will provide feedback to a client / board / player about the expected or allowed parameter values and ranges for the move.

In games I have seen so far, a lot of this logic exists in the client/board implementation, and then for complex moves it remains possible for the player to attempt a move then find out it is INVALID_MOVE.

This feature request is meant to more solidly separate game implementation from interface. I want multiple clients to all be able to take advantage of the same code to enumerate valid moves, or at least get some information with which to narrow down the possibilities.

For a game like checkers, it would probably be possible to return an enumeration of every possible move.

For a more complex game, you might return some ranges of values, e.g. indicating that paramA can be 100-500 and paramB can be 100-200, without necessarily mentioning that B>A will return INVALID_MOVE.

delucis avatar Apr 24 '20 21:04 delucis

On the original topic, not my copied tangent above... What if there was a way to ask for a "dry run" of a move? The return value would be INVALID_MOVE* or a new G, but not a change to the actual G.

    • or whatever we later replace INVALID_MOVE with for better feedback

sparr avatar Apr 24 '20 21:04 sparr

I think that approach you outlined in #636 is definitely one possibility. At the moment we probably need to address #592 first to establish a standard way for the game master to return information to the client. At the moment even INVALID_MOVE isn’t returned. Things like waiting for move success etc. would also be useful additions and we can then build other ways for the game master to communicate things like “dry runs”. It’s not possible currently because the only thing sent to clients from the master are full state updates.

delucis avatar Apr 24 '20 21:04 delucis

My personal concern is not only whether to check if the move is legal, but also to give the client the list of possible moves. The client shouldn't have to test every possible move and check if it's valid or not before displaying them in the UI.

It's especially true if the backend / client run separate stacks, i.e. the logic would be on the server side, and the client doesn't run bgio (because someone made a bot to play the game, or wants to make an android application for the game, ...), then the client absolutely needs to have a list of possible moves.

My suggested interface is this, for tic-tac-toe:

{
  moves: {
    check: {
      move(G, ctx, data) {
         // ...
      },
      available(G, ctx) {
        return [0,1,2,3,4,5,6,7,8].filter(x => !G.cells[x]);
      }
    }
  }
}

Then boardgame.io needs to have the list of available moves somewhere, probably in Context, generated like this:

function generateAvailableMoves (Game, G, ctx) {
  // maybe not needed? Or done elsewhere
  delete ctx.availableMoves;

  const moveDefs = currentPhaseStageMoveDefs(Game, G, ctx);

  const availableMoves = {};

  for (const [moveName, moveDef] of Object.entries(moveDefs)) {
    if (moveDef.available) {
      const moves = moveDef.available(G, ctx);
      if (moves.length > 0) {
        availableMoves[moveName] = moveDef.available(G, ctx);
      }
    } else {
      // Game didn't define the `available` function on the move, we just 
      // say that the move is possible 
      availableMoves[moveName] = true;
    }
  }

  ctx.availableMoves = availableMoves;
}

So for a tictactoe game, ctx.availableMoves would look like this:

// If moves.check.available was defined
{
  cells: [0, 2, 3, 6]
}

// If moves.check.available was NOT defined
{
  cells: true
}

Then to check if it's an illegal move, on boardgame.io's side:

function executeMove(Game, G, ctx, move: {name: string, arg: any}) {
  // ...

  if (! (move.name) in ctx.availableMoves) {
    throw InvalidMove(...);
  }

  const available: any[] | true = ctx.availableMoves[move.name];

  if (Array.isArray(available)) {
    if (!available.some(item => equalsDeep(item, move.data))) {
      throw InvalidMove(...);
    }
  }

  // Can even be combined with an additional .isValid function in the move definition
  const moveDef = currentPhaseStageMoveDefs(Game, G, ctx)[move.name];

  if (moveDef.isValid && moveDef.isValid(G, ctx, move.data) === false) {
      throw InvalidMove(...);
  }

  // Then, execute the move
}

With that, for tic tac toe, all invalid inputs, like an already check cell, or even invalid cell ids like 1.2, "abcd", 10000 are easily filtered.

Some games, like Dixit, can have free text input. In which case, the availableMoves API is not enough, and the isValid API (included in the code snippet above) makes a lot of sense for that particular move:

{
  phases: {
    hinting: {
      moves: {
        hint: {
          move(G, ctx, hint) {
            G.hint = hint;
          },
          isValid(G, ctx, hint) {
            if (typeof hint !== "string" || !hint.length) {
              // No specific error message, the error is obvious
              return false;
            }

            // Throws an assertion error with a message
            assert(hint.length <= 20, "Hint is too long");

            // Probably not needed, since `false` and `undefined` are different values
            return true;
          }
        }
      }
    },
    guessing: {
      moves: {
        guess: {
          move(G, ctx, imageId) {
            G.players[ctx.currentPlayer].guess = imageId;
          },
          available(G, ctx) {
             return G.images;
          }
        }
      }
    }
  }
}

I think I would love those APIs. And keep the possibility to return INVALID_MOVE / throw an exception in the actual move code, for more complex situations.

Currently, to check if a move's data corresponds to the availableMoves given, I suggested an equalsDeep function, but it can be more elaborate in a later API. For example the game can provide a custom match function and return something like [{range: [10, 200]}, ...] in move.available(...) and the game's match function would match {range: [10, 200]} to 10, 11, 12, ...

coyotte508 avatar May 09 '20 10:05 coyotte508

Just a random thought, but with regard to typing and other things, it might be useful to have the arguments of moves be a single typed object, which would allow a hypothetical available function to return a typed array of possible arguments. The whole thing just reminds me a lot of some parts of xstate and the internals of a variety of different store libraries (Redux, Vuex, ...).

Obviously one main issue is how feasible it is to enumerate all possible arguments, although it would be quite interesting to see how this fares in real usage. I would assume many games could get by just fine enumerating all possible options for a move at a fairly low cost (in memory and cpu). The decision space for each individual move usually is fairly low, given how most boardgames use (by necessity) a low number of discrete things/values for pretty much everything. While games with continuous values/objects do of course exist, I guess they are the minority (dexterity games? The dial in Wavelength?).

senritsu avatar Aug 22 '21 14:08 senritsu

Having thought a bit more about the topic, it seems this issue bleeds a lot into a few different related topics which may or may not warrant a separate proposal/rfc.

At its core of both this, #592, and #636 are attempts to improve the DX to provide good UX on the client. As it is right now, the client seems to have few ways of providing good UX, apart from working a lot around (or outside of) the framework.

Problems

Lack of affordance visibility

One key issue is how to properly communicate all affordances to the player. Fundamentally "moves" in the framework right now are lacking a way of exposing the necessary metadata to allow the client of looking at a move in the context of the state to derive the possible affordances. There are a few different but related angles to this problem:

  • Player validation: The client does not know if its player is allowed to do a move right now (which is sort of like authorization)
  • Parameter validation: The client does not know what parameters for a move are valid/invalid

Both can of course apply at the same time. Without them, the only choice is to use basically trial and error (handling).

Lack of preview

For good UX, previewing the consequences of an interaction can be extremely helpful. This is one of the key differences where digital implementations of analog games can actually provide something really useful beyond reducing fiddliness and improving accessibility via global multiplayer.

Benefits of addressing those problems

Supposing a solution that allows the client to access full information about all possible moves, complete with a full set of possible parameters for each move, as well as being able to dry-run the move (if possible, limitations for server-only moves of course still apply, relates to #636 and may also relate to #950 for consistency between client/server execution).

Going off what @coyotte508 proposed above:

// purely hypothetical, using `moveContext` for brevity as per #662
moves: {
  playCard: {
    move (moveContext, cardId, targetLocation) { /* snip */ },
    possibleParameters (moveContext) {
       // framework should probably wrap this and return empty array if player is not active

       // what game components a player can use to make a move usually depends on game state
       const cards = someModule.getCardsInPlayersHand(moveContext)

       // parameters may depend on each other and the game state
       // returns a lot of parameter lists like
       // ['card-id-3', 'player-3-army']
       // ['card-id-7', `player-1-building-3`]
       // those could directly be fed into a move invocation using spread syntax
       return cards.flatMap(({id, type}) => someModule.getValidTargets(moveContext, type).map(location => [id, location]))
    }
  }
}

Affordances can be communicated to the player cleanly

The client UI can

  • hide impossible/invalid moves, focusing attention on the current decision space
  • choose to display certain moves as disabled, possibly even providing some reason why they cannot be made at the moment
  • display moves right at the game components where they are relevant (identified by the move parameters), in a context menu or using other modes of input (each game component/piece would basically have a "bound" partial application of the function)
  • Offer "autocomplete" for moves in form of a hierarchical select or similar, to narrow down the choices of a player bit by bit ("Move army > from area X > to area Y > expedite movement using resouce Z")
  • Result preview allows

Prevention instead of error handling

Apart from edge cases like simultaneous conflicting updates from different clients, illegal moves should be pretty much impossible if the client decides to use the information to prevent the moves from being made in the first place.

Input for AI

If moves themselves have a built-in way of communicating available options, a bot can of course use those as well to collect all possible moves in the current situation and select one of them. This would more or less decentralize the current way of providing a bot with an enumeration of possible moves.

Promotes well-defined rules

Fully defining/specifying all moves would not only serve to solidify the game rules (by forcing the implementer to think things through), but could also be unit tested to make the rules implementation more robust.

All this would of course be strictly optional, since it obviously may be a lot of work to fully specify all moves in a more complex game. Probably worth it though in most cases.


Any thoughts on that matter?

senritsu avatar Sep 04 '21 15:09 senritsu

Seems like a thorough overview. Couple of additional thoughts:

  • Any API should account for moves that don’t require any parameters. I suppose returning [[]] from possibleParameters would work, but a cleaner API would just involve a move flagging if it is currently available via a boolean. Perhaps an available function could return boolean | Parameters[] or it could be more stuctured like { active: boolean; parameters?: Parameters[] }.

  • The clearer benefits here to me are if the client can do something that is not possible already, which I think is only potentially the case for AI enumeration? For many of the other use cases, no client API is essential. For the UI uses, a possibleParameters function can already be called with G & ctx from the client. In fact, that may be preferable as possibleParameters could be expensive and depending on the UI currently displayed, calling it for every move may be superfluous. So this is more a case of code structure rather than functionality.

    That said, we don’t currently expose a simple way to check which moves are available. That should be possible. Where possibleParameters is entirely in the hands of the user, move availability is the explicit responsibility of the framework and it could well be non-trivial for a developer to duplicate all the logic to know which move is available when. client.moves always contains a dispatcher for all moves even if the current phase etc. only makes some of them available. That helps protect against calling a move that is undefined. Having a way to check which moves are actually active, would no doubt be helpful. This could be as simple as an array of available move names:

    onsole.log(client.moves); // => { playCard: Function, otherMove: Function, ... }
    onsole.log(client.availableMoves); // => ['playCard']
    f (client.availableMoves.includes('playCard')) {
     const params = playCardPossibleParameters(G, ctx);
     // render play card UI
    
    

    That functionality could be augmented by a long-form active method, so by default availableMoves contains any moves in the current phase/stage, but that can be additionally filtered if any of those moves have an active method that returns false.

  • One additional hesitancy I’d offer around an official possibleParameters API is that I’m not sure it’s always easier to map parameters to UI. For example, in our Tic-Tac-Toe demo, a cell is clickable if it isn’t filled. Is it cleaner for the client to map filled/unfilled state to a possible move, or check for the presence of the index of the cell in an array of valid parameters? I think both approaches have their strengths in different scenarios so wonder if this should be left up to developers.

  • Move “previews” (#636) are orthogonal to this really — they could definitely be useful, but should be callable like moves are currently whether or not the game has implemented possibleParameters or an equivalent API.

delucis avatar Sep 04 '21 19:09 delucis

Perhaps an available function could return boolean | Parameters[] or it could be more stuctured like { active: boolean; parameters?: Parameters[] }.

Something like that could work well, and could separate certain aspects cleanly (like distinguishing between the player-specific authorization and "possible moves", or other not-yet-thought-of reasons of things being not available).

framework-internal vs. external

I do agree that the whole "legal parameters" functionality could very well live in user-land at first, experiments with different ways of handling this may well crystallize into functionality that could eventually be absorbed into the core. Having it as a plugin or similar for convenient access would probably be best, to treat it as an optional tool for developers to do things one specific way. If time allows, I will start tinkering with this a bit, and see how it works out.

client.availableMoves

One thing that would probably be useful here is, similar to the topic of #592, some way of returning a reason for a move being not available. Disabling things without clearly communicating the reason often leads to more confusion than anything else. Completely hiding them works around this, but the client may well prefer to just disable some interaction for layout reasons, so this should be an option. Whatever #592 ends up returning, would probably be a good (optional?) return value for a hypothetical active method as well.

Move previews

Completely agree, functionally they are unrelated. It was included here solely because when thinking about this I looked at it from more of a holistic client UX perspective.

Sorry for kind of derailing the issue here a bit, I was considering just opening a separate discussion, but it seemed quite related to what was already being discussed here in the comments 😀

senritsu avatar Sep 05 '21 06:09 senritsu

some way of returning a reason for a move being not available

That's true. I wonder then if it should just indicate what moves are available in the current phase/stage for this player. That way availableMoves is just a framework level feature that always means one thing. Any further filtering would be up to the user along the lines of exposing valid parameters.

Sorry for kind of derailing the issue here a bit

This is very much a brainstorming issue, so no problem at all.

delucis avatar Sep 05 '21 08:09 delucis