Skip to content

Audit quick wins: unexported imports, dt clamp, perf bracket, FFI/alloc hoists, mojibake repair - #28

Merged
proggeramlug merged 2 commits into
mainfrom
fix/audit-quick-wins
Jul 16, 2026
Merged

Audit quick wins: unexported imports, dt clamp, perf bracket, FFI/alloc hoists, mojibake repair#28
proggeramlug merged 2 commits into
mainfrom
fix/audit-quick-wins

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

First slice of the 2026-07-16 full audit (game + engine). All quality-neutral by design — no visual or gameplay behavior changes.

Correctness

  • Four functions imported but never exported (countAlive/spawnEnemy/despawnAllEnemies/damageEnemy in director.ts) — hud/combat/main call them every frame; only Perry's indifference to export semantics made it work. Exported the three that cross modules; spawnEnemy stays internal.
  • dt clamped to 100 ms — a stalled frame (SDF clipmap re-bake does 20–60 ms CPU; worse spikes exist) stepped the Euler integrators far enough to carry a charging dragoon over a wall's repulsion ring.
  • PERFTEST perfTB was never assigned — phase B reported negative, phase C absorbed it. Now bracketed at beginMode3D.
  • Score table knew 5 of 7 kinds — both ranged upgrade kinds scored the 10-point fallback, less than a mantis. Now 40/90.
  • commitRun(0)/bestScore(0) hardcoded arena slot 0 at 5 sites — per-arena bests would merge silently the day a second arena ships. Now ARENA_INDEX.
  • Removed the duplicate const moving (one block, two meanings — tsc would reject it) and the never-implemented DIR.pending* fields.

Perf hygiene (perf-audit finding 11 — all quality-neutral)

  • One playerPosition() FFI read per frame (the enemy-projectile loop alone did up to 24/frame).
  • Forest + static-mesh draws reuse load-time position/tint objects (~180 fresh objects/frame eliminated).
  • Diag strings built only where drawn (MOBILE paid for two lines it never drew).
  • ~45 stranded imports removed, each verified unused by word-boundary occurrence count (count 1 = import line only).

Source repair

  • 261 mojibake sequences + one BOM repaired across 5 files (committed PowerShell round-trip damage; â€" etc.). Repair inverts the exact corruption (cp1252 byte-image must decode as valid UTF-8), so clean text is untouchable by construction. Verified comment-only — no string literal was affected.
  • perry-quirks.md no longer tells the reader the runtime world loader is unusable and to use a tool deleted months ago.

Verification

  • perry compile green (necessary, not sufficient here).
  • 25 s title-screen batch run: boots clean, 120k grass placed, nav flights found, no ReferenceError.
  • 75 s AITEST batch run: auto-plays a wave — enemies spawn, close 57 m → melee, cycle AI states (approach → orbit → dart → recover), damage path fires. Zero errors on stdout/stderr.
  • Boot log confirms ray_query=true on DX12 — the DXC DLLs in the repo root are live (separate ticket coming for deploying them properly).

Summary by CodeRabbit

  • New Features

    • Added arena-specific best-score display and run results.
    • Added scoring support for additional enemy types.
    • Added optional graphics overrides and a launch option to disable selected rendering effects.
    • Improved environmental visuals, including moisture-based grass variation, lighting, fog, shadows, and wind effects.
  • Bug Fixes

    • Corrected score tracking when runs end in player death or victory.
    • Improved audio loading and calm/combat music transitions.
  • Performance

    • Reduced per-frame overhead and improved rendering, simulation, and enemy projectile updates.

Ralph Kuepper added 2 commits July 16, 2026 10:40
…nd-trip damage)

Every non-ASCII char in these files' comments had been double-encoded
(UTF-8 read as cp1252 and re-written: em dash -> 'â€"', x -> '×',
pi -> 'Ï€'), and weapons.ts additionally carried the BOM the same
round-trip adds. 87 sequences repaired by inverting the exact
transformation (cp1252-encode each suspect run, require the byte image
to decode as valid multi-byte UTF-8); clean text has no such image, so
the repair cannot touch it by construction. Comment-only damage - a
targeted regex sweep confirmed no string literal was affected.

main.ts and director.ts had the same damage (a further ~170 sequences);
their repair rides the next commit alongside functional changes to the
same files.
Correctness:
- director.ts exported countAlive/despawnAllEnemies/damageEnemy - all
  three were imported (hud, combat, main) and called every frame while
  never being exported; it worked only because Perry ignores export
  semantics, and would break under tsc, an LSP, or a stricter Perry.
- dt is now clamped to 100 ms. A stalled frame (SDF-clipmap re-bake,
  window drag) hands back 300 ms+, and one Euler step that big walks a
  charging dragoon clean over a wall's repulsion ring into the house.
- PERFTEST's perfTB was declared and never assigned, so phase B always
  reported a huge negative and phase C silently absorbed it - the perf
  instrument of record could not tell sim time from draw time. It now
  brackets at beginMode3D.
- score.ts knew 5 of the 7 kinds: both SH-042 ranged upgrade kinds fell
  to the 10-point fallback (less than a mantis). Now 40/90 points.
- commitRun/bestScore hardcoded arena slot 0 at five sites while the
  menu read the real ARENA_INDEX - per-arena bests would have silently
  merged the day a second arena ships.
- removed the dead duplicate 'const moving' (same block, two meanings)
  and the never-implemented DIR.pending* accumulate-then-apply fields.

Perf hygiene (perf-audit finding 11, quality-neutral):
- one authoritative playerPosition() read per frame after stepPhysics;
  the per-projectile loop alone was re-crossing the FFI up to 24x/frame.
- forest + static-mesh draw loops reuse hoisted position/tint objects
  built once at load (was ~180 fresh objects per frame for static data).
- diag-bar strings are built inside the !MOBILE gate that draws them.
- removed ~45 imports verified unused by word-boundary count (the
  SH-025 split left them stranded).

Docs: perry-quirks.md 'Impact on the shooter's design' described the
deleted build-world.ts bake step as the shipped answer; it now states
the runtime-loadWorld reality and the rules that still bind.

Also repairs the remaining ~170 mojibake sequences in main.ts and
director.ts (see previous commit for the mechanism and verification).

Verified: perry compile green; 25 s title-screen batch run boots clean
(120k grass, nav flights found, no ReferenceError); 75 s AITEST run
auto-plays a wave - enemies spawn, close 57 m to melee, cycle AI states,
damage path fires - with zero errors on stdout/stderr.
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR updates runtime world rendering, frame timing, environment grass variation, director exports, arena-indexed score commits, enemy scoring, HUD best-score lookup, and Perry world-loading documentation. Several files also receive comment and formatting corrections.

Changes

Runtime arena updates

Layer / File(s) Summary
World boot and render-loop changes
src/main.ts
Startup graphics, render passes, cached world draw data, diagnostics, harness behavior, player/enemy rendering, and simulation timing are updated.
Environment and grass variation
src/environment.ts
Deterministic moisture noise affects grass appearance, while environment, water, glass, reflection, and GI setup documentation is revised.
Combat director and enemy behavior
src/director.ts, src/combat.ts, src/enemies.ts, src/weapons.ts
Director helpers are exported, projectile position lookup is hoisted, death commits use W.ARENA_INDEX, and enemy/weapon comments are normalized.
Arena scoring and summary display
src/score.ts, src/hud.ts
Scoring supports seven enemy kinds, and the HUD reads the best score for the selected arena.
Runtime world-loading documentation
docs/perry-quirks.md
World loading is documented as runtime JSON loading through loadWorld, replacing the former build-time generator description.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 64.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title references several real changes in the PR, including dt clamping, FFI/alloc hoists, and mojibake repairs, even though it is a bit list-like.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/audit-quick-wins

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/score.ts (1)

52-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use KIND_POINTS.length instead of a hardcoded magic number.

This makes the bounds check robust against future additions to the KIND_POINTS array, preventing cases where point configurations are expanded but the length check remains stale.

♻️ Proposed fix
-  const base = kind >= 0 && kind < 7 ? KIND_POINTS[kind] : 10;
+  const base = kind >= 0 && kind < KIND_POINTS.length ? KIND_POINTS[kind] : 10;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/score.ts` at line 52, Update the bounds check in the score calculation to
compare kind against KIND_POINTS.length instead of the hardcoded value 7, while
preserving the existing fallback behavior for out-of-range kinds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/main.ts`:
- Around line 397-417: Update the mobile profile configuration in the
surrounding rendering setup to disable GTAO, alongside the existing mobile
exclusions for SSGI, SSR, and sun shafts. Preserve mobile shadows and bloom, and
leave GTAO enabled for non-MOBILE profiles.

---

Nitpick comments:
In `@src/score.ts`:
- Line 52: Update the bounds check in the score calculation to compare kind
against KIND_POINTS.length instead of the hardcoded value 7, while preserving
the existing fallback behavior for out-of-range kinds.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f0aa4042-2bb8-4891-9b3c-b926d6aa9974

📥 Commits

Reviewing files that changed from the base of the PR and between bf23a71 and 6d87c0a.

📒 Files selected for processing (9)
  • docs/perry-quirks.md
  • src/combat.ts
  • src/director.ts
  • src/enemies.ts
  • src/environment.ts
  • src/hud.ts
  • src/main.ts
  • src/score.ts
  • src/weapons.ts

Comment thread src/main.ts
Comment on lines +397 to +417
// Lumen SW-GI is the big one it re-bakes an SDF clipmap as the view moves,
// which is a GPU stall the phone has no headroom to absorb. SSR goes with it.
//
// GTAO stays ON. It was cut with the other two at first, but measured on an
// iPhone 16 Pro it doesn't cost a frame-rate tier: mid-wave the frame sits at
// 25.0 ms either way (the same ~40 fps), and the title screen holds 60. It buys
// back the contact shadows that seat the grass, trees and aliens on the ground
// instead of leaving them looking pasted over it — the cheapest of the three
// instead of leaving them looking pasted over it the cheapest of the three
// screen-space passes by some margin, and the one with the best return.
//
// Read that "free" precisely, though: present mode is Fifo on a 120 Hz panel,
// so every frame snaps to a multiple of 8.33 ms and GTAO is being absorbed by
// slack inside a bucket rather than costing nothing. It eats margin. If a
// heavier wave starts tipping frames from the 25 ms bucket into the 33 ms one,
// this is the first thing to put back. (Per-pass GPU timings would settle it,
// but the profiler reports -1 on iOS — the Metal backend doesn't get
// TIMESTAMP_QUERY — so wall-clock at a fixed point in the wave is the honest
// but the profiler reports -1 on iOS the Metal backend doesn't get
// TIMESTAMP_QUERY so wall-clock at a fixed point in the wave is the honest
// instrument here.)
//
// Render scale stays at 0.5 (TSR reconstructs to native in the TAA pass), which
// on a 2622x1206 iPhone means a ~1311x603 internal buffer — the same
// on a 2622x1206 iPhone means a ~1311x603 internal buffer the same

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

Disable GTAO on the MOBILE profile.

The comment indicates a deliberate choice to keep GTAO enabled. As per coding guidelines, the MOBILE profile must disable SSGI, SSR, GTAO, and sun shafts while retaining shadows and bloom. Please ensure GTAO is disabled on mobile platforms to comply with the project's performance budgets.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main.ts` around lines 397 - 417, Update the mobile profile configuration
in the surrounding rendering setup to disable GTAO, alongside the existing
mobile exclusions for SSGI, SSR, and sun shafts. Preserve mobile shadows and
bloom, and leave GTAO enabled for non-MOBILE profiles.

Source: Coding guidelines

@proggeramlug
proggeramlug merged commit ec06847 into main Jul 16, 2026
1 check passed
@proggeramlug
proggeramlug deleted the fix/audit-quick-wins branch July 16, 2026 10:18
proggeramlug pushed a commit that referenced this pull request Jul 16, 2026
Import-block conflict: took main's post-#28 trimmed list + KIND_TINTO
(the one name this branch adds to main.ts). Both changesets verified
coexisting: ppFrame hoists (#28) and the kind-tint draws (this PR).
proggeramlug pushed a commit that referenced this pull request Jul 16, 2026
Two things the textually-clean auto-merge would have shipped broken:
- #28 removed KIND_COUNT/ALIEN_GLB from main.ts imports; this branch's
  stageModels batch uses both. Re-added - a green Perry compile does not
  catch absent cross-module imports (ReferenceError at boot).
- This branch (and #30) accidentally committed dxcompiler.dll/dxil.dll:
  both were branched before #29's .gitignore landed, so 'git add -A'
  swept the untracked DLLs in, and #30's merge carried them onto main.
  Untracked again here (files stay on disk - the game needs them beside
  the exe; see EN-058).

Verified on the merged tree: compile green, 14 s batch run boots to the
menu with [music] menu=1 calm=2 combat=3 and [anim] clipset=2 slot0=9
slot1=10 both live.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant