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

chooseSeed silently ignores an unparseable SEED instead of failing loudly #25

New Issue
Open
opened 2026-08-09 08:10:34 +02:00 by clawbot · 0 comments
clawbot commented 2026-08-09 08:10:34 +02:00
Collaborator
Copy Link

Problem

chooseSeed in cmd/rogue/main.go falls back to the time+pid default when
SEED is set but cannot be parsed. The user asked for a specific dungeon and
silently got a random one.

Set-but-invalid configuration should fail loudly. Silently substituting a
different value is the worst of the three options (honour it, reject it, or
quietly ignore it) because nothing distinguishes "I typo'd SEED" from "the
seed worked".

This matters more here than in a typical program: SEED exists precisely so a
dungeon can be reproduced — for bug reports, for the seed-compat work, and
for wizard-mode debugging. A silently-ignored seed produces an unreproducible
session while the user believes it is pinned, which can send someone chasing a
"non-deterministic" bug that is nothing of the kind.

Found by the reworker of PR #23 (recorded there as M5) and correctly left
alone as out of scope.

Definition of done

  1. SEED set and unparseable → the program exits non-zero with a message that
    names SEED and shows the offending value. It must not start a game.
  2. SEED unset → unchanged behaviour (time+pid default).
  3. SEED set and valid → unchanged behaviour.
  4. Decide and document, in a code comment, what counts as valid: is an empty
    SEED= an error or the same as unset? Is a negative value accepted? State
    the choice rather than letting it fall out of strconv.
  5. Tests cover all three cases in item 1-3 plus whatever item 4 settles.
  6. The error goes to stderr, and the terminal is left usable — note the
    check should happen before raw mode is raised, or after Fini; do not
    introduce a path that prints an error into a raw terminal.
  7. make check fully green.
  8. TODO.md updated in the same commit — Completed Steps entry, and do not
    rotate "Next Step".
  9. Commit title ends with (closes #N).

Implementation requirements

  • Check what C does with a bad seed first (git show origin/c-master:main.c,
    and the ROGUEOPTS/seed handling). If C was permissive, this is a
    deliberate divergence and must be commented as such — but "fail loudly on
    invalid config" is the right call regardless, since C had no SEED
    environment variable to be faithful to in the first place. Confirm that.
  • Do NOT change the signal-handling or pendingSaver code settled in PR #23.
  • Do NOT regenerate goldens under game/testdata/.
  • Leave c-master and modern-rogue alone.
  • Lint environment warning: golangci-lint on this host shares one cache and
    lock across ~18 concurrent sessions. Run make lint/make check in a retry
    loop and only accept a run that neither reports
    parallel golangci-lint is running nor mentions paths outside your own
    worktree. Otherwise you may report green on a red branch.
  • make targets only. Do NOT modify .golangci.yml. No Dockerfile/CI/script/.
  • Never mention Claude or Anthropic anywhere.

Depends on

PR #23 (#12) — it touches the same file; land that first to avoid a conflict.

Priority

Low-medium. Small, self-contained, and removes a genuine debugging trap.

## Problem `chooseSeed` in `cmd/rogue/main.go` falls back to the time+pid default when `SEED` is set but cannot be parsed. The user asked for a specific dungeon and silently got a random one. Set-but-invalid configuration should fail loudly. Silently substituting a different value is the worst of the three options (honour it, reject it, or quietly ignore it) because nothing distinguishes "I typo'd `SEED`" from "the seed worked". This matters more here than in a typical program: `SEED` exists precisely so a dungeon can be **reproduced** — for bug reports, for the seed-compat work, and for wizard-mode debugging. A silently-ignored seed produces an unreproducible session while the user believes it is pinned, which can send someone chasing a "non-deterministic" bug that is nothing of the kind. Found by the reworker of PR #23 (recorded there as M5) and correctly left alone as out of scope. ## Definition of done 1. `SEED` set and unparseable → the program exits non-zero with a message that names `SEED` and shows the offending value. It must not start a game. 2. `SEED` unset → unchanged behaviour (time+pid default). 3. `SEED` set and valid → unchanged behaviour. 4. Decide and document, in a code comment, what counts as valid: is an empty `SEED=` an error or the same as unset? Is a negative value accepted? State the choice rather than letting it fall out of `strconv`. 5. Tests cover all three cases in item 1-3 plus whatever item 4 settles. 6. The error goes to **stderr**, and the terminal is left usable — note the check should happen before raw mode is raised, or after `Fini`; do not introduce a path that prints an error into a raw terminal. 7. `make check` fully green. 8. `TODO.md` updated in the same commit — Completed Steps entry, and do **not** rotate "Next Step". 9. Commit title ends with ` (closes #N)`. ## Implementation requirements - Check what C does with a bad seed first (`git show origin/c-master:main.c`, and the `ROGUEOPTS`/seed handling). If C was permissive, this is a deliberate divergence and must be commented as such — but "fail loudly on invalid config" is the right call regardless, since C had no `SEED` environment variable to be faithful to in the first place. Confirm that. - Do NOT change the signal-handling or `pendingSaver` code settled in PR #23. - Do NOT regenerate goldens under `game/testdata/`. - Leave `c-master` and `modern-rogue` alone. - **Lint environment warning:** golangci-lint on this host shares one cache and lock across ~18 concurrent sessions. Run `make lint`/`make check` in a retry loop and only accept a run that neither reports `parallel golangci-lint is running` nor mentions paths outside your own worktree. Otherwise you may report green on a red branch. - `make` targets only. Do NOT modify `.golangci.yml`. No Dockerfile/CI/`script/`. - Never mention Claude or Anthropic anywhere. ## Depends on PR #23 (#12) — it touches the same file; land that first to avoid a conflict. ## Priority Low-medium. Small, self-contained, and removes a genuine debugging trap.
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#25
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?