fix(client): refactor event handling and stabilize release workflow - #51
Conversation
…orkflow - Deduplicated GameEvent logic in lib.rs using dispatch_game_event helper - Updated EntityStatus to include dynamic max_hp/max_shield from type defaults - Fixed health bar calculations in playground.ts to use actual max vitals - Simplified release-plz.yml to resolve double release issue and match workspace standards
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 9 minutes and 0 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCentralizes WASM client GameEvent handling into a deferred dispatcher, enriches exported entity statuses with protocol-derived Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,220,255,0.5)
participant Transport
participant Decoder
participant WASM_Tick as "WASM tick"
participant Dispatcher
participant WorldState
end
Transport->>Decoder: poll & read bytes
Decoder->>WASM_Tick: decode NetworkEvent(s)
WASM_Tick->>WASM_Tick: buffer GameEvent(s) into collected_game_events
WASM_Tick->>Dispatcher: iterate collected_game_events
Dispatcher->>WorldState: dispatch_game_event(event)
WorldState-->>Dispatcher: apply side effects (despawn, manifest update, handlers)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/aetheris-client-wasm/src/lib.rs (1)
659-815:⚠️ Potential issue | 🔴 CriticalDefer game-event dispatch until the
transportborrow is released.This loop holds
&mut self.transportfrom line 659 to 852, so the new calls on lines 700 and 809 toself.dispatch_game_event(...)(which takes&mut self) create an E0499 borrow conflict and prevent compilation. Queue the decodedGameEvents during polling and dispatch them after theif let Some(transport) = &mut self.transportscope ends.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/aetheris-client-wasm/src/lib.rs` around lines 659 - 815, The borrow error is caused by calling self.dispatch_game_event(&game_event) while holding &mut self.transport in the poll loop; change the poll block to collect decoded GameEvent instances (e.g., push game_event into a local Vec<GameEvent> like collected_game_events) instead of calling self.dispatch_game_event inside the match arms (references: transport.poll_events, NetworkEvent::GameEvent, self.dispatch_game_event); after the if let Some(transport) = &mut self.transport { ... } scope ends, iterate the collected_game_events and call self.dispatch_game_event for each to release the transport borrow before mutably borrowing self again.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/aetheris-client-wasm/src/lib.rs`:
- Around line 1017-1039: The hard-coded match that derives max_hp/max_shield
inside the entities.into_iter().map producing EntityStatus is out-of-sync and
must be replaced with a lookup from the authoritative entity definitions; remove
the local match on slot.entity_type and instead fetch the canonical stats
(max_hp/max_shield) from the central entity definitions API/registry (use the
module/function that exposes the authoritative definitions) using
slot.entity_type as the key, fall back to a sensible default if the lookup is
missing, and keep the rest of the EntityStatus construction (network_id, hp,
shield, entity_type, is_player) unchanged so exported maxima reflect the real
entity definitions.
---
Outside diff comments:
In `@crates/aetheris-client-wasm/src/lib.rs`:
- Around line 659-815: The borrow error is caused by calling
self.dispatch_game_event(&game_event) while holding &mut self.transport in the
poll loop; change the poll block to collect decoded GameEvent instances (e.g.,
push game_event into a local Vec<GameEvent> like collected_game_events) instead
of calling self.dispatch_game_event inside the match arms (references:
transport.poll_events, NetworkEvent::GameEvent, self.dispatch_game_event); after
the if let Some(transport) = &mut self.transport { ... } scope ends, iterate the
collected_game_events and call self.dispatch_game_event for each to release the
transport borrow before mutably borrowing self again.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6bbd9bd2-76d4-465d-bbf2-3375632b5116
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.github/workflows/release-plz.ymlcrates/aetheris-client-wasm/src/lib.rsplayground/src/playground.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/aetheris-client-wasm/src/lib.rs`:
- Line 669: collected_game_events is declared inside the if-let transport block
but later referenced at dispatch time, so move its declaration to an outer scope
so it lives across the transport block and the dispatch use; specifically,
declare let mut collected_game_events: Vec<...> (use the existing GameEvent
type) before the if let Some(transport) { ... } block and remove the inner
declaration so the same mutable vector is populated inside the block and still
available later when dispatching.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e11160ec-7487-4607-879f-e68148502e9e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
Cargo.tomlcrates/aetheris-client-wasm/CHANGELOG.mdcrates/aetheris-client-wasm/src/lib.rsdocs/CLIENT_DESIGN.md
✅ Files skipped from review due to trivial changes (2)
- crates/aetheris-client-wasm/CHANGELOG.md
- docs/CLIENT_DESIGN.md
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/aetheris-client-wasm/src/lib.rs (1)
659-669:⚠️ Potential issue | 🔴 CriticalMove
collected_game_eventsoutside the transport block.Line 669 declares the buffer inside
if let Some(transport), but Lines 857-859 consume it after that scope ends. That makes the deferred-dispatch refactor a compile blocker.🐛 Proposed fix
+ let mut collected_game_events = Vec::new(); if let Some(transport) = &mut self.transport { let events = match transport.poll_events().await { Ok(e) => e, Err(e) => { tracing::error!("Transport poll failure: {:?}", e); @@ }; let mut updates: Vec<(ClientId, aetheris_protocol::events::ComponentUpdate)> = Vec::new(); - let mut collected_game_events = Vec::new();Also applies to: 855-859
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/aetheris-client-wasm/src/lib.rs` around lines 659 - 669, The buffer variable collected_game_events is declared inside the if let Some(transport) { ... } scope but later used after that block (in the deferred-dispatch code), causing a borrow/ownership error; move the declaration of collected_game_events (and any other buffers that must survive beyond the transport block) so it is created before the if let Some(transport) = &mut self.transport { ... } block, then push into it from inside the transport.poll_events() handling (functions/methods to look at: transport.poll_events, the surrounding async method in lib.rs, and any code that consumes collected_game_events around lines 855–859), ensuring the variable is in the outer scope and not dropped when the transport block ends.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/aetheris-client-wasm/src/lib.rs`:
- Around line 659-669: The buffer variable collected_game_events is declared
inside the if let Some(transport) { ... } scope but later used after that block
(in the deferred-dispatch code), causing a borrow/ownership error; move the
declaration of collected_game_events (and any other buffers that must survive
beyond the transport block) so it is created before the if let Some(transport) =
&mut self.transport { ... } block, then push into it from inside the
transport.poll_events() handling (functions/methods to look at:
transport.poll_events, the surrounding async method in lib.rs, and any code that
consumes collected_game_events around lines 855–859), ensuring the variable is
in the outer scope and not dropped when the transport block ends.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a0d13e4a-07d3-4dd1-a6c4-4518fafd9199
📒 Files selected for processing (1)
crates/aetheris-client-wasm/src/lib.rs
2 participants
Changes
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores