|
| 1 | +# V2 Bug TODO |
| 2 | + |
| 3 | +--- |
| 4 | + |
| 5 | +## 8. Dragging a placed object shows no grid-snapping feedback |
| 6 | + |
| 7 | +**File:** `src/ui/drag-drop.ts` — `setupGridObjectDrag` |
| 8 | + |
| 9 | +The old implementation used interact.js's `snap` modifier which locked `event.dx/dy` to grid increments before they were applied, so the element itself jumped cell-by-cell giving live visual feedback of exactly which cell it would land in. |
| 10 | + |
| 11 | +V2's `setupGridObjectDrag` accumulates raw `e.movementX/Y` pixel deltas and applies a free-form `transform: translate()`. The element slides freely with no indication of where it will snap on drop. |
| 12 | + |
| 13 | +**Root cause:** No snapping logic in `pointermove`. The transform is applied from raw pixel deltas rather than snapped-to-grid deltas. |
| 14 | + |
| 15 | +**Fix needed:** On `pointerdown`, record `baseLeft` and `baseBottom` from the element's current inline `style.left` / `style.bottom` values. In `pointermove`, compute the raw pixel position (`baseLeft + dx`), snap it to the nearest grid cell (`Math.round(rawLeft / cellSize) * cellSize`), then compute the snapped offset (`snappedLeft - baseLeft`) and apply that as the transform instead of the raw `dx/dy`. This makes the element jump in cell-size increments during the drag, matching the old behaviour. |
| 16 | + |
| 17 | +--- |
| 18 | + |
| 19 | +## 9. Bots still cannot be picked up from the grid |
| 20 | + |
| 21 | +**Files:** `src/ui/drag-drop.ts` line ~191 — `handleDrop` / `src/ui/game-setup.ts` — `setupNewBot` |
| 22 | + |
| 23 | +`handleDrop` constructs the bot object passed to `_grid.add_bot()` **without a `userId` field**. This flows into `onReplaceBot` → `setupNewBot`. Inside `setupNewBot`, the guard `if (bot.userId === userId)` is always false (userId is `undefined`), so: |
| 24 | + |
| 25 | +- `setupGridObjectDrag` is never called on the bot element — it has no drag listeners |
| 26 | +- `currentBotId` is never updated to `bot.id` on line 150 of `game-setup.ts` |
| 27 | +- `chosen-bot` attribute is never set on `<body>` |
| 28 | + |
| 29 | +`_onBotIdChange` is called from `handleDrop` which does update `_currentBotId` in `drag-drop.ts`, but without drag listeners on the element this does nothing. |
| 30 | + |
| 31 | +**Fix needed:** Store `userId` in `drag-drop.ts` (alongside `_cellSize`, `_grid`) and set it in `initDragDrop`. Pass `userId: _userId` in the bot object literal inside `handleDrop`. Update `initDragDrop`'s signature and the call site in `game-setup.ts` to supply `userId`. |
| 32 | + |
| 33 | +--- |
| 34 | + |
| 35 | +## 10. Players can place more than one bot from the sidebar |
| 36 | + |
| 37 | +**Files:** `src/ui/drag-drop.ts` line 43 — `setupTemplateDrag` / `src/ui/game-setup.ts` — `setupNewBot` |
| 38 | + |
| 39 | +The JS guard in `setupTemplateDrag`: |
| 40 | +```ts |
| 41 | +if (document.body.hasAttribute("chosen-bot") && target.getAttribute("type") === BOT_TYPE) return; |
| 42 | +``` |
| 43 | +relies on `chosen-bot` being set on `<body>`. But because `setupNewBot` never reaches `bot.userId === userId` (issue 9 — userId is missing), `document.body.setAttribute("chosen-bot", ...)` is never called. So the guard never fires and a second bot can be dropped. |
| 44 | + |
| 45 | +**Root cause:** Downstream symptom of issue 9. No independent fix needed — fixing issue 9 (adding `userId` to the placed bot) causes `setupNewBot` to set `chosen-bot` on body, which re-enables this guard automatically. |
| 46 | + |
| 47 | +--- |
| 48 | + |
| 49 | +## 11. Bot is not highlighted green when placed on the grid |
| 50 | + |
| 51 | +**Files:** `src/ui/grid-render.ts` — `drawBot` / `src/game.css` |
| 52 | + |
| 53 | +`drawBot` adds `classList.add("current-bot")` only when `bot.userId === userId`. Because `handleDrop` omits `userId` from the bot object (issue 9), `bot.userId` is `undefined`, so the class is never added and the CSS rule `.current-bot { background-color: rgb(144, 255, 127); }` never applies. |
| 54 | + |
| 55 | +**Root cause:** Same as issue 9. Fixing issue 9 causes `drawBot` to add `.current-bot` automatically. No separate fix needed. |
| 56 | + |
| 57 | +--- |
| 58 | + |
| 59 | +## 12. Delete and rotate icons are barely clickable — clicking them shifts the object instead |
| 60 | + |
| 61 | +**Files:** `src/ui/drag-drop.ts` — `setupGridObjectDrag` / `src/ui/grid-render.ts` — `addDeleteIcon`, `addRotateBotIcon` |
| 62 | + |
| 63 | +`setupGridObjectDrag` attaches a `pointerdown` listener to the entire container div. The delete and rotate icons are children of that container. When the user clicks an icon, `pointerdown` bubbles up to the container, which sets `dragging = true` and calls `el.setPointerCapture()`. The subsequent `pointerup` then fires `dropGridObject`, shifting the object to whichever cell the pointer happened to be over. The icons have only a `click` listener with no `stopPropagation`, so the drag always wins. |
| 64 | + |
| 65 | +**Fix needed:** In `addDeleteIcon` and `addRotateBotIcon` (grid-render.ts), add a `pointerdown` listener on the icon element that calls `e.stopPropagation()`. This prevents the event from reaching the container's drag listener, so the icon click registers as a click rather than a drag start. |
| 66 | + |
| 67 | +--- |
| 68 | + |
| 69 | +## 13. Dragging a placed object to a new cell lands it offset from where the user expects |
| 70 | + |
| 71 | +**File:** `src/ui/drag-drop.ts` — `dropGridObject` |
| 72 | + |
| 73 | +`dropGridObject` computes the target grid cell from the raw **pointer position** (`clientX`, `clientY`) using `Math.floor`. The cell chosen therefore depends on where inside the object the user's pointer happens to be — grabbing the top-right corner of a 3×3 bot gives a different landing cell than grabbing the bottom-left, even when the element itself is snapped to the same visual position. |
| 74 | + |
| 75 | +The old implementation computed the target cell from the **element's bounding rect bottom-left** after the drag (`elementRect.left - gridRect.left`, `gridRect.bottom - elementRect.bottom`) using `Math.round`. Because the snap modifier had already snapped the element to a grid-aligned position, the element's bottom-left was always exactly on a cell boundary and the result was consistent regardless of grab point. |
| 76 | + |
| 77 | +**Fix needed:** In `dropGridObject`, replace the pointer-based cell computation with an element-rect-based one: |
| 78 | +```ts |
| 79 | +const elRect = el.getBoundingClientRect(); |
| 80 | +const gx = Math.round((elRect.left - Math.round(gridRect.left)) / _cellSize); |
| 81 | +const gy = Math.round((Math.round(gridRect.bottom) - elRect.bottom) / _cellSize); |
| 82 | +``` |
| 83 | +This is safe because `pointermove` already applies a snapped transform before `pointerup` fires, so `elRect` at drop time reflects a grid-aligned position. |
| 84 | + |
| 85 | +--- |
| 86 | + |
| 87 | +## 14. The rotate button on the bot is not visible |
| 88 | + |
| 89 | +**File:** `src/game.css` — `.rotation-handle` rule |
| 90 | + |
| 91 | +The current `game.css` has only `right: 20px` for `.rotation-handle`. There is no background colour, border-radius, colour, padding, or position offset — so the icon renders as invisible/transparent text overlapping the bot image and is likely clipped by the container. |
| 92 | + |
| 93 | +The old implementation positioned the handle at `left: 50%; bottom: -30px` with `background-color: #ff1661; border-radius: 10rem; color: #fff`, making it a visible pink pill below the bot. |
| 94 | + |
| 95 | +**Fix needed:** |
| 96 | +- Replace the `.rotation-handle` rule in `game.css` with the full old-style rule: centred horizontally, placed 28 px below the bot, pink background, white text. |
| 97 | +- Add `overflow: visible` to `.bot-container` so the handle is not clipped by the container boundary. |
| 98 | + |
| 99 | +--- |
| 100 | + |
| 101 | +## 15. Bots don't move when the game starts (two sub-causes) |
| 102 | + |
| 103 | +### 15a — `startMovingBot` emits wrong botId from stale room snapshot |
| 104 | + |
| 105 | +**File:** `src/firebase/firebase-sync.ts` lines 319–355 |
| 106 | + |
| 107 | +`start_game_for_everyone()` emits `gameEvents.emit("startMovingBot", this.get_bot_id())`. `get_bot_id()` derives the numeric ID from `this.roomInfo.users` — a snapshot taken once at `join_room_page`. If that snapshot predates the current user's entry in `users`, `sorted.indexOf(userId)` returns `-1`, making `get_bot_id()` return `0`. `grid.bots[0]` is always `undefined`, so `startMovingBot_virtual` exits every tick silently. |
| 108 | + |
| 109 | +The reliable numeric botId is already in the URL as `?bot_id=` (`botIdParam` in `main.ts` line 28). Fix: store it on the `RealtimeUpdates` instance and use it in `start_game_for_everyone` / `stop_moving_bot`. |
| 110 | + |
| 111 | +### 15b — Firebase receives stale pre-move bot position, overwriting every move |
| 112 | + |
| 113 | +**File:** `src/ui/bot-movement.ts` line 114 |
| 114 | + |
| 115 | +`_onEmitReplaceBot?.(bot)` — `bot` was captured at line 94 before the move. `apply_next_move_to_bot` writes a **new object** into `grid.bots[botId][0]`, so the local `bot` reference is stale. Firebase receives the old position; the other client's listener pushes it back, overwriting the move. Result: bot appears frozen on both screens. |
| 116 | + |
| 117 | +**Fix needed:** Change line 114 to `_onEmitReplaceBot?.(grid.bots[botId][0])`. |
| 118 | + |
| 119 | +--- |
| 120 | + |
| 121 | +## 16. Timer (`#countdown`) and phase indicator never become visible |
| 122 | + |
| 123 | +**Files:** `game.html` lines 54–55, `src/ui/phase-manager.ts` lines 117–118 and 139–153 |
| 124 | + |
| 125 | +`#countdown` and `#phaseIndicator` both have `style="display:none;"` inline in `game.html`. The `phaseStartTimeChanged` handler only sets `.textContent`, never removes `display:none`. Same for the `phaseChanged` handler and `#phaseIndicator`. Both elements remain invisible throughout the entire game. |
| 126 | + |
| 127 | +**Fix needed:** In `phase-manager.ts`, set `element.style.display = ""` before (or alongside) setting `textContent`, in both the `phaseChanged` and `phaseStartTimeChanged` handlers. |
| 128 | + |
| 129 | +--- |
| 130 | + |
| 131 | +## 17. Delete/rotate icons visible but unclickable during movement |
| 132 | + |
| 133 | +**File:** `src/game.css` |
| 134 | + |
| 135 | +The `body[is-moving]` CSS rule sets `pointer-events: none` on the grid object containers, making the icons non-interactive — but they remain fully rendered and visible, giving a misleading impression that the user can interact with them. |
| 136 | + |
| 137 | +The old implementation in `virtual-board/test-style.css` line 216 had `body[is-moving="true"] .edit-icon { display: none; }` to hide them entirely. |
| 138 | + |
| 139 | +**Fix needed:** Add to `game.css`: |
| 140 | +```css |
| 141 | +body[is-moving] .edit-icon, |
| 142 | +body[is-moving] .rotation-handle { display: none; } |
| 143 | +``` |
| 144 | + |
| 145 | +--- |
| 146 | + |
| 147 | +## 18. Bots can move outside the grid boundaries |
| 148 | + |
| 149 | +**File:** `src/grid/virtual-grid.ts` |
| 150 | + |
| 151 | +Two cascading bugs: |
| 152 | + |
| 153 | +**18a — `is_valid_bot_position` never checks `isInsideBoard` (line 200):** Only collision with other bots/obstacles is checked. `future_position_after_move` therefore returns `valid_position: true` for moves that exit the grid, and `random_move` plus all policy methods accept them. |
| 154 | + |
| 155 | +**18b — OOB recovery in `get_next_move_using_policies` is a broken stub (line 422):** When a bot is already outside, the code returns `["move", 1]` unconditionally — if the bot is facing outward this pushes it further outside. The old `grid.js` had a full `get_next_move_to_be_inside_board` method (lines 1325–1383) that analysed which corners/sides were inside and returned the correct turn or move to steer back. |
| 156 | + |
| 157 | +**Fix needed:** |
| 158 | +1. Add `if (!this.isInsideBoard(...)) return { valid: false, message: "Out of bounds" }` at the top of `is_valid_bot_position`. |
| 159 | +2. Port `get_next_move_to_be_inside_board` from `grid.js` and call it in `get_next_move_using_policies` in place of the unconditional `["move", 1]` stub. |
0 commit comments