Show numbers on the wheel, streamline UI, add SFX#2
Merged
Conversation
- render: draw each pocket number rotated radially; brighter Stake-style palette; last result large in the hub centre - ui: replace the bulky right sidebar with a slim left control rail (amount + ½/2×, Lancer) and a top bar (balance pill + recent-numbers chip strip); bet by clicking the table directly - audio: new self-contained SFX engine, loads libpulse-simple via dlopen (no link-time dep, degrades silently); synthesised chip/spin/win cues played on detached threads; mute toggle - simplify HistoryEntry to value-only (no heap strings)
📝 WalkthroughWalkthroughThis PR adds a runtime-loaded audio SFX engine, integrates it into AppState, replaces heap-allocated history strings with numeric entries, restructures the UI from a sidebar to a control rail + play area, and updates rendering and styling to match the new layout. ChangesRoulette UI Reorganization with Audio Engine
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/render.zig (1)
132-146: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftRoute table labels through enum
label()methods instead of hardcoded literals.
drawTablestill embeds UI text directly ("PAIR","ROUGE","NOIR","IMPAIR","COL 1", etc.). Please move these strings behindlabel()methods on the relevant game enums and call those methods here to keep localization and terminology centralized.As per coding guidelines: "
**/*.{zig,css}: UI strings must be in French ...; provide these strings vialabel()methods on game enums".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/render.zig` around lines 132 - 146, The drawZone calls embed UI strings directly; add label() methods on the relevant enums (dozen, range, parity, color, column) that return the French label strings, then replace the literal arguments in drawZone with calls to those enum label() methods (e.g., .{ .parity = .even }.parity.label(), .{ .color = .red }.color.label(), .{ .column = .first }.column.label(), .{ .dozen = .first }.dozen.label(), .{ .range = .low }.range.label()) so all UI text is centralized behind enum.label().Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app.zig`:
- Line 81: The call to self.audio.deinit() can free PCM slices/dynlib while
detached threads spawned by Audio.play are still running; modify the Audio type
to track in-flight workers (atomic counter) and a shutting_down flag, increment
the counter at the start of each detached worker created by Audio.play and
decrement on worker exit, have workers check the shutting_down flag as
appropriate, and change Audio.deinit to set shutting_down=true, wait for the
in-flight counter to reach zero (spin/sleep or use a condition/async wait) and
only then free PCM buffers and close the dynamic library so no live worker can
access freed resources.
In `@src/style.css`:
- Line 57: The CSS currently uses a GTK type selector "spinbutton" which doesn't
match the widget because state.amount_spin has no class; add a CSS class to the
GtkSpinButton in src/ui.zig (e.g., add "amount-spin" or "spin-button" to
state.amount_spin via its style context) and then update src/style.css to target
that class instead of the type selector (e.g., change ".control-rail spinbutton
{" to ".control-rail .amount-spin {"), ensuring the class name matches exactly
between the Zig code and the CSS.
In `@src/ui.zig`:
- Around line 98-105: Replace symbol-only labels with French text by using the
enum label() helpers or explicit French strings when creating the buttons:
instead of gtk_button_new_with_label("½") and gtk_button_new_with_label("2×")
(the half_button/double_button created around halveClicked and doubleClicked),
call the enum's label() or pass French labels like "Réduire de moitié" and
"Multiplier par deux"; do the same for the volume buttons (replace "🔊"/"🔇"
with "Activer le son"/"Désactiver le son" or use the label() accessor). If you
must keep glyphs visually, also set an explicit French accessible name via the
appropriate GTK accessible API so assistive tech sees the French action. Ensure
all changes use the same label() method pattern used elsewhere in the UI code.
---
Outside diff comments:
In `@src/render.zig`:
- Around line 132-146: The drawZone calls embed UI strings directly; add label()
methods on the relevant enums (dozen, range, parity, color, column) that return
the French label strings, then replace the literal arguments in drawZone with
calls to those enum label() methods (e.g., .{ .parity = .even }.parity.label(),
.{ .color = .red }.color.label(), .{ .column = .first }.column.label(), .{
.dozen = .first }.dozen.label(), .{ .range = .low }.range.label()) so all UI
text is centralized behind enum.label().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fbd30ed3-79e4-4de9-9ca1-2026db846aea
📒 Files selected for processing (7)
CLAUDE.mdsrc/app.zigsrc/audio.zigsrc/gtk.zigsrc/render.zigsrc/style.csssrc/ui.zig
Detached play() threads read the PCM buffers and the dlopen'd lib; deinit could free them mid-playback (use-after-free at shutdown). Track live workers with an atomic counter (incremented before spawn, decremented on worker exit) and have deinit yield until it reaches zero before freeing.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Opengrep zig-swallowed-error flagged the 'catch {}' on std.Thread.yield().
Replace the fallible yield with a non-fallible 1ms nanosleep syscall: no
error to swallow, lower CPU than a busy spin, same shutdown-wait behaviour.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/audio.zig`:
- Around line 95-96: Replace the Linux-specific nanosleep loop with the
cross-platform std.time.sleep: stop constructing the local `poll` timespec and
calling `std.os.linux.nanosleep`; instead, when waiting for
`self.in_flight.load(.acquire) != 0` call `std.time.sleep` with the desired
millisecond duration (e.g., 1ms) to simplify and make the wait portable while
preserving the same backoff behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0fbed31c-57a9-4877-b9a1-965b2c6f4cf1
📒 Files selected for processing (1)
src/audio.zig
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a self-contained audio engine for sound effects, updates the application architecture to include audio, and refactors the UI and data model to improve performance and maintainability. It also includes visual improvements to the roulette wheel and table rendering.
Audio engine integration and architecture updates:
audiomodule (src/audio.zig) that provides synthesized sound effects (chip, spin, win), loads PulseAudio at runtime viadlopen, and ensures sound is optional and non-blocking. TheAudioinstance is now part ofAppStateand is properly initialized and deinitialized. [1] [2] [3] [4] [5] [6] [7]UI and data model refactoring:
HistoryEntrystruct, reducing memory management complexity and improving performance. The history is now displayed as colored chips, with color derived from the number, and the corresponding UI widget was renamed for clarity. [1] [2] [3]GTK and Cairo API enhancements:
gtk.zigwith additional widget and drawing bindings (e.g.,GtkScrolledWindow,gtk_box_remove,gtk_button_set_label, new Cairo functions) to support richer UI features and improved rendering. [1] [2] [3] [4] [5]Visual improvements:
These changes collectively modernize the codebase, improve user experience with sound and visuals, and simplify internal data handling.- render: draw each pocket number rotated radially; brighter Stake-style palette; last result large in the hub centre
Summary by CodeRabbit
New Features
UI/UX Improvements
Documentation