From d8cb51bb3858ae5847ea0eeea6fb7834493571f7 Mon Sep 17 00:00:00 2001 From: Kenneth Sinder Date: Sun, 14 Jun 2026 13:45:21 -0700 Subject: [PATCH] fix: make generate_mobs hang-proof for any level data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mob-placement loop in World.generate_mobs rejection-sampled a non-spawn platform whose x differed from the longest platform's, looping until it found one. If every non-spawn platform shared the longest's x (e.g. a hand-built level whose platforms all stack at one x), no roll could ever satisfy the exit condition and the build hung forever — an unrecoverable freeze. Previously this was only guarded at the editor boundary (EditorModel.playability refused to test-play such levels). That left World.from_level_data itself unsafe for any other caller. Fix it structurally in the engine instead: - generate_mobs pre-checks whether any eligible platform exists and places no ground mobs when none do, so the loop is provably terminating for any input. When eligible platforms exist it consumes RNG identically to before, so procedurally-generated levels stay bit-for-bit reproducible. - from_level_data now raises a clear ValueError for <2 platforms instead of an opaque IndexError deep in gather_platform_info (which seeds from platforms[1]). - With the hang impossible in the engine, EditorModel.playability drops the distinct-x rule and just enforces the real floor (>=2 platforms); levels that used to be rejected on safety grounds are now allowed. Tests: engine-level no-hang regression (watchdog thread, so a regression fails loudly rather than freezing the suite) + single-platform ValueError; editor playability tests updated to match the relaxed, honest contract. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/exit_dash/world/editing.py | 26 ++++++++++--------- src/exit_dash/world/world.py | 16 ++++++++++++ tests/test_editor.py | 15 ++++++----- tests/test_world.py | 46 ++++++++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 20 deletions(-) diff --git a/src/exit_dash/world/editing.py b/src/exit_dash/world/editing.py index 6c74a2b..edaae42 100644 --- a/src/exit_dash/world/editing.py +++ b/src/exit_dash/world/editing.py @@ -4,10 +4,11 @@ the same records the ``.dat`` format stores (see :mod:`exit_dash.world.level`), enforces the format's fixed-slot limits, and round-trips through :mod:`exit_dash.world.loader`. -:meth:`EditorModel.playability` encodes the structural rules a level must satisfy to be -*built* by :func:`exit_dash.world.world.World.from_level_data` without crashing or -hanging — most importantly that there are enough platforms with distinct x-positions -that the mob-placement loop in ``generate_mobs`` can always terminate. +:meth:`EditorModel.playability` encodes the structural floor a level must meet to be +*built* by :func:`exit_dash.world.world.World.from_level_data` (at least two platforms, +since the build seeds its platform extremes from ``platforms[1]``). Mob placement is now +hang-proof in the engine itself, so the editor no longer needs to police platform +x-positions to keep ``generate_mobs`` terminating. """ from __future__ import annotations @@ -150,14 +151,15 @@ def _in(rec: object, x: float, y: float, w: float, h: float) -> bool: # -- validation & persistence ----------------------------------------------------- def playability(self) -> tuple[bool, str]: - """Return ``(ok, reason)``. A non-ok level would crash/hang the level builder.""" - if len(self.platforms) < 3: - return False, "need at least 3 platforms" - # generate_mobs() loops until it finds a non-spawn platform whose x differs from - # the longest platform's; that only terminates if the non-spawn platforms hold at - # least two distinct x-positions. - if len({p.x for p in self.platforms[1:]}) < 2: - return False, "non-spawn platforms need at least 2 distinct x-positions" + """Return ``(ok, reason)``. A non-ok level would fail to build at all. + + The only hard requirement is a spawn platform plus at least one more: the build + seeds its platform extremes from ``platforms[1]``. Mob placement is hang-proof in + the engine regardless of where the platforms sit, so there are no other structural + rules to enforce here — winnability is left to the level designer. + """ + if len(self.platforms) < 2: + return False, "need at least 2 platforms" return True, "ready to play" def save(self, path: Path) -> None: diff --git a/src/exit_dash/world/world.py b/src/exit_dash/world/world.py index 70ea224..7e77857 100644 --- a/src/exit_dash/world/world.py +++ b/src/exit_dash/world/world.py @@ -123,7 +123,18 @@ def generate_mobs(self, longest: Platform, farthest: Platform) -> None: quantity = len(self.platforms) - 1 sample = AICharacter(0, 0, 0, 0, ("slime", -1, -1)) self.mobs = [] + # Ground mobs are placed on a random non-spawn platform that does not share the + # longest platform's x (the original avoided clustering them there). The rejection + # loop below re-rolls until it finds such a platform, so it only terminates if at + # least one exists; with none it would spin forever. Pre-checking eligibility makes + # generation provably terminating for any level data (e.g. a hand-built level whose + # non-spawn platforms all stack at one x) — we simply place no ground mobs instead. + # When eligible platforms exist this consumes RNG identically to the original, so + # procedurally-generated levels remain bit-for-bit reproducible. + eligible = any(p[0] != longest[0] for p in self.platforms[1:]) for _ in range(quantity): + if not eligible: + break which = self.rng.randint(1, len(self.platforms) - 1) while self.platforms[which][0] == longest[0]: which = self.rng.randint(1, len(self.platforms) - 1) @@ -240,6 +251,11 @@ def from_level_data( background: Background | None = None, hint: str = "", ) -> World: + # gather_platform_info() seeds its running extremes from platforms[1], so a level + # needs a spawn platform plus at least one more to build at all. Fail with a clear + # message rather than an opaque IndexError deep in the build. + if len(data.platforms) < 2: + raise ValueError("a level needs at least two platforms to build") world = cls( player, screen_w, diff --git a/tests/test_editor.py b/tests/test_editor.py index 3810834..041edbd 100644 --- a/tests/test_editor.py +++ b/tests/test_editor.py @@ -60,23 +60,22 @@ def test_round_trips_through_loader(tmp_path): def test_too_few_platforms_is_unplayable(): - model = EditorModel( - platforms=[PlatformRec(0, 0, 100), PlatformRec(0, 0, 100)], - door=DoorRec(0, 0), - ) + # A lone spawn platform can't build: gather_platform_info() seeds from platforms[1]. + model = EditorModel(platforms=[PlatformRec(0, 0, 100)], door=DoorRec(0, 0)) ok, reason = model.playability() assert not ok assert "platform" in reason -def test_identical_x_platforms_is_unplayable(): - # All non-spawn platforms sharing an x would hang generate_mobs; must be rejected. +def test_identical_x_platforms_is_playable(): + # All non-spawn platforms sharing an x used to hang generate_mobs; the engine now + # handles it (placing no ground mobs), so the editor no longer rejects such levels. model = EditorModel( platforms=[PlatformRec(0, 0, 100), PlatformRec(500, 100, 100), PlatformRec(500, 300, 100)], door=DoorRec(0, 0), ) ok, _ = model.playability() - assert not ok + assert ok def test_editor_scene_places_and_can_test_play(pygame_ready): @@ -99,7 +98,7 @@ def test_editor_scene_places_and_can_test_play(pygame_ready): def test_custom_level_runs_headless(): # The full editor->play path: a level built from editor data boots and runs without - # crashing or hanging (the latter being the generate_mobs risk playability() guards). + # crashing. (generate_mobs is hang-proof in the engine; see test_world.py.) data = EditorModel.blank().to_level_data() app = Application(headless=True) scene = LevelScene( diff --git a/tests/test_world.py b/tests/test_world.py index 01bc893..a8dac8b 100644 --- a/tests/test_world.py +++ b/tests/test_world.py @@ -3,12 +3,14 @@ from __future__ import annotations import random +import threading import pytest from exit_dash.core.paths import asset_path from exit_dash.entities.player import PlayableCharacter from exit_dash.world import loader +from exit_dash.world.level import DoorRec, LevelData, PlatformRec from exit_dash.world.world import World @@ -62,3 +64,47 @@ def test_generated_level_is_reproducible_from_seed(player): assert len(w1.blocks) == len(w2.blocks) assert [e.mob_type for e in w1.enemies] == [e.mob_type for e in w2.enemies] assert w1.door is not None and w1.keys + + +def test_degenerate_level_does_not_hang(player): + # Every non-spawn platform shares one x, so the longest platform is among them and the + # rejection loop in generate_mobs can never find a differently-placed platform. The old + # code spun forever here; the engine must now build the level (placing no ground mobs) + # instead of hanging. Built on a watchdog thread so a regression fails loudly rather + # than freezing the whole suite (pytest-timeout is not a dependency). + data = LevelData( + door=DoorRec(800, 400), + platforms=[ + PlatformRec(-2000, 600, 300), # spawn platform, at a distinct x + PlatformRec(800, 600, 200), + PlatformRec(800, 400, 400), # widest non-spawn -> the "longest", at x=800 + PlatformRec(800, 200, 300), + ], + seed=1.0, + ) + result: dict[str, World] = {} + + def build() -> None: + result["world"] = World.from_level_data( + data, player=player, screen_w=1280, screen_h=720, rng=random.Random(1) + ) + + worker = threading.Thread(target=build, daemon=True) + worker.start() + worker.join(timeout=15.0) + assert not worker.is_alive(), ( + "generate_mobs hung on a degenerate level (infinite-loop regression)" + ) + + world = result["world"] + # The level still builds and is winnable-shaped; only the patrol fly is placed, since + # no platform is eligible to host a ground mob. + assert [e.mob_type for e in world.enemies] == ["fly"] + assert world.door is not None and world.keys + + +def test_single_platform_level_raises(player): + # gather_platform_info() seeds its extremes from platforms[1]; one platform can't build. + data = LevelData(door=DoorRec(0, 0), platforms=[PlatformRec(0, 600, 300)]) + with pytest.raises(ValueError, match="two platforms"): + World.from_level_data(data, player=player, screen_w=1280, screen_h=720)