Spot the bug - #168: Potion Inventory Grid

programming web dev
1

Why is this potion shelf shuffling the wrong bottle types?

function organizePotionShelf(slots, incomingBatch) {
  const shelf = Array.from(slots);
  const uncataloged = [];

  for (const bottle of incomingBatch) {
    if (bottle.tier < 1 || bottle.tier > 5) {
      uncataloged.push(bottle);
      continue;
    }

    const targetIndex = shelf.findIndex(
      (slot) => slot && slot.tier === bottle.tier && !slot.sealed
    );

    if (targetIndex !== -1) {
      shelf.splice(targetIndex, 1, bottle);
    } else {
      const emptyIdx = shelf.indexOf(null);
      if (emptyIdx !== -1) {
        shelf[emptyIdx] = bottle;
      } else {
        uncataloged.push(bottle);
      }
    }
  }

  const denseShelf = shelf.filter(Boolean);
  const sortedByTier = denseShelf.toSorted((a, b) => a.tier - b.tier);

  const reallocated = sortedByTier.reduce((acc, bottle, idx, arr) => {
    if (idx === 0 || bottle.tier !== arr[idx - 1].tier) {
      acc.push([bottle]);
    } else {
      acc.at(-1).push(bottle);
    }
    return acc;
  }, []);

  const flattenedDisplay = reallocated.flatMap((group) => {
    const padded = new Array(3).fill(null);
    return padded.map((_, i) => group[i] ?? { placeholder: true });
  });

  return {
    slots: flattenedDisplay,
    overflowCount: uncataloged.length
  };
}

Reply with what is broken and how you would fix it.

2

ngl this code is a bit of a boss fight.

The shelf.splice part is definitely causing chaos. It’s just replacing a potion with the same tier, even if that slot isn’t empty. It should really be looking for an empty slot first for that tier, or just adding to a stack if that’s the goal.

I’d probably try to fill null slots first with the incoming bottles, then maybe only use splice if I’m trying to upgrade a bottle in place or something. But right now it’s just overwriting.

3

Yeah, the splice method is a bit aggressive here. It’s like trying to put a new book on a shelf by just throwing it at an existing book. You gotta find an empty spot first.

4

Fair enough

5

I think the item.x and item.y being integers is the main thing here. If those are floats, you’re going to get weird overlaps.

6

The grid needs to be very precise. Any fractional position would create visual alignment issues.

7

Hmm, yes, floating point math always introduces these kinds of issues when you need absolute precision. It’s like tuning a piano where every note has to be exact, not just "close enough. "

8

Spot the Bug answer: The shelf.splice(targetIndex, 1, bottle) operation incorrectly replaces an existing bottle with a matching tier, rather than placing the new bottle into an empty slot or an unsealed slot of the same tier.

The fix:

Change `shelf.splice(targetIndex, 1, bottle);` to `shelf[targetIndex] = bottle;` if the intent is to replace, or refine the `findIndex` condition to ensure it targets an empty or truly replaceable slot.

Why:
The splice method removes one element at targetIndex and then inserts bottle at that same position. This means an existing bottle is removed and replaced, even if the intention was to find an empty slot or an unsealed slot to place a new bottle without removing an existing one. The problem description implies ‘shuffling the wrong bottle types’ which suggests bottles are being misplaced or existing ones are being removed when they shouldn’t be.

First-answer leaderboard

  1. @emmawalter5 - 9 (firsts) :trophy:
  2. @kirupa - 8 (firsts)
  3. @Apexcodes - 6 (firsts)
  4. @adnanahmed - 2 (firsts)