Allowed moves as function

[jstacoder]

Checklist

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

here it is only on the phase level, im having trouble getting it working on the top level in flow, i want to return all move names by default, or should it just stay undefined by default at the top level? any suggestions would be very appreicated

[coveralls]

Coverage remained the same at 100.0% when pulling 9b96f0e on jstacoder:allowedMoves_as_function into e5c567b on google:master.

[nicolodavis]

Let's also make this available on a per-game basis (not just on a per-phase basis)?

[nicolodavis]

Oops, forgot to wait for this. No worries, I'll just add this myself.

[nicolodavis]

Hijacking this thread for a moment to discuss something broader. We don't need to implement any of the following in this PR. One of us will follow up later:

We now have a few different mechanisms to disallow moves:

1. allowedMoves
2. canMakeMove
3. Returning undefined in a move function.

1 is designed to handle disabling types of moves. 2 and 3 are designed to disable specific instances of a move (with a particular argument combination for example). Both are valuable, but I would recommend that we only use one out of 2 and 3 to do the latter. I'm leaning toward retiring 2 (i.e. not allowing the user to override canMakeMove).

Also, we should probably expose 1 through ctx so that it is a helpful hint to the UI to figure out what moves to highlight.

@Stefan-Hanke, what do you think?

@felizardo, this might also be useful for the AI code. Let's consolidate what we have there as well.

[jstacoder]

I think exposing it to ctx is perfect, exactly what’s needed to make it work at the top level, and I like returning undefined from a move to specifically disallow it, but I was wondering if at the top level of the config should it maybe be a disallowedMoves array? Which is then checked, possibly overridden in the phase level allowedMoves? It just seems like the use case would be to disallow one or two moves, which would then conditionally be allowed in certain phases or specific parts of turns, but I can also see how maybe having the top level value and phase level value being different (disallowedMoves vs allowedMoves) could be confusing as well. Ideas?

[nicolodavis]

Let's just use allowedMoves everywhere. It's much less confusing that way. About the semantics of the override, let's just have the phase level config override the global config whenever it is defined. We can use null to indicate that all moves are allowed (instead of undefined).

For example, if a game has three moves: A, B and C.

Game -> allowedMoves: ['A', 'B'],
phase 1 -> allowedMoves: null       // A B C allowed
phase 2 -> allowedMoves: ['A']      // A allowed
phase 3                             // A and B allowed
phase 4 -> allowedMoves: []         // nothing allowed

[vdfdev]

@nicolodavis Sorry I've been a little bit busy IRL, so I lost a little bit the track of the changes, I will try to catch up this weekend