Skip to content

Fix Battle Tanks combat edge cases: laser damage, impact accounting, and server tick timing - #71

Merged
bashmohandes merged 1 commit into
masterfrom
codex/conduct-extensive-code-review-for-battle-tank
Aug 16, 2026
Merged

Fix Battle Tanks combat edge cases: laser damage, impact accounting, and server tick timing#71
bashmohandes merged 1 commit into
masterfrom
codex/conduct-extensive-code-review-for-battle-tank

Conversation

@bashmohandes

Copy link
Copy Markdown
Owner

Motivation

  • Ensure laser/ray weapons apply the exact damage modifiers captured when fired so authoritative resolution matches the fired projectile payload.
  • Prevent stale or unrelated previous impacts from influencing the turn announcement after an out-of-bounds miss.
  • Centralize online impact statistics so both instantaneous ray resolutions and simulated projectile flights update stats consistently.
  • Make the tank simulation robust to event-loop delays by advancing tankRooms.tick using actual elapsed time instead of a fixed step.

Description

  • Apply the projectile's authoritative damage payload when resolving ricochet laser shots by using projectile.weapon?.baseDamage as the ray damage source in resolveLaser (battle-tanks/scripts/game.js).
  • Stop announcing damage from a prior impact on an out-of-bounds miss by capturing the resolved impact locally in resolveShot and using it for the turn announcement (battle-tanks/scripts/game.js).
  • Add recordImpact(room, shooter) to server/battle-tanks-rooms.js and replace duplicated impact-stat update code paths (ray fire handling and physics tick) to centralize hits and damageTaken accounting.
  • Advance the online tank simulation using actual elapsed time by passing (now - lastTankTick) / 1000 to tankRooms.tick and include now in the broadcast call (server/index.js).
  • Add regression tests for the new behaviors in tests/battle-tanks.test.js covering laser damage modifiers and stale miss announcements.

Testing

  • Ran node --test tests/battle-tanks.test.js tests/battle-tanks-rooms.test.js and all Battle Tanks unit tests passed (64 tests related to Battle Tanks passed).
  • Ran npm run check (syntax checks) and it completed without errors.
  • Ran the full suite npm test and all repository tests passed (159 tests total).
  • Verified no whitespace or diff issues with git diff --check and confirmed a clean working tree after committing the changes.

Codex Task

@bashmohandes
bashmohandes merged commit 6d04e87 into master Aug 16, 2026
3 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2437e897f7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

function predictProjectile(projectile, elapsed = 0) { if (!projectile) return null; const seconds = Number.isFinite(elapsed) ? Math.max(0, elapsed) : 0; return { ...projectile, x: projectile.x + projectile.vx * seconds, y: projectile.y + projectile.vy * seconds + .5 * GRAVITY * seconds * seconds, vy: projectile.vy + GRAVITY * seconds }; }
function circleRect(x, y, r, rect) { return x + r >= rect.x && x - r <= rect.x + rect.w && y + r >= rect.y && y - r <= rect.y + rect.h; }
function resolveShot(state, hit) { const completedPlayer = state.activePlayer, projectile = state.projectile, point = projectile && Number.isFinite(projectile.x) ? { x: projectile.x, y: projectile.y, type: hit?.type, index: hit?.index } : null; state.projectile = null; if (hit && point) { state.impactSerial = (state.impactSerial || 0) + 1; const explosion = resolveExplosion(state, point, projectile); state.lastImpact = { ...explosion, serial: state.impactSerial }; state.impacts = [...(state.impacts || []), state.lastImpact].slice(-14); if (explosion.affected.some(item => item.healthDamage > 0)) state.hits += 1; } endTurnEffects(state, completedPlayer); const destroyed = state.tanks.map((tank, index) => tank.health <= 0 ? index : -1).filter(index => index >= 0); if (destroyed.length) { state.phase = 'game-over'; if (destroyed.length === state.tanks.length) { state.winner = null; state.draw = true; state.announcement = 'Draw! Both tanks were destroyed.'; } else { state.winner = 1 - destroyed[0]; state.draw = false; state.announcement = `Player ${state.winner + 1} wins!`; } return hit; } advancePickupSchedule(state); state.activePlayer = 1 - state.activePlayer; state.phase = 'aiming'; const damage = state.lastImpact?.totalDamage || 0; state.announcement = damage ? `${damage} splash damage. Player ${state.activePlayer + 1}'s turn.` : `Shot ended. Player ${state.activePlayer + 1}'s turn.`; return hit; }
function resolveShot(state, hit) { const completedPlayer = state.activePlayer, projectile = state.projectile, point = projectile && Number.isFinite(projectile.x) ? { x: projectile.x, y: projectile.y, type: hit?.type, index: hit?.index } : null; let currentImpact = null; state.projectile = null; if (hit && point) { state.impactSerial = (state.impactSerial || 0) + 1; const explosion = resolveExplosion(state, point, projectile); currentImpact = state.lastImpact = { ...explosion, serial: state.impactSerial }; state.impacts = [...(state.impacts || []), state.lastImpact].slice(-14); if (explosion.affected.some(item => item.healthDamage > 0)) state.hits += 1; } endTurnEffects(state, completedPlayer); const destroyed = state.tanks.map((tank, index) => tank.health <= 0 ? index : -1).filter(index => index >= 0); if (destroyed.length) { state.phase = 'game-over'; if (destroyed.length === state.tanks.length) { state.winner = null; state.draw = true; state.announcement = 'Draw! Both tanks were destroyed.'; } else { state.winner = 1 - destroyed[0]; state.draw = false; state.announcement = `Player ${state.winner + 1} wins!`; } return hit; } advancePickupSchedule(state); state.activePlayer = 1 - state.activePlayer; state.phase = 'aiming'; const damage = currentImpact?.totalDamage || 0; state.announcement = damage ? `${damage} splash damage. Player ${state.activePlayer + 1}'s turn.` : `Shot ended. Player ${state.activePlayer + 1}'s turn.`; return hit; }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Retain the just-resolved laser impact for announcements

When a nonlethal laser deals damage, resolveLaser first stores its result in state.lastImpact and then calls resolveShot(state, null). Because currentImpact is initialized to null and is populated only when hit is truthy, the turn now incorrectly announces “Shot ended” instead of the laser's damage. The stale-impact guard should still recognize an impact created by the current ray resolution, for example by comparing impact serials.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant