Fix Gin Rummy stale-index drag bug and add Pipe Puzzle unhover regressio

- Resolve hand card index from the container at event time instead of
  capturing it when handlers are bound, preventing the "wrong card on
  second drag" bug after a reorder.
- Enable drag-to-arrange in both draw and discard phases; clicking a card
  during draw now shows guidance rather than discarding.
- Add discard-pile click target so a selected card can be discarded by
  clicking the pile, and extract shared `_setupPileClick` helper to reduce
  duplication.
- Add `tools/regressionPipePuzzleUnhover.cjs`, a Playwright regression
  test verifying that Phaser's `killTweensOf(target, 'prop')` no longer
  clobbers in-flight rotation tweens on pointerout/pointerover.
This commit is contained in:
Brian Fertig 2026-08-26 22:38:46 -06:00
parent 759b70733a
commit 199bbc40fb
2 changed files with 195 additions and 20 deletions

View File

@ -297,22 +297,32 @@ export default class GinRummyGame extends Phaser.Scene {
this.input.on('pointerup', () => { this.input.on('pointerup', () => {
if (this.potentialDrag) { if (this.potentialDrag) {
const idx = this.potentialDrag.handIdx; const pd = this.potentialDrag;
this.potentialDrag = null; this.potentialDrag = null;
this.onHandClick(idx); // Re-resolve the index from the card container at event time. No reorder
// can happen between pointerdown and a plain pointerup, but resolving from
// the (stable) container is the safest way to stay correct.
const idx = this.humanCards.indexOf(pd.card);
this.onHandClick(idx === -1 ? pd.handIdx : idx);
return; return;
} }
if (this.dragState) this.endCardDrag(); if (this.dragState) this.endCardDrag();
}); });
} }
onHandPointerDown(handIdx, pointer) { onHandPointerDown(card, pointer) {
if (this.humanMode !== 'discard') return; // Drag-to-arrange works in both the draw and discard phases (Phase 10 pattern);
// dropping onto the discard pile only takes effect in the discard phase.
if (this.humanMode !== 'discard' && this.humanMode !== 'draw') return;
if (this.dragState) return; if (this.dragState) return;
const card = this.humanCards[handIdx];
if (!card) return; if (!card) return;
// Resolve the card's index NOW, from the container. The container is stable
// across reorders, but an index captured when the handlers were bound would go
// stale after the hand is rearranged (the "wrong card on second drag" bug).
const handIdx = this.humanCards.indexOf(card);
if (handIdx === -1) return;
this.potentialDrag = { this.potentialDrag = {
handIdx, startX: pointer.x, startY: pointer.y, card, handIdx, startX: pointer.x, startY: pointer.y,
offsetX: pointer.x - card.x, offsetY: pointer.y - card.y, offsetX: pointer.x - card.x, offsetY: pointer.y - card.y,
}; };
} }
@ -656,41 +666,51 @@ export default class GinRummyGame extends Phaser.Scene {
beginHumanTurn() { beginHumanTurn() {
if (this.logic.phase === 'draw') { if (this.logic.phase === 'draw') {
this.humanMode = 'draw'; this.humanMode = 'draw';
this.setStatus('Draw from the stock pile or take the discard'); this.setStatus('Drag cards to arrange your hand, then draw from the stock or discard');
this.knockBtn.setVisible(false); this.knockBtn.setVisible(false);
this.ginBtn.setVisible(false); this.ginBtn.setVisible(false);
// Hand is interactive so the player can arrange cards before drawing
// (Phase 10 pattern — drag reorders; drop targets are inactive until discard phase)
this._setupHandInteraction();
// Make stock clickable // Make stock clickable
if (this.pileObjs.stock) { if (this.pileObjs.stock) {
this.pileObjs.stock.setSize(CARD_W, CARD_H); this._setupPileClick(this.pileObjs.stock, () => this.onDrawStock());
this.pileObjs.stock.setInteractive({ useHandCursor: true });
this.pileObjs.stock.on('pointerdown', () => this.onDrawStock());
this.pileObjs.stock.on('pointerover', () => { this.pileObjs.stock?.setAlpha(0.8); });
this.pileObjs.stock.on('pointerout', () => { this.pileObjs.stock?.setAlpha(1); });
} }
// Make discard pile clickable // Make discard pile clickable
if (this.pileObjs.discard) { if (this.pileObjs.discard) {
this.pileObjs.discard.setSize(CARD_W, CARD_H); this._setupPileClick(this.pileObjs.discard, () => this.onDrawDiscard());
this.pileObjs.discard.setInteractive({ useHandCursor: true });
this.pileObjs.discard.on('pointerdown', () => this.onDrawDiscard());
this.pileObjs.discard.on('pointerover', () => { this.pileObjs.discard?.setAlpha(0.8); });
this.pileObjs.discard.on('pointerout', () => { this.pileObjs.discard?.setAlpha(1); });
} }
} else if (this.logic.phase === 'discard') { } else if (this.logic.phase === 'discard') {
this.humanMode = 'discard'; this.humanMode = 'discard';
this.setStatus('Select a card to discard, or Knock / Gin'); this.setStatus('Select a card to discard, or Knock / Gin');
this.updateActionButtons(); this.updateActionButtons();
this._setupHandInteraction(); this._setupHandInteraction();
// Clicking the discard pile discards the currently selected card
if (this.pileObjs.discard) {
this._setupPileClick(this.pileObjs.discard, () => this.onDiscardPileClick());
}
} }
} }
_setupPileClick(pile, handler) {
pile.setSize(CARD_W, CARD_H);
pile.setInteractive({ useHandCursor: true });
pile.on('pointerdown', handler);
pile.on('pointerover', () => pile.setAlpha(0.8));
pile.on('pointerout', () => pile.setAlpha(1));
}
_setupHandInteraction() { _setupHandInteraction() {
const hand = this.logic.players[0].hand; this.humanCards.forEach((c) => {
this.humanCards.forEach((c, i) => {
c.setSize(CARD_W, CARD_H); c.setSize(CARD_W, CARD_H);
c.setInteractive({ useHandCursor: true }); c.setInteractive({ useHandCursor: true });
c.on('pointerdown', (pointer) => this.onHandPointerDown(i, pointer)); // Pass the card container (stable identity), not the captured index — the
// index is resolved at event time so it stays correct after a reorder.
c.on('pointerdown', (pointer) => this.onHandPointerDown(c, pointer));
c.on('pointerover', () => { if (!this.dragState) c.setAlpha(0.85); }); c.on('pointerover', () => { if (!this.dragState) c.setAlpha(0.85); });
c.on('pointerout', () => { c.setAlpha(1); }); c.on('pointerout', () => { c.setAlpha(1); });
}); });
@ -715,6 +735,10 @@ export default class GinRummyGame extends Phaser.Scene {
} }
onHandClick(handIdx) { onHandClick(handIdx) {
if (this.humanMode === 'draw') {
this.setStatus('Draw a card first, then you can discard');
return;
}
if (this.humanMode !== 'discard') return; if (this.humanMode !== 'discard') return;
if (this.selectedCard) { if (this.selectedCard) {
// Deselect previous // Deselect previous
@ -733,6 +757,21 @@ export default class GinRummyGame extends Phaser.Scene {
} }
} }
onDiscardPileClick() {
if (this.humanMode !== 'discard') return;
if (!this.selectedCard) {
this.setStatus('Select a card to discard first, or drag one onto the pile');
return;
}
const hand = this.logic.players[0].hand;
const handIdx = hand.findIndex(c => c.key === this.selectedCard);
if (handIdx === -1) {
this.selectedCard = null;
return;
}
this.commitDiscard(handIdx);
}
onDrawStock() { onDrawStock() {
if (this.humanMode !== 'draw') return; if (this.humanMode !== 'draw') return;
if (!this.logic.drawStock(0)) return; if (!this.logic.drawStock(0)) return;

View File

@ -0,0 +1,136 @@
// Regression test — Pipe Puzzle: a tile's rotation animation must keep running
// after the pointer leaves the tile (mouse-out) mid-spin.
// node tools/regressionPipePuzzleUnhover.cjs [baseURL]
//
// Background: Phaser 3.90's `tweens.killTweensOf(target, 'prop')` no longer
// supports the prop filter — the argument is silently ignored and EVERY tween
// on the target is destroyed. PipePuzzleGame used it as
// `killTweensOf(tile, 'scaleX')` in the pointerout handler, which killed the
// in-flight rotation tween and froze the tile at a half-rotated angle.
//
// This test drives the same code path the pointerout event hits
// (`_setHover(i, false)`) while a rotation tween is live, and asserts:
// 1. the rotation tween SURVIVES the un-hover and completes to target;
// 2. the hover scale tweens are the only ones replaced;
// 3. mirror case: clicking/re-hovering mid-spin also must not kill the
// in-flight scale tween.
//
// Completion is verified by manually ticking the tween manager
// (`tweens.tick()`), which is deterministic even when headless RAF is
// throttled. Exits non-zero on failure.
const { chromium } = require('/home/brianfertig/.npm/_npx/e41f203b7505f1fb/node_modules/playwright');
const BASE = process.argv[2] || 'http://localhost:8123';
(async () => {
const browser = await chromium.launch({
headless: true,
executablePath: '/home/brianfertig/.cache/ms-playwright/chromium-1208/chrome-linux64/chrome',
args: ['--no-sandbox', '--disable-gpu'],
});
const page = await browser.newPage({ viewport: { width: 1280, height: 720 } });
const errors = [];
page.on('pageerror', (e) => errors.push('pageerror: ' + e.message));
page.on('console', (m) => { if (m.type() === 'error') errors.push('console: ' + m.text()); });
await page.goto(BASE + '/', { waitUntil: 'load', timeout: 30000 });
await page.waitForFunction(
() => window.game && (window.game.isRunning === true || window.game.isRunning === 'running'),
{ timeout: 25000 },
);
await page.waitForTimeout(600);
// Start the game scene and jump to the Easy board.
await page.evaluate(() => { window.game.scene.start('PipePuzzleGame', {}); });
await page.waitForTimeout(500);
await page.evaluate(() => { window.game.scene.getScene('PipePuzzleGame')._startGame('easy'); });
await page.waitForTimeout(400);
const r = await page.evaluate(() => {
const rel = (a) => ((a % 360) + 360) % 360;
const sc = window.game.scene.getScene('PipePuzzleGame');
const { source, drain } = sc._board;
let i = 0;
while (i === source || i === drain) i++;
const img = sc._cells[i];
const tweensOf = () => sc.tweens.getTweensOf(img);
const hasKey = (key) => tweensOf().some((t) => t.data && t.data.some((d) => d.key === key));
// Advance every live tween on this tile to completion, deterministically.
// (Driven via Tween.update() so the check works even when the headless
// RAF clock is throttled to a crawl. `update` returns true once the
// tween has finished; the manager reaps it on its next step.)
const settle = (maxSteps = 200) => {
for (let g = 0; g < maxSteps; g++) {
let anyRunning = false;
for (const tw of sc.tweens.getTweensOf(img)) {
if (tw.isDestroyed()) continue;
if (!tw.update(16)) anyRunning = true; // advanced, still running
}
if (!anyRunning) break;
}
};
// ── Case 1: rotate, then mouse OUT mid-animation (the reported bug) ──
sc._setHover(i, true); // pointerover
const hoverTween = tweensOf().at(-1); // the hover scale tween
const moves = sc._moves;
sc._rotateCell(i); // pointerdown → 240ms spin starts
const target = img._targetAngle;
const spinAtStart = tweensOf().find((t) => t.data && t.data.some((d) => d.key === 'angle'));
sc._setHover(i, false); // pointerout while spin is live
const spinSurvivesUnhover = tweensOf().some((t) => t === spinAtStart && !t.isDestroyed());
const hoverTweenKilled = hoverTween.isDestroyed();
const restoreTweenPresent = hasKey('scaleX');
settle();
const completedAfterUnhover = rel(img.angle) === rel(target);
// ── Case 2 (mirror): rotate, then mouse IN again mid-animation ──
sc._rotateCell(i); // pre-fix this killed EVERY tween (incl. scale)
const scaleAtStart = tweensOf().find((t) => t.data && t.data.some((d) => d.key === 'scaleX'));
sc._setHover(i, true); // pointerover again mid-spin
const spinSurvivesRehover = tweensOf().some((t) => t.data && t.data.some((d) => d.key === 'angle') && !t.isDestroyed());
const scaleTweenReplaced = scaleAtStart != null && scaleAtStart.isDestroyed() && hasKey('scaleX');
settle();
const completedAfterRehover = rel(img.angle) === rel(img._targetAngle);
// ── Case 3: game state must have advanced cleanly ──
const stateAdvanced =
sc._moves === moves + 2 &&
!sc._won &&
sc._board.sockets.every((s) => Number.isInteger(s));
return {
spinSurvivesUnhover, hoverTweenKilled, restoreTweenPresent, completedAfterUnhover,
spinSurvivesRehover, scaleTweenReplaced, completedAfterRehover, stateAdvanced,
};
});
console.log('tween survival/kill state:', r);
const checks = [
['un-hover does NOT kill the in-flight rotation tween', r.spinSurvivesUnhover],
['un-hover DOES kill the old hover-scale tween', r.hoverTweenKilled],
['un-hover still starts the scale-restore tween', r.restoreTweenPresent],
['rotation COMPLETES to target angle after un-hover', r.completedAfterUnhover],
['re-hover does NOT kill the in-flight rotation tween', r.spinSurvivesRehover],
['re-hover replaces the previous scale tween', r.scaleTweenReplaced],
['rotation COMPLETES to target angle after re-hover', r.completedAfterRehover],
['game state advanced cleanly (2 moves, not won)', r.stateAdvanced],
];
let failures = 0;
for (const [name, ok] of checks) {
console.log(`${ok ? ' ok ' : 'FAIL '}${name}`);
if (!ok) failures++;
}
const relErrs = errors.filter((e) => !/favicon|404|net::ERR|ERR_NAME|Failed to load resource/.test(e));
if (relErrs.length) { console.error('JS errors:'); relErrs.forEach((e) => console.error(' ' + e)); failures++; }
await browser.close();
console.log(failures === 0 ? 'PIPE PUZZLE UNHOVER REGRESSION: PASS' : 'PIPE PUZZLE UNHOVER REGRESSION: FAIL');
process.exit(failures === 0 ? 0 : 1);
})().catch((e) => { console.error(e); process.exit(1); });