From 3c2c25461dd656e17af5cf3ccd06144c1a94054a Mon Sep 17 00:00:00 2001 From: dmccoystephenson Date: Sun, 26 Jul 2026 06:02:46 +0000 Subject: [PATCH] Fix direction lock with tick limit off; record the run on restart 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) --- src/ophidian.py | 46 +++-- src/progression/obituary.py | 1 + tests/progression/test_obituary.py | 10 ++ tests/rendering/test_pygame_keydown_events.py | 13 ++ tests/rendering/test_pygame_run_loop.py | 27 +++ tests/test_ophidian_run_lifecycle.py | 162 ++++++++++++++++++ 6 files changed, 249 insertions(+), 10 deletions(-) create mode 100644 tests/rendering/test_pygame_run_loop.py create mode 100644 tests/test_ophidian_run_lifecycle.py diff --git a/src/ophidian.py b/src/ophidian.py index 3f20c5f..1fdcfce 100644 --- a/src/ophidian.py +++ b/src/ophidian.py @@ -164,6 +164,21 @@ def checkForLevelProgressAndReinitialize(self): self.level += 1 self.initialize() + def restartRun(self): + """Ends the current run on the player's 'r' key. + + The run is recorded first, so restarting banks its currency, + obituary and lifetime stats exactly like dying or quitting does - + previously 'r' jumped straight to reinitializing and silently threw + all of that away (see issue #113). + + Recording lives here rather than inside + checkForLevelProgressAndReinitialize because the collision path + calls that method too, and has already recorded the run by then. + """ + self.recordCurrentRun("restart") + self.checkForLevelProgressAndReinitialize() + def recordCurrentRun(self, causeOfDeath): # bank currency earned this run before folding it into lifetime stats; # recordRun() below calls saveManager.save() which persists both @@ -499,7 +514,7 @@ def handleKeyDownEvent(self, key): else: self.config.limitTickSpeed = True elif key == 'r': - self.checkForLevelProgressAndReinitialize() + self.restartRun() return "restart" elif key == 'c': self.cycleSelectedCosmetic() @@ -550,7 +565,7 @@ def handleKeyDownEvent(self, key): else: self.config.limitTickSpeed = True elif key == self.pygame.K_r: - self.checkForLevelProgressAndReinitialize() + self.restartRun() return "restart" elif key == self.pygame.K_c: self.cycleSelectedCosmetic() @@ -743,6 +758,23 @@ def initialize(self): tail = self.selectedSnakePart.getTail() self.spawnSnakePart(tail, tail.getColor()) + def endOfTick(self): + """Closes out one movement step, for both UI loops. + + Only the sleep is gated on limitTickSpeed - one loop iteration is + exactly one moveEntity call either way, so the tick counter and the + per-tick direction latch have to advance every iteration. Gating + those on limitTickSpeed too meant that pressing 'l' left + changedDirectionThisTick permanently True after the first turn (it + is reset nowhere else, not even in initialize()), locking the snake + into one direction, and froze self.tick so runs recorded a stale + ticksSurvived (see issue #112). + """ + if self.config.limitTickSpeed: + time.sleep(self.config.tickSpeed) + self.tick += 1 + self.changedDirectionThisTick = False + def run(self): if self.config.useTextUI: self.runTextUI() @@ -791,10 +823,7 @@ def runTextUI(self): ) self.textRenderer.renderControls() - if self.config.limitTickSpeed: - time.sleep(self.config.tickSpeed) - self.tick += 1 - self.changedDirectionThisTick = False + self.endOfTick() self.quitApplication() @@ -851,10 +880,7 @@ def runPygameUI(self): self.drawUiMessage() self.pygame.display.update() - if self.config.limitTickSpeed: - time.sleep(self.config.tickSpeed) - self.tick += 1 - self.changedDirectionThisTick = False + self.endOfTick() self.quitApplication() diff --git a/src/progression/obituary.py b/src/progression/obituary.py index 3ca0b37..f16cdb1 100644 --- a/src/progression/obituary.py +++ b/src/progression/obituary.py @@ -14,6 +14,7 @@ CAUSE_OF_DEATH_PHRASES = { "collision": "colliding with itself", "quit": "the player's own hand", + "restart": "a deliberate restart", } diff --git a/tests/progression/test_obituary.py b/tests/progression/test_obituary.py index 7073847..8837376 100644 --- a/tests/progression/test_obituary.py +++ b/tests/progression/test_obituary.py @@ -41,6 +41,10 @@ def test_cause_of_death_phrase_quit(): assert causeOfDeathPhrase("quit") == "the player's own hand" +def test_cause_of_death_phrase_restart(): + assert causeOfDeathPhrase("restart") == "a deliberate restart" + + def test_cause_of_death_phrase_unknown_code_falls_back_to_raw_code(): assert causeOfDeathPhrase("something-new") == "something-new" @@ -67,6 +71,12 @@ def test_format_obituary_lines_interpolates_fields_for_quit(): assert "the player's own hand" in narrative +def test_format_obituary_lines_interpolates_fields_for_restart(): + lines = formatObituaryLines(sampleObituary(causeOfDeath="restart")) + narrative = lines[1] + assert "a deliberate restart" in narrative + + def test_format_obituary_lines_falls_back_when_name_missing(): lines = formatObituaryLines(sampleObituary(name=None)) assert "Unnamed Ophidian" in lines[1] diff --git a/tests/rendering/test_pygame_keydown_events.py b/tests/rendering/test_pygame_keydown_events.py index 7c067c4..66ffccb 100644 --- a/tests/rendering/test_pygame_keydown_events.py +++ b/tests/rendering/test_pygame_keydown_events.py @@ -95,6 +95,19 @@ def test_pygame_r_key_reinitializes_level_and_signals_restart(pygameGame, monkey assert result == "restart" +def test_pygame_r_key_records_the_run_before_restarting(pygameGame): + # regression test: the pygame branch of 'r' discarded the run's + # obituary, currency and lifetime stats along with the text UI's (see + # issue #113) + game = pygameGame + runsBefore = game.saveManager.data["lifetimeStats"]["totalRuns"] + + game.handleKeyDownEvent(pygame.K_r) + + assert game.saveManager.data["lifetimeStats"]["totalRuns"] == runsBefore + 1 + assert game.saveManager.data["obituaries"][-1]["causeOfDeath"] == "restart" + + def test_pygame_c_key_cycles_selected_cosmetic(pygameGame, monkeypatch): game = pygameGame calls = [] diff --git a/tests/rendering/test_pygame_run_loop.py b/tests/rendering/test_pygame_run_loop.py new file mode 100644 index 0000000..df2c32b --- /dev/null +++ b/tests/rendering/test_pygame_run_loop.py @@ -0,0 +1,27 @@ +from ophidian import Ophidian + + +def test_pygame_loop_runs_end_of_tick_without_the_tick_limit(pygameGame, monkeypatch): + # regression test: self.tick and changedDirectionThisTick used to be + # updated inside the `if limitTickSpeed:` block next to the sleep, so + # pressing 'l' in the graphical UI locked the snake into one direction + # forever and froze the tick counter (see issue #112) + game = pygameGame + game.config.limitTickSpeed = False + game.tick = 0 + game.changedDirectionThisTick = True + + monkeypatch.setattr(Ophidian, "quitApplication", lambda self: None) + # one pass only: stopping the loop from the movement step still leaves + # the end-of-tick bookkeeping to run before the while condition is + # re-checked + monkeypatch.setattr( + Ophidian, + "moveEntity", + lambda self, entity, direction: setattr(self, "running", False), + ) + + game.runPygameUI() + + assert game.tick == 1 + assert game.changedDirectionThisTick is False diff --git a/tests/test_ophidian_run_lifecycle.py b/tests/test_ophidian_run_lifecycle.py new file mode 100644 index 0000000..fdc7ab1 --- /dev/null +++ b/tests/test_ophidian_run_lifecycle.py @@ -0,0 +1,162 @@ +import time + +from textui.textrenderer import TextRenderer + +from ophidian import Ophidian +from snake.snakePart import SnakePart + + +def _makeGame(monkeypatch, tmp_path): + monkeypatch.chdir(tmp_path) + monkeypatch.setattr(TextRenderer, "enableRawMode", lambda self: None) + monkeypatch.setattr(TextRenderer, "disableRawMode", lambda self: None) + return Ophidian(useTextUI=True) + + +def _silenceTextRenderer(monkeypatch): + monkeypatch.setattr(TextRenderer, "renderGrid", lambda self, *args: None) + monkeypatch.setattr(TextRenderer, "renderMessage", lambda self, message: None) + monkeypatch.setattr(TextRenderer, "renderStats", lambda self, *args: None) + monkeypatch.setattr(TextRenderer, "renderHud", lambda self, *args: None) + monkeypatch.setattr(TextRenderer, "renderControls", lambda self: None) + monkeypatch.setattr(TextRenderer, "getKeyPress", lambda self, timeout=0: None) + + +def _runOneTextUiIteration(game, monkeypatch): + """Runs exactly one pass of runTextUI by having the movement step stop + the loop, so the end-of-tick bookkeeping still runs before the while + condition is re-checked.""" + _silenceTextRenderer(monkeypatch) + monkeypatch.setattr(Ophidian, "quitApplication", lambda self: None) + monkeypatch.setattr( + Ophidian, + "moveEntity", + lambda self, entity, direction: setattr(self, "running", False), + ) + game.runTextUI() + + +def test_end_of_tick_advances_tick_and_clears_latch_without_the_tick_limit( + tmp_path, monkeypatch +): + # regression test: self.tick and changedDirectionThisTick used to be + # incremented/reset inside the `if limitTickSpeed:` block next to the + # sleep, so pressing 'l' locked the snake into one direction forever and + # froze the tick counter (see issue #112) + game = _makeGame(monkeypatch, tmp_path) + game.config.limitTickSpeed = False + game.tick = 0 + game.changedDirectionThisTick = True + + slept = [] + monkeypatch.setattr(time, "sleep", lambda seconds: slept.append(seconds)) + + game.endOfTick() + + assert game.tick == 1 + assert game.changedDirectionThisTick is False + assert slept == [] + + +def test_end_of_tick_still_sleeps_when_the_tick_limit_is_on(tmp_path, monkeypatch): + game = _makeGame(monkeypatch, tmp_path) + game.config.limitTickSpeed = True + game.config.tickSpeed = 0.25 + game.tick = 0 + game.changedDirectionThisTick = True + + slept = [] + monkeypatch.setattr(time, "sleep", lambda seconds: slept.append(seconds)) + + game.endOfTick() + + assert slept == [0.25] + assert game.tick == 1 + assert game.changedDirectionThisTick is False + + +def test_direction_can_be_changed_again_after_a_tick_without_the_tick_limit( + tmp_path, monkeypatch +): + # the player-facing symptom of issue #112: with the limit off, the first + # turn latched changedDirectionThisTick and every later direction key + # was ignored for the rest of the process + game = _makeGame(monkeypatch, tmp_path) + game.config.limitTickSpeed = False + monkeypatch.setattr(time, "sleep", lambda seconds: None) + game.selectedSnakePart.setDirection(3) # facing right + game.changedDirectionThisTick = False + + game.handleKeyDownEvent("w") + assert game.selectedSnakePart.getDirection() == 0 + assert game.changedDirectionThisTick is True + + game.endOfTick() + + game.handleKeyDownEvent("a") + assert game.selectedSnakePart.getDirection() == 1 + + +def test_text_ui_loop_runs_end_of_tick_without_the_tick_limit(tmp_path, monkeypatch): + # the loop itself must reach the end-of-tick bookkeeping when + # limitTickSpeed is off, not just endOfTick() in isolation + game = _makeGame(monkeypatch, tmp_path) + game.config.limitTickSpeed = False + game.tick = 0 + game.changedDirectionThisTick = True + + _runOneTextUiIteration(game, monkeypatch) + + assert game.tick == 1 + assert game.changedDirectionThisTick is False + + +def test_restart_records_the_run_with_a_restart_cause_of_death(tmp_path, monkeypatch): + # regression test: 'r' used to jump straight to + # checkForLevelProgressAndReinitialize, discarding the run's obituary, + # currency and lifetime stats (see issue #113) + game = _makeGame(monkeypatch, tmp_path) + runsBefore = game.saveManager.data["lifetimeStats"]["totalRuns"] + obituariesBefore = len(game.saveManager.data["obituaries"]) + + game.handleKeyDownEvent("r") + + assert game.saveManager.data["lifetimeStats"]["totalRuns"] == runsBefore + 1 + assert len(game.saveManager.data["obituaries"]) == obituariesBefore + 1 + assert game.saveManager.data["obituaries"][-1]["causeOfDeath"] == "restart" + + +def test_restart_banks_the_currency_earned_this_run(tmp_path, monkeypatch): + game = _makeGame(monkeypatch, tmp_path) + game.saveManager.data["currency"] = 0 + # currencyEarnedForRun is length - 1, mirroring "one per food eaten" + game.snakeParts = [SnakePart((0, 0, 0)) for _ in range(4)] + + game.handleKeyDownEvent("r") + + assert game.saveManager.data["currency"] == 3 + + +def test_restart_records_the_run_before_reinitializing_the_board(tmp_path, monkeypatch): + # order matters: initialize() resets snakeParts back to a single head, + # so recording after the reset would log every restart as length 1 + game = _makeGame(monkeypatch, tmp_path) + game.snakeParts = [SnakePart((0, 0, 0)) for _ in range(6)] + + game.restartRun() + + assert game.saveManager.data["obituaries"][-1]["length"] == 6 + assert len(game.snakeParts) == 1 + + +def test_restart_still_reinitializes_and_signals_restart(tmp_path, monkeypatch): + game = _makeGame(monkeypatch, tmp_path) + calls = [] + monkeypatch.setattr( + game, "checkForLevelProgressAndReinitialize", lambda: calls.append("reinit") + ) + + result = game.handleKeyDownEvent("r") + + assert calls == ["reinit"] + assert result == "restart"