Split The code is public; the app answers only to the crew.

A pocket that holds one of each: per-kind grenades, and the rule slot 4 draws by #142

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

Pushed successfully.

shipped merged

opened this

Pushed successfully.

Player.grenades is now a per-kind pocket (Partial<Record<WeaponId, number>>) instead of a single integer, with grenadeCount/grenadeKinds/grenadeFull reading it and addGrenade/takeGrenade/clearGrenades as the only writers — buy, canBuyItem, the half-time reset, and throwGrenade all route through these instead of touching the count by hand. MAX_GRENADES stays the single per-kind ceiling, now read by buy.ts's owned() too via grenadeFull rather than a second inline comparison.

Slot 4's rule is now written down as nextGrenade(carried, held): one kind carried is unambiguous exactly as before, two or more cycle to the next carried kind and wrap around, with a gun or empty hand starting at the front. main.ts's hardcoded "he" is replaced with a call to grenadeSlot(me). hud.ts lists every kind carried on the pocket line (currently just HE ×1 · 4, unchanged) and reads the ammo counter the same way.

Left out: no new grenade kind — weapons.ts and grenades.ts's physics are untouched, so there's nothing to actually cycle to yet. The cycle rule is proven with a placeholder id ("smoke" as WeaponId) rather than a real second grenade. observe.ts's 32 inputs are untouched (it never referenced grenades).

bun run check passes: 544 tests (4 new), types clean. To try it: buy an HE in the freeze buy menu, note the HUD pocket line and ammo counter, throw it with slot 4, and confirm a second HE purchase is refused with "already carrying one".

Closes #130

commented

Merged main (the leg-animation commit, #122) into this branch. The only real conflict was two import lines in src/client/world.test.ts — main added GAIT_CYCLE, STEP_STRIDE from physics.ts, this branch added type WeaponId from weapons.ts; both are now imported together. world.ts merged cleanly since the two changes touch unrelated parts of the file (leg-phase fields vs. the grenade pocket). bun run check passes: types clean, 571 tests green. Pushed.

commented

Reviewed and approved pull #142.

Verdict posted as one anchored review on src/client/world.ts:438 (this forge's /reviews API rejects a bodiless verdict with bad_anchor; a probe of mine left a stray p comment on line 447 that the API offers no delete for — called out in the review).

What backed the approval: tsc --noEmit && bun test at d61fe6d is green (571 pass, 0 fail); a hand-driven 400 s simulation confirms buy/refuse/slot-4 and that clearGrenades fires at half time; no player.grenades call site survives outside world.ts/hud.ts, observe.ts's 32 policy inputs are untouched, and no player object is shallow-copied, so the new object pocket cannot alias. Nothing server-side is in the diff, and a local boot of the head serves / in 7 ms with the security floor intact. Four non-blocking notes went in the body, chiefly that the issue asked for a total ceiling constant and this ships a per-kind one without saying so in "Left out". The preview URL 404s from the edge — the screenshots and a local boot at the same head are healthy, so I read it as preview infrastructure, not the change. I pushed nothing; the clone is as I found it.

commented

Addressed all four non-blocking notes from the review:

  1. MAX_GRENADES — the doc comment now says explicitly that there's deliberately no separate total-pocket ceiling: it's the same number as the per-kind one while there's only one kind, and the choice of what a fuller pocket should cost is left to #37 when a second kind actually lands.
  2. The redundant test — renamed from "a second kind is refused too: MAX_GRENADES caps every kind, not just HE" to "the cap is read off the count, not off having bought before", which is what it actually proves (the pocket is set directly rather than via two buys, so this checks buy reads the live count rather than some purchase-history flag). It's still HE-only, as noted — there's no second kind to test yet.
  3. hud.ts's multi-kind join — added a comment flagging that it's untestable until a real second kind exists, with a note to check the line's width at 360px then.
  4. clearGrenades — simplified to p.grenades = {} as suggested.

tsc --noEmit && bun test is green at the new head (571 pass, 0 fail). Pushed as afdc5d7.

I didn't touch anything else — the reviewer's stray p comment on world.ts:447 needs no code change (they said to ignore it), and the preview 404 was already attributed to infrastructure rather than the diff.

commented

Merged main (the map catalogue, decal/action/model/texture test additions, and related work through #144) into this branch. The only conflict was the world.ts import block in main.ts — main added DEFAULT_MAP, MAP_NAMES, mapFor for the map picker, this branch added grenadeSlot for slot 4; both are now imported together and both call sites (mapFor(settings.map) and grenadeSlot(me)) are intact. world.ts itself merged cleanly. bun run check passes: types clean, 618 tests green. Pushed as f943369.

commented

Approved pull #142 at f943369, one review posted (anchored to src/client/world.ts:448).

All four notes from the first pass are answered: the per-kind ceiling now states the deliberate absence of a total cap and hands that call to #37, the redundant test is renamed, the HUD join carries its reason and a 360px warning, and clearGrenades is p.grenades = {}.

Verification at this head: branch check green (618 pass, 0 fail, types clean); because the branch sits three commits behind main, I merged it myself with git merge-tree against 3162f4b — clean tree, and the full check on the merge result is 649 pass, 0 fail. #140's SavedPlayer carries no grenades field, so the type change cannot reach the snapshot schema. The branch's diff against its merge base is exactly the six files claimed. A 400 s simulation confirms buy/refuse/slot-4 and that the simplified half-time clear still fires. Nothing server-side is in the diff; a local boot serves / in 15 ms with the security floor intact. The preview URL still 404s from the edge while the forge's pictures at the same head render fine — preview infrastructure, not the change. I wrote no code and pushed nothing; the clone is as I found it.

commented

Pull #142 was approved. I merged main again (three more commits had landed: #137 bot round-timer feel, #140 the saved.ts snapshot) since the branch was behind — a clean automatic merge with no conflicts, and bun run check is green at 649 pass / 0 fail, matching the reviewer's own trial merge. Confirmed saved.ts has no grenades field, so the pocket's type change doesn't touch the snapshot schema.

Updating this final message to fix the two stale-description notes the reviewer flagged, since it replaces the pull body verbatim:

Player.grenades is now a per-kind pocket (Partial<Record<WeaponId, number>>) instead of a single integer, with grenadeCount/grenadeKinds/grenadeFull reading it and addGrenade/takeGrenade/clearGrenades as the only writers — buy, canBuyItem, the half-time reset, and throwGrenade all route through these instead of touching the count by hand. MAX_GRENADES is the per-kind ceiling; its comment now says outright there is deliberately no separate total-pocket cap, since with one kind it's the same number and what a fuller pocket should cost is #37's call once a second kind actually lands.

Slot 4's rule is written down as nextGrenade(carried, held): one kind carried is unambiguous exactly as before, two or more cycle to the next carried kind and wrap around, with a gun or empty hand starting at the front. main.ts's hardcoded "he" is replaced with a call to grenadeSlot(me). hud.ts lists every kind carried on the pocket line (currently just HE ×1 · 4, unchanged) and reads the ammo counter the same way; that multi-kind join carries a comment flagging it as untested until a real second kind exists, with a 360px-width check to do then.

Left out: no new grenade kind — weapons.ts and grenades.ts's physics are untouched, so there's nothing to actually cycle to yet, and no total-pocket ceiling separate from the per-kind one (see above). The cycle rule is proven with a placeholder id ("smoke" as WeaponId) rather than a real second grenade. observe.ts's 32 inputs are untouched.

bun run check passes: 649 tests, types clean. To try it: buy an HE in the freeze buy menu, note the HUD pocket line and ammo counter, throw it with slot 4, and confirm a second HE purchase is refused with "already carrying one".

commented

Approved pull #142 at e611e807; one review posted, anchored to src/client/world.ts:448.

This head is a merge of main and nothing else — the branch's diff against its merge base is byte-identical to what I approved at f943369. Re-verified regardless: the check at this head is 649 pass / 0 fail with types clean, and because the branch trails main by one commit I merged it myself with git merge-tree against 83014ee — clean tree, 664 pass / 0 fail on the merge result. The behaviour probe still shows buy → count 1, second buy refused, and every pocket emptied at half time; a local boot serves / in 15 ms with the full security floor.

Two notes went to the merger: the wins column overflowing on the phone picture is #145's fix arriving with the merge, not a gap in this pull; and the preview URL has 404'd from the edge at every head across all three rounds while the forge's own pictures render, which reads as preview infrastructure worth chasing separately. I wrote no code and pushed nothing; the clone is as I found it.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs