sneak/rgoue
1
0
Fork 0
Code Issues 10 Pull Requests Actions Packages Projects Releases Wiki Activity

showMap diverges from C's standout handling — decide after PR #38 lands the corrected analysis #39

New Issue
Open
opened 2026-08-09 17:43:36 +02:00 by clawbot · 2 comments
clawbot commented 2026-08-09 17:43:36 +02:00
Collaborator
Copy Link

STATUS: the original body of this issue was WRONG and has been replaced.

The analysis below is the corrected one, verified twice in review (PR #38,
two rounds) and now landed in the tree at 13caec4. The original text — which
claimed C's standout latches on at the first secret square and never turns
off — was mine, written from a subagent's summary rather than from the C, and
is preserved in the comment history rather than here. Do not work from the
earlier comments' description of the mechanism.

The divergence

C's show_map (wizard.c) tests two different things either side of the draw:

if (!(real & F_REAL)) wstandout(hw);   /* the BIT */
...
if (!real) wstandend(hw);              /* the whole FLAG WORD */

The Go port (game/wizard.go) tests isReal on both sides, so it turns
standout off correctly. The port is better than C here — which, under a
faithfulness contract, is itself the divergence.

What C actually does

There are exactly three sites that clear F_REAL, and the only whole-word
write to p_flags anywhere in the C is new_level.c:39 = F_REAL — so there is
no fourth route to a zero flag word:

Site Resulting p_flags
passages.c:281 putpass() — sets F_PASS, then clears F_REAL 0x80, non-zero
passages.c:309 door(), secret arm — clears F_REAL, sets nothing exactly 0
new_level.c:79 trap loop — *sp &= ~F_REAL; *sp |= rnd(NTRAPS) 0 one time in eight (T_DOOR is 00, NTRAPS is 8)

be_trapped (move.c:276) is what ORs F_SEEN later, so the trap case only
yields zero while the trap is unsprung; likewise door()'s case only while
the secret door is unfound.

Consequence: C's wstandend does fire — at secret doors and unsprung
trapdoors. What C gets wrong is leaking standout forward from a secret
passage (or a non-trapdoor trap) until the row-major scan reaches one of those
zero-word squares. The result is intermittent bands of reverse video, not a
permanently reversed map.

The Go counterparts match C at all three sites (game/passages.go:265-271,
:277-297, game/newlevel.go:53-55), so these squares exist in both
languages.

The decision

Option A — keep the port's correct behaviour and document the deliberate
divergence — subject to a final ruling now that the corrected analysis has
landed. Rationale: the faithfulness contract protects what a player can
observe (seed compatibility, message text, RNG call order, save format), and
this touches none of them. It is display-only, wizard-only, and unambiguously
a C typo rather than a C decision. Reproducing intermittent reversed bands in
a debug-only command buys nothing.

MEMORY.md permits this — "behavior must not change ... unless a TODO step
says so" — so deviations are contemplated; they must be deliberate and
recorded.

Definition of done

  1. A comment at showMap giving C's asymmetric test verbatim, the three-site
    mechanism above, the intermittent-bands consequence, and the decision not
    to reproduce it.
  2. An ARCHITECTURE.md §9 entry (§9 is where deliberate departures live).
  3. A test that asserts the chosen behaviour — standout off after a secret
    square — replacing the deliberately-agnostic assertion PR #38 left in
    place. That assertion was correct while this question was open: a reviewer
    confirmed empirically that rewriting the Go to the C-faithful
    *g.Level.FlagsAt(y, x) == 0 leaves the entire suite green, so the current
    test pins only the intersection of both behaviours.
  4. make check green; TODO.md Completed Steps entry; do not rotate
    "Next Step".
  5. Commit title ends with (closes #N).

Implementation requirements

  • Verify the three sites yourself before writing the write-up. This
    analysis has been wrong once already and the write-up is the deliverable —
    documenting a mechanism incorrectly would defeat the entire point.
  • Read C via git show origin/c-master:<file>; do NOT check out or modify
    c-master/modern-rogue.
  • Lint: export GOLANGCI_LINT_CACHE to a fresh empty private dir AND retry
    on parallel golangci-lint is running; a private cache does not prevent the
    lock collision. Treat any result naming paths outside your worktree as void.
  • make targets only. Do NOT modify .golangci.yml. No Dockerfile/CI/script/.
  • Nothing under game/testdata/.
  • Never mention Claude or Anthropic anywhere.
## STATUS: the original body of this issue was WRONG and has been replaced. The analysis below is the **corrected** one, verified twice in review (PR #38, two rounds) and now landed in the tree at `13caec4`. The original text — which claimed C's standout latches on at the first secret square and never turns off — was mine, written from a subagent's summary rather than from the C, and is preserved in the comment history rather than here. Do not work from the earlier comments' description of the mechanism. ## The divergence C's `show_map` (`wizard.c`) tests two different things either side of the draw: ```c if (!(real & F_REAL)) wstandout(hw); /* the BIT */ ... if (!real) wstandend(hw); /* the whole FLAG WORD */ ``` The Go port (`game/wizard.go`) tests `isReal` on **both** sides, so it turns standout off correctly. The port is *better* than C here — which, under a faithfulness contract, is itself the divergence. ## What C actually does There are exactly **three** sites that clear `F_REAL`, and the only whole-word write to `p_flags` anywhere in the C is `new_level.c:39 = F_REAL` — so there is no fourth route to a zero flag word: | Site | Resulting `p_flags` | | --- | --- | | `passages.c:281` `putpass()` — sets `F_PASS`, then clears `F_REAL` | `0x80`, non-zero | | `passages.c:309` `door()`, secret arm — clears `F_REAL`, sets nothing | **exactly `0`** | | `new_level.c:79` trap loop — `*sp &= ~F_REAL; *sp \|= rnd(NTRAPS)` | **`0`** one time in eight (`T_DOOR` is `00`, `NTRAPS` is 8) | `be_trapped` (`move.c:276`) is what ORs `F_SEEN` later, so the trap case only yields zero while the trap is **unsprung**; likewise `door()`'s case only while the secret door is **unfound**. **Consequence:** C's `wstandend` *does* fire — at secret doors and unsprung trapdoors. What C gets wrong is leaking standout **forward** from a secret passage (or a non-trapdoor trap) until the row-major scan reaches one of those zero-word squares. The result is **intermittent bands of reverse video, not a permanently reversed map.** The Go counterparts match C at all three sites (`game/passages.go:265-271`, `:277-297`, `game/newlevel.go:53-55`), so these squares exist in both languages. ## The decision **Option A — keep the port's correct behaviour and document the deliberate divergence** — subject to a final ruling now that the corrected analysis has landed. Rationale: the faithfulness contract protects what a *player* can observe (seed compatibility, message text, RNG call order, save format), and this touches none of them. It is display-only, wizard-only, and unambiguously a C typo rather than a C decision. Reproducing intermittent reversed bands in a debug-only command buys nothing. `MEMORY.md` permits this — "behavior must not change ... **unless a TODO step says so**" — so deviations are contemplated; they must be *deliberate and recorded*. ## Definition of done 1. A comment at `showMap` giving C's asymmetric test verbatim, the three-site mechanism above, the intermittent-bands consequence, and the decision not to reproduce it. 2. An `ARCHITECTURE.md` §9 entry (§9 is where deliberate departures live). 3. A test that **asserts the chosen behaviour** — standout off after a secret square — replacing the deliberately-agnostic assertion PR #38 left in place. That assertion was correct while this question was open: a reviewer confirmed empirically that rewriting the Go to the C-faithful `*g.Level.FlagsAt(y, x) == 0` leaves the entire suite green, so the current test pins only the intersection of both behaviours. 4. `make check` green; `TODO.md` Completed Steps entry; do **not** rotate "Next Step". 5. Commit title ends with ` (closes #N)`. ## Implementation requirements - **Verify the three sites yourself** before writing the write-up. This analysis has been wrong once already and the write-up *is* the deliverable — documenting a mechanism incorrectly would defeat the entire point. - Read C via `git show origin/c-master:<file>`; do NOT check out or modify `c-master`/`modern-rogue`. - **Lint:** export `GOLANGCI_LINT_CACHE` to a fresh empty private dir AND retry on `parallel golangci-lint is running`; a private cache does not prevent the lock collision. Treat any result naming paths outside your worktree as void. - `make` targets only. Do NOT modify `.golangci.yml`. No Dockerfile/CI/`script/`. - Nothing under `game/testdata/`. - Never mention Claude or Anthropic anywhere.
sneak was assigned by clawbot 2026-08-09 17:43:36 +02:00
clawbot commented 2026-08-09 17:44:03 +02:00
Author
Collaborator
Copy Link

Decision: Option A — keep the port's correct behaviour, document the
divergence. sneak has delegated repo-local calls, so I am ruling rather than
parking this. Unassigning him.

I filed this assigned to sneak out of reflex because it touches the
faithfulness contract. On reflection that was wrong: unlike #29 (which changes
the canonical config shared by every repo) this is entirely local to rgoue,
and MEMORY.md already contemplates deviations — "behavior must not change
... unless a TODO step says so". The contract asks for deviations to be
deliberate and recorded, not for them never to happen. So the decision is
in scope; only the recording is mandatory.

Why A over B, restated as a rule rather than a preference: the
faithfulness contract exists to protect what a player can observe — seed
compatibility, message text, RNG call order, save format. This touches none of
those. It is display-only, wizard-only, and unambiguously a C typo rather than
a C decision (!(real & F_REAL) against !real in adjacent lines). Faithfully
reproducing a rendering bug in a debug command buys nothing and costs
readability for the exact person the command exists to help.

The part that actually matters is the recording. An undocumented
improvement is indistinguishable from an unnoticed porting error the next time
somebody diffs the port against C — which is precisely how '+' (#11) stayed
hidden until it was found by accident. So this must not land as "the Go code
happens to be right"; it lands as "the Go code is deliberately right, here is
C's bug, here is why we did not copy it".

Scope for whoever implements this (small, and NOT part of PR #38 — that
PR's tests deliberately pin neither behaviour, which was the right call while
the question was open):

  1. A comment at showMap giving C's asymmetric test verbatim, the mechanism
    (new_level.c seeds p_flags = F_REAL; squares losing the bit keep
    F_PASS or a non-zero rnd(NTRAPS), so real is rarely zero), the visible
    consequence in C (standout latches on at the first secret square), and the
    decision not to reproduce it.
  2. An ARCHITECTURE.md §9-style entry, since §9 is where deliberate
    departures live.
  3. A test that asserts the chosen behaviour — standout off after a secret
    square — replacing PR #38's deliberately-agnostic assertion that stops at
    the first secret square.
  4. make check green; TODO.md Completed Steps entry; do not rotate
    "Next Step".

Verify the analysis before writing any of it. Every step above is
second-hand from PR #38's author and is currently being checked by that PR's
reviewer. If the reviewer's adjudication contradicts any of it, this decision
is void and should be re-opened rather than implemented from this comment. On
this repo, briefs have been wrong twelve times, several of them mine.

Filing the implementation as its own issue once PR #38's review lands and the
analysis is confirmed.

**Decision: Option A — keep the port's correct behaviour, document the divergence.** sneak has delegated repo-local calls, so I am ruling rather than parking this. Unassigning him. I filed this assigned to sneak out of reflex because it touches the faithfulness contract. On reflection that was wrong: unlike #29 (which changes the canonical config shared by every repo) this is entirely local to rgoue, and `MEMORY.md` already contemplates deviations — "behavior must not change ... **unless a TODO step says so**". The contract asks for deviations to be *deliberate and recorded*, not for them never to happen. So the decision is in scope; only the recording is mandatory. **Why A over B**, restated as a rule rather than a preference: the faithfulness contract exists to protect what a *player* can observe — seed compatibility, message text, RNG call order, save format. This touches none of those. It is display-only, wizard-only, and unambiguously a C typo rather than a C decision (`!(real & F_REAL)` against `!real` in adjacent lines). Faithfully reproducing a rendering bug in a debug command buys nothing and costs readability for the exact person the command exists to help. **The part that actually matters is the recording.** An undocumented improvement is indistinguishable from an unnoticed porting error the next time somebody diffs the port against C — which is precisely how `'+'` (#11) stayed hidden until it was found by accident. So this must not land as "the Go code happens to be right"; it lands as "the Go code is deliberately right, here is C's bug, here is why we did not copy it". **Scope for whoever implements this** (small, and NOT part of PR #38 — that PR's tests deliberately pin neither behaviour, which was the right call while the question was open): 1. A comment at `showMap` giving C's asymmetric test verbatim, the mechanism (`new_level.c` seeds `p_flags = F_REAL`; squares losing the bit keep `F_PASS` or a non-zero `rnd(NTRAPS)`, so `real` is rarely zero), the visible consequence in C (standout latches on at the first secret square), and the decision not to reproduce it. 2. An `ARCHITECTURE.md` §9-style entry, since §9 is where deliberate departures live. 3. A test that **asserts the chosen behaviour** — standout off after a secret square — replacing PR #38's deliberately-agnostic assertion that stops at the first secret square. 4. `make check` green; `TODO.md` Completed Steps entry; do not rotate "Next Step". **Verify the analysis before writing any of it.** Every step above is second-hand from PR #38's author and is currently being checked by that PR's reviewer. If the reviewer's adjudication contradicts any of it, this decision is void and should be re-opened rather than implemented from this comment. On this repo, briefs have been wrong twelve times, several of them mine. Filing the implementation as its own issue once PR #38's review lands and the analysis is confirmed.
clawbot changed title from DECISION: showMap is more correct than C — reproduce C's broken standout, or keep the fix? to DECIDED: keep showMap's correct standout; document the deliberate divergence from C 2026-08-09 17:44:03 +02:00
sneak was unassigned by clawbot 2026-08-09 17:44:03 +02:00
clawbot commented 2026-08-09 17:56:16 +02:00
Author
Collaborator
Copy Link

VOID — my ruling above is withdrawn. The analysis it rested on is wrong.

I wrote that the decision was conditional: "if the reviewer's adjudication
contradicts any of it, this decision is void and should be re-opened rather
than implemented from this comment." PR #38's review contradicted it. The
condition has fired, so I am withdrawing rather than quietly patching.

What is still true: the divergence is real. C's show_map tests
!(real & F_REAL) before the draw and !real after; game/wizard.go:129-138
tests isReal both times. The port does not reproduce C's asymmetry.

What is false in my issue body and my ruling — both of which I wrote from
PR #38's author's account without checking:

The enumeration of squares that lose F_REAL names two sites. There are
three:

  1. passages.c putpass() — sets F_PASS, then clears F_REAL → 0x80.
    Correct as stated.
  2. passages.c door(), the secret-door arm — omitted entirely from my
    analysis.     It clears F_REAL on a square whose flags are exactly
    F_REAL, leaving p_flags == 0. It is the more common case:
    rnd(10)+1 < level && rnd(5) == 0 against putpass's rnd(40) == 0.
    game/passages.go:285-296 reproduces it, so such squares exist in both
    languages.
  3. The new_level.c trap loop — *sp |= rnd(NTRAPS) yields 0..7 and T_DOOR
    is 00, so an unseen trapdoor square is also p_flags == 0. My "a
    non-zero rnd(NTRAPS)" is wrong one time in eight.

So the consequence I described is wrong. I said standout latches on at the
first secret square and never turns off, "rendering the remainder of the
wizard map reversed". It does not. C leaks forward from a secret passage (or
a non-trapdoor trap) and clears again at the next secret door or unseen
trapdoor — intermittent bands, not a permanently reversed map.

Does the conclusion survive? I think Option A still wins, and for a
stronger reason than before — but I am not re-ruling from this comment,
because that is the mistake I just made.

The corrected severity cuts both ways. It makes C's bug less damaging, which
weakens "the wizard map is unusable, fix it". But it also makes reproducing
the bug even less valuable: faithfully rendering intermittent reversed bands
in a debug-only command buys nothing at all. What it does change is the
ARCHITECTURE.md text, which under Option A is the entire deliverable — and
writing that from a false description of C would defeat the purpose of
recording it.

Re-ruling once PR #38's rework lands with the corrected analysis in the
tree, so the decision is made against text that has been through review
rather than against a summary. Nothing depends on the timing: PR #38's tests
deliberately pin neither behaviour, and that was independently confirmed —
rewriting the Go to the C-faithful *g.Level.FlagsAt(y, x) == 0 leaves the
entire suite green.

Leaving this open and unassigned. Retitling to drop "DECIDED".

Two lessons I am recording rather than absorbing silently. First: I filed this
issue quoting C line-by-line as though I had read it, when I had read a
subagent's summary of it — the issue reads far more authoritative than my
evidence warranted. Second: stating the void condition is what made this
recoverable. It cost one sentence and saved a wrong decision from being
implemented as fact.

**VOID — my ruling above is withdrawn. The analysis it rested on is wrong.** I wrote that the decision was conditional: "if the reviewer's adjudication contradicts any of it, this decision is void and should be re-opened rather than implemented from this comment." PR #38's review contradicted it. The condition has fired, so I am withdrawing rather than quietly patching. **What is still true:** the divergence is real. C's `show_map` tests `!(real & F_REAL)` before the draw and `!real` after; `game/wizard.go:129-138` tests `isReal` both times. The port does not reproduce C's asymmetry. **What is false in my issue body and my ruling** — both of which I wrote from PR #38's author's account without checking: The enumeration of squares that lose `F_REAL` names two sites. There are **three**: 1. `passages.c putpass()` — sets `F_PASS`, then clears `F_REAL` → `0x80`. Correct as stated. 2. `passages.c door()`, the secret-door arm — **omitted entirely from my analysis.** It clears `F_REAL` on a square whose flags are exactly `F_REAL`, leaving `p_flags == 0`. It is the *more common* case: `rnd(10)+1 < level && rnd(5) == 0` against `putpass`'s `rnd(40) == 0`. `game/passages.go:285-296` reproduces it, so such squares exist in both languages. 3. The `new_level.c` trap loop — `*sp |= rnd(NTRAPS)` yields 0..7 and `T_DOOR` is `00`, so an unseen trapdoor square is also `p_flags == 0`. My "a non-zero `rnd(NTRAPS)`" is wrong one time in eight. **So the consequence I described is wrong.** I said standout latches on at the first secret square and **never** turns off, "rendering the remainder of the wizard map reversed". It does not. C leaks forward from a secret *passage* (or a non-trapdoor trap) and clears again at the next secret *door* or unseen trapdoor — **intermittent bands, not a permanently reversed map.** **Does the conclusion survive?** I think Option A still wins, and for a *stronger* reason than before — but I am not re-ruling from this comment, because that is the mistake I just made. The corrected severity cuts both ways. It makes C's bug less damaging, which weakens "the wizard map is unusable, fix it". But it also makes reproducing the bug even less valuable: faithfully rendering intermittent reversed bands in a debug-only command buys nothing at all. What it *does* change is the `ARCHITECTURE.md` text, which under Option A is the entire deliverable — and writing that from a false description of C would defeat the purpose of recording it. **Re-ruling once PR #38's rework lands with the corrected analysis in the tree**, so the decision is made against text that has been through review rather than against a summary. Nothing depends on the timing: PR #38's tests deliberately pin neither behaviour, and that was independently confirmed — rewriting the Go to the C-faithful `*g.Level.FlagsAt(y, x) == 0` leaves the entire suite green. Leaving this open and unassigned. Retitling to drop "DECIDED". Two lessons I am recording rather than absorbing silently. First: I filed this issue quoting C line-by-line as though I had read it, when I had read a subagent's summary of it — the issue reads far more authoritative than my evidence warranted. Second: stating the void condition is what made this recoverable. It cost one sentence and saved a wrong decision from being implemented as fact.
clawbot changed title from DECIDED: keep showMap's correct standout; document the deliberate divergence from C to showMap diverges from C's standout handling — decide after PR #38 lands the corrected analysis 2026-08-09 17:56:16 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
Review passed and gate is green; ready to merge
 needs-checks
make check is failing on this PR
 needs-rebase
PR conflicts with main and must be rebased
 needs-review
PR awaiting independent adversarial review
 needs-rework
Review found problems; author must rework
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/rgoue#39
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?