From 072dafddb31883d7cc25fcee956366a891d9966d Mon Sep 17 00:00:00 2001 From: Errilaz Date: Tue, 4 Aug 2026 10:30:39 +0200 Subject: [PATCH] chore: small fixes --- .agents/history/jerklint-01/review.md | 137 ++++++++++++++++++++++++++ .agents/rules/commits.md | 4 +- tsconfig.base.json | 2 +- tsconfig.json | 6 ++ 4 files changed, 146 insertions(+), 3 deletions(-) create mode 100644 .agents/history/jerklint-01/review.md diff --git a/.agents/history/jerklint-01/review.md b/.agents/history/jerklint-01/review.md new file mode 100644 index 0000000..de6ccb8 --- /dev/null +++ b/.agents/history/jerklint-01/review.md @@ -0,0 +1,137 @@ +# Jerklint Review 01 + +Reviewed 2026-08-04. Scope: handwritten source under `engine/`, `app/`, +`server/`, `shared/`, and `scripts/`; root runtime, build, lint, editor, and +project configuration; `README.md`. Generated assets, build output, +dependencies, and lockfile were excluded. + +**Jerklint Findings** + +1. **High** `engine/render/Color.ts:37-43`, + `engine/render/Texture.ts:46-56`, `engine/render/Rasterizer.ts:199-203`, + `engine/render/RenderConfig.ts:104-114`, `scripts/gen-assets.ts:136-143`: + `Color.lerp` promises to interpolate a `Color` but omits alpha, so `Color.rgb` + silently supplies `255`. Every bilinear sample therefore becomes opaque. + The `clean` preset enables linear filtering, while the NPC asset has a + transparent background and alpha cutout happens after sampling. This is a + false generic abstraction with a direct behavior change hidden behind an + unrelated render knob. Smallest fix: interpolate alpha in `Color.lerp`; add + an explicitly named RGB-only blend only if fog or sky ever needs one. + +2. **High** `engine/render/RenderConfig.ts:16-58`, + `engine/render/RenderConfig.ts:64-115`, `app/main.ts:19-29`, + `engine/render/Framebuffer.ts:43-73`: live render configuration has no + authoritative validity or application boundary. Public presets are mutable + singleton objects, `useConfig` retains the supplied alias, dimensions are + snapshotted into framebuffer resources, and documented numeric ranges are + not enforced. A direct width change leaves resources stale; values such as + `colorDepth = 0` make quantization divide by zero. This will become immediate + pressure when planned live sliders arrive. Smallest fix: make presets + readonly templates, create a fresh validated config for each update, and + route updates through one app-side apply function that rebuilds resources + when dimensions change. + + +4. **Medium** `app/level.ts:18-27`, `app/level.ts:66-105`, + `app/player.ts:2`, `app/player.ts:19-31`, `app/player.ts:61-97`, + `app/main.ts:48-52`: collision policy has no single owner. `Player.update` + accepts the render-heavy `Level`, then reconstructs gameplay facts using a + hardcoded ground height and NPC radius; NPC visual size lives in `main.ts`. + `buildLevel` also constructs geometry, collision, NPC placement, and sky in + one body. NPC or floor changes therefore require cross-file agreement and + isolated player tests need unrelated render data. Smallest fix: let `Level` + own a compact NPC descriptor and a narrow collision world containing ground, + boxes, and circles; pass only that collision value to `Player.update`. + +5. **Medium** `app/main.ts:54-69`, `app/player.ts:23-55`: browser key codes are + the player simulation API. Physics owns WASD and Space bindings, receives a + mutable `Set`, cannot represent a press edge, and therefore treats a + held jump key as a fresh jump whenever landing. Lost `keyup` events also + leave state stuck because no blur path clears the set. Rebinding, gamepad + input, and deterministic tests all require physics edits. Smallest fix: map + browser events in `main.ts` to a semantic `PlayerInput` value with movement + axes and `jumpPressed`; clear raw input on blur. + +6. **Medium** `engine/scene/Camera.ts:27-33`, + `engine/math/Mat4.ts:23-34`, `engine/render/Rasterizer.ts:9-19`, + `engine/render/Rasterizer.ts:58-68`, `engine/render/Rasterizer.ts:77-115`: + projection limits and clipping have separate authorities. Camera projection + declares near/far values of `0.05` and `100`, but `Rasterizer.project` + discards clip-space `z` and clips only against unrelated `w >= 0.01`; far + clipping is absent. `Rasterizer.draw` looks matrix-generic while relying on + perspective-specific `w` semantics for clipping, depth, and fog. Smallest + fix: retain clip-space `z` and clip against canonical near/far planes, or + narrow and name the API so its perspective-matrix contract is explicit. + +7. **Medium** `engine/render/RenderConfig.ts:48-54`, + `engine/render/Texture.ts:36-38`, `engine/render/Rasterizer.ts:51-57`, + `engine/render/Sky.ts:34-36`, `engine/render/Sky.ts:93-98`, + `engine/render/Sky.ts:121-128`: closed option unions fail open. New texture + filters silently become nearest, new lighting modes silently become unlit, + and new cloud kinds silently become basic because each dispatcher uses a + binary predicate plus fallback. `basicCumulus` accepts the full `CloudLayer` + union, preventing TypeScript from exposing the missing branch. Smallest fix: + use exhaustive switches with a `never` check and narrow cloud helpers to + their concrete variant types. + +11. **Low** `engine/scene/Sprite.ts:9-23`, `app/main.ts:48-52`, + `app/main.ts:107-112`: `Sprite.texture` is assigned but never read; drawing + separately reaches back to `textures.npc`. Two sources can diverge, and the + type promises ownership the render path ignores. Smallest fix: draw with + `npc.texture`, or remove the field if material binding is intentionally + external. + +12. **Low** `engine/scene/Camera.ts:4-13`, `engine/math/Mat4.ts:37-43`, + `engine/render/Sky.ts:62-66`, `app/main.ts:73-79`: camera pitch validity is + documented by `Camera` but enforced by one caller through the unexplained + literal `1.4`. Both camera and sky basis construction rely on that ritual. + Any second camera producer can create a degenerate basis. Smallest fix: own + the pitch limit and look-delta/clamp operation in the `Camera` namespace. + +14. **Low** `engine/math/Vec3.ts:3-4`, `engine/math/Vec3.ts:34-37`: + `Vec3.normalize` violates its namespace-wide fresh-result guarantee only for + zero vectors by returning the mutable input alias. Callers can safely mutate + ordinary results but unexpectedly mutate source state at one edge. Smallest + fix: return a fresh zero vector. + +**Scorecard** + +- DRY: concern - collision, release, and resource-update policy have split owners. +- KISS: pass - core engine stays direct, data-oriented, and framework-free. +- YAGNI: concern - two unused public placeholders remain. +- SOC: concern - render config application and release artifacts mix concerns. +- Cohesion: concern - player physics consumes unrelated level rendering data. +- Coupling: fail - config/resources and projection/clipping rely on hidden cross-module rules. +- Dependency Direction: pass - browser code points inward and engine remains DOM-free. +- Law of Demeter: pass - shallow plain-data access; no traversal chains or service locators. +- Immutability: concern - mutable preset aliases and zero-vector aliasing weaken boundaries. +- Declarative Shape: concern - option unions and release phases are not exhaustively interpreted. +- Implicit Contracts: fail - alpha, dimensions, pitch, clipping, checks, and publishing depend on rituals. +- Abstraction Pressure: concern - important rules are under-centralized while stale exports remain. +- Naming/API Clarity: fail - `Color.lerp`, `check`, `publish`, and `Sprite.texture` overpromise. +- Locality: concern - NPC behavior and configuration changes require cross-file coordination. +- Testability: fail - zero tests, false-green test command, and broad player fixtures. +- File/API Shape: concern - type/namespace pattern is strong; ignored and stale exports weaken it. +- Predicate Accuracy: concern - binary fallback dispatch silently accepts future union variants. +- Construction Phase Separation: concern - level and release construction each hide multiple phases. + +**Strengths** + +- Engine/browser dependency boundary is clean and enforced by separate TypeScript libraries. +- Domain types generally own behavior through matching namespaces and matching filenames. +- Hot-loop mutation is local, explicit, and appropriate for a software rasterizer. +- Rasterizer, sky, texture loading, and procedural mesh generation remain cohesive despite numeric code. +- `main.ts` is still a reasonable composition root; splitting presentation or FPS code now would add ceremony. +- No ECS, service locator, class hierarchy, utility bag, dependency cycle, or speculative plugin system appeared. + +**Validation** + +- `bun run build`: passed. +- `bunx tsc --build --dry`: selected engine, app, and config only. +- `bun run lint`: exited with one `typescript(array-type)` warning at `scripts/gen-assets.ts:162`. +- `bun run test`: found zero tests and emitted Bun's tsconfig directory-mismatch internal error; `--pass-with-no-tests` kept the command green. + +**Verdict** + +Refactor soon. Keep the core architecture. Fix lying primitives and quality +commands first, then centralize configuration, input, and collision contracts. diff --git a/.agents/rules/commits.md b/.agents/rules/commits.md index eef866f..e1fea2c 100644 --- a/.agents/rules/commits.md +++ b/.agents/rules/commits.md @@ -6,6 +6,6 @@ condition: "(?i)git commit|commit message|conventional commit" When commiting changes, follow these rules to write the messages: -- Conventional Commit (reference available at .ai/resources/conventional-commit.md) -- `@commitlint/config-conventional` (reference available at .ai/resources/commitlint.md) +- Conventional Commit (reference available at .agents/resources/conventional-commit.md) +- `@commitlint/config-conventional` (reference available at .agents/resources/commitlint.md) diff --git a/tsconfig.base.json b/tsconfig.base.json index b561c88..efca3d5 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -11,6 +11,6 @@ "skipDefaultLibCheck": true, "skipLibCheck": true, "strict": true, - "outDir": "dist" + "outDir": "out" } } diff --git a/tsconfig.json b/tsconfig.json index f308062..02330e4 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -12,6 +12,12 @@ }, { "path": "./tsconfig.app.json" + }, + { + "path": "./tsconfig.server.json" + }, + { + "path": "./tsconfig.shared.json" } ] }