Skip to content

Make the walkthrough seed opt-in: unseeded FrotzEnv is now stochastic (v4.0) - #86

Closed
MarcCote with Copilot wants to merge 2 commits into
masterfrom
copilot/add-warning-for-fixed-seed
Closed

Make the walkthrough seed opt-in: unseeded FrotzEnv is now stochastic (v4.0)#86
MarcCote with Copilot wants to merge 2 commits into
masterfrom
copilot/add-warning-for-fixed-seed

Conversation

Copilot AI commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

FrotzEnv(rom) without a seed silently applied the walkthrough seed (e.g. 12 for Zork I) for any game with bindings, i.e. the whole benchmark suite. Callers who reasonably assumed "no seed → stochastic" were unknowingly running deterministic episodes, applying the fixed-seed handicap the Jericho paper asks researchers to disclose. reset()'s docstring already advertised a use_walkthrough_seed parameter that was never implemented.

This is a breaking change; version bumped to 4.0.0.

API changes (jericho/jericho.py)

  • seed(None) now resolves to -1 (time-dependent) instead of bindings['seed']. As a side effect, an explicit seed=0 is now honored — the previous seed or bindings.get(...) treated it as unset.
  • reset(use_walkthrough_seed=False) implements the documented toggle; True seeds the emulator with the walkthrough seed for that episode, and warns (UnsupportedGameWarning) if the game has none.
  • New FrotzEnv.walkthrough_seed property exposing the game's walkthrough seed, or None.
  • New ImplicitRandomSeedWarning raised on reset() when the game has a walkthrough seed and the caller supplied neither an explicit seed nor use_walkthrough_seed — silent for anyone who made the choice explicit.

Callers and docs

  • tools/test_games.py, tools/find_walkthrough.py and tests/test_jericho.py::test_copy replay walkthroughs, so they now reset with use_walkthrough_seed=True.
  • README gained a "Breaking changes in Jericho 4.0" section; the quickstart walkthrough section documents the new reset argument and flags seeding as a handicap.
  • tests/test_seed.py covers seed resolution, the new property, warning behavior, and walkthrough reproducibility.
env = FrotzEnv("zork1.z5")            # stochastic (time-dependent seed)
env = FrotzEnv("zork1.z5", seed=-1)   # stochastic, explicit → no warning
env.walkthrough_seed                  # 12
env.reset()                           # warns: walkthrough seed no longer implicit
env.reset(use_walkthrough_seed=True)  # deterministic, reproduces get_walkthrough()

Co-authored-by: MarcCote <660004+MarcCote@users.noreply.github.com>
Copilot AI changed the title [WIP] Add warning for silent fixed-seed default in Jericho Make the walkthrough seed opt-in: unseeded FrotzEnv is now stochastic (v4.0) Jul 29, 2026
Copilot AI requested a review from MarcCote July 29, 2026 12:55
@MarcCote MarcCote closed this Jul 29, 2026
@MarcCote MarcCote reopened this Jul 29, 2026
@MarcCote
MarcCote marked this pull request as ready for review July 29, 2026 13:35
@abahgat

abahgat commented Jul 30, 2026

Copy link
Copy Markdown

I reviewed this and folded it into #88, keeping the commit intact since it was a good start. The downstream PR includes a few fixes and stronger tests. I am happy to continue on #88 if that looks good to you (there are a couple open questions for you)

@MarcCote

Copy link
Copy Markdown
Collaborator

Superseded by #88

@MarcCote MarcCote closed this Jul 31, 2026
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.

Silent fixed-seed default prevents independent runs for most games. Consider adding a warning?

3 participants