Skip to content

Fix direction lock with the tick limit off; record the run on restart - #116

Merged
dmccoystephenson merged 1 commit into
mainfrom
feature/tick-loop-and-restart-run-recording
Jul 26, 2026
Merged

Fix direction lock with the tick limit off; record the run on restart#116
dmccoystephenson merged 1 commit into
mainfrom
feature/tick-loop-and-restart-run-recording

Conversation

@dmccoystephenson

@dmccoystephenson dmccoystephenson commented Jul 26, 2026

Copy link
Copy Markdown
Member

Two run-lifecycle correctness bugs in the game loop.

Summary

#112 — turning off the tick-speed limit locked the snake's direction. self.tick += 1 and self.changedDirectionThisTick = False lived inside the if self.config.limitTickSpeed: block next to time.sleep(), in both runTextUI and runPygameUI. handleKeyDownEvent latches changedDirectionThisTick to True on every accepted turn and nothing else ever clears it (initialize() doesn't either), so pressing l meant the snake ignored every direction key after its first turn — for the rest of the process, across restarts. It also froze self.tick, so runs recorded a stale ticksSurvived and totalTicksSurvived stopped accumulating (which gates the frost cosmetic unlock at 300 ticks).

Both loops now call a shared endOfTick() that gates only the sleep on limitTickSpeed. One loop iteration is exactly one moveEntity call either way, so the counter and the latch advance once per iteration. The latch itself is kept — it is what stops the player turning twice between movement steps and reversing into themselves behind the reversal guard's back.

#113 — restarting with r discarded the run. The r handler went straight to checkForLevelProgressAndReinitialize(), so the run's currency, obituary and lifetime stats were silently thrown away, unlike the collision and quit paths which both call recordCurrentRun. A player who restarts rather than dies could never accumulate currency for the shop.

Both UI branches now call a new restartRun() that records the run as cause-of-death "restart" and then reinitializes. Recording lives in restartRun() rather than inside checkForLevelProgressAndReinitialize because the collision path calls that method too and has already recorded by then. "restart" is added to CAUSE_OF_DEATH_PHRASES ("a deliberate restart") so the obituary line reads as narrative instead of falling through to the raw code.

Gameplay logic stays out of both UI layers: endOfTick() and restartRun() live on Ophidian and are called identically from each loop, and no renderer internals are touched.

Test plan

  • python3 -m pytest — 150 passed (138 before, 12 new)
  • python3 -m compileall src clean
  • Verified all 11 new assertions fail against main's src/ (stashed the source change, re-ran: 11 failed)
  • New coverage in tests/test_ophidian_run_lifecycle.py: endOfTick skips the sleep but still advances tick/latch with the limit off; still sleeps tickSpeed with it on; a second direction change is accepted after a tick with the limit off; runTextUI reaches the bookkeeping with the limit off; r records the run, banks its currency, and records before the board resets (so length isn't logged as 1)
  • New coverage in tests/rendering/test_pygame_run_loop.py: same loop-level regression against the real headless pygame loop
  • tests/rendering/test_pygame_keydown_events.py: K_r records the run
  • tests/progression/test_obituary.py: "restart" phrase and its narrative line
  • black/autoflake clean on the new and changed files (not run repo-wide: src/ophidian.py is not black-formatted at main, so ./format.sh would bury this diff in unrelated reformatting)

Notes

README.md needs no change — r is still documented as "restart" and still restarts; it just no longer loses the run's bookkeeping on the way.

Closes #112
Closes #113


This PR description was drafted during a Gardener session (Stephenson-Software/gardener).

Move the tick counter and per-tick direction latch out of the
limitTickSpeed-gated block (issue #112), and record the current run
before 'r' reinitializes the board (issue #113).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dmccoystephenson

dmccoystephenson commented Jul 26, 2026

Copy link
Copy Markdown
Member Author

Self-review

Both fixes are correct and each has a regression test confirmed to fail against main's src/ (the source change was stashed and rerun: 11 of the 12 new assertions fail, the twelfth being a preserved-behaviour check). No UI/gameplay coupling introduced — endOfTick() and restartRun() both live on Ophidian and are called identically from each loop, and no renderer internals are touched from gameplay code.

(Posted as a single comment rather than anchored inline review comments — the review API was not reachable from this session. File and line references below are against the head commit.)

src/ophidian.py:179 — recording order is load-bearing

test_restart_records_the_run_before_reinitializing_the_board pins it: recordCurrentRun reads len(self.snakeParts) for both the obituary length and currencyEarnedForRun, and checkForLevelProgressAndReinitializeinitialize() resets snakeParts to a single head. Recording second would log every restart as length 1 and bank 0 currency.

Note also that checkForLevelProgressAndReinitialize can level up or trigger an ascension when the progress threshold is already met, so r at the threshold still grants the level-up. That behaviour is unchanged by this PR — flagging it only because the run is now recorded at the pre-advance self.level, which is exactly what the collision path already does (recordCurrentRun at src/ophidian.py:341 runs before checkForLevelProgressAndReinitialize at src/ophidian.py:349).

src/ophidian.py:794 — text UI continue now skips endOfTick() as a unit

Pre-existing, not introduced here, but worth noting since the diff moves the tick bookkeeping into endOfTick(): in the text UI, r/p skip both the movement step and endOfTick() for that iteration, so changedDirectionThisTick stays latched for one extra iteration if a direction key and r land in the same pass. Self-correcting on the next iteration, so not worth churning this PR over.

src/ophidian.py:841 — the pygame loop does not behave that way at all

Pre-existing divergence that the tick refactor made visible: this continue sits inside for event in self.pygame.event.get(), not inside while self.running. So it advances to the next event, not the next frame — the "restart" return value from handleKeyDownEvent is effectively a no-op in the graphical UI, while at src/ophidian.py:794 the identical return value skips the whole rest of the text UI's iteration.

Concretely: after r, the pygame loop falls straight through to moveEntity and moves the snake once on the just-initialized board in the same frame; the text UI does not. Same for p (the shop). This is the kind of text-vs-graphical drift PRs #92/#95/#99 kept having to correct.

Out of scope for this PR — filed separately rather than widening the diff.

Considered and deliberately left as-is

  1. No self.collision guard on restartRun(). quitApplication guards with if not self.collision: to avoid double-recording. Tracing whether r can reach restartRun() while self.collision is True shows it cannot: moveEntity sets collision = True, records, and then either calls checkForLevelProgressAndReinitialize() (which reaches initialize(), clearing the flag) or sets running = False, which exits the loop straight into quitApplication without another key ever being handled. Adding the guard would be dead code.

  2. Restarting does not show the pygame obituary overlay. The collision and quit paths both call renderObituaryScreen(), which blocks for 1.5 s. Putting that behind a restart keypress would make r feel broken, and the player still gets immediate feedback that the run was banked because drawHud re-reads the currency every frame and the balance visibly jumps. Console output still carries the full obituary via printObituaryToConsole.

  3. ./format.sh not run repo-wide. src/ophidian.py is not black-formatted at main, so a full run would bury this diff in unrelated reformatting. black/autoflake are clean on the new and changed files.


This comment was drafted during a Gardener session (Stephenson-Software/gardener).

@dmccoystephenson

dmccoystephenson commented Jul 26, 2026

Copy link
Copy Markdown
Member Author

Follow-up for the src/ophidian.py:794 / src/ophidian.py:841 finding above filed as #117. Nothing else from the self-review requires a code change on this branch: findings 1 and 2 were traced to "correct as written" and finding 3 is the deliberate scope of ./format.sh.


This comment was drafted during a Gardener session (Stephenson-Software/gardener).

@dmccoystephenson
dmccoystephenson merged commit 0ee6826 into main Jul 26, 2026
@dmccoystephenson
dmccoystephenson deleted the feature/tick-loop-and-restart-run-recording branch July 26, 2026 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant