Skip to content

Rename dev/ to demo/, split moq-boy into rs/ and js/#1204

Merged
kixelated merged 1 commit into
mainfrom
dev-to-demo
Apr 3, 2026
Merged

Rename dev/ to demo/, split moq-boy into rs/ and js/#1204
kixelated merged 1 commit into
mainfrom
dev-to-demo

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Summary

  • Rename dev/demo/ to better reflect the folder's purpose
  • Split moq-boy into rs/moq-boy (Rust emulator publisher) and js/moq-boy (web viewer), matching repo conventions
  • Keep ROM hosting infrastructure (justfile, wrangler.jsonc, worker.ts) in demo/boy/
  • Flatten src/worker.tsworker.ts in demo/boy/ and demo/pub/
  • Add just dev as alias for just demo web
  • Update all docs and config references (CLAUDE.md, README.md, doc/, Cargo.toml, package.json)

Test plan

  • just check passes
  • just fix passes
  • just demo starts relay + pub + web
  • just dev starts web demo
  • just demo boy starts boy demo
  • cargo build --bin moq-boy builds from rs/moq-boy

🤖 Generated with Claude Code

Rename the dev/ folder to demo/ to better reflect its purpose.
Split the moq-boy hybrid project into rs/moq-boy (Rust emulator)
and js/moq-boy (web viewer), matching the repo's convention.
Keep ROM hosting infrastructure in demo/boy/.
Flatten worker.ts files out of src/ in demo/boy/ and demo/pub/.
Add `just dev` as alias for `just demo web`.

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

coderabbitai Bot commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

This pull request reorganizes the repository structure by renaming the /dev/ directory to /demo/ and splitting the Game Boy emulator (moq-boy) into two separate packages: a Rust native publisher at /rs/moq-boy/ and a JavaScript web viewer at /js/moq-boy/. Configuration files for workspaces (Cargo.toml, package.json, justfile) are updated to reflect the new directory layout. Wrangler, TypeScript, and Vite configurations are adjusted for the relocated components. All documentation files and path references throughout the repository are updated to point to the new /demo/ and component-specific directories.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 61.54% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately captures the two main changes: renaming dev/ to demo/ and splitting moq-boy into separate Rust and JavaScript packages.
Description check ✅ Passed The description provides a clear summary of the changes, test plan, and relevant context directly related to the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev-to-demo
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch dev-to-demo

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (1)
rs/moq-boy/src/emulator.rs (1)

68-71: Prefer .context(...) over re-wrapping these Boytacean errors.

These map_err(anyhow!(...)) calls flatten the source chain, which makes emulator startup/reset failures harder to diagnose. If the boytacean error types implement std::error::Error, switching these to .context("failed to initialize emulator")?, .context("failed to load ROM")?, and .context("failed to reload ROM")? will preserve the underlying cause. As per coding guidelines, Use anyhow::Context (.context("msg")) instead of .map_err(|_| anyhow::anyhow!("msg")) for Rust error conversion.

Also applies to: 85-90

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@rs/moq-boy/src/emulator.rs` around lines 68 - 71, Replace the .map_err(|e|
anyhow::anyhow!("...: {e}"))? patterns with anyhow::Context so the original
Boytacean errors are preserved; specifically change the
gb.load(false).map_err(...) call to gb.load(false).context("failed to initialize
emulator")? and the gb.load_rom(&rom, None).map_err(...) call to
gb.load_rom(&rom, None).context("failed to load ROM")? (and similarly update the
reload ROM call around the gb.load_rom usage at the 85-90 region to
.context("failed to reload ROM")?); also ensure the anyhow::Context trait is in
scope (use anyhow::Context) so .context(...) compiles.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@doc/setup/demo/boy.md`:
- Around line 34-35: Update the example ROM invocation in the "demo boy start"
docs to use the named input required by the recipe/binary: replace the
positional path usage with the named parameter rom=... (or the equivalent flag
--rom ...) so the examples like "demo boy start path/to/game.gb" become "demo
boy start rom=path/to/game.gb" (apply the same change to the other example
occurrences in this file).

In `@rs/moq-boy/src/audio.rs`:
- Around line 51-75: The code currently reuses the Opus encoder frame_size
(derived from ffmpeg_encoder.frame_size(), OPUS_FRAME_SAMPLES) for buffering
input at self.input_sample_rate, causing timing drift for 44.1 kHz input; define
a separate input_frame_size (e.g. compute samples for a 20 ms chunk:
(input_sample_rate * 20) / 1000 or special-case 44100→882) and use that for
sample_buffer accumulation and when creating input frames, then after resampling
use the actual resampled output length (or compute the output duration from
input_frame_size and sample rates) to advance frame_count/timestamps instead of
assuming 960/OPUS_SAMPLE_RATE; update places referencing frame_size,
sample_buffer, frame_count, input_sample_rate, resampler and ffmpeg_encoder to
use the new input_frame_size vs the existing Opus frame_size so timestamps
reflect resampled output duration.

In `@rs/moq-boy/src/main.rs`:
- Around line 268-285: emu.pressed_buttons() returns a HashSet so the collected
Vec held has non-deterministic ordering causing new_status_str to flip even when
buttons unchanged; to fix, after creating held (the Vec from
emu.pressed_buttons()), sort it before building new_status (either by deriving
Ord on Button and calling held.sort() or by calling held.sort_by_key(|b|
format!("{:?}", b))) so the serialized new_status/new_status_str and comparison
with last_status are stable.
- Around line 224-250: The blocking emulator loop currently drains commands with
while let Ok(cmd) = cmd_rx.try_recv() but never exits the surrounding infinite
loop when the channel is closed; update the task that processes cmd_rx (the code
using cmd_rx.try_recv() inside the loop that matches on
input::Command::{Buttons, ViewerLeft, Reset}) to detect
TryRecvError::Disconnected and break out or return Ok(()) so the blocking task
can shut down cleanly (i.e., check the Err variant of try_recv() and on
Disconnected perform an early return or explicit shutdown instead of looping
forever).
- Around line 105-106: Replace the separate AtomicBool and (Mutex, Condvar) pair
with a single Arc<(Mutex<bool>, Condvar)> (reuse the name resume_notify) and
change all uses accordingly: initialize with Mutex::new(true) to start paused;
in the emulator thread, lock the mutex and use a loop that checks the bool guard
and calls cvar.wait(&mut guard) while the flag indicates paused; in every resume
path that previously did paused.store(...) and resume_notify.1.notify_all(),
instead lock the same mutex, set *guard = false (or true per your semantics)
while holding the lock, then call notify_all() on the Condvar; update all
locations that previously accessed paused (AtomicBool) to use the Mutex<bool>
guard so the pause predicate is always guarded by the same mutex.

---

Nitpick comments:
In `@rs/moq-boy/src/emulator.rs`:
- Around line 68-71: Replace the .map_err(|e| anyhow::anyhow!("...: {e}"))?
patterns with anyhow::Context so the original Boytacean errors are preserved;
specifically change the gb.load(false).map_err(...) call to
gb.load(false).context("failed to initialize emulator")? and the
gb.load_rom(&rom, None).map_err(...) call to gb.load_rom(&rom,
None).context("failed to load ROM")? (and similarly update the reload ROM call
around the gb.load_rom usage at the 85-90 region to .context("failed to reload
ROM")?); also ensure the anyhow::Context trait is in scope (use anyhow::Context)
so .context(...) compiles.
🪄 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: CHILL

Plan: Pro

Run ID: b7cd5f3d-e3da-4f85-8482-85cae8299fe6

📥 Commits

Reviewing files that changed from the base of the PR and between adc4ac3 and 650564c.

⛔ Files ignored due to path filters (3)
  • bun.lock is excluded by !**/*.lock
  • demo/pub/bun.lock is excluded by !**/*.lock
  • demo/web/src/favicon.svg is excluded by !**/*.svg
📒 Files selected for processing (64)
  • CLAUDE.md
  • Cargo.toml
  • README.md
  • demo/boy/.gitignore
  • demo/boy/CHANGELOG.md
  • demo/boy/justfile
  • demo/boy/worker.ts
  • demo/boy/wrangler.jsonc
  • demo/justfile
  • demo/pub/justfile
  • demo/pub/media/.gitignore
  • demo/pub/package.json
  • demo/pub/tsconfig.json
  • demo/pub/worker.ts
  • demo/pub/wrangler.jsonc
  • demo/relay/.gitignore
  • demo/relay/justfile
  • demo/relay/leaf0.toml
  • demo/relay/leaf1.toml
  • demo/relay/localhost.toml
  • demo/relay/prod.toml
  • demo/relay/root.toml
  • demo/throttle/enable
  • demo/web/.env
  • demo/web/README.md
  • demo/web/justfile
  • demo/web/package.json
  • demo/web/src/discover.ts
  • demo/web/src/highlight.ts
  • demo/web/src/index.css
  • demo/web/src/index.html
  • demo/web/src/index.ts
  • demo/web/src/mse.html
  • demo/web/src/publish.html
  • demo/web/src/publish.ts
  • demo/web/src/vite-env.d.ts
  • demo/web/tailwind.config.js
  • demo/web/tsconfig.json
  • demo/web/vite.config.ts
  • dev/boy/src/audio.rs
  • dev/boy/src/emulator.rs
  • dev/boy/src/input.rs
  • dev/boy/src/main.rs
  • dev/boy/src/video.rs
  • doc/app/web.md
  • doc/concept/standard/interop.md
  • doc/js/@moq/lite.md
  • doc/js/index.md
  • doc/setup/demo/boy.md
  • doc/setup/demo/web.md
  • js/moq-boy/package.json
  • js/moq-boy/src/index.html
  • js/moq-boy/src/index.ts
  • js/moq-boy/tsconfig.json
  • js/moq-boy/vite.config.ts
  • justfile
  • package.json
  • rs/moq-boy/Cargo.toml
  • rs/moq-boy/src/audio.rs
  • rs/moq-boy/src/emulator.rs
  • rs/moq-boy/src/input.rs
  • rs/moq-boy/src/main.rs
  • rs/moq-boy/src/video.rs
  • rs/moq-relay/README.md
💤 Files with no reviewable changes (5)
  • dev/boy/src/input.rs
  • dev/boy/src/emulator.rs
  • dev/boy/src/main.rs
  • dev/boy/src/video.rs
  • dev/boy/src/audio.rs

Comment thread doc/setup/demo/boy.md
Comment on lines +34 to 35
just demo boy start path/to/game.gb
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

ROM invocation syntax is incorrect in the docs.

The examples use a positional ROM path, but the recipe and binary currently require named rom=... input, which maps to --rom. As written, these commands won’t run successfully.

Proposed doc fix
-just demo boy start path/to/game.gb
+just demo boy start rom=path/to/game.gb
-just demo boy start path/to/other.gb
+just demo boy start rom=path/to/other.gb

Also applies to: 42-43

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doc/setup/demo/boy.md` around lines 34 - 35, Update the example ROM
invocation in the "demo boy start" docs to use the named input required by the
recipe/binary: replace the positional path usage with the named parameter
rom=... (or the equivalent flag --rom ...) so the examples like "demo boy start
path/to/game.gb" become "demo boy start rom=path/to/game.gb" (apply the same
change to the other example occurrences in this file).

Comment thread rs/moq-boy/src/audio.rs
Comment on lines +51 to +75
let frame_size = ffmpeg_encoder.frame_size() as usize;

// Set up resampler if input rate differs from Opus rate.
let resampler = if input_sample_rate != OPUS_SAMPLE_RATE {
Some(ffmpeg_next::software::resampling::Context::get(
ffmpeg_next::format::Sample::I16(ffmpeg_next::format::sample::Type::Packed),
ffmpeg_next::ChannelLayout::STEREO,
input_sample_rate,
ffmpeg_next::format::Sample::I16(ffmpeg_next::format::sample::Type::Packed),
ffmpeg_next::ChannelLayout::STEREO,
OPUS_SAMPLE_RATE,
)?)
} else {
None
};

Ok(Self {
opus,
ffmpeg_encoder,
resampler,
sample_buffer: Vec::new(),
frame_size: if frame_size > 0 { frame_size } else { OPUS_FRAME_SAMPLES },
frame_count: 0,
input_sample_rate,
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

wc -l rs/moq-boy/src/audio.rs

Repository: moq-dev/moq

Length of output: 82


🏁 Script executed:

cat -n rs/moq-boy/src/audio.rs

Repository: moq-dev/moq

Length of output: 5700


🏁 Script executed:

grep -n "frame_size\|push_samples\|encode_frame\|frame_count" rs/moq-boy/src/audio.rs | head -40

Repository: moq-dev/moq

Length of output: 723


🏁 Script executed:

sed -n '80,140p' rs/moq-boy/src/audio.rs

Repository: moq-dev/moq

Length of output: 2176


🏁 Script executed:

sed -n '80,100p' rs/moq-boy/src/main.rs

Repository: moq-dev/moq

Length of output: 852


🏁 Script executed:

python3 <<'EOF'
# Verify the timestamp math
frame_size = 960
opus_rate = 48000
input_rate = 44100

# Time per frame at 44.1 kHz (actual input consumption)
input_frame_ms = (frame_size / input_rate) * 1000

# Time per frame according to timestamp calculation
timestamp_advance_us = (frame_size * 1_000_000) // opus_rate
timestamp_advance_ms = timestamp_advance_us / 1000

# Drift per frame
drift_ms = input_frame_ms - timestamp_advance_ms
drift_per_second = drift_ms * (1000 / frame_size / (1000 / input_rate))

print(f"Input frame duration at 44.1 kHz: {input_frame_ms:.2f} ms")
print(f"Timestamp advance (line 131): {timestamp_advance_ms:.2f} ms")
print(f"Drift per frame: {drift_ms:.2f} ms")
print(f"Frames per second: {input_rate / frame_size:.1f}")
print(f"Cumulative drift per second: {drift_ms * (input_rate / frame_size):.2f} ms")
EOF

Repository: moq-dev/moq

Length of output: 223


Separate the 44.1 kHz input frame size from the 48 kHz Opus frame size to fix audio timing drift.

self.frame_size is derived from the 48 kHz Opus encoder (line 51, storing 960 samples), but it is reused for input buffering at 44.1 kHz (line 91). This causes each input batch to contain 960 samples at 44.1 kHz = ~21.77 ms of audio, while the timestamp at line 131 advances by only 20 ms (960 * 1_000_000 / 48000). The result is 81+ ms of cumulative audio desynchronization per second.

Additionally, line 108 marks the input frame with self.input_sample_rate (44.1 kHz), which is then resampled to 48 kHz before encoding. The resampler will produce approximately 1043 samples at the output, but the timestamp calculation still assumes 960 samples at 48 kHz were generated.

Define a separate input frame size (882 samples/channel for 20 ms at 44.1 kHz) and buffer input accordingly, or derive output timestamps from the actual resampled frame duration rather than the input frame size.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@rs/moq-boy/src/audio.rs` around lines 51 - 75, The code currently reuses the
Opus encoder frame_size (derived from ffmpeg_encoder.frame_size(),
OPUS_FRAME_SAMPLES) for buffering input at self.input_sample_rate, causing
timing drift for 44.1 kHz input; define a separate input_frame_size (e.g.
compute samples for a 20 ms chunk: (input_sample_rate * 20) / 1000 or
special-case 44100→882) and use that for sample_buffer accumulation and when
creating input frames, then after resampling use the actual resampled output
length (or compute the output duration from input_frame_size and sample rates)
to advance frame_count/timestamps instead of assuming 960/OPUS_SAMPLE_RATE;
update places referencing frame_size, sample_buffer, frame_count,
input_sample_rate, resampler and ffmpeg_encoder to use the new input_frame_size
vs the existing Opus frame_size so timestamps reflect resampled output duration.

Comment thread rs/moq-boy/src/main.rs
Comment on lines +105 to +106
let paused = Arc::new(AtomicBool::new(true)); // Start paused until first viewer.
let resume_notify = Arc::new((Mutex::new(()), Condvar::new()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's find the file and examine the relevant code sections
find . -type f -name "main.rs" -path "*/moq-boy/*"

Repository: moq-dev/moq

Length of output: 79


🏁 Script executed:

# Extract the code at the specified line ranges to understand the pause/resume mechanism
# Lines 105-106, 118-121, 141-145, 165-176, 199-206
sed -n '100,210p' rs/moq-boy/src/main.rs | cat -n

Repository: moq-dev/moq

Length of output: 4379


🏁 Script executed:

# Let me get more context around the wait and notify patterns
# to understand the full scope of the race condition
sed -n '100,112p' rs/moq-boy/src/main.rs | cat -n

Repository: moq-dev/moq

Length of output: 673


🏁 Script executed:

# Now let's see all the notify paths to verify the pattern
rg "notify_all" rs/moq-boy/src/main.rs -B 2 -A 1

Repository: moq-dev/moq

Length of output: 559


🏁 Script executed:

# Let me get the complete context of the wait implementation including the outer if
sed -n '99,115p' rs/moq-boy/src/main.rs | cat -n

Repository: moq-dev/moq

Length of output: 774


🏁 Script executed:

# Get all the notify_all() call locations with full context
rg "notify_all" rs/moq-boy/src/main.rs -B 3 -A 0

Repository: moq-dev/moq

Length of output: 669


🏁 Script executed:

# Let me extract the full implementation including the outer if statement
sed -n '99,110p' rs/moq-boy/src/main.rs

Repository: moq-dev/moq

Length of output: 518


🏁 Script executed:

# Get the absolute line numbers by using grep with line numbers
rg "let paused = Arc::new\(AtomicBool" rs/moq-boy/src/main.rs -n

Repository: moq-dev/moq

Length of output: 141


🏁 Script executed:

# Now get the actual lines with proper context
cat -n rs/moq-boy/src/main.rs | sed -n '100,120p'

Repository: moq-dev/moq

Length of output: 957


🏁 Script executed:

# Get the wait loop implementation that starts around line 101-110
cat -n rs/moq-boy/src/main.rs | sed -n '130,165p'

Repository: moq-dev/moq

Length of output: 1274


🏁 Script executed:

# Get all the resume/notify patterns to understand all code paths
cat -n rs/moq-boy/src/main.rs | sed -n '118,125p'

Repository: moq-dev/moq

Length of output: 331


🏁 Script executed:

# Get the pause/resume task at the "Monitor overall pause state" section
cat -n rs/moq-boy/src/main.rs | sed -n '165,185p'

Repository: moq-dev/moq

Length of output: 827


🏁 Script executed:

# Get the blocking emulator loop where it waits
cat -n rs/moq-boy/src/main.rs | sed -n '185,220p'

Repository: moq-dev/moq

Length of output: 1632


Guard the pause predicate with the same mutex as the Condvar.

The emulator thread's wait loop at lines 204–205 holds the mutex while checking paused and calling cvar.wait(). However, all resume paths (lines 121, 144, 176, 180) modify paused via store() and immediately call notify_all() without acquiring the same mutex. This creates a race: if a resume happens after the check at line 204 but before wait() is called at line 205, the notification is lost and the emulator thread blocks indefinitely. Restructure as a single Arc<(Mutex<bool>, Condvar)> and acquire the mutex before modifying paused on all resume paths.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@rs/moq-boy/src/main.rs` around lines 105 - 106, Replace the separate
AtomicBool and (Mutex, Condvar) pair with a single Arc<(Mutex<bool>, Condvar)>
(reuse the name resume_notify) and change all uses accordingly: initialize with
Mutex::new(true) to start paused; in the emulator thread, lock the mutex and use
a loop that checks the bool guard and calls cvar.wait(&mut guard) while the flag
indicates paused; in every resume path that previously did paused.store(...) and
resume_notify.1.notify_all(), instead lock the same mutex, set *guard = false
(or true per your semantics) while holding the lock, then call notify_all() on
the Condvar; update all locations that previously accessed paused (AtomicBool)
to use the Mutex<bool> guard so the pause predicate is always guarded by the
same mutex.

Comment thread rs/moq-boy/src/main.rs
Comment on lines +224 to +250
// Drain pending commands.
while let Ok(cmd) = cmd_rx.try_recv() {
match cmd {
input::Command::Buttons {
buttons,
viewer_id,
ts_ms,
} => {
emu.set_buttons(&viewer_id, buttons.into_iter().collect());
last_input = std::time::Instant::now();

let latency = current_ts_ms - ts_ms;
if latency >= 0.0 {
viewer_latency.insert(viewer_id, (latency, std::time::Instant::now()));
}
}
input::Command::ViewerLeft { viewer_id } => {
emu.viewer_left(&viewer_id);
viewer_latency.remove(&viewer_id);
}
input::Command::Reset => {
tracing::info!("resetting emulator (viewer request)");
emu.reset()?;
last_input = std::time::Instant::now();
}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find the main.rs file
find . -path "*moq-boy*" -name "main.rs" -type f

Repository: moq-dev/moq

Length of output: 79


🏁 Script executed:

# Get the file size to understand how much we need to read
wc -l rs/moq-boy/src/main.rs

Repository: moq-dev/moq

Length of output: 81


🏁 Script executed:

# Read the context around lines 224-250 and 315-319
sed -n '200,280p' rs/moq-boy/src/main.rs | cat -n

Repository: moq-dev/moq

Length of output: 3118


🏁 Script executed:

# Need to see more context - looking at a wider range to understand the outer loop
sed -n '170,260p' rs/moq-boy/src/main.rs | cat -n

Repository: moq-dev/moq

Length of output: 3595


🏁 Script executed:

# Also need to check lines around 315-319 mentioned in the comment
sed -n '305,330p' rs/moq-boy/src/main.rs | cat -n

Repository: moq-dev/moq

Length of output: 836


🏁 Script executed:

# Search for the spawn_blocking and tokio::select! context to understand the full picture
rg -A 5 -B 5 "spawn_blocking" rs/moq-boy/src/main.rs | head -60

Repository: moq-dev/moq

Length of output: 441


🏁 Script executed:

# Let me examine the full outer loop to see all possible exit conditions
sed -n '170,331p' rs/moq-boy/src/main.rs | cat -n

Repository: moq-dev/moq

Length of output: 6268


🏁 Script executed:

# Let's also check the exact line numbers for context by looking at the raw lines
sed -n '224,250p' rs/moq-boy/src/main.rs | nl -v 224

Repository: moq-dev/moq

Length of output: 97


🏁 Script executed:

# And lines 315-319 as mentioned
sed -n '315,320p' rs/moq-boy/src/main.rs | nl -v 315

Repository: moq-dev/moq

Length of output: 97


🏁 Script executed:

# Get exact content of lines 224-250 and 315-319
cat rs/moq-boy/src/main.rs | sed -n '224,250p'

Repository: moq-dev/moq

Length of output: 818


🏁 Script executed:

# Check lines 315-319
cat rs/moq-boy/src/main.rs | sed -n '315,320p'

Repository: moq-dev/moq

Length of output: 258


Exit the blocking emulator loop when the command channel disconnects.

When session.closed() or input::handle_viewers() completes in the tokio::select!, cmd_tx is dropped. However, the outer loop in the blocking task lacks an exit condition for channel disconnection. After cmd_rx.try_recv() begins returning Err(TryRecvError::Disconnected), the while loop exits but the surrounding infinite loop continues, preventing clean shutdown. Return Ok(()) when receiving TryRecvError::Disconnected, or add an explicit shutdown signal.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@rs/moq-boy/src/main.rs` around lines 224 - 250, The blocking emulator loop
currently drains commands with while let Ok(cmd) = cmd_rx.try_recv() but never
exits the surrounding infinite loop when the channel is closed; update the task
that processes cmd_rx (the code using cmd_rx.try_recv() inside the loop that
matches on input::Command::{Buttons, ViewerLeft, Reset}) to detect
TryRecvError::Disconnected and break out or return Ok(()) so the blocking task
can shut down cleanly (i.e., check the Err variant of try_recv() and on
Disconnected perform an early return or explicit shutdown instead of looping
forever).

Comment thread rs/moq-boy/src/main.rs
Comment on lines +268 to +285
let held: Vec<_> = emu.pressed_buttons().iter().copied().collect();
let idle_secs = idle_time.as_secs();
let remaining = timeout_secs.saturating_sub(idle_secs);

let latency_map: serde_json::Map<String, serde_json::Value> = viewer_latency
.iter()
.map(|(k, (ms, _))| (k.clone(), serde_json::json!((*ms as u32))))
.collect();

let new_status = serde_json::json!({
"buttons": held,
"reset_in": remaining,
"latency": latency_map,
});
let new_status_str = new_status.to_string();

if new_status_str != last_status {
last_status = new_status_str.clone();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n rs/moq-boy/src/main.rs | sed -n '260,290p'

Repository: moq-dev/moq

Length of output: 1286


🏁 Script executed:

# Find the definition of pressed_buttons
rg "fn pressed_buttons" rs/moq-boy

Repository: moq-dev/moq

Length of output: 133


🏁 Script executed:

# Search for the emulator type definition
rg "struct.*[Ee]mu|impl.*pressed_buttons" rs/moq-boy -A 3

Repository: moq-dev/moq

Length of output: 243


🏁 Script executed:

rg "enum Button|struct Button" rs/moq-boy/src/emulator.rs -A 10

Repository: moq-dev/moq

Length of output: 127


🏁 Script executed:

# Check if Button implements Ord or has any sorting traits
rg "impl.*Ord|impl.*PartialOrd|#\[derive.*Ord" rs/moq-boy/src/emulator.rs

Repository: moq-dev/moq

Length of output: 37


🏁 Script executed:

# Look for any sorting of buttons before line 268
sed -n '240,285p' rs/moq-boy/src/main.rs

Repository: moq-dev/moq

Length of output: 1470


Sort the button collection before serializing to ensure stable status payloads.

emu.pressed_buttons() returns a HashSet<Button>, and collecting it directly to a Vec produces a non-deterministic order. This causes new_status_str to change even when the pressed buttons haven't changed, triggering redundant status publishes. Sort the buttons before serializing:

let mut held: Vec<_> = emu.pressed_buttons().iter().copied().collect();
held.sort_by_key(|btn| format!("{:?}", btn));  // or derive Ord and use held.sort()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@rs/moq-boy/src/main.rs` around lines 268 - 285, emu.pressed_buttons() returns
a HashSet so the collected Vec held has non-deterministic ordering causing
new_status_str to flip even when buttons unchanged; to fix, after creating held
(the Vec from emu.pressed_buttons()), sort it before building new_status (either
by deriving Ord on Button and calling held.sort() or by calling
held.sort_by_key(|b| format!("{:?}", b))) so the serialized
new_status/new_status_str and comparison with last_status are stable.

@kixelated kixelated merged commit 815cfe6 into main Apr 3, 2026
0 of 2 checks passed
@kixelated kixelated deleted the dev-to-demo branch April 3, 2026 18:06
This was referenced Apr 3, 2026
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