Skip to content

Fix food spawning on occupied cells; surface notifications in the text UI - #111

Merged
dmccoystephenson merged 2 commits into
mainfrom
feature/food-spawn-placement-and-text-ui-messages
Jul 25, 2026
Merged

dmccoystephenson merged 2 commits into
mainfrom
feature/food-spawn-placement-and-text-ui-messages

Conversation

@dmccoystephenson

@dmccoystephenson dmccoystephenson commented Jul 25, 2026 •

Copy link
Copy Markdown
Member

Summary

  • spawnFood() now places food on an empty location. It previously searched for an empty location and then threw the result away, handing placement to Environment.addEntity()'s independent random draw. Food therefore landed under snake parts regularly — and moveEntity() checks the destination for a SnakePart before it ever looks for food, so those cells end the run (or burn the second_wind charge) instead of feeding the player. Placement now picks from the set of empty locations directly, and falls back to the old behaviour only when the grid is completely full.
  • The discarded while notFound loop is gone. Wired up as written it hung forever on a full grid; the new test for that case reproduced the hang against the pre-fix source (pytest had to be killed at the 2-minute mark).
  • Text-UI notifications are visible again. notify() only print()ed in text mode, and TextRenderer.renderGrid()'s clearScreen() wipes that print later in the same tick — so speed boosts, second_wind saves, skin unlocks, ascensions and biome-arrival flavor text were all silently dropped for text-UI players. Messages are now queued on the shared uiBanner in both modes, and runTextUI() renders the current one through a new TextRenderer.renderMessage().
  • UI/gameplay decoupling preserved (per the PR Add text-based UI option with clean architecture, comprehensive testing, CI/CD verification, and performance optimizations #95 review decision): renderMessage() takes an already-resolved plain string, so src/textui never imports from src/ui, and gameplay code still only ever calls notify().

Notes

  • test_notify_is_console_only_in_text_ui_mode asserted the now-reversed behaviour and was replaced by test_notify_queues_the_message_in_text_ui_mode.
  • ./format.sh was not run: the repo is not currently black-clean (9 files, including the vendored src/lib/pyenvlib/environment.py, would be reformatted). black --diff confirms it would not touch any line added here, so the diff is left focused on the actual change.

Test plan

  • python3 -m pytest — 138 passed (was 132)
  • python3 -m compileall src — clean
  • New: food lands on the only empty cell; never lands on an occupied cell over 20 spawns on a near-full grid; still spawns when the grid is full
  • New: renderMessage() prints a message, and prints nothing for None/""
  • New: notify() queues in text mode, and runTextUI() hands the queued message to the renderer
  • Verified the pre-fix source fails/hangs on the new food tests

Closes #109
Closes #110


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

spawnFood() searched for an empty location and then discarded it, letting
Environment.addEntity()'s independent random draw place the food - so food
routinely landed under a snake part, on a cell that ends the run instead of
feeding the player. The discarded while-loop also hung forever on a full
grid. Placement now picks from the set of empty locations directly.

notify() only printed in text mode, and renderGrid()'s clearScreen() wipes
that print later in the same tick, so every notification was invisible
there. Messages are now queued on the shared banner in both modes, and the
text loop renders the current one via TextRenderer.renderMessage() - which
takes a plain string, so textui stays independent of the pygame ui package.

Closes #109
Closes #110

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

dmccoystephenson commented Jul 25, 2026 •

Copy link
Copy Markdown
Member Author

Self-review

The fix is correct on both counts and well covered by tests — python3 -m pytest gives 138 passed, python3 -m compileall src is clean.

Verified against source (not from the diff alone):

  • Grid.locations is a dict keyed by location ID (src/lib/pyenvlib/grid.py:105-112), so for locationId in grid.getLocations() yields IDs — the comprehension is correct, and it matches the existing iteration idiom at src/ophidian.py:95 and src/textui/textrenderer.py:56.
  • Environment.addEntityToLocation(entity, location) exists (src/lib/pyenvlib/environment.py:54) and delegates to the grid, so placement is now deterministic instead of a second independent random draw.
  • Ordering in checkForLevelProgressAndReinitialize() puts the head on the grid (src/ophidian.py:728) before spawnFood() (:738), so the head's cell is correctly excluded from the empty set on the very first spawn of each level.
  • UiBanner.current() is documented as "call once per frame — advances/expires the queue as a side effect" (src/ui/banner.py:25-39) and is now called exactly once per loop iteration in each mode: renderMessage() in runTextUI(), UiBanner.draw() in runPygameUI(). No double-advance.
  • renderMessage() takes an already-resolved plain string, so src/textui still imports nothing from src/ui — the PR Add text-based UI option with clean architecture, comprehensive testing, CI/CD verification, and performance optimizations #95 decoupling decision holds.
  • Only renderGrid() calls clearScreen() in TextRenderer, and renderMessage() runs after it, so the message survives the frame it is printed in.
  • README.md's Controls table and the --text-ui flag (src/ophidian.py:864) are unaffected by this change and still match renderControls() / argparse — no doc drift introduced.
  • Collision path checked for the same class of bug: printObituaryToConsole()'s output is followed by time.sleep(self.config.tickSpeed * 20) at src/ophidian.py:328 before the next clearScreen(), so the obituary is already visible in text mode and needs no equivalent fix here.

Non-blocking notes

src/ophidian.py:635-639 — grid.getLocation(locationId) is evaluated twice per cell (once in the filter, once in the result). getLocations() returns the ID→Location dict, so iterating grid.getLocations().values() would be one lookup per cell and read a bit more directly. Purely cosmetic: the grid is small, and the current form matches the getLocations() / getLocation(id) idiom used elsewhere in this file.

src/ophidian.py:208 — in text mode this print() is now guaranteed to be wiped by the next frame's clearScreen(); the banner is the only thing the player actually sees. Worth keeping (it still matters for piped/redirected stdout, and in pygame mode the console is a genuine second channel), but the docstring above reads as if the console print is player-facing in both modes. Addressed below.

(Posted as a PR comment rather than a formal review: the review-submission API call is not available to this session.)


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

- spawnFood() iterates grid.getLocations().values() instead of resolving
  each location ID twice (once in the filter, once in the result)
- notify()'s docstring now says explicitly that in text mode the console
  print only survives in redirected output; the banner is the player-
  facing channel

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

dmccoystephenson commented Jul 25, 2026 •

Copy link
Copy Markdown
Member Author

Both self-review notes applied in 284a0bb:

  • spawnFood() now iterates grid.getLocations().values(), so each location is resolved once instead of twice.
  • notify()'s docstring states explicitly that in text mode the console print only survives in redirected/piped output, and the banner is the player-facing channel.

Re-verified after the change: python3 -m pytest → 138 passed, python3 -m compileall src clean, and black --diff src/ophidian.py still touches none of the lines added by this PR.

Documentation pass (Phase 7) found nothing to fix: README.md's Controls table matches handleKeyDownEvent() for both modes (w/↑, a/←, s/↓, d/→, l, c, p, r, q in both; f11 only in the pygame branch, which is how the table labels it), it matches TextRenderer.renderControls(), and the two usage commands match the single --text-ui argparse flag at src/ophidian.py:864.

This PR is ready for a human to review and merge — this session is not authorized to merge.


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

@dmccoystephenson
dmccoystephenson merged commit 8ebf4b5 into main Jul 25, 2026
@dmccoystephenson
dmccoystephenson deleted the feature/food-spawn-placement-and-text-ui-messages branch July 25, 2026 16:39
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