brainray: add rl_draw_text_int and rl_measure_text_int - #292
Conversation
brainray could draw text but not a number. rl_draw_text takes a rant, and Brainrot has no string concatenation and no sprintf, so the only strings a program could ever hand it were literals -- a game could compute a score and had no way to show it. Formatting is fixed at one literal prefix followed by exactly one integer rather than exposing a format string. A Brainrot-supplied "%s" would make the host read an argument that isn't there, and there is no way to check a user-supplied format against the single rizz actually passed, so the format never comes from user code. `pad` is the minimum digit count, zero-padded, so a HUD doesn't jitter as a counter grows; it is clamped so a wild value can't request an enormous allocation. rl_measure_text_int mirrors rl_measure_text so numeric text can be centred the way a literal can. These are the only wrappers besides rl_init_window that allocate. The allocation is deliberately outside the LeakSanitizer brackets, so a missed free() is still reported as brainray's own leak rather than disappearing into the graphics stack's disclaimed globals -- and test_brainray_windowed_run_is_leak_clean now calls both every iteration so a per-call leak accumulates into a failure. Verified by removing the free() and watching that test go red. Also adds a docs guard: docs/brainray.md calls itself the single source of truth for the binding and both READMEs defer to it, which only holds if the table keeps up. The new test asserts every STDROT_EXPORT_SIG name appears there, checked by name rather than by parsing the table so reformatting can't break it or hide a gap. Related to #291 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
COMMENT
The wrappers do what #291 asked: a host-owned "%s%0*d" join, not a Brainrot-supplied format string; pad clamped before snprintf; malloc outside the LSan window and free after DrawText/MeasureText return, so the STDROT_CSTRING scratch is never retained. I ran the documented table through the same format string. It matches, including the negative row.
That is not the same as the tests proving it. The only runtime exercise would still pass if pad and value were ignored, as long as something was allocated and freed. GitHub CI will not save you: test runs pytest -v test_brainrot.py with no $DISPLAY, so neither the windowed leak smoke nor test_docs_consistency.py is on the merge gate. lint / static-analysis / build are already green on this HEAD; static-analysis does not compile brainray/raylib.c.
1. The formatting contract is documented, not tested
MAJOR.
rl_draw_text_int / rl_measure_text_int exist to produce a specific string. The new tests never look at that string. An implementation that strdups the prefix, or that uses %d and drops pad, would still get a green leak smoke and a green docs-name check.
The windowed program already has InitWindow and rl_measure_text. Compare widths against the documented literals ("SCORE 000450", "SCORE -00450") and bet() the equality. That is not a pixel test. It is the cheapest way to make the table in docs/brainray.md load-bearing.
2. Prose says "digits"; %0*d is field width
The table is correct. The sentence above it is not, for any negative value. HUD stability wants field width, so the runtime is the right choice — the words should match it.
VERDICT
This is a tests-and-wording problem, not an implementation or ABI problem. The binding is the issue's B1 API, the format never comes from user code, and the LSan cut around the new heap buffer is the one this file's header actually promised. It can merge as-is. It should not merge with the formatting table treated as tested, because it is not.
Merge: acceptable. Do not treat CI on this PR as coverage of the new wrappers.
Sent by Cursor Automation: Code Reviewer
Addresses the review on #292. The MAJOR finding was right: the leak smoke proved the wrappers allocate and free, not what they render. It would have stayed green against an implementation that ignored pad and value. Adds a separate windowed test that measures rl_measure_text_int against rl_measure_text of the exact literals in docs/brainray.md's table, every row, so the table is load bearing rather than decorative. Verified by mutation, not by assertion: replacing "%s%0*d" with "%s%d" fails the pad-6 row; dropping `value` fails it too; and adding 1 to pad for negatives -- i.e. digit-count semantics instead of field width -- fails specifically on the -450 row, which is the row that distinguishes them. A final assertion requires padded and unpadded to measure differently, so pad cannot be dead code and a degenerate MeasureText returning 0 for everything cannot make the equalities trivially true. The test states what it cannot do as well. Text width is the sum of glyph widths, so a swapped prefix and number measure identically; that is not reachable without reading back the bytes, and the docstring says so rather than implying the contract is fully pinned. Second finding, also right: "minimum number of digits" is wrong for any negative value. -450 at pad 6 is "-00450", six columns holding five digits and a sign, not "-000450". The table, the "exactly as printf's %0*d" sentence and the C were all correct; only the lead sentence and the code comment were not. Fixed the words, not the format string -- field width is the behaviour a HUD wants, since "000450" and "-00450" occupy the same space. Third finding: the docs guard matched the name anywhere in the file, so a prose mention or a half-deleted row satisfied it. It now requires an actual table row, and also fails on the reverse -- a row for a function brainray no longer exports. Verified both directions by mutation; deleting the row while three prose mentions remain now fails. That guard also was not on the merge gate. CI ran `pytest -v test_brainrot.py`, so test_docs_consistency.py never ran there even though `make test` has always collected it -- CI was strictly narrower than the command contributors are told to run, and the pre-existing test_make_help_lists_developer_targets shared the blind spot. CI now collects the whole tests/ directory. The new format test is skipped in headless CI, like every other brainray runtime test, because MeasureText needs InitWindow to load the default font. Putting raylib behind xvfb on the runner would close that and is worth doing, but it is a CI change with its own flakiness risk and does not belong in this PR. Nit from the review: the second snprintf's return is now explicitly discarded. Related to #291 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Thanks — all four findings were correct. Addressed: 1. The formatting contract is documented, not tested (MAJOR). Agreed; the leak smoke only ever proved ownership. Added I verified it by mutation rather than by asserting it works:
That third one is the case your second comment is about, and it's why the negative row earns its place. I also added the distinctness assertion you implied — padded and unpadded must measure differently. That makes "pad is dead code" a definite catch rather than a lucky one, and it fails loudly if On what it can't do: you suggested it would catch concat in the wrong order. It won't — text width is the sum of glyph widths, so 2. "digits" vs field width. Correct, and the split was in both the prose and the code comment while the table, the 3. The docs guard is weaker than advertised, and not on the merge gate. Both true. It now requires an actual table row (anchored to the leading pipe), and additionally fails on the reverse — a row for a function brainray no longer exports. Verified by deleting the row while three prose mentions of the name remain; the old check passed that, the new one fails it. For the gate: CI ran 4. Nit — ignored One thing I did not do. The new format test is still Local: /cursor-review |
There was a problem hiding this comment.
APPROVE
The four findings from the last look are closed on HEAD a5176a8.
The format test now pins every documented row through rl_measure_text_int vs rl_measure_text of the table literals, including -450 / 6 → SCORE -00450, plus a distinctness check so pad cannot be dead and MeasureText cannot be uniformly 0. I re-ran the format string against the table in C; it matches, and the digit-count mutation (pad++ on negatives) produces SCORE -000450, which is the row that test exists to reject. Docs and the br_format_text_int comment now say field width. The docs guard requires a leading-pipe table row and fails the reverse (stale row). CI test now runs pytest -v from tests/, so test_docs_consistency.py is on the merge gate. I ran those three tests here: all passed; deleting the rl_draw_text_int row while leaving the name in prose makes the new check fail.
The windowed format test is still skipif'd without $DISPLAY / raylib. That is the same skip every brainray runtime test already has. Putting raylib behind xvfb is a CI change, not a hole in this PR.
CI on this HEAD: lint / static-analysis / build green; test still running. I could not execute the format test here (no raylib). The leak smoke and the format test will skip on the runner for the same reason they always have.
VERDICT
Implementation, ABI, ownership, and now the documented format contract are aligned. The remaining gap is process: GitHub still will not execute the windowed tests. That is not a reason to hold this merge.
Merge: yes.
Sent by Cursor Automation: Code Reviewer


Description
brainraycould draw text but not a number.rl_draw_texttakes arant, and Brainrot has no string concatenation and nosprintf, so the only strings a program could ever hand it were literals — a game could compute a score and had no way to show it.Adds two wrappers mirroring the existing
rl_draw_text/rl_measure_textpair:valuepad4500SCORE 4504506SCORE 000450-4506SCORE -0045006SCORE 000000Formatting is fixed at one literal prefix plus exactly one integer, not a format string. A Brainrot-supplied
"%s"would make the host read an argument that isn't there, and there's no way to validate a user-supplied format against the singlerizzactually passed — so the format never comes from user code. Narrower thanTextFormat, and it covers the case that actually exists.padis clamped to 32 so a wild value can't request an enormous allocation, and a negative value keeps its sign inside the padded field exactly as printf's%0*ddoes.On the allocation
These are the only wrappers besides
rl_init_windowthat allocate. The allocation sits outside the LeakSanitizer brackets on purpose, per the existing policy in this file's header: raylib's own globals are disclaimed, brainray's own memory stays tracked. So a missedfree()here is still reported as brainray's leak instead of vanishing into the graphics stack.To make sure that's not just a claim,
test_brainray_windowed_run_is_leak_cleannow calls both functions every iteration, so a per-call leak accumulates. Verified by deleting thefree()and watching the test go red, then restoring it.Docs guard
docs/brainray.mdcalls itself the single source of truth for the binding, and bothREADME.mdandexamples/raylib/README.mddefer to it rather than repeating the function list. That only holds if the table keeps up — a wrapper added without a row is undiscoverable. New test asserts everySTDROT_EXPORT_SIGname appears in the doc, checked by name rather than by parsing the table's columns, so reformatting can't break it or hide a gap. Also verified to fail by deleting a row.Related Issue
Related to #291
Type of Change
Checklist
make format-checklocally (ormake formatto fix)make test415 passed.make valgrind400 test cases, 0 errors, exit 0.make format-checkclean.make cppchecknot run — cppcheck isn't installed on this machine, so CI'sstatic-analysisjob is the first to see it.🤖 Generated with Claude Code