Server-side array shuffling.

[philihp]

Adds a method for performing a deterministic random shuffle for the 3rd point in #68. The game logic requests that one of the attributes in G be shuffled. The server then assumes it's an array and does a shallow shuffle of it.

To keep the interface simple, only arrays at the root of G are able to be shuffled. If the user wants to shuffle a nested array, they'll have to do it themselves.

This should be rebased on top of #113 prior to merging.

@nicolodavis @Stefan-Hanke

[nicolodavis]

seed should be inside a key called random.

[googlebot]

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

[coveralls]

Coverage remained the same at 100.0% when pulling 9055361 on philihp:pb--shuffle-deck into 1a3ced2 on google:master.

[nicolodavis]

Why not allow nested fields as well? You can just split on . and use the index operator to retrieve deeper fields without changing the interface.

[nicolodavis]

Philihp, can you merge master in? There are some conflicting changes.

[philihp]

@nicolodavis I started to do nested deep fields but it got tricky with having to clone every level of the object to preserve immutability of the source G for Redux, and the added complexity for little gain made me think I was overengineering it. Here's the test I was trying to satisfy:

test('Random.Shuffle works on a nested attribute', () => {
  let ctx = { random: { seed: 'some_predetermined_seed' } };
  const tiles = ['A', 'B', 'C', 'D', 'E']
  let G = {
    players: {
      0: tiles,
      1: tiles
    }
  }

  let G2 = Random.Shuffle(G, 'players.1')
  let { G: G3, ctx: ctx2 } = RunRandom(G2, ctx)
  expect(G.players['0']).toMatchObject(tiles)
  expect(G.players['1']).toMatchObject(tiles) // this was the tricky one :(
  expect(G3.players['0']).toMatchObject(tiles)
  expect(G3.players['1']).not.toMatchObject(tiles)
})

[Stefan-Hanke]

Ah, there's nothing like a small little recursion problem. :)
Warning: I've omitted error handling.

function fisheryates_shuffle(array) {
  var currentIndex = array.length, temporaryValue, randomIndex;
  while (0 !== currentIndex) {
    randomIndex = Math.floor(Math.random() * currentIndex);
    currentIndex -= 1;
    temporaryValue = array[currentIndex];
    array[currentIndex] = array[randomIndex];
    array[randomIndex] = temporaryValue;
  }
  return array;
}

const tiles = ['A', 'B', 'C', 'D', 'E'];
let G = {
  players: {
    0: tiles,
    1: tiles
  }
}

function shuffle(datastructure, structuredname) {
    // recursion end-case
    if(structuredname.indexOf(".") < 0) {
        let data = fisheryates_shuffle([...datastructure[structuredname]]);
        return { ...datastructure, [structuredname]: data };
    }

    // strip one level, recurse
    const [car, ...cdr] = structuredname.split(".")
    let data = datastructure[car];
    let shuffleddata = shuffle(data, cdr.join("."));

    // and merge
    return { ...datastructure, [car]: shuffleddata};
}

let G2 = shuffle(G, "players.1");

console.log(JSON.stringify(G, undefined, 3));
console.log(JSON.stringify(G2, undefined, 3));

[philihp]

Yeah, doing that only works when there's only one Random op. In RunRandom, this line also doesn't work with deep attributes... I think it's a lot of complexity we can live without for the time being.

[nicolodavis]

I think it's ok if we leave this PR to just handle top level field names.

We'll eventually need to support deeper fields before we can properly release the Random API, though (I think it's too restrictive to game designers to have all dice and card decks at the top level). For example, some game designers will prefer the following model for a card game (I personally think this is quite natural too):

Game({
  setup: numPlayers => ({
    players: {
      '0': { hand: [...], discard: [...] },
      '1': { hand: [...], discard: [...] },
       ...
    }
  })
})

If we don't support nested fields, everyone will have to flatten their states:

Game({
  setup: numPlayers => ({
    '0-hand': [...],
    '1-hand': [...],
    ...
    
    '0-discard': [...],
    '1-discard: [...],
    ...
  })
})

So, maybe let's just get the shuffling logic in this PR and one of us can follow up with a broader change to support nested fields across the entire Random API later?

[philihp]

SGTM. Good point, shuffling a discard is a common thing. For Liar's dice, it's already awkward to roll into secret player info.

[Stefan-Hanke]

@philihp I agree. If you have suggestions for making the API more dev-friendly, feel free to propose. An extension of the random language e.g. would be to allow terms like 5D6 which would reduce the boiler plate in Liar's Dice. Or even 4D6 2D12 1D20.

If you mean by awkward the nature of the API itself, that random values are not just returned, we'd need to change where game moves are executed - in case a server is present, it needs to be restricted to running only there.

[nicolodavis]

Now that I think about it, this is actually possible!

Context: We want moves to execute on both client and server so that there is no lag (optimistic update). The server state is still authoritative and the client state is overwritten eventually (if it differs).

We could disable optimistic updates for random calls. For example, if G contains a random op, we could have the client just wait for the server update.

This way the random API can just return values directly. I think this will simplify things quite a bit, including our problem of not being able to call random code in the flow section at the moment.

[philihp]

@Stefan-Hanke by awkward, I mean to your first point, if the user wants 5d6, they have to call Random.D6 5 times (e.g.)... then they show up at the root of G, and have to be moved out during onMove (e.g.)