Add location label to game viewer stats display#1219
Merged
Conversation
Adds a --location CLI flag (e.g. --location "Dallas, TX") that gets included in the status track JSON and displayed in the viewer debug stats above the latency section. https://claude.ai/code/session_01NqVUKRxJYpgtK8hRtPpwxw
Contributor
WalkthroughThe change adds optional location reporting across the project. The Rust backend 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rs/moq-boy/src/main.rs (1)
294-301: Consider usingserde_json's#[serde(skip_serializing_if)]for cleaner optional field handling.The current approach of mutating the JSON value works correctly, but a more idiomatic approach would be to define a struct with
#[serde(skip_serializing_if = "Option::is_none")]on the location field. This avoids manual mutation and clearly expresses intent.That said, the current implementation is functional and the scope of change is minimal, so this is optional.
♻️ Optional: Define a status struct for cleaner serialization
// At module level #[derive(serde::Serialize)] struct StatusPayload<'a> { buttons: &'a [&'a str], reset_in: u64, latency: &'a serde_json::Map<String, serde_json::Value>, #[serde(skip_serializing_if = "Option::is_none")] location: Option<&'a str>, } // Then in the loop: let new_status = StatusPayload { buttons: &held.iter().map(|s| s.as_str()).collect::<Vec<_>>(), reset_in: remaining, latency: &latency_map, location: location.as_deref(), }; let new_status_str = serde_json::to_string(&new_status).unwrap();🤖 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 294 - 301, Replace the manual JSON mutation with a serde-serializable struct to let serde handle optional fields: create a StatusPayload struct (derive Serialize) with fields matching buttons, reset_in, latency and an Option<String> location annotated with #[serde(skip_serializing_if = "Option::is_none")], then construct StatusPayload using held, remaining, latency_map and location.as_deref()/cloned as appropriate and serialize it with serde_json::to_string instead of building/ mutating new_status; this removes the if let Some(loc) = &location block and makes serialization for location automatic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@rs/moq-boy/src/main.rs`:
- Around line 294-301: Replace the manual JSON mutation with a
serde-serializable struct to let serde handle optional fields: create a
StatusPayload struct (derive Serialize) with fields matching buttons, reset_in,
latency and an Option<String> location annotated with
#[serde(skip_serializing_if = "Option::is_none")], then construct StatusPayload
using held, remaining, latency_map and location.as_deref()/cloned as appropriate
and serialize it with serde_json::to_string instead of building/ mutating
new_status; this removes the if let Some(loc) = &location block and makes
serialization for location automatic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0cf97814-5547-4cbb-9a5a-ebd4d63dce71
📒 Files selected for processing (3)
js/moq-boy/src/index.tsjs/moq-boy/src/styles.tsrs/moq-boy/src/main.rs
…lag-lMn8O # Conflicts: # js/moq-boy/src/index.ts # js/moq-boy/src/styles.ts # rs/moq-boy/src/main.rs
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
js/moq-boy/src/index.ts (1)
13-18: Document the new publicGameStatus.locationfield semantics
GameStatusis exported, andlocationsemantics (omitted vs present) should be documented to keep API consumers aligned.Suggested API doc addition
export interface GameStatus { buttons: string[]; latency: Record<string, number>; + /** Optional location label; omitted when backend has no configured location. */ location?: string; stats?: GameStats; }As per coding guidelines, "Document public APIs with clear docstrings or comments".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/moq-boy/src/index.ts` around lines 13 - 18, Add a docstring to the exported GameStatus interface documenting the semantics of the optional location field: describe when location will be present versus omitted, the expected string format/values (examples if applicable), how consumers should interpret an omitted value versus an empty string, and any guarantees (e.g., never null). Update the comment directly above the location?: string; declaration in the GameStatus interface in index.ts so API consumers can rely on consistent behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/moq-boy/src/index.ts`:
- Around line 265-271: The current truthy check for json.location hides the
element when the payload contains an empty string; change the condition to an
explicit presence check (e.g., testing json.location !== undefined or
Object.prototype.hasOwnProperty.call(json, "location")) so that locationEl is
shown whenever the location field exists in the payload (even if empty) and
still hidden only when the field is absent; update the block around locationEl
and json.location to use that explicit check.
---
Nitpick comments:
In `@js/moq-boy/src/index.ts`:
- Around line 13-18: Add a docstring to the exported GameStatus interface
documenting the semantics of the optional location field: describe when location
will be present versus omitted, the expected string format/values (examples if
applicable), how consumers should interpret an omitted value versus an empty
string, and any guarantees (e.g., never null). Update the comment directly above
the location?: string; declaration in the GameStatus interface in index.ts so
API consumers can rely on consistent 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: CHILL
Plan: Pro
Run ID: c619e9d1-36d6-4907-be4b-d072a48da8b9
📒 Files selected for processing (3)
js/moq-boy/src/index.tsjs/moq-boy/src/styles.tsrs/moq-boy/src/main.rs
✅ Files skipped from review due to trivial changes (1)
- js/moq-boy/src/styles.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- rs/moq-boy/src/main.rs
Comment on lines
+265
to
+271
| // Show location if available. | ||
| if (json.location) { | ||
| locationEl.style.display = "block"; | ||
| locationEl.textContent = json.location; | ||
| } else { | ||
| locationEl.style.display = "none"; | ||
| } |
Contributor
There was a problem hiding this comment.
Use explicit presence check for location instead of truthiness
Line [266] uses a truthy check, so "" is treated as “missing” even when the field is present in payload. Prefer checking for undefined.
Proposed fix
- if (json.location) {
+ if (json.location !== undefined) {
locationEl.style.display = "block";
locationEl.textContent = json.location;
} else {
locationEl.style.display = "none";
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/moq-boy/src/index.ts` around lines 265 - 271, The current truthy check for
json.location hides the element when the payload contains an empty string;
change the condition to an explicit presence check (e.g., testing json.location
!== undefined or Object.prototype.hasOwnProperty.call(json, "location")) so that
locationEl is shown whenever the location field exists in the payload (even if
empty) and still hidden only when the field is absent; update the block around
locationEl and json.location to use that explicit check.
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.
Summary
This PR adds support for displaying a location label in the game viewer's stats panel. The location is configurable via a command-line argument and is dynamically shown/hidden based on the game status updates.
Key Changes
--locationCLI argument to allow specifying a location label (e.g., "Dallas, TX") that gets included in the game status JSONGameStatusinterface with optionallocationfieldlocationElDOM element in the controls panel to display the location#buildControls()return type to include the location element.locationclass with monospace font, green color (#8bac0f), and appropriate sizing to match the latency list stylingImplementation Details
--locationargument was providedhttps://claude.ai/code/session_01NqVUKRxJYpgtK8hRtPpwxw