DEV Community

Vera Yang for BeGoodTool.com

Posted on

The shuffle in my memory-training game isn't fully random — and nobody would ever notice

I built a small card-flip memory game a while back: nine tiles numbered 1–9 flash their numbers for ten seconds, then go blank, and you have to click them back in ascending order purely from memory of where each number was sitting. Recently I went back into the source to clean it up and actually read the shuffle function line by line instead of skimming it. Turns out it has a real bug — one that's completely invisible while playing, but is a textbook example of how easy it is to get a "just shuffle an array" loop subtly wrong.

It's not "repeat the sequence" — it's "reconstruct it from memory"

The game doesn't show you an order to repeat, like Simon. It shows you nine tiles in scrambled positions, each briefly displaying its number, and your job during recall is to click the tiles in ascending numeric order (1, 2, 3…9) even though the numbers are now hidden. The click handler is bound once, on the container, using event delegation:

box_num.addEventListener(
  "click",
  (addbox = (e) => {
    if (e.target.tagName.toLowerCase() === "div") {
      ccount.value++;
      if (Number(e.target.id) != ccount.value) {
        // wrong tile — game over
      } else {
        e.target.style.background = "#ff4e50";
        e.target.innerHTML = e.target.id;
        if (Number(e.target.id) == length) {
          // last tile clicked correctly — win
        }
      }
    }
  }),
);
Enter fullscreen mode Exit fullscreen mode

Each tile's id was set to its real number when it was created (divtag.id = nums[key]), it just isn't displayed anymore. ccount counts how many correct clicks you've made in a row, and every click has to match ccount exactly — so there's no partial credit and no way to click out of order and self-correct. The tagName check exists because the listener sits on the parent #box_num, not on individual tiles, so any click that bubbles up from outside an actual .div_num (padding, gaps between tiles) has to be filtered out rather than miscounted as a wrong tile.

The shuffle that's almost, but not quite, Fisher-Yates

Before the tiles are drawn, the numbers get shuffled with this:

let nums = Array.from(Array(length + 1).keys()).slice(1); // [1, 2, ..., length]

for (let j, i = 0; i < length; i++) {
  j = Math.floor(Math.random() * i);
  [nums[i], nums[j]] = [nums[j], nums[i]];
}
Enter fullscreen mode Exit fullscreen mode

That looks like the standard in-place swap shuffle, and for most of the array it behaves like one. But walk through the first two iterations by hand:

  • i = 0: j = Math.floor(Math.random() * 0), which is always 0. Swapping nums[0] with itself — a no-op.
  • i = 1: j = Math.floor(Math.random() * 1). Math.random() returns a value in [0, 1), so Math.random() * 1 is also in [0, 1), and Math.floor() of anything in [0, 1) is always 0. j isn't random at all here — it's a hardcoded 0 in disguise.

So the second tile always swaps with the first tile, every single game, regardless of the RNG. From i = 2 onward j does range properly over [0, i), so the bias fades out fast and by the time you're looking at nine tiles it's not something a human could ever detect just by playing — but it means the first position isn't shuffled with the same freedom as the rest. A correct inside-out Fisher-Yates needs j to range over [0, i] inclusive, not [0, i); missing that one boundary is exactly what turns "shuffle" into "shuffle, except the first step."

The on-screen timer only ever measures the recall phase

The ten-second memorization window and the timer you see on screen are two different clocks, and they don't overlap:

window.startTimer = () => {
  numTimeout = setInterval(() => {
    tt.value++;
    time.value = tt.value;
  }, 1000);
};

window.stopTimer = () => {
  tt.value = 0;
  ccount.value = 0;
  time.value = "0";
  if (numTimeout) {
    clearInterval(numTimeout);
    numTimeout = null;
  }
};
Enter fullscreen mode Exit fullscreen mode

startTimer runs immediately when the tiles first appear, but stopTimer — which resets the counter to zero — fires again right when the ten seconds are up, just before the tiles go blank and the click listener gets attached. So the number you see counting up during recall never includes the memorization window; it's purely "how long did clicking the right sequence take you," which is also the number baked into the win message. It's a small design choice, but it's the reason the "success" time can be genuinely fast (2–3 seconds) even though the whole round took thirteen.

That reset function doing double duty (as both "pause between phases" and "reset on game over") is also why addbox — the click handler — is captured in an outer-scoped variable instead of being an anonymous inline function. The #box_num container itself is never destroyed between games, only its child tiles are cleared out, so on closeMask() the code has to explicitly call removeEventListener("click", addbox) using that exact same function reference. Without holding onto it, a second playthrough would stack a second click listener on top of the first, and every click would silently count twice.

Two things that look configurable but aren't

The shuffle function takes a length parameter, and num_click(length) reads like it's built to support different board sizes. In practice the template only ever calls num_click(9) — there's no way in the current UI to play a smaller or larger board, and the ten-second memorization window (RememberTime = 10) is a flat constant that doesn't scale with length either. So despite the code being written in a reusable, parameterized way, there's currently exactly one difficulty level.

The other honest gotcha: the tiles themselves are built with raw document.createElement and innerHTML calls straight into a #box_num div, sitting inside an otherwise normal Vue component. That works, but it means Vue's reactivity system has no idea those tiles exist — which is exactly why cleanup (removing the old listener, clearing innerHTML by hand) has to be done manually instead of just letting Vue's own re-render/unmount handle it.

I turned the cleaned-up version into a small free tool if you want to see whether you can beat the "less than 2 seconds" tier without tripping the shuffle's one deterministic swap: Memory Test & Training. No sign-up, just nine cards and a clock.


Available in other languages

Top comments (0)