Skip to content

docs: state what pygame.quit() and the caches do to draw calls - #69

Merged
dmccoystephenson merged 1 commit into
mainfrom
docs/pin-restart-and-cache-history-behavior
Nov 7, 2026
Merged

dmccoystephenson merged 1 commit into
mainfrom
docs/pin-restart-and-cache-history-behavior

Conversation

@dmccoystephenson

Copy link
Copy Markdown
Member

Summary

Lifecycle probes of each public method found three behaviors the docs either omitted or misstated. No executable code changes: graphik.py changes only in docstrings.

  • Destroyed surfaces (omitted): pygame.quit() destroys the display surface and its subsurfaces, and a Graphik bound to either stays bound to it. On pygame 2.1.2, drawRectangle and drawButton then segfault the process with no traceback, and on a subsurface drawText does too. The class docstring and README now state the hazard and the recovery (assign a live surface to gameDisplay, or build a new Graphik), and that an ordinary off-screen surface keeps working.
  • Whole-number float font sizes (misstated): the docs said a float raises TypeError, but 16.0 is served from the cache once 16 has been drawn. The exception is now documented and pinned. It is not changed, because making it raise could break a caller that works today. The direction is left to a maintainer in drawText: a whole-number float size raises or succeeds depending on what the font cache already holds #68.
  • drawImage display mode (misstated): the docs said it raises unless a display mode is set. Only the first load of a path needs one, because the conversion happens alongside the load. Docs are corrected and the behavior is pinned.
  • Tests: six test cases are added (148 passed, 142 before).
    • test_drawing_to_a_destroyed_surface_fails_after_a_pygame_restart (two cases), in a subprocess so that a segfault cannot end the test run
    • test_rebinding_game_display_restores_every_draw_method_after_a_restart
    • test_off_screen_surface_keeps_drawing_across_a_pygame_restart
    • test_draw_text_accepts_a_whole_float_size_only_once_its_font_is_cached
    • test_draw_image_needs_a_display_mode_only_for_the_first_load_of_a_path

No public member is changed, so the consumer-impact gate does not apply.

No tracking issue was filed for the destroyed-surface and drawImage gaps, which were found during triage. Refs #68, which stays open for the maintainer decision.

Test plan

  • python3 -m py_compile src/main/python/preponderous/graphik/graphik.py
  • python3 -m pytest: 148 passed. The conftest forces the dummy SDL drivers, so this is headless.
  • Temporary mutants of graphik.py (float-aware font key, convert_alpha on every call, reload when no display is set): each was caught by the new tests, and the source was restored.
  • CI matrix (3.9 to 3.13)

Local validation used pygame 2.1.2 on Python 3.8, which is below the supported floor. Installing a newer pygame was not available in this dispatch, so CI is the only check against current pygame versions. The segfault is documented as a 2.1.2 observation for that reason, and the destroyed-surface test asserts only a nonzero exit.

Backlog

Each open issue not picked was skipped for a stated reason:

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

🤖 Generated with Claude Code


drafted by Claude on behalf of Daniel Stephenson

Document and pin three behaviors the docs left out: a Graphik bound to
the display or a subsurface must be rebound after pygame.quit() (on
pygame 2.1.2 drawRectangle segfaults otherwise), a whole-number float
font size is accepted once its font is cached, and drawImage needs a
display mode only for the first load of a path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dmccoystephenson

Copy link
Copy Markdown
Member Author

Self-review rubric, scored against the diff at e1059e1 and CI run 42361748515. This is an inline review by the authoring session, not an independent one. A Copilot review could not be requested (Could not resolve user with login 'copilot').

  • Scope: PASS. Three files changed: docstrings in graphik.py, three README statements, and new tests. Every hunk maps to one of the three behaviors in the PR description.
  • Tests-new: PASS (not applicable). No public method was added.
  • Tests-fix: PASS (not applicable). No bug fix is included. The new tests were instead mutation-checked: a float-aware font key, convert_alpha on every call, and a reload with no display each failed one of them. The source was then restored and 148 tests passed.
  • Sibling structure: PASS. The tests sit beside the font-cache, image-cache and fractional-size groups, reuse _make_graphik, _write_solid_image, _rgb and _count_font_constructions, and follow the subprocess pattern of test_draw_text_survives_a_pygame_restart_with_no_display_mode.
  • Sibling renames: PASS (not applicable). Nothing was renamed.
  • Docs: PASS. The README and docstrings agree on all three behaviors. RELEASING.md is unaffected, and the Dependencies section is unchanged.
  • Issue resolution: PASS (not applicable). No Closes reference is used, and drawText: a whole-number float size raises or succeeds depending on what the font cache already holds #68 stays open for the maintainer decision.
  • Manual validation: PASS. py_compile and pytest (148 passed) are green locally, and all five CI jobs are green on e1059e1.
  • camelCase: PASS. No public method was added or renamed.
  • Backward-compat: PASS. No executable line changed, so no call behaves differently. gh search code was not run: it needs approval that this dispatch cannot grant, and the change that would have required it (making whole floats raise) was left out for that reason.
  • Headless: PASS. The new tests ran under the dummy driver locally and in CI. The subprocess test inherits the conftest's environment.
  • Version sync: PASS. No version string was touched.
  • No new deps: PASS. No import was added.

Do-not-auto-merge paths: none matched. .github/workflows/, LICENSE and the version source are untouched, and no file deletes more than 50 lines.

Findings accepted without change:

  • src/main/python/preponderous/graphik/graphik.py:28 — the library could detect a dead display (get_size() raises on one) and raise instead of crashing, but the same check does not cover subsurfaces, and a per-call guard is a design change. It is documented rather than guarded here.
  • src/main/python/preponderous/graphik/graphik.py:159 — documenting a quirk makes it look intended. The test's comment says "Characterization, not endorsement", and drawText: a whole-number float size raises or succeeds depending on what the font cache already holds #68 records the options.
  • src/test/python/preponderous/graphik/test_graphik.py:819 — the test accepts any nonzero exit, so it cannot tell a segfault from pygame.error. That is deliberate: the outcome varies by pygame version, and CI confirmed only that it is nonzero on the installed versions, not which failure occurred there.
  • drawRectangle(2**31 - 5, 0, 100, 10, color) also segfaulted on pygame 2.1.2, which conflicts with the README's statement that off-surface positions raise nothing. It is not documented here, because a coordinate near the 32-bit limit is not a realistic input.

The PR description's CI checkbox was written before CI finished and is left unticked. CI's result is recorded here.

Verdict: ready to merge.

This review comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).


drafted by Claude on behalf of Daniel Stephenson

@dmccoystephenson
dmccoystephenson merged commit 5c2cbf0 into main Nov 7, 2026
5 checks passed
@dmccoystephenson
dmccoystephenson deleted the docs/pin-restart-and-cache-history-behavior branch November 7, 2026 09:56
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.

1 participant