docs/superpowers/notes/ws8e-checkpoint-review-feedback.md
PR: https://github.com/Dacode45/ms-raylib-rs/pull/2 Reviewer: owner (Dacode45) Reviewed at: 2026-05-29
Owner left 10 inline comments on the WS8 checkpoint PR. Per owner direction ("fix if small, otherwise add to future workstreams"), this note records the disposition of each.
6.0-rc)book.yml caching"Should use an apt cache here" / "should use rust caching here"
Disposition: Fixed. Added awalsh128/cache-apt-pkgs-action@v1 for the
Linux build deps (cmake + X11 + audio libs) and Swatinem/rust-cache@v2
(shared-key: book) between toolchain install and cargo build. Both
caches keyed for the WS8 release prep cycle. Other workflows (check.yml,
test.yml, web.yml, sanitizers.yml) can adopt the same pattern in a
follow-up if owner wants symmetric coverage — see future workstream below.
release-sys.yml: verify vendored examples aren't packaged"Need to validate that publish doesn't publish all the native raylib examples. Those should be excluded."
Disposition: Fixed. The existing raylib-sys/Cargo.toml exclude list
already covers raylib/examples/*, raylib/projects/*, raylib/templates/*,
but the workflow now also runs cargo package --list -p raylib-sys and
greps for those paths immediately after the dry-run, failing the build if
anything leaks through. The validation runs in both dry_run=true and
dry_run=false paths, so the safety net catches a regression before any
real publish.
glam_conv.rs: glam matrix index ordering"TODO: Double check that the glam ordering for matrix indicies are the same as the ordering for raylib."
Disposition: Already verified — no code change needed. The semantic
test at raylib-sys/tests/conversions.rs:251
(glam_matrix_translation_matches_semantically) asserts that
Matrix::translate(1.0, 2.0, 3.0) produces the same transform as
glam::Mat4::from_translation(vec3(1.0, 2.0, 3.0)) using
abs_diff_eq at 1e-5. If the column-major ordering disagreed, the
matrices would differ at the translation column — the test forces the
mapping to be correct. The module-level docs in glam_conv.rs:8-19 also
spell out the mapping explicitly.
rgui/controls.rs: debug_assert for ranged inputs"I want a debuh_assert here"
Disposition: Fixed. Added debug_assert!(min_value <= max_value, ...)
with a formatted error message to the 5 ranged controls in
raylib/src/rgui/controls.rs: gui_spinner, gui_value_box,
gui_slider, gui_slider_bar, gui_progress_bar. Each asserts the
range invariant before the FFI call, so misuse panics loudly in debug
builds with a clear message naming the function + the offending values.
These five items are too large or too cross-cutting for the WS8 checkpoint; each gets its own future workstream as appropriate.
test.yml: nobuild-mode CI matrix"Need to validate that tests work with nobuild mode where users bring their own raylib library and we link against it. We can use the libraries released here https://github.com/raysan5/raylib/releases/tag/6.0"
Workstream: "Nobuild-mode CI matrix" (post-release follow-up). Adds a
new CI matrix dimension that downloads the raylib 6.0 prebuilt libraries
from https://github.com/raysan5/raylib/releases/tag/6.0, sets
RAYLIB_BINDGEN_LOCATION (or equivalent for the nobuild feature wiring),
builds + tests the safe crate against the external library, and proves
the nobuild feature is a viable production path. Non-trivial: per-OS
fetch logic, link-flag management, and the matrix expansion across the
existing 3-OS × feature surface.
raylib-sys/src/color.rs: revisit From<&Color> impl"This probably isn't necessary if we implement clone on Color"
Workstream: "Color/Vector conversion ergonomics audit". Color
already derives Copy + Clone, so impl From<&Color> for Color at
raylib-sys/src/color.rs:56-65 is technically redundant for callers
that can do *color or color.clone(). But removing it would break any
caller that uses Into<Color> constraints with a &Color receiver. A
proper resolution requires auditing all Into<Color> callsites (and the
analogous From<&T> impls on Vector2/3/4, Rectangle, etc.) and either
removing the impls + migrating callsites, or keeping them with a comment
explaining the ergonomic use case. Punt to a focused refactor workstream
post-release.
audio_stream_callback.rs: thiserror migration"TODO: Use this_error for all error types."
Workstream: "thiserror migration" (post-release). thiserror = "2.0.12"
is already a [dependencies] entry; the migration is to convert remaining
ad-hoc error types across the safe crate (callbacks/, error returns from
Image::from_*, font loading, etc.) to #[derive(thiserror::Error)].
Crate-wide refactor; needs its own brainstorm to enumerate the affected
error types and decide on a canonical error-hierarchy shape.
databuf.rs: more tests for edge cases"There need to be a lot more tests for edge cases and various uses of databuf. We can use the software renderer mode here to ensure allocations. Maybe this is where we pull in valgrind."
Workstream: "DataBuf testing + memory profiling" (post-release).
Owner confirmed in the WS8e checkpoint reply: "I definitely want to see
more tests (can be a future workstream)". This workstream uses the
existing software_renderer Tier-2 harness (no GPU required) to drive
allocation-heavy paths through DataBuf, plus integrates valgrind (or
the Miri/ASAN equivalents) to validate the lifetime invariants. Likely
pairs with the broader "more tests" intent — could be one umbrella
workstream covering DataBuf, Mesh accessors, FilePathList, ImageBuf,
and any other ManuallyDrop<Box<[T]>>-based wrappers.
rlgl/immediate.rs: coverage audit"Double check that no rlgl functions are missing"
Workstream: "rlgl safe-module coverage audit" (post-release). The safe
rlgl module shipped in WS5 covers the immediate-mode + matrix-stack
surface. A coverage audit compares the full ffi::rlgl_* (or
raylib-sys::rl*) function list against the safe module's public surface
and gaps are either (a) wrapped safely, (b) explicitly documented as
"escape-hatch FFI only", or (c) recorded as future work. Pairs naturally
with the "more tests" workstream — coverage audit + tests for each newly
exposed function.
raylib 6.0.0 (not a local path).
Starts after the final-release workstream lands the publish.The roadmap originally had: WS8 (release prep) → WS9 (showcase + Pages finale) → final-release (publish + canonical merge).
After the WS8 checkpoint review and the cheatsheet parity audit
(cheatsheet-parity-audit.md), the ordering is:
Quality / audit / test workstreams ship before the WS9 showcase deploy finale, so the published 6.0 crate covers the full cheatsheet surface AND has the soundness / coverage debt paid down before users start consuming the gallery examples.
pixel-pointers — wrap GetPixelColor / SetPixelColor with a
safe abstraction over the void * + PixelFormat
(cheatsheet audit §4 #1).hashes — wrap Compute{CRC32,MD5,SHA1,SHA256}, or formally
decline in favor of Rust crates (cheatsheet audit §4 #2).mixed-audio — wrap Attach/DetachAudioMixedProcessor with
a RAII handle distinct from the per-stream variant
(cheatsheet audit §4 #3).raylib-test delete-or-fix — promoted from long-tail by owner
directive (2026-05-29). The integration-xvfb job in test.yml
is non-required; decide whether to fix the window-opening
integration tests under xvfb or delete the crate entirely.-C linker=gcc + libubsan wiring. Currently informational
only; make it green if not gating.GuiGetIcons/GuiLoadIcons + PR #296
(GuiLoadStyleFromMemory) — promoted from long-tail. Currently
unsafe + # Safety doc; replace with proper safe wrappers.6.0.0
(owner intent saved as post-release-bevy-raylib memory).check.yml /
test.yml / web.yml / sanitizers.yml — PR review comments 1+2.paste rewrite or library swap — from WS8d Fold-in 2.AsRef/AsMut/Deref/DerefMut on pointer-owning wrappers
(WS3-scale).get_gamepad_button_pressed transmute — fold opportunistically
if another workstream touches input.rs.rlsw on wasm32 — build.rs platform_from_target reordering.Items 10+ are flexible — owner can reorder when each starts based on what's most painful at the time. The pre-WS9 queue (items 1-9) is owner-locked.