fix: make generate_mobs hang-proof for any level data#3
Merged
Conversation
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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The mob-placement loop in
World.generate_mobsrejection-sampled a non-spawn platform whose x differed from the longest platform's, looping until it found one:If every non-spawn platform shares the longest's x (e.g. a hand-built level whose platforms all stack at one x), no roll can ever satisfy the exit condition and the build hangs forever — an unrecoverable freeze, not a catchable crash.
Until now this was only guarded at the editor boundary (
EditorModel.playabilityrefused to test-play such a level). That leftWorld.from_level_dataitself unsafe for every other caller (tests, future code, a hand-edited.dat). This fixes it structurally in the engine so the editor guard is no longer load-bearing for safety.Changes
generate_mobsis now provably terminating for any input. It pre-computes whether any eligible (differently-placed) non-spawn platform exists and places no ground mobs when none do, instead of spinning. When eligible platforms exist it consumes RNG identically to before — so procedurally-generated levels stay bit-for-bit reproducible (the determinism tests confirm this).from_level_datafails legibly.<2platforms now raises a clearValueErrorinstead of an opaqueIndexErrordeep insidegather_platform_info(which seeds its extremes fromplatforms[1]).EditorModel.playabilitydrops the distinct-x rule and just enforces the real floor (≥2 platforms). Levels previously rejected on safety grounds are now allowed.Tests
test_degenerate_level_does_not_hang— builds the exact all-same-x level that used to hang, on a watchdog thread (so a regression fails loudly rather than freezing the whole suite, sincepytest-timeoutisn't a dep) and asserts only the patrol fly is placed.test_single_platform_level_raises— theValueErrorfloor.98 passed locally; ruff / ruff format / mypy clean; headless
--frames 120smoke exit 0.🤖 Generated with Claude Code