alee/golfgame
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 2 Wiki Activity

Add inline comments across client and server codebase

Browse Source 
Full-codebase commenting pass focused on the tricky, fragile, and
non-obvious spots: animation coordination flags in app.js, AI decision
safety checks in ai.py, scoring evaluation order in game.py, animation
engine magic numbers in card-animations.js, and server infrastructure
coupling in main.py/handlers.py/room.py. No logic changes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
adlee-was-taken
2026-02-25 22:17:19 -05:00
parent 72eab2c811
commit 215849703c
9 changed files with 199 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
client/animation-queue.js
View File

@@ -31,14 +31,17 @@ class AnimationQueue {
};
}
// Add movements to the queue and start processing
// Add movements to the queue and start processing.
// The onComplete callback only fires after the LAST movement in this batch —
// intermediate movements don't trigger it. This is intentional: callers want
// to know when the whole sequence is done, not each individual step.
async enqueue(movements, onComplete) {
if (!movements || movements.length === 0) {
if (onComplete) onComplete();
return;
}
// Add completion callback to last movement
// Attach callback to last movement only
const movementsWithCallback = movements.map((m, i) => ({
...m,
onComplete: i === movements.length - 1 ? onComplete : null
@@ -185,7 +188,9 @@ class AnimationQueue {
await this.delay(this.timing.flipDuration);
}
// Step 2: Quick crossfade swap
// Step 2: Quick crossfade swap.
// 150ms is short enough to feel instant but long enough for the eye to
// register the transition. Shorter looks like a glitch, longer looks laggy.
handCard.classList.add('fade-out');
heldCard.classList.add('fade-out');
await this.delay(150);

74
client/app.js
View File

@@ -30,7 +30,14 @@ class GolfGame {
this.soundEnabled = true;
this.audioCtx = null;
// Swap animation state
// --- Animation coordination flags ---
// These flags form a system: they block renderGame() from touching the discard pile
// while an animation is in flight. If any flag gets stuck true, the discard pile
// freezes and the UI looks broken. Every flag MUST be cleared in every code path:
// animation callbacks, error handlers, fallbacks, and the `your_turn` safety net.
// If you're debugging a frozen discard pile, check these first.
// Swap animation state — local player's swap defers state updates until animation completes
this.swapAnimationInProgress = false;
this.swapAnimationCardEl = null;
this.swapAnimationFront = null;
@@ -44,19 +51,19 @@ class GolfGame {
// Animation lock - prevent overlapping animations on same elements
this.animatingPositions = new Set();
// Track opponent swap animation in progress (to apply swap-out class after render)
// Blocks discard update: opponent swap animation in progress
this.opponentSwapAnimation = null; // { playerId, position }
// Track draw pulse animation in progress (defer held card display until pulse completes)
// Blocks held card display: draw pulse animation hasn't finished yet
this.drawPulseAnimation = false;
// Track local discard animation in progress (prevent renderGame from updating discard)
// Blocks discard update: local player discarding drawn card to pile
this.localDiscardAnimating = false;
// Track opponent discard animation in progress (prevent renderGame from updating discard)
// Blocks discard update: opponent discarding without swap
this.opponentDiscardAnimating = false;
// Track deal animation in progress (suppress flip prompts until dealing complete)
// Blocks discard update + suppresses flip prompts: deal animation in progress
this.dealAnimationInProgress = false;
// Track round winners for visual highlight
@@ -818,7 +825,11 @@ class GolfGame {
hasDrawn: newState.has_drawn_card
});
// V3_03: Intercept round_over transition to defer card reveals
// V3_03: Intercept round_over transition to defer card reveals.
// The problem: the last turn's swap animation flips a card, and then
// the round-end reveal animation would flip it again. We snapshot the
// old state, patch it to mark the swap position as already face-up,
// and use that as the "before" for the reveal animation.
const roundJustEnded = oldState?.phase !== 'round_over' &&
newState.phase === 'round_over';
@@ -834,7 +845,8 @@ class GolfGame {
}
// Build preRevealState from oldState, but mark swap position as
// already handled so reveal animation doesn't double-flip it
// already handled so reveal animation doesn't double-flip it.
// Without this patch, the card visually flips twice in a row.
const preReveal = JSON.parse(JSON.stringify(oldState));
if (this.opponentSwapAnimation) {
const { playerId, position } = this.opponentSwapAnimation;
@@ -1332,14 +1344,16 @@ class GolfGame {
this.heldCardFloating.classList.add('hidden');
this.heldCardFloating.style.cssText = '';
// Pre-emptively skip the flip animation - the server may broadcast the new state
// before our animation completes, and we don't want renderGame() to trigger
// the flip-in animation (which starts with opacity: 0, causing a flash)
// Three-part race guard. All three are needed, and they protect different things:
// 1. skipNextDiscardFlip: prevents the CSS flip-in animation from firing
// (it starts at opacity:0, which causes a visible flash)
// 2. lastDiscardKey: prevents renderGame() from detecting a "change" to the
// discard pile and re-rendering it mid-animation
// 3. localDiscardAnimating: blocks renderGame() from touching the discard DOM
// entirely until our animation callback fires
// Remove any one of these and you get a different flavor of visual glitch.
this.skipNextDiscardFlip = true;
// Also update lastDiscardKey so renderGame() won't see a "change"
this.lastDiscardKey = `${discardedCard.rank}-${discardedCard.suit}`;
// Block renderGame from updating discard during animation (prevents race condition)
this.localDiscardAnimating = true;
// Animate held card to discard using anime.js
@@ -2414,7 +2428,13 @@ class GolfGame {
};
}
// Fire-and-forget animation triggers based on state changes
// Fire-and-forget animation triggers based on state diffs.
// Two-step detection:
// STEP 1: Did someone draw? (drawn_card goes null -> something)
// STEP 2: Did someone finish their turn? (discard pile changed + turn advanced)
// Critical: if STEP 1 detects a draw-from-discard, STEP 2 must be skipped.
// The discard pile changed because a card was REMOVED, not ADDED. Without this
// suppression, we'd fire a phantom discard animation for a card nobody discarded.
triggerAnimationsForStateChange(oldState, newState) {
if (!oldState) return;
@@ -2522,18 +2542,18 @@ class GolfGame {
}
// STEP 2: Detect when someone FINISHES their turn (discard changes, turn advances)
// Skip if we just detected a draw - the discard change was from REMOVING a card, not adding one
// Skip if we just detected a draw — see comment at top of function.
if (discardChanged && wasOtherPlayer && !justDetectedDraw) {
// Check if the previous player actually SWAPPED (has a new face-up card)
// vs just discarding the drawn card (no hand change)
// Figure out if the previous player SWAPPED (a card in their hand changed)
// or just discarded their drawn card (hand is identical).
// Three cases to detect a swap:
// Case 1: face-down -> face-up (normal swap into hidden position)
// Case 2: both face-up but different card (swap into already-revealed position)
// Case 3: card identity null -> known (race condition: face_up flag lagging behind)
const oldPlayer = oldState.players.find(p => p.id === previousPlayerId);
const newPlayer = newState.players.find(p => p.id === previousPlayerId);
if (oldPlayer && newPlayer) {
// Find the position that changed
// Could be: face-down -> face-up (new reveal)
// Or: different card at same position (replaced visible card)
// Or: card identity became known (null -> value, indicates swap)
let swappedPosition = -1;
let wasFaceUp = false; // Track if old card was already face-up
@@ -3993,7 +4013,11 @@ class GolfGame {
// Not holding - show normal discard pile
this.discard.classList.remove('picked-up');
// Skip discard update during any discard-related animation - animation handles the visual
// The discard pile is touched by four different animation paths.
// Each flag represents a different in-flight animation that "owns" the discard DOM.
// renderGame() must not update the discard while any of these are active, or you'll
// see the card content flash/change underneath the animation overlay.
// Priority order doesn't matter — any one of them is reason enough to skip.
const skipReason = this.localDiscardAnimating ? 'localDiscardAnimating' :
this.opponentSwapAnimation ? 'opponentSwapAnimation' :
this.opponentDiscardAnimating ? 'opponentDiscardAnimating' :
@@ -4017,7 +4041,9 @@ class GolfGame {
const discardCard = this.gameState.discard_top;
const cardKey = `${discardCard.rank}-${discardCard.suit}`;
// Only animate discard flip during active gameplay, not at round/game end
// Only animate discard flip during active gameplay, not at round/game end.
// lastDiscardKey is pre-set by discardDrawn() to prevent a false "change"
// detection when the server confirms what we already animated locally.
const isActivePlay = this.gameState.phase !== 'round_over' &&
this.gameState.phase !== 'game_over';
const shouldAnimate = isActivePlay && this.lastDiscardKey &&

26
client/card-animations.js
View File

@@ -43,10 +43,14 @@ class CardAnimations {
const discardRect = this.getDiscardRect();
if (!deckRect || !discardRect) return null;
// Center the held card between deck and discard pile
const centerX = (deckRect.left + deckRect.right + discardRect.left + discardRect.right) / 4;
const cardWidth = deckRect.width;
const cardHeight = deckRect.height;
const isMobilePortrait = document.body.classList.contains('mobile-portrait');
// Overlap percentages: how much the held card peeks above the deck/discard row.
// 48% on mobile (tighter vertical space, needs more overlap to fit),
// 35% on desktop (more breathing room). Tuned by eye, not by math.
const overlapOffset = cardHeight * (isMobilePortrait ? 0.48 : 0.35);
return {
@@ -463,6 +467,9 @@ class CardAnimations {
}
});
// Register a no-op entry so cancelAll() can find and stop this animation.
// The actual anime.js instance doesn't need to be tracked (fire-and-forget),
// but we need SOMETHING in the map or cleanup won't know we're animating.
this.activeAnimations.set(`initialFlip-${Date.now()}`, { pause: () => {} });
} catch (e) {
console.error('Initial flip animation error:', e);
@@ -786,7 +793,11 @@ class CardAnimations {
});
};
// Delay first shake, then repeat at interval
// Two-phase timing: wait initialDelay, then shake on an interval.
// Edge case: if stopTurnPulse() is called between the timeout firing and
// the interval being stored on the entry, the interval would leak. That's
// why we re-check activeAnimations.has(id) after the timeout fires — if
// stop was called during the delay, we bail before creating the interval.
const timeout = setTimeout(() => {
if (!this.activeAnimations.has(id)) return;
doShake();
@@ -1114,18 +1125,18 @@ class CardAnimations {
return;
}
// Wait for any in-progress draw animation to complete
// Check if there's an active draw animation by looking for overlay cards
// Collision detection: if a draw animation is still in flight (its overlay cards
// are still in the DOM), we can't start the swap yet — both animations touch the
// same visual space. 350ms is enough for the draw to finish its arc and land.
// This happens when the server sends the swap state update before the draw
// animation's callback fires (network is faster than anime.js, sometimes).
const existingDrawCards = document.querySelectorAll('.draw-anim-card[data-animating="true"]');
if (existingDrawCards.length > 0) {
// Draw animation still in progress - wait a bit and retry
setTimeout(() => {
// Clean up the draw animation overlay
existingDrawCards.forEach(el => {
delete el.dataset.animating;
el.remove();
});
// Now run the swap animation
this._runUnifiedSwap(handCardData, heldCardData, handRect, heldRect, discardRect, T, rotation, wasHandFaceDown, onComplete);
}, 350);
return;
@@ -1208,6 +1219,9 @@ class CardAnimations {
],
width: discardRect.width,
height: discardRect.height,
// Counter-rotate from the card's grid tilt back to 0. The -3 intermediate
// value adds a slight overshoot that makes the arc feel physical.
// Do not "simplify" this to [rotation, 0]. It will look robotic.
rotate: [rotation, rotation - 3, 0],
duration: T.arc,
easing: this.getEasing('arc'),

15
client/card-manager.js
View File

@@ -100,12 +100,14 @@ class CardManager {
}
}
// Get the deck color class for a card based on its deck_id
// Get the deck color class for a card based on its deck_id.
// Reads from window.currentDeckColors, which app.js sets from game state.
// This global coupling is intentional — card-manager shouldn't know about
// game state directly, and passing it through every call site isn't worth it.
getDeckColorClass(cardData) {
if (!cardData || cardData.deck_id === undefined || cardData.deck_id === null) {
return null;
}
// Get deck colors from game state (set by app.js)
const deckColors = window.currentDeckColors || ['red', 'blue', 'gold'];
const colorName = deckColors[cardData.deck_id] || deckColors[0] || 'red';
return `deck-${colorName}`;
@@ -126,7 +128,10 @@ class CardManager {
cardEl.style.width = `${rect.width}px`;
cardEl.style.height = `${rect.height}px`;
// On mobile, scale font proportional to card width so rank/suit fit
// On mobile, scale font proportional to card width so rank/suit fit.
// This must stay in sync with the CSS .card font-size on desktop — if CSS
// sets a fixed size and we set an inline style, the inline wins. Clearing
// fontSize on desktop lets the CSS rule take over.
if (document.body.classList.contains('mobile-portrait')) {
cardEl.style.fontSize = `${rect.width * 0.35}px`;
} else {
@@ -235,7 +240,9 @@ class CardManager {
await this.delay(flipDuration);
}
// Step 2: Move card to discard
// Step 2: Move card to discard.
// The +50ms buffer accounts for CSS transition timing jitter — without it,
// we occasionally remove the 'moving' class before the transition finishes.
cardEl.classList.add('moving');
this.positionCard(cardEl, discardRect);
await this.delay(duration + 50);