Skip to content

Fix MissionSpace validation with overlapping placeholders - #525

Merged
mwydmuch merged 1 commit into
Farama-Foundation:mainfrom
jadenfix:fix/mission-overlap-removal
Oct 8, 2026
Merged

mwydmuch merged 1 commit into
Farama-Foundation:mainfrom
jadenfix:fix/mission-overlap-removal

Conversation

@jadenfix

@jadenfix jadenfix commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Description

MissionSpace.contains() can raise IndexError on a valid mission when placeholder words overlap. For example, a space with green, darkgreen and verydarkgreen raises on Get the verydarkgreen ball.. The overlap scan records some removal indices more than once, and deleting earlier entries shifts the remaining indices.

Remove each index once, working backwards. Added regressions for three nested words and for overlaps in two separate placeholder groups, with valid and invalid missions. Both crash cases fail before the fix. No new dependencies are required.

Validation: pytest (3,237 passed, 192 skipped), pytest --doctest-modules minigrid/ (15 passed), and pre-commit run --all-files all pass. The full Linux test matrix also passes on the exact PR commit: Python 3.10–3.14, plus Python 3.10 with Gymnasium 0.29.1 / NumPy 1.26.4 (3,237 passed, 192 skipped in each configuration). All latest-version doctests, pre-commit checks and package build pass.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have run the pre-commit checks with pre-commit run --all-files
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective
  • New and existing unit tests pass locally with my changes

No documentation update is needed for this internal fix.

Upstream pre-commit and the old-dependency / Python 3.10–3.13 test jobs pass. The Python 3.14 job was cancelled during tests; a maintainer rerun has been requested. The full fork matrix above passes against the exact PR commit.

Prepared with OpenAI Codex assistance.

Signed-off-by: Jaden Fix <jadenfix20@gmail.com>
@jadenfix

jadenfix commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

@mwydmuch, could you approve the pending fork CI for this MissionSpace fix when convenient? It removes duplicate indices in reverse order so overlapping placeholders no longer raise IndexError. The full local suite and regression tests pass; validation is in the description. Thanks!

@jadenfix
jadenfix marked this pull request as ready for review October 7, 2026 22:34
@jadenfix

jadenfix commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

@mwydmuch, upstream pre-commit and the old-dependency / Python 3.10–3.13 test jobs now pass. Only the Python 3.14 job was cancelled during tests; could you rerun that job when convenient? The full matrix on my fork also passes against this PR's exact commit, linked in the description. Thanks!

@mwydmuch mwydmuch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank @jadenfix fot the fix

@mwydmuch
mwydmuch merged commit 748510c into Farama-Foundation:main Oct 8, 2026
12 of 13 checks passed
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.

2 participants