Update project to 2024 edition#99
Merged
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (118)
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughWorkspace-wide edition bump to Rust 2024, systematic import/re-export reordering across crates, and focused refactors: FOV scanline immutability, fastnoise simplifications, WebGPU lifetime parameterization, wasm-specific gating and state-swap, and explicit unsafe blocks for OpenGL calls. ChangesRust 2024 Edition Upgrade and Workspace Import Refactor
Algorithmic and Safety Improvements
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~75 minutes Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
bracket-terminal/src/hal/webgpu/platform.rs (1)
37-38:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing lifetime parameter on trait implementations.
The
PlatformGLstruct was updated toPlatformGL<'a>, but theseunsafe impldeclarations still referencePlatformGLwithout the lifetime parameter. This will cause a compilation error.🐛 Proposed fix
-unsafe impl Send for PlatformGL {} -unsafe impl Sync for PlatformGL {} +unsafe impl<'a> Send for PlatformGL<'a> {} +unsafe impl<'a> Sync for PlatformGL<'a> {}🤖 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 `@bracket-terminal/src/hal/webgpu/platform.rs` around lines 37 - 38, The Send/Sync impls refer to PlatformGL without the required lifetime; change the unsafe impls to use the lifetime form matching the struct (e.g., unsafe impl<'a> Send for PlatformGL<'a> {} and unsafe impl<'a> Sync for PlatformGL<'a> {}) so the implementations compile with the updated PlatformGL<'a> definition and any bounds remain consistent with the struct.bracket-pathfinding/src/field_of_view/symmetric_shadowcasting.rs (1)
125-141:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
current_start_slopein the symmetry check path.
current_start_slopeis updated at Line 136, but Line 131 still evaluates symmetry againstscanline.start_slope. That desynchronizes slope updates from reveal decisions and can produce incorrect visibility after opaque→transparent transitions.🔧 Proposed fix
- if Rational32::from_integer(dx * dx + dy * dy) <= self.radius_plus_half_2 { - if self.is_opaque(tile) || is_symmetric(&scanline, tile) { + if Rational32::from_integer(dx * dx + dy * dy) <= self.radius_plus_half_2 { + let column = Rational32::from_integer(tile.column); + let depth = Rational32::from_integer(scanline.depth); + let is_symmetric_now = + column >= depth * current_start_slope + && column <= depth * scanline.end_slope; + if self.is_opaque(tile) || is_symmetric_now { self.reveal(tile); }🤖 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 `@bracket-pathfinding/src/field_of_view/symmetric_shadowcasting.rs` around lines 125 - 141, The symmetry check currently uses scanline.start_slope but should use the up-to-date current_start_slope so reveal decisions stay in sync after opacity transitions; update the call to is_symmetric inside the loop to use current_start_slope (either by passing current_start_slope into is_symmetric or by constructing a scanline-like value whose start_slope is current_start_slope) instead of relying on scanline.start_slope, keeping the rest of the logic around current_start_slope, is_opaque, reveal, and slope(tile) unchanged.
🧹 Nitpick comments (6)
bracket-pathfinding/examples/dijkstra_weighted/main.rs (1)
38-41: ⚡ Quick winPrefer idiomatic
.min()for clamping.The inline
if n > 255.0 { 255.0 } else { n }can be replaced withn.min(255.0)for more concise, idiomatic Rust.♻️ Proposed simplification
RGB::from_u8( 0, - 255 - { - let n = flow_map.map[idx] * 12.0; - if n > 255.0 { 255.0 } else { n } - } as u8, + 255 - (flow_map.map[idx] * 12.0).min(255.0) as u8, 0, )🤖 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 `@bracket-pathfinding/examples/dijkstra_weighted/main.rs` around lines 38 - 41, Replace the manual clamp expression that computes n = flow_map.map[idx] * 12.0 and then uses if n > 255.0 { 255.0 } else { n } with the idiomatic Rust min method (n.min(255.0)) before casting to u8; update the expression that builds the byte (the block creating n and casting to u8) to use n.min(255.0) to make the code concise and idiomatic while preserving the cast to u8.bracket-terminal/src/hal/native/mainloop.rs (1)
280-286: ⚡ Quick winRemove unused private functions
physical_key_to_virtual_keycodeandkeycode_to_virtual_keycode.After switching input handling to
PhysicalKey::Codeat line 280, these translation helpers (lines 321-327) are no longer called. They represent dead code that should be removed for cleanliness.Cleanup diff
-fn physical_key_to_virtual_keycode(key: &PhysicalKey) -> Option<VirtualKeyCode> { - match key { - PhysicalKey::Code(code) => keycode_to_virtual_keycode(code), - PhysicalKey::Unidentified(_) => None, - } -} - -fn keycode_to_virtual_keycode(code: &KeyCode) -> Option<VirtualKeyCode> { - use VirtualKeyCode::*; - match code { - // ... - _ => None, - } -}🤖 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 `@bracket-terminal/src/hal/native/mainloop.rs` around lines 280 - 286, Remove the now-dead helper functions physical_key_to_virtual_keycode and keycode_to_virtual_keycode from mainloop.rs (they're no longer used after switching input handling to winit::keyboard::PhysicalKey::Code in the event handling block that calls bterm.on_key), and also remove any imports or references that become unused as a result; after deletion, run a build/format to ensure no remaining references or unused-import warnings.bracket-terminal/src/hal/crossterm_be/scancode_helper.rs (1)
3-186: ⚡ Quick winAdd table-driven tests for key-to-scan and crossterm-to-KeyCode translation.
This mapping table is large and manual; without tests it’s easy to regress key identity during future migrations. Add a small colocated test set covering representative keys (letters, digits, arrows, punctuation, numpad, backspace/delete).
As per coding guidelines, Add tests for new functionality in the relevant module; for split domains, prefer colocated
tests.rs.🤖 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 `@bracket-terminal/src/hal/crossterm_be/scancode_helper.rs` around lines 3 - 186, Add colocated table-driven unit tests in a new tests.rs in this module that assert correctness of the two mapping functions: virtual_key_code_to_scan and keycode_to_key. Create small arrays of representative cases (letters like KeyA/Char('a'), digits like Digit1/Char('1'), arrows ArrowLeft/Left, punctuation like Backquote/Char('`'), Backspace/Delete, numpad keys like Numpad1/NumpadAdd) and iterate them to assert virtual_key_code_to_scan returns the expected scan code and keycode_to_key converts crossterm::event::KeyCode variants to the expected KeyCode; place tests next to the module (colocated tests.rs) and keep them table-driven to easily extend coverage.bracket-terminal/src/bterm.rs (1)
1084-1114: ⚡ Quick winAdd focused tests for
letter_to_option(KeyCode)mapping.This public helper changed key type but the colocated tests only cover
iclamp. Add unit tests forKeyCode::KeyA,KeyCode::KeyZ, and a non-letter key fallback to-1.✅ Proposed test additions
#[cfg(test)] mod tests { - use super::iclamp; + use super::{iclamp, letter_to_option}; + use crate::prelude::KeyCode; #[test] // Tests that we make an RGB triplet at defaults and it is black. fn test_iclamp() { assert!(iclamp(1, 0, 2) == 1); assert!(iclamp(5, 0, 2) == 2); assert!(iclamp(-5, 0, 2) == 0); } + + #[test] + fn test_letter_to_option_keycode_mapping() { + assert_eq!(letter_to_option(KeyCode::KeyA), 0); + assert_eq!(letter_to_option(KeyCode::KeyZ), 25); + assert_eq!(letter_to_option(KeyCode::Escape), -1); + } }As per coding guidelines, Add tests for new functionality in the relevant module; for split domains, prefer colocated
tests.rs.🤖 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 `@bracket-terminal/src/bterm.rs` around lines 1084 - 1114, Add focused unit tests for the public helper function letter_to_option in bterm.rs: create colocated tests (e.g., in tests.rs or inside bterm.rs under #[cfg(test)]) that assert letter_to_option(KeyCode::KeyA) == 0, letter_to_option(KeyCode::KeyZ) == 25, and letter_to_option for a non-letter key (e.g., KeyCode::Enter or KeyCode::Escape) returns -1; use assert_eq! and keep tests small and named to indicate the mapping and fallback behavior.bracket-terminal/examples/walking_with_is_pressed.rs (1)
18-18: ⚡ Quick winType declaration uses deprecated alias.
The field is declared as
HashMap<VirtualKeyCode, (i32, i32)>while the code insertsKeyCodevalues (lines 45-56). AlthoughVirtualKeyCodeis now aliased toKeyCodeinlib.rs, using the old alias in the type declaration while using the new enum name for values is inconsistent.♻️ Suggested consistency fix
struct State { map: Vec<TileType>, player_position: usize, - keymap: HashMap<VirtualKeyCode, (i32, i32)>, + keymap: HashMap<KeyCode, (i32, i32)>, }🤖 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 `@bracket-terminal/examples/walking_with_is_pressed.rs` at line 18, The struct field keymap is typed as HashMap<VirtualKeyCode, (i32, i32)> but the code inserts KeyCode values; update the type to HashMap<KeyCode, (i32, i32)> and adjust imports/usages accordingly (replace VirtualKeyCode with KeyCode in the type declaration and ensure KeyCode is imported where the struct is defined) so the key type matches the inserted enum values in functions that populate keymap.bracket-terminal/examples/tiles.rs (1)
81-96: ⚡ Quick winInconsistent key enum usage in the same match block.
Lines 81-90 use
VirtualKeyCode::Numpad*variants while lines 93-96 useKeyCode::Arrow*variants. AlthoughVirtualKeyCodeis now aliased toKeyCode(defined inlib.rs), mixing both forms in the same match expression is inconsistent and may confuse readers.♻️ Suggested consistency fix
match key { // Numpad - VirtualKeyCode::Numpad8 => self.move_player(0, -1), - VirtualKeyCode::Numpad4 => self.move_player(-1, 0), - VirtualKeyCode::Numpad6 => self.move_player(1, 0), - VirtualKeyCode::Numpad2 => self.move_player(0, 1), + KeyCode::Numpad8 => self.move_player(0, -1), + KeyCode::Numpad4 => self.move_player(-1, 0), + KeyCode::Numpad6 => self.move_player(1, 0), + KeyCode::Numpad2 => self.move_player(0, 1), // Numpad diagonals - VirtualKeyCode::Numpad7 => self.move_player(-1, -1), - VirtualKeyCode::Numpad9 => self.move_player(1, -1), - VirtualKeyCode::Numpad1 => self.move_player(-1, 1), - VirtualKeyCode::Numpad3 => self.move_player(1, 1), + KeyCode::Numpad7 => self.move_player(-1, -1), + KeyCode::Numpad9 => self.move_player(1, -1), + KeyCode::Numpad1 => self.move_player(-1, 1), + KeyCode::Numpad3 => self.move_player(1, 1), // Cursors KeyCode::ArrowUp => self.move_player(0, -1),🤖 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 `@bracket-terminal/examples/tiles.rs` around lines 81 - 96, The match mixes VirtualKeyCode::Numpad* variants with KeyCode::Arrow* variants — standardize to one enum form; replace the KeyCode::ArrowUp/ArrowDown/ArrowLeft/ArrowRight arms with the corresponding VirtualKeyCode::Up/VirtualKeyCode::Down/VirtualKeyCode::Left/VirtualKeyCode::Right variants so all match arms use VirtualKeyCode (leave calls to self.move_player as-is).
🤖 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 `@bracket-random/Cargo.toml`:
- Line 5: The Cargo.toml currently targets edition = "2024" but lists outdated
dependencies; update Cargo.toml to bump regex to 1.12.3 or later, remove or
replace lazy_static = "1.4.0" (migrate code using lazy_static! to
std::sync::LazyLock or another modern alternative in places referenced by those
macros), keep or verify rand_chacha = "0.10" compatibility, and add an explicit
rust-version = "1.85" field; also update any source files that import
lazy_static::lazy_static! to use LazyLock initializers (or a crate alternative)
so the project builds under Rust 1.85+/edition 2024.
In `@bracket-terminal/examples/walking.rs`:
- Line 96: Replace the truncated comment "and applying movement vi" in
examples/walking.rs with a complete sentence that clearly describes the intent
(for example: "and applying vi-style movement keys to control the cursor" or
"and applying movement inputs to update position"), ensuring the comment matches
surrounding context and style; update the single-line comment near the walking
example so it reads as a full, grammatical phrase rather than a clipped
fragment.
In `@bracket-terminal/src/hal/crossterm_be/scancode_helper.rs`:
- Around line 150-175: The mapping currently only handles
CrossKeyCode::Char('a'..'z') and drops uppercase chars; update the Char arm in
the scancode mapping function (the match handling CrossKeyCode::Char) to either
normalize the incoming char to lowercase before mapping or add explicit arms for
'A'..'Z' mapping to KeyCode::KeyA..KeyZ (referencing CrossKeyCode::Char and
KeyCode::KeyA..KeyCode::KeyZ to locate the logic), and add unit tests exercising
both uppercase and lowercase inputs for each letter to ensure Shift+Letter
events are preserved; keep the existing separate Shift modifier handling
unchanged.
In `@bracket-terminal/src/hal/native/mainloop.rs`:
- Around line 280-289: Add a colocated regression test in this module (tests.rs
within mainloop.rs) that exercises the new PhysicalKey::Code path and the
event.text handling: construct synthetic winit keyboard events with
PhysicalKey::Code(key_code) for both Pressed and Released states, assert that
bterm.on_key is called with the correct key_code and the correct pressed flag
for each event (use the same helper/mocking approach used elsewhere in this
module), and verify that event.text is only processed when event.state ==
ElementState::Pressed (i.e., include an event with Some(text) on Pressed and
ensure the text-handling branch runs, and include a Released event with
Some(text) and ensure it does not). Ensure the test is colocated in this file's
tests module and uses the same setup/init helpers for bterm and mainloop as
other tests in this module.
In `@bracket-terminal/src/hal/webgpu/init.rs`:
- Around line 18-24: EventLoop::new now returns Result and windows must be
created via ActiveEventLoop inside the resumed callback, so change the init flow
to (1) handle the Result from EventLoop::new() (use .unwrap() or propagate the
error) and remove any direct call to EventLoop::create_window(), and (2) move
the Window creation logic (currently using
Window::default_attributes()/with_title()/with_inner_size() and create_window)
into ApplicationHandler::resumed() where you receive an ActiveEventLoop and call
ActiveEventLoop::create_window(), passing scaler.new_window_size() and
window_title there; update any places that assumed a Window created earlier
(store it in your handler/state instead) so initialization compiles with winit
0.30.
---
Outside diff comments:
In `@bracket-pathfinding/src/field_of_view/symmetric_shadowcasting.rs`:
- Around line 125-141: The symmetry check currently uses scanline.start_slope
but should use the up-to-date current_start_slope so reveal decisions stay in
sync after opacity transitions; update the call to is_symmetric inside the loop
to use current_start_slope (either by passing current_start_slope into
is_symmetric or by constructing a scanline-like value whose start_slope is
current_start_slope) instead of relying on scanline.start_slope, keeping the
rest of the logic around current_start_slope, is_opaque, reveal, and slope(tile)
unchanged.
In `@bracket-terminal/src/hal/webgpu/platform.rs`:
- Around line 37-38: The Send/Sync impls refer to PlatformGL without the
required lifetime; change the unsafe impls to use the lifetime form matching the
struct (e.g., unsafe impl<'a> Send for PlatformGL<'a> {} and unsafe impl<'a>
Sync for PlatformGL<'a> {}) so the implementations compile with the updated
PlatformGL<'a> definition and any bounds remain consistent with the struct.
---
Nitpick comments:
In `@bracket-pathfinding/examples/dijkstra_weighted/main.rs`:
- Around line 38-41: Replace the manual clamp expression that computes n =
flow_map.map[idx] * 12.0 and then uses if n > 255.0 { 255.0 } else { n } with
the idiomatic Rust min method (n.min(255.0)) before casting to u8; update the
expression that builds the byte (the block creating n and casting to u8) to use
n.min(255.0) to make the code concise and idiomatic while preserving the cast to
u8.
In `@bracket-terminal/examples/tiles.rs`:
- Around line 81-96: The match mixes VirtualKeyCode::Numpad* variants with
KeyCode::Arrow* variants — standardize to one enum form; replace the
KeyCode::ArrowUp/ArrowDown/ArrowLeft/ArrowRight arms with the corresponding
VirtualKeyCode::Up/VirtualKeyCode::Down/VirtualKeyCode::Left/VirtualKeyCode::Right
variants so all match arms use VirtualKeyCode (leave calls to self.move_player
as-is).
In `@bracket-terminal/examples/walking_with_is_pressed.rs`:
- Line 18: The struct field keymap is typed as HashMap<VirtualKeyCode, (i32,
i32)> but the code inserts KeyCode values; update the type to HashMap<KeyCode,
(i32, i32)> and adjust imports/usages accordingly (replace VirtualKeyCode with
KeyCode in the type declaration and ensure KeyCode is imported where the struct
is defined) so the key type matches the inserted enum values in functions that
populate keymap.
In `@bracket-terminal/src/bterm.rs`:
- Around line 1084-1114: Add focused unit tests for the public helper function
letter_to_option in bterm.rs: create colocated tests (e.g., in tests.rs or
inside bterm.rs under #[cfg(test)]) that assert letter_to_option(KeyCode::KeyA)
== 0, letter_to_option(KeyCode::KeyZ) == 25, and letter_to_option for a
non-letter key (e.g., KeyCode::Enter or KeyCode::Escape) returns -1; use
assert_eq! and keep tests small and named to indicate the mapping and fallback
behavior.
In `@bracket-terminal/src/hal/crossterm_be/scancode_helper.rs`:
- Around line 3-186: Add colocated table-driven unit tests in a new tests.rs in
this module that assert correctness of the two mapping functions:
virtual_key_code_to_scan and keycode_to_key. Create small arrays of
representative cases (letters like KeyA/Char('a'), digits like Digit1/Char('1'),
arrows ArrowLeft/Left, punctuation like Backquote/Char('`'), Backspace/Delete,
numpad keys like Numpad1/NumpadAdd) and iterate them to assert
virtual_key_code_to_scan returns the expected scan code and keycode_to_key
converts crossterm::event::KeyCode variants to the expected KeyCode; place tests
next to the module (colocated tests.rs) and keep them table-driven to easily
extend coverage.
In `@bracket-terminal/src/hal/native/mainloop.rs`:
- Around line 280-286: Remove the now-dead helper functions
physical_key_to_virtual_keycode and keycode_to_virtual_keycode from mainloop.rs
(they're no longer used after switching input handling to
winit::keyboard::PhysicalKey::Code in the event handling block that calls
bterm.on_key), and also remove any imports or references that become unused as a
result; after deletion, run a build/format to ensure no remaining references or
unused-import warnings.
🪄 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: c1b59149-0ceb-4b02-97cb-d0f5fbf465b3
📒 Files selected for processing (120)
Cargo.tomlbracket-algorithm-traits/Cargo.tomlbracket-algorithm-traits/src/algorithm2d.rsbracket-algorithm-traits/src/algorithm3d.rsbracket-algorithm-traits/src/lib.rsbracket-bevy/Cargo.tomlbracket-bevy/examples/bevy_roguelike_tutorial_4/map.rsbracket-bevy/src/builder/bterm_builder.rsbracket-bevy/src/builder/loader_system.rsbracket-bevy/src/consoles/common_draw.rsbracket-bevy/src/consoles/simple_console/back_end/simple_no_background.rsbracket-bevy/src/consoles/simple_console/back_end/simple_with_background.rsbracket-bevy/src/consoles/simple_console/front_end.rsbracket-bevy/src/consoles/sparse_console/back_end/sparse_no_background.rsbracket-bevy/src/consoles/sparse_console/back_end/sparse_with_background.rsbracket-bevy/src/consoles/sparse_console/front_end.rsbracket-bevy/src/consoles/virtual_console.rsbracket-bevy/src/context.rsbracket-bevy/src/lib.rsbracket-bevy/src/textblock.rsbracket-color/Cargo.tomlbracket-color/examples/colors.rsbracket-color/examples/lerp.rsbracket-color/examples/lerpit.rsbracket-color/examples/lerpit_hsv.rsbracket-color/examples/named_color.rsbracket-color/examples/shades_of_grey.rsbracket-color/src/palette.rsbracket-color/src/rgba.rsbracket-embedding/Cargo.tomlbracket-geometry/Cargo.tomlbracket-geometry/examples/bresenham_circle.rsbracket-geometry/examples/bresenham_line.rsbracket-geometry/examples/bresenham_sweep.rsbracket-geometry/examples/vector_line.rsbracket-geometry/src/angles.rsbracket-geometry/src/lines.rsbracket-noise/Cargo.tomlbracket-noise/examples/cellular.rsbracket-noise/examples/perlin_fractal.rsbracket-noise/examples/simplex_billow_quintic.rsbracket-noise/examples/simplex_fractal.rsbracket-noise/examples/simplex_rigid_hermite.rsbracket-noise/examples/value.rsbracket-noise/examples/value_fractal.rsbracket-noise/examples/whitenoise.rsbracket-noise/src/fastnoise.rsbracket-pathfinding/Cargo.tomlbracket-pathfinding/benches/astar_benchmark.rsbracket-pathfinding/benches/fov_benchmark.rsbracket-pathfinding/examples/astar/common.rsbracket-pathfinding/examples/astar_manhattan/common.rsbracket-pathfinding/examples/dijkstra/common.rsbracket-pathfinding/examples/dijkstra/main.rsbracket-pathfinding/examples/dijkstra_weighted/common.rsbracket-pathfinding/examples/dijkstra_weighted/main.rsbracket-pathfinding/examples/fov/common.rsbracket-pathfinding/src/field_of_view/mod.rsbracket-pathfinding/src/field_of_view/symmetric_shadowcasting.rsbracket-pathfinding/src/lib.rsbracket-random/Cargo.tomlbracket-random/benches/dice.rsbracket-random/src/parsing.rsbracket-random/src/random.rsbracket-rex/Cargo.tomlbracket-rex/src/rex.rsbracket-terminal/Cargo.tomlbracket-terminal/benches/batching_benchmark.rsbracket-terminal/examples/fontswitch.rsbracket-terminal/examples/no_cls.rsbracket-terminal/examples/postprocess.rsbracket-terminal/examples/sparse_cls.rsbracket-terminal/examples/squishy.rsbracket-terminal/examples/tiles.rsbracket-terminal/examples/walking.rsbracket-terminal/examples/walking_with_is_pressed.rsbracket-terminal/src/bterm.rsbracket-terminal/src/consoles/command_buffer.rsbracket-terminal/src/consoles/flexible_console.rsbracket-terminal/src/consoles/simple_console.rsbracket-terminal/src/consoles/sparse_console.rsbracket-terminal/src/consoles/text/gui_helpers.rsbracket-terminal/src/consoles/text/multi_tile_sprite.rsbracket-terminal/src/consoles/text/textblock.rsbracket-terminal/src/consoles/virtual_console.rsbracket-terminal/src/hal/crossterm_be/init.rsbracket-terminal/src/hal/crossterm_be/main_loop.rsbracket-terminal/src/hal/crossterm_be/mod.rsbracket-terminal/src/hal/crossterm_be/scancode_helper.rsbracket-terminal/src/hal/curses/main_loop.rsbracket-terminal/src/hal/curses/mod.rsbracket-terminal/src/hal/gl_common/backing/fancy_console_backing.rsbracket-terminal/src/hal/gl_common/backing/shared_main_loop.rsbracket-terminal/src/hal/gl_common/backing/simple_console_backing.rsbracket-terminal/src/hal/gl_common/backing/sparse_console_backing.rsbracket-terminal/src/hal/gl_common/backing/sprite_console_backing.rsbracket-terminal/src/hal/gl_common/font.rsbracket-terminal/src/hal/gl_common/shader.rsbracket-terminal/src/hal/native/init.rsbracket-terminal/src/hal/native/mainloop.rsbracket-terminal/src/hal/native/mod.rsbracket-terminal/src/hal/wasm/events/external.rsbracket-terminal/src/hal/wasm/events/mod.rsbracket-terminal/src/hal/wasm/init.rsbracket-terminal/src/hal/wasm/mainloop.rsbracket-terminal/src/hal/wasm/mod.rsbracket-terminal/src/hal/webgpu/backend.rsbracket-terminal/src/hal/webgpu/backing/fancy_console_backing.rsbracket-terminal/src/hal/webgpu/backing/simple_console_backing.rsbracket-terminal/src/hal/webgpu/backing/sparse_console_backing.rsbracket-terminal/src/hal/webgpu/backing/sprite_console_backing.rsbracket-terminal/src/hal/webgpu/init.rsbracket-terminal/src/hal/webgpu/mainloop.rsbracket-terminal/src/hal/webgpu/mod.rsbracket-terminal/src/hal/webgpu/platform.rsbracket-terminal/src/hal/webgpu/quadrender.rsbracket-terminal/src/initializer.rsbracket-terminal/src/input/event_queue.rsbracket-terminal/src/input/input_handler.rsbracket-terminal/src/lib.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format all Rust code usingcargo fmt --all
Fix allcargo clippywarnings — the CI enforces-D warnings
Add tests for new functionality in the relevant module; for split domains, prefer colocatedtests.rs
Files:
bracket-bevy/src/builder/loader_system.rsbracket-geometry/examples/vector_line.rsbracket-color/examples/colors.rsbracket-geometry/src/angles.rsbracket-geometry/examples/bresenham_sweep.rsbracket-noise/examples/cellular.rsbracket-geometry/examples/bresenham_line.rsbracket-terminal/src/hal/webgpu/backend.rsbracket-pathfinding/examples/dijkstra_weighted/main.rsbracket-color/src/rgba.rsbracket-noise/examples/value_fractal.rsbracket-color/examples/shades_of_grey.rsbracket-noise/examples/simplex_billow_quintic.rsbracket-color/examples/lerpit.rsbracket-noise/examples/whitenoise.rsbracket-terminal/src/hal/gl_common/font.rsbracket-algorithm-traits/src/algorithm2d.rsbracket-pathfinding/examples/dijkstra/main.rsbracket-geometry/src/lines.rsbracket-terminal/src/hal/wasm/init.rsbracket-algorithm-traits/src/algorithm3d.rsbracket-terminal/src/consoles/virtual_console.rsbracket-geometry/examples/bresenham_circle.rsbracket-bevy/src/consoles/simple_console/back_end/simple_with_background.rsbracket-bevy/src/consoles/simple_console/back_end/simple_no_background.rsbracket-bevy/src/consoles/sparse_console/back_end/sparse_no_background.rsbracket-bevy/src/consoles/sparse_console/back_end/sparse_with_background.rsbracket-pathfinding/examples/astar/common.rsbracket-noise/examples/simplex_rigid_hermite.rsbracket-bevy/src/builder/bterm_builder.rsbracket-random/src/random.rsbracket-terminal/src/hal/webgpu/mod.rsbracket-terminal/src/hal/webgpu/backing/fancy_console_backing.rsbracket-random/benches/dice.rsbracket-bevy/examples/bevy_roguelike_tutorial_4/map.rsbracket-terminal/src/consoles/simple_console.rsbracket-terminal/src/hal/webgpu/quadrender.rsbracket-terminal/src/hal/crossterm_be/mod.rsbracket-pathfinding/src/lib.rsbracket-terminal/src/hal/crossterm_be/init.rsbracket-noise/examples/perlin_fractal.rsbracket-terminal/src/hal/native/mod.rsbracket-terminal/src/hal/gl_common/backing/shared_main_loop.rsbracket-bevy/src/consoles/sparse_console/front_end.rsbracket-terminal/src/hal/gl_common/shader.rsbracket-terminal/src/consoles/sparse_console.rsbracket-pathfinding/examples/dijkstra_weighted/common.rsbracket-color/examples/named_color.rsbracket-bevy/src/consoles/virtual_console.rsbracket-terminal/src/consoles/text/textblock.rsbracket-noise/examples/simplex_fractal.rsbracket-terminal/src/hal/webgpu/backing/sparse_console_backing.rsbracket-terminal/benches/batching_benchmark.rsbracket-terminal/src/consoles/command_buffer.rsbracket-terminal/src/consoles/text/gui_helpers.rsbracket-terminal/src/hal/gl_common/backing/simple_console_backing.rsbracket-pathfinding/examples/dijkstra/common.rsbracket-terminal/src/hal/curses/mod.rsbracket-terminal/examples/tiles.rsbracket-pathfinding/src/field_of_view/mod.rsbracket-color/examples/lerpit_hsv.rsbracket-terminal/src/consoles/flexible_console.rsbracket-terminal/src/hal/gl_common/backing/sprite_console_backing.rsbracket-terminal/src/hal/wasm/mod.rsbracket-bevy/src/consoles/simple_console/front_end.rsbracket-bevy/src/textblock.rsbracket-terminal/src/hal/gl_common/backing/fancy_console_backing.rsbracket-terminal/src/hal/wasm/mainloop.rsbracket-terminal/src/consoles/text/multi_tile_sprite.rsbracket-color/examples/lerp.rsbracket-noise/examples/value.rsbracket-terminal/src/initializer.rsbracket-pathfinding/examples/fov/common.rsbracket-terminal/src/hal/wasm/events/mod.rsbracket-terminal/src/hal/wasm/events/external.rsbracket-bevy/src/lib.rsbracket-pathfinding/benches/fov_benchmark.rsbracket-terminal/examples/sparse_cls.rsbracket-algorithm-traits/src/lib.rsbracket-terminal/examples/squishy.rsbracket-terminal/src/hal/webgpu/mainloop.rsbracket-terminal/src/input/event_queue.rsbracket-noise/src/fastnoise.rsbracket-random/src/parsing.rsbracket-pathfinding/examples/astar_manhattan/common.rsbracket-terminal/src/hal/webgpu/backing/sprite_console_backing.rsbracket-terminal/examples/walking_with_is_pressed.rsbracket-terminal/src/hal/webgpu/backing/simple_console_backing.rsbracket-terminal/examples/walking.rsbracket-terminal/src/lib.rsbracket-terminal/examples/no_cls.rsbracket-terminal/src/hal/gl_common/backing/sparse_console_backing.rsbracket-pathfinding/benches/astar_benchmark.rsbracket-bevy/src/consoles/common_draw.rsbracket-terminal/src/hal/curses/main_loop.rsbracket-terminal/examples/fontswitch.rsbracket-terminal/examples/postprocess.rsbracket-rex/src/rex.rsbracket-terminal/src/hal/webgpu/platform.rsbracket-terminal/src/hal/webgpu/init.rsbracket-terminal/src/hal/native/init.rsbracket-terminal/src/hal/native/mainloop.rsbracket-pathfinding/src/field_of_view/symmetric_shadowcasting.rsbracket-terminal/src/input/input_handler.rsbracket-terminal/src/hal/crossterm_be/scancode_helper.rsbracket-terminal/src/bterm.rsbracket-bevy/src/context.rsbracket-terminal/src/hal/crossterm_be/main_loop.rsbracket-color/src/palette.rs
🔇 Additional comments (108)
bracket-terminal/src/hal/webgpu/platform.rs (2)
9-13: LGTM!
27-29: LGTM!bracket-pathfinding/examples/dijkstra/main.rs (1)
31-31: LGTM!bracket-pathfinding/src/field_of_view/mod.rs (1)
222-226: LGTM!Cargo.toml (1)
5-5: LGTM!bracket-algorithm-traits/Cargo.toml (1)
5-5: LGTM!bracket-bevy/Cargo.toml (1)
4-4: LGTM!bracket-color/Cargo.toml (1)
5-5: LGTM!bracket-embedding/Cargo.toml (1)
4-4: LGTM!bracket-geometry/Cargo.toml (1)
5-5: LGTM!bracket-noise/Cargo.toml (1)
5-5: LGTM!bracket-pathfinding/Cargo.toml (1)
5-5: LGTM!bracket-rex/Cargo.toml (1)
4-4: LGTM!bracket-terminal/Cargo.toml (1)
5-5: LGTM!bracket-algorithm-traits/src/lib.rs (1)
45-45: LGTM!bracket-pathfinding/src/lib.rs (1)
15-15: LGTM!bracket-color/src/rgba.rs (1)
1-1: LGTM!bracket-bevy/examples/bevy_roguelike_tutorial_4/map.rs (1)
3-6: LGTM!bracket-bevy/src/builder/bterm_builder.rs (1)
2-7: LGTM!bracket-bevy/src/builder/loader_system.rs (1)
2-3: LGTM!bracket-bevy/src/consoles/common_draw.rs (1)
4-4: LGTM!bracket-bevy/src/consoles/simple_console/back_end/simple_no_background.rs (1)
2-2: LGTM!bracket-bevy/src/consoles/simple_console/back_end/simple_with_background.rs (1)
1-1: LGTM!bracket-bevy/src/consoles/simple_console/front_end.rs (1)
2-9: LGTM!bracket-bevy/src/consoles/sparse_console/back_end/sparse_no_background.rs (1)
2-2: LGTM!bracket-bevy/src/consoles/sparse_console/back_end/sparse_with_background.rs (1)
2-2: LGTM!bracket-bevy/src/consoles/sparse_console/front_end.rs (1)
6-7: LGTM!bracket-bevy/src/consoles/virtual_console.rs (1)
4-4: LGTM!bracket-bevy/src/context.rs (1)
2-2: LGTM!bracket-bevy/src/lib.rs (1)
21-22: LGTM!bracket-bevy/src/textblock.rs (1)
6-6: LGTM!bracket-color/examples/colors.rs (1)
2-2: LGTM!bracket-color/examples/lerp.rs (1)
2-2: LGTM!bracket-color/examples/lerpit.rs (1)
2-2: LGTM!bracket-color/examples/lerpit_hsv.rs (1)
2-2: LGTM!bracket-color/examples/named_color.rs (1)
2-2: LGTM!bracket-color/examples/shades_of_grey.rs (1)
2-2: LGTM!bracket-color/src/palette.rs (1)
2-91: LGTM!bracket-geometry/examples/bresenham_circle.rs (1)
4-4: LGTM!bracket-geometry/examples/bresenham_line.rs (1)
4-4: LGTM!bracket-geometry/examples/bresenham_sweep.rs (1)
4-4: LGTM!bracket-geometry/examples/vector_line.rs (1)
4-4: LGTM!bracket-geometry/src/angles.rs (1)
20-20: LGTM!bracket-geometry/src/lines.rs (1)
70-70: LGTM!bracket-noise/examples/cellular.rs (1)
7-7: LGTM!bracket-noise/examples/perlin_fractal.rs (1)
7-7: LGTM!bracket-noise/examples/simplex_billow_quintic.rs (1)
7-7: LGTM!bracket-noise/examples/simplex_fractal.rs (1)
7-7: LGTM!bracket-noise/examples/simplex_rigid_hermite.rs (1)
7-7: LGTM!bracket-noise/examples/value.rs (1)
7-7: LGTM!bracket-noise/examples/value_fractal.rs (1)
7-7: LGTM!bracket-noise/examples/whitenoise.rs (1)
7-7: LGTM!bracket-noise/src/fastnoise.rs (1)
1688-1688: LGTM!Also applies to: 3009-3009
bracket-random/benches/dice.rs (1)
5-5: LGTM!bracket-random/src/parsing.rs (1)
100-100: LGTM!bracket-random/src/random.rs (1)
2-3: LGTM!bracket-rex/src/rex.rs (1)
15-15: LGTM!bracket-terminal/benches/batching_benchmark.rs (1)
5-5: LGTM!bracket-terminal/examples/sparse_cls.rs (1)
1-1: LGTM!bracket-terminal/src/consoles/text/gui_helpers.rs (1)
1-1: LGTM!bracket-terminal/src/consoles/text/multi_tile_sprite.rs (1)
1-1: LGTM!bracket-terminal/src/consoles/text/textblock.rs (1)
1-1: LGTM!bracket-terminal/src/consoles/virtual_console.rs (1)
5-6: LGTM!bracket-terminal/src/initializer.rs (1)
1-4: LGTM!bracket-terminal/src/consoles/command_buffer.rs (1)
6-6: LGTM!bracket-terminal/src/consoles/flexible_console.rs (1)
2-3: LGTM!bracket-terminal/src/consoles/simple_console.rs (1)
2-3: LGTM!bracket-terminal/src/consoles/sparse_console.rs (1)
2-3: LGTM!bracket-terminal/src/hal/crossterm_be/init.rs (1)
1-8: LGTM!bracket-terminal/src/hal/crossterm_be/main_loop.rs (1)
2-2: LGTM!Also applies to: 5-6, 8-8, 10-10, 16-16, 27-29, 155-155
bracket-terminal/src/hal/curses/mod.rs (1)
3-3: LGTM!bracket-terminal/src/hal/gl_common/backing/fancy_console_backing.rs (1)
1-1: LGTM!Also applies to: 3-3
bracket-terminal/src/hal/gl_common/backing/shared_main_loop.rs (1)
1-1: LGTM!Also applies to: 4-5, 7-7
bracket-terminal/src/hal/gl_common/backing/simple_console_backing.rs (1)
1-1: LGTM!Also applies to: 3-3
bracket-terminal/src/hal/gl_common/backing/sparse_console_backing.rs (1)
1-3: LGTM!bracket-terminal/src/hal/gl_common/backing/sprite_console_backing.rs (1)
1-1: LGTM!bracket-terminal/src/hal/gl_common/font.rs (1)
1-1: LGTM!bracket-terminal/src/hal/gl_common/shader.rs (1)
1-1: LGTM!Also applies to: 63-64, 72-77, 84-85, 91-92, 98-106, 112-113, 119-120, 126-133
bracket-terminal/src/hal/native/init.rs (1)
3-7: LGTM!bracket-terminal/src/hal/native/mainloop.rs (1)
4-5: LGTM!bracket-terminal/src/hal/native/mod.rs (1)
9-9: LGTM!bracket-terminal/src/hal/wasm/events/external.rs (1)
1-1: LGTM!bracket-terminal/src/hal/wasm/events/mod.rs (1)
8-8: LGTM!bracket-terminal/src/hal/wasm/init.rs (1)
2-2: LGTM!bracket-terminal/src/hal/wasm/mainloop.rs (1)
4-5: LGTM!bracket-terminal/src/hal/wasm/mod.rs (1)
11-11: LGTM!bracket-terminal/src/hal/webgpu/backend.rs (1)
3-3: LGTM!bracket-terminal/src/hal/webgpu/backing/fancy_console_backing.rs (1)
5-8: LGTM!bracket-terminal/src/hal/webgpu/backing/simple_console_backing.rs (1)
5-8: LGTM!bracket-terminal/src/hal/webgpu/backing/sparse_console_backing.rs (1)
4-7: LGTM!bracket-terminal/src/hal/webgpu/backing/sprite_console_backing.rs (1)
4-4: LGTM!bracket-terminal/src/hal/webgpu/mainloop.rs (1)
4-5: LGTM!Also applies to: 8-8, 11-11, 13-13
bracket-terminal/src/hal/curses/main_loop.rs (1)
5-5: LGTM!Also applies to: 7-7
bracket-terminal/src/hal/webgpu/quadrender.rs (1)
3-3: LGTM!bracket-algorithm-traits/src/algorithm2d.rs (1)
45-47: LGTM!bracket-algorithm-traits/src/algorithm3d.rs (1)
47-49: LGTM!bracket-terminal/src/hal/crossterm_be/mod.rs (1)
3-3: LGTM!bracket-terminal/src/hal/webgpu/mod.rs (1)
15-15: LGTM!bracket-terminal/src/input/event_queue.rs (1)
1-1: LGTM!Also applies to: 45-45
bracket-terminal/src/bterm.rs (1)
4-8: LGTM!Also applies to: 60-60, 329-343
bracket-terminal/src/input/input_handler.rs (1)
2-2: LGTM!Also applies to: 17-17, 43-45, 69-71, 136-145
bracket-terminal/examples/fontswitch.rs (1)
26-31: LGTM!bracket-terminal/examples/no_cls.rs (1)
11-11: LGTM!bracket-terminal/examples/postprocess.rs (1)
47-51: LGTM!bracket-terminal/examples/squishy.rs (1)
64-64: LGTM!bracket-terminal/examples/walking.rs (1)
98-113: LGTM!bracket-terminal/examples/walking_with_is_pressed.rs (1)
45-56: LGTM!bracket-terminal/src/lib.rs (1)
12-54: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bracket-pathfinding/src/field_of_view/symmetric_shadowcasting.rs (1)
117-156: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd regression tests for scanline slope transition behavior.
This refactor changes how start slopes are carried across opaque/translucent transitions, but no colocated tests were added for these edge paths. Please add focused tests for transition cases to lock behavior.
As per coding guidelines, "Add tests for new functionality in the relevant module; for split domains, prefer colocated
tests.rs."🤖 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 `@bracket-pathfinding/src/field_of_view/symmetric_shadowcasting.rs` around lines 117 - 156, Add regression tests in a colocated tests.rs for symmetric_shadowcasting that exercise scan()’s slope-transition logic across opaque/transparent boundaries: create minimal field maps (use the same types/constructors used by symmetric_shadowcasting: Scanline, Quadrant/transform, and the FOV struct that owns scan()) and add focused cases for (1) opaque->transparent transition, (2) transparent->opaque transition, and (3) edge-case grazing tiles near radius limit; in each test set up the tile opacity pattern, call scan(first_line) (or the public entry that invokes scan), and assert the visible tiles and that subsequent scanlines would carry the expected start_slope/end_slope (i.e., verify which tiles are revealed and that transitions spawn next scanlines with correct slopes). Place these tests in the module’s tests.rs so they run with cargo test and lock the behavior introduced by scan(), using the existing helpers (is_opaque, reveal, Scanline::next, slope, quadrant transform) to construct expectations.
♻️ Duplicate comments (2)
bracket-terminal/src/hal/crossterm_be/scancode_helper.rs (1)
108-186:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize alphabetic
Charinput before mapping.
keycode_to_keystill only handlesCrossKeyCode::Char('a'..'z'). Crossterm models typed characters asKeyCode::Char(char), and upstream tracks that shifted-letter reporting varies by terminal, so uppercaseChar('A'..'Z')can legitimately appear here. In the current match those events fall through toNone, which drops input. Normalize ASCII letters before matching, and add colocated tests for both lowercase and uppercase cases. (docs.rs)Proposed fix
pub fn keycode_to_key(c: crossterm::event::KeyCode) -> Option<KeyCode> { use crossterm::event::KeyCode as CrossKeyCode; match c { @@ - CrossKeyCode::Char('a') => Some(KeyCode::KeyA), - CrossKeyCode::Char('b') => Some(KeyCode::KeyB), - CrossKeyCode::Char('c') => Some(KeyCode::KeyC), - CrossKeyCode::Char('d') => Some(KeyCode::KeyD), - CrossKeyCode::Char('e') => Some(KeyCode::KeyE), - CrossKeyCode::Char('f') => Some(KeyCode::KeyF), - CrossKeyCode::Char('g') => Some(KeyCode::KeyG), - CrossKeyCode::Char('h') => Some(KeyCode::KeyH), - CrossKeyCode::Char('i') => Some(KeyCode::KeyI), - CrossKeyCode::Char('j') => Some(KeyCode::KeyJ), - CrossKeyCode::Char('k') => Some(KeyCode::KeyK), - CrossKeyCode::Char('l') => Some(KeyCode::KeyL), - CrossKeyCode::Char('m') => Some(KeyCode::KeyM), - CrossKeyCode::Char('n') => Some(KeyCode::KeyN), - CrossKeyCode::Char('o') => Some(KeyCode::KeyO), - CrossKeyCode::Char('p') => Some(KeyCode::KeyP), - CrossKeyCode::Char('q') => Some(KeyCode::KeyQ), - CrossKeyCode::Char('r') => Some(KeyCode::KeyR), - CrossKeyCode::Char('s') => Some(KeyCode::KeyS), - CrossKeyCode::Char('t') => Some(KeyCode::KeyT), - CrossKeyCode::Char('u') => Some(KeyCode::KeyU), - CrossKeyCode::Char('v') => Some(KeyCode::KeyV), - CrossKeyCode::Char('w') => Some(KeyCode::KeyW), - CrossKeyCode::Char('x') => Some(KeyCode::KeyX), - CrossKeyCode::Char('y') => Some(KeyCode::KeyY), - CrossKeyCode::Char('z') => Some(KeyCode::KeyZ), + CrossKeyCode::Char(ch) if ch.is_ascii_alphabetic() => Some(match ch.to_ascii_lowercase() { + 'a' => KeyCode::KeyA, + 'b' => KeyCode::KeyB, + 'c' => KeyCode::KeyC, + 'd' => KeyCode::KeyD, + 'e' => KeyCode::KeyE, + 'f' => KeyCode::KeyF, + 'g' => KeyCode::KeyG, + 'h' => KeyCode::KeyH, + 'i' => KeyCode::KeyI, + 'j' => KeyCode::KeyJ, + 'k' => KeyCode::KeyK, + 'l' => KeyCode::KeyL, + 'm' => KeyCode::KeyM, + 'n' => KeyCode::KeyN, + 'o' => KeyCode::KeyO, + 'p' => KeyCode::KeyP, + 'q' => KeyCode::KeyQ, + 'r' => KeyCode::KeyR, + 's' => KeyCode::KeyS, + 't' => KeyCode::KeyT, + 'u' => KeyCode::KeyU, + 'v' => KeyCode::KeyV, + 'w' => KeyCode::KeyW, + 'x' => KeyCode::KeyX, + 'y' => KeyCode::KeyY, + 'z' => KeyCode::KeyZ, + _ => unreachable!(), + }),As per coding guidelines,
Add tests for new functionality in the relevant module; for split domains, prefer colocated tests.rs.🤖 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 `@bracket-terminal/src/hal/crossterm_be/scancode_helper.rs` around lines 108 - 186, The keycode_to_key function currently ignores uppercase letters because it only matches Char('a'..'z'); update key handling by matching CrossKeyCode::Char(c) once, normalize ASCII alphabetic chars with c.to_ascii_lowercase() (or equivalent) before mapping to KeyCode::KeyA..KeyZ, and keep all other Char mappings unchanged; then add colocated tests in the module's tests.rs that assert both lowercase and uppercase Char inputs (e.g., 'a' and 'A') map to the same KeyCode (use keycode_to_key in tests).bracket-terminal/src/hal/webgpu/init.rs (1)
18-24:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftHandle
EventLoop::new()properly and stop using the deprecated window-creation path.Line 18 assigns
EventLoop::new()directly, but in winit 0.30 that call returnsResult<EventLoop<_>, EventLoopError>. Line 24 then callscreate_windowon that result, so this block does not type-check as written. On top of that,EventLoop::create_windowis only a deprecated compatibility shim in 0.30; the supported path isActiveEventLoop::create_window, and leaving the deprecated call here will still trip CI because clippy is enforced with-D warnings. (docs.rs)As per coding guidelines,
Fix all cargo clippy warnings — the CI enforces -D warnings.🤖 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 `@bracket-terminal/src/hal/webgpu/init.rs` around lines 18 - 24, EventLoop::new() now returns Result and you must handle the EventLoopError instead of assigning directly; call EventLoop::new().expect("...") or propagate the Result to handle errors from EventLoop::new() (reference EventLoop::new and EventLoopError), and stop using the deprecated EventLoop::create_window compatibility shim by calling the supported ActiveEventLoop::create_window API with the created event loop (reference ActiveEventLoop::create_window and the existing create_window parameters) so the code type-checks and avoids clippy deprecation warnings.
🧹 Nitpick comments (2)
bracket-terminal/examples/tiles.rs (1)
81-90: ⚡ Quick win
tiles.rs: Numpad arms don’t need “deprecated VirtualKeyCode” migration (it’s already aKeyCodealias).In this crate’s
bracket_terminal::prelude,VirtualKeyCodeis re-exported aswinit::keyboard::KeyCode(pub use winit::keyboard::KeyCode as VirtualKeyCode), soVirtualKeyCode::Numpad*is already the same asKeyCode::Numpad*. The only issue atbracket-terminal/examples/tiles.rslines 81-90 is enum inconsistency vs the cursor arms (a minor style refactor: switch Numpad arms toKeyCode::Numpad*).🤖 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 `@bracket-terminal/examples/tiles.rs` around lines 81 - 90, The match arms in tiles.rs use VirtualKeyCode::Numpad* but the crate re-exports winit::keyboard::KeyCode as VirtualKeyCode, so for consistency with the cursor handling switch the Numpad arms to use KeyCode::Numpad* instead; update the match cases (the lines calling self.move_player(...) for Numpad8, Numpad4, Numpad6, Numpad2 and the diagonal Numpad7/Numpad9/Numpad1/Numpad3) to reference KeyCode::NumpadX rather than VirtualKeyCode::NumpadX so enum usage is consistent with the rest of the file.bracket-terminal/src/bterm.rs (1)
1085-1115: ⚡ Quick winAdd unit tests for
letter_to_optionmapping.This function’s behavior changed with the key API migration; please add tests for at least A/Z and non-letter keys to prevent silent input regressions.
As per coding guidelines, "Add tests for new functionality in the relevant module; for split domains, prefer colocated
tests.rs."🤖 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 `@bracket-terminal/src/bterm.rs` around lines 1085 - 1115, Add unit tests for the letter_to_option function: create a colocated tests module (tests.rs or a #[cfg(test)] mod tests in the same file) and add assertions that letter_to_option(KeyCode::KeyA) == 0, letter_to_option(KeyCode::KeyZ) == 25, and that a non-letter key (e.g. KeyCode::Escape or another non-alphabetic KeyCode variant) returns -1; put these tests next to the letter_to_option function so future key-API changes are covered.
🤖 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 `@bracket-terminal/src/hal/webgpu/platform.rs`:
- Around line 9-13: The unsafe impls for PlatformGL are missing the lifetime
parameter; change the implementations from unsafe impl Send for PlatformGL {} /
unsafe impl Sync for PlatformGL {} to include the generic lifetime: use unsafe
impl<'a> Send for PlatformGL<'a> {} and unsafe impl<'a> Sync for PlatformGL<'a>
{} so the impls match the struct declaration PlatformGL<'a> (affecting the
implementations that reference PlatformGL and WgpuLink<'a>).
- Around line 9-13: PlatformGL currently mixes an owned WrappedContext { window:
Window } with WgpuLink<'a> { surface: Surface<'a> } produced from a borrowed
&Window, which makes the lifetime unsatisfiable and the static BACKEND invalid;
fix by decoupling the lifetime: either remove the lifetime parameter from
PlatformGL and WgpuLink (make WgpuLink own an owned Surface type instead of
Surface<'a>) so PlatformGL becomes a non-generic PlatformGL that can be stored
in BACKEND, or if you prefer to keep the generic, change the unsafe impls to
include the lifetime (unsafe impl<'a> Send for PlatformGL<'a> {} and unsafe
impl<'a> Sync for PlatformGL<'a> {}); update the types referenced
(PlatformGL<'a>, WgpuLink<'a>, WrappedContext, Surface<'a>, Window,
instance.create_surface, and the BACKEND static ref) to be consistent with the
chosen approach.
---
Outside diff comments:
In `@bracket-pathfinding/src/field_of_view/symmetric_shadowcasting.rs`:
- Around line 117-156: Add regression tests in a colocated tests.rs for
symmetric_shadowcasting that exercise scan()’s slope-transition logic across
opaque/transparent boundaries: create minimal field maps (use the same
types/constructors used by symmetric_shadowcasting: Scanline,
Quadrant/transform, and the FOV struct that owns scan()) and add focused cases
for (1) opaque->transparent transition, (2) transparent->opaque transition, and
(3) edge-case grazing tiles near radius limit; in each test set up the tile
opacity pattern, call scan(first_line) (or the public entry that invokes scan),
and assert the visible tiles and that subsequent scanlines would carry the
expected start_slope/end_slope (i.e., verify which tiles are revealed and that
transitions spawn next scanlines with correct slopes). Place these tests in the
module’s tests.rs so they run with cargo test and lock the behavior introduced
by scan(), using the existing helpers (is_opaque, reveal, Scanline::next, slope,
quadrant transform) to construct expectations.
---
Duplicate comments:
In `@bracket-terminal/src/hal/crossterm_be/scancode_helper.rs`:
- Around line 108-186: The keycode_to_key function currently ignores uppercase
letters because it only matches Char('a'..'z'); update key handling by matching
CrossKeyCode::Char(c) once, normalize ASCII alphabetic chars with
c.to_ascii_lowercase() (or equivalent) before mapping to KeyCode::KeyA..KeyZ,
and keep all other Char mappings unchanged; then add colocated tests in the
module's tests.rs that assert both lowercase and uppercase Char inputs (e.g.,
'a' and 'A') map to the same KeyCode (use keycode_to_key in tests).
In `@bracket-terminal/src/hal/webgpu/init.rs`:
- Around line 18-24: EventLoop::new() now returns Result and you must handle the
EventLoopError instead of assigning directly; call
EventLoop::new().expect("...") or propagate the Result to handle errors from
EventLoop::new() (reference EventLoop::new and EventLoopError), and stop using
the deprecated EventLoop::create_window compatibility shim by calling the
supported ActiveEventLoop::create_window API with the created event loop
(reference ActiveEventLoop::create_window and the existing create_window
parameters) so the code type-checks and avoids clippy deprecation warnings.
---
Nitpick comments:
In `@bracket-terminal/examples/tiles.rs`:
- Around line 81-90: The match arms in tiles.rs use VirtualKeyCode::Numpad* but
the crate re-exports winit::keyboard::KeyCode as VirtualKeyCode, so for
consistency with the cursor handling switch the Numpad arms to use
KeyCode::Numpad* instead; update the match cases (the lines calling
self.move_player(...) for Numpad8, Numpad4, Numpad6, Numpad2 and the diagonal
Numpad7/Numpad9/Numpad1/Numpad3) to reference KeyCode::NumpadX rather than
VirtualKeyCode::NumpadX so enum usage is consistent with the rest of the file.
In `@bracket-terminal/src/bterm.rs`:
- Around line 1085-1115: Add unit tests for the letter_to_option function:
create a colocated tests module (tests.rs or a #[cfg(test)] mod tests in the
same file) and add assertions that letter_to_option(KeyCode::KeyA) == 0,
letter_to_option(KeyCode::KeyZ) == 25, and that a non-letter key (e.g.
KeyCode::Escape or another non-alphabetic KeyCode variant) returns -1; put these
tests next to the letter_to_option function so future key-API changes are
covered.
🪄 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: d5b681ae-d4a2-499d-8e89-1c21f2ad6530
📒 Files selected for processing (121)
Cargo.tomlbracket-algorithm-traits/Cargo.tomlbracket-algorithm-traits/src/algorithm2d.rsbracket-algorithm-traits/src/algorithm3d.rsbracket-algorithm-traits/src/lib.rsbracket-bevy/Cargo.tomlbracket-bevy/examples/bevy_roguelike_tutorial_4/map.rsbracket-bevy/src/builder/bterm_builder.rsbracket-bevy/src/builder/loader_system.rsbracket-bevy/src/consoles/common_draw.rsbracket-bevy/src/consoles/simple_console/back_end/simple_no_background.rsbracket-bevy/src/consoles/simple_console/back_end/simple_with_background.rsbracket-bevy/src/consoles/simple_console/front_end.rsbracket-bevy/src/consoles/sparse_console/back_end/sparse_no_background.rsbracket-bevy/src/consoles/sparse_console/back_end/sparse_with_background.rsbracket-bevy/src/consoles/sparse_console/front_end.rsbracket-bevy/src/consoles/virtual_console.rsbracket-bevy/src/context.rsbracket-bevy/src/lib.rsbracket-bevy/src/textblock.rsbracket-color/Cargo.tomlbracket-color/examples/colors.rsbracket-color/examples/lerp.rsbracket-color/examples/lerpit.rsbracket-color/examples/lerpit_hsv.rsbracket-color/examples/named_color.rsbracket-color/examples/shades_of_grey.rsbracket-color/src/palette.rsbracket-color/src/rgb.rsbracket-color/src/rgba.rsbracket-embedding/Cargo.tomlbracket-geometry/Cargo.tomlbracket-geometry/examples/bresenham_circle.rsbracket-geometry/examples/bresenham_line.rsbracket-geometry/examples/bresenham_sweep.rsbracket-geometry/examples/vector_line.rsbracket-geometry/src/angles.rsbracket-geometry/src/lines.rsbracket-noise/Cargo.tomlbracket-noise/examples/cellular.rsbracket-noise/examples/perlin_fractal.rsbracket-noise/examples/simplex_billow_quintic.rsbracket-noise/examples/simplex_fractal.rsbracket-noise/examples/simplex_rigid_hermite.rsbracket-noise/examples/value.rsbracket-noise/examples/value_fractal.rsbracket-noise/examples/whitenoise.rsbracket-noise/src/fastnoise.rsbracket-pathfinding/Cargo.tomlbracket-pathfinding/benches/astar_benchmark.rsbracket-pathfinding/benches/fov_benchmark.rsbracket-pathfinding/examples/astar/common.rsbracket-pathfinding/examples/astar_manhattan/common.rsbracket-pathfinding/examples/dijkstra/common.rsbracket-pathfinding/examples/dijkstra/main.rsbracket-pathfinding/examples/dijkstra_weighted/common.rsbracket-pathfinding/examples/dijkstra_weighted/main.rsbracket-pathfinding/examples/fov/common.rsbracket-pathfinding/src/field_of_view/mod.rsbracket-pathfinding/src/field_of_view/symmetric_shadowcasting.rsbracket-pathfinding/src/lib.rsbracket-random/Cargo.tomlbracket-random/benches/dice.rsbracket-random/src/parsing.rsbracket-random/src/random.rsbracket-rex/Cargo.tomlbracket-rex/src/rex.rsbracket-terminal/Cargo.tomlbracket-terminal/benches/batching_benchmark.rsbracket-terminal/examples/fontswitch.rsbracket-terminal/examples/no_cls.rsbracket-terminal/examples/postprocess.rsbracket-terminal/examples/sparse_cls.rsbracket-terminal/examples/squishy.rsbracket-terminal/examples/tiles.rsbracket-terminal/examples/walking.rsbracket-terminal/examples/walking_with_is_pressed.rsbracket-terminal/src/bterm.rsbracket-terminal/src/consoles/command_buffer.rsbracket-terminal/src/consoles/flexible_console.rsbracket-terminal/src/consoles/simple_console.rsbracket-terminal/src/consoles/sparse_console.rsbracket-terminal/src/consoles/text/gui_helpers.rsbracket-terminal/src/consoles/text/multi_tile_sprite.rsbracket-terminal/src/consoles/text/textblock.rsbracket-terminal/src/consoles/virtual_console.rsbracket-terminal/src/hal/crossterm_be/init.rsbracket-terminal/src/hal/crossterm_be/main_loop.rsbracket-terminal/src/hal/crossterm_be/mod.rsbracket-terminal/src/hal/crossterm_be/scancode_helper.rsbracket-terminal/src/hal/curses/main_loop.rsbracket-terminal/src/hal/curses/mod.rsbracket-terminal/src/hal/gl_common/backing/fancy_console_backing.rsbracket-terminal/src/hal/gl_common/backing/shared_main_loop.rsbracket-terminal/src/hal/gl_common/backing/simple_console_backing.rsbracket-terminal/src/hal/gl_common/backing/sparse_console_backing.rsbracket-terminal/src/hal/gl_common/backing/sprite_console_backing.rsbracket-terminal/src/hal/gl_common/font.rsbracket-terminal/src/hal/gl_common/shader.rsbracket-terminal/src/hal/native/init.rsbracket-terminal/src/hal/native/mainloop.rsbracket-terminal/src/hal/native/mod.rsbracket-terminal/src/hal/wasm/events/external.rsbracket-terminal/src/hal/wasm/events/mod.rsbracket-terminal/src/hal/wasm/init.rsbracket-terminal/src/hal/wasm/mainloop.rsbracket-terminal/src/hal/wasm/mod.rsbracket-terminal/src/hal/webgpu/backend.rsbracket-terminal/src/hal/webgpu/backing/fancy_console_backing.rsbracket-terminal/src/hal/webgpu/backing/simple_console_backing.rsbracket-terminal/src/hal/webgpu/backing/sparse_console_backing.rsbracket-terminal/src/hal/webgpu/backing/sprite_console_backing.rsbracket-terminal/src/hal/webgpu/init.rsbracket-terminal/src/hal/webgpu/mainloop.rsbracket-terminal/src/hal/webgpu/mod.rsbracket-terminal/src/hal/webgpu/platform.rsbracket-terminal/src/hal/webgpu/quadrender.rsbracket-terminal/src/initializer.rsbracket-terminal/src/input/event_queue.rsbracket-terminal/src/input/input_handler.rsbracket-terminal/src/lib.rs
✅ Files skipped from review due to trivial changes (85)
- bracket-embedding/Cargo.toml
- bracket-color/examples/lerp.rs
- bracket-algorithm-traits/Cargo.toml
- bracket-color/examples/named_color.rs
- bracket-bevy/examples/bevy_roguelike_tutorial_4/map.rs
- bracket-geometry/examples/bresenham_sweep.rs
- bracket-noise/examples/simplex_billow_quintic.rs
- bracket-bevy/src/consoles/virtual_console.rs
- bracket-terminal/src/consoles/text/multi_tile_sprite.rs
- bracket-bevy/src/consoles/simple_console/back_end/simple_no_background.rs
- bracket-geometry/examples/bresenham_circle.rs
- bracket-color/examples/lerpit.rs
- bracket-terminal/src/hal/native/mod.rs
- bracket-terminal/src/consoles/text/textblock.rs
- bracket-random/src/random.rs
- bracket-random/src/parsing.rs
- bracket-terminal/src/hal/gl_common/font.rs
- bracket-terminal/src/hal/curses/mod.rs
- bracket-bevy/src/consoles/simple_console/front_end.rs
- bracket-color/src/rgb.rs
- bracket-bevy/Cargo.toml
- bracket-noise/Cargo.toml
- bracket-pathfinding/benches/fov_benchmark.rs
- bracket-color/examples/lerpit_hsv.rs
- bracket-terminal/benches/batching_benchmark.rs
- bracket-terminal/src/hal/webgpu/mod.rs
- bracket-geometry/examples/vector_line.rs
- bracket-terminal/src/hal/gl_common/backing/sprite_console_backing.rs
- bracket-color/examples/shades_of_grey.rs
- bracket-terminal/src/consoles/virtual_console.rs
- bracket-terminal/src/hal/wasm/mod.rs
- bracket-geometry/src/lines.rs
- bracket-noise/examples/perlin_fractal.rs
- bracket-terminal/src/hal/wasm/events/external.rs
- bracket-terminal/src/hal/gl_common/backing/shared_main_loop.rs
- bracket-pathfinding/benches/astar_benchmark.rs
- bracket-color/src/rgba.rs
- bracket-noise/examples/cellular.rs
- bracket-bevy/src/textblock.rs
- bracket-bevy/src/consoles/simple_console/back_end/simple_with_background.rs
- bracket-bevy/src/builder/bterm_builder.rs
- bracket-noise/examples/whitenoise.rs
- bracket-pathfinding/src/field_of_view/mod.rs
- bracket-terminal/src/hal/native/init.rs
- bracket-terminal/src/hal/gl_common/backing/fancy_console_backing.rs
- bracket-algorithm-traits/src/lib.rs
- bracket-pathfinding/examples/dijkstra/common.rs
- bracket-bevy/src/context.rs
- bracket-bevy/src/consoles/common_draw.rs
- bracket-terminal/src/consoles/simple_console.rs
- bracket-bevy/src/consoles/sparse_console/back_end/sparse_with_background.rs
- bracket-pathfinding/examples/fov/common.rs
- bracket-terminal/src/hal/wasm/init.rs
- bracket-terminal/src/hal/webgpu/backing/sparse_console_backing.rs
- bracket-pathfinding/src/lib.rs
- bracket-terminal/src/consoles/flexible_console.rs
- bracket-terminal/src/hal/webgpu/backing/fancy_console_backing.rs
- bracket-terminal/src/consoles/text/gui_helpers.rs
- bracket-geometry/examples/bresenham_line.rs
- bracket-noise/examples/simplex_rigid_hermite.rs
- bracket-terminal/src/hal/wasm/mainloop.rs
- bracket-pathfinding/examples/astar_manhattan/common.rs
- bracket-pathfinding/Cargo.toml
- bracket-rex/src/rex.rs
- bracket-terminal/src/hal/gl_common/backing/sparse_console_backing.rs
- bracket-noise/examples/simplex_fractal.rs
- bracket-terminal/src/hal/crossterm_be/main_loop.rs
- bracket-terminal/src/hal/webgpu/mainloop.rs
- bracket-bevy/src/consoles/sparse_console/front_end.rs
- bracket-bevy/src/consoles/sparse_console/back_end/sparse_no_background.rs
- bracket-color/examples/colors.rs
- bracket-pathfinding/examples/dijkstra/main.rs
- bracket-terminal/src/initializer.rs
- bracket-terminal/src/hal/wasm/events/mod.rs
- bracket-terminal/src/consoles/command_buffer.rs
- bracket-terminal/src/hal/curses/main_loop.rs
- bracket-terminal/examples/sparse_cls.rs
- bracket-terminal/src/hal/crossterm_be/init.rs
- bracket-terminal/src/hal/webgpu/backing/sprite_console_backing.rs
- bracket-algorithm-traits/src/algorithm2d.rs
- bracket-pathfinding/examples/dijkstra_weighted/common.rs
- bracket-pathfinding/examples/dijkstra_weighted/main.rs
- bracket-noise/src/fastnoise.rs
- bracket-terminal/src/hal/webgpu/backing/simple_console_backing.rs
- bracket-bevy/src/lib.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format all Rust code usingcargo fmt --all
Fix allcargo clippywarnings — the CI enforces-D warnings
Add tests for new functionality in the relevant module; for split domains, prefer colocatedtests.rs
Files:
bracket-pathfinding/examples/astar/common.rsbracket-algorithm-traits/src/algorithm3d.rsbracket-terminal/src/hal/webgpu/quadrender.rsbracket-geometry/src/angles.rsbracket-terminal/src/hal/webgpu/backend.rsbracket-terminal/src/hal/crossterm_be/mod.rsbracket-noise/examples/value_fractal.rsbracket-noise/examples/value.rsbracket-terminal/examples/postprocess.rsbracket-terminal/examples/tiles.rsbracket-terminal/src/consoles/sparse_console.rsbracket-terminal/examples/fontswitch.rsbracket-random/benches/dice.rsbracket-bevy/src/builder/loader_system.rsbracket-pathfinding/src/field_of_view/symmetric_shadowcasting.rsbracket-terminal/src/bterm.rsbracket-terminal/examples/no_cls.rsbracket-terminal/src/input/event_queue.rsbracket-terminal/examples/walking.rsbracket-terminal/src/lib.rsbracket-color/src/palette.rsbracket-terminal/examples/squishy.rsbracket-terminal/src/input/input_handler.rsbracket-terminal/examples/walking_with_is_pressed.rsbracket-terminal/src/hal/gl_common/backing/simple_console_backing.rsbracket-terminal/src/hal/gl_common/shader.rsbracket-terminal/src/hal/webgpu/init.rsbracket-terminal/src/hal/native/mainloop.rsbracket-terminal/src/hal/webgpu/platform.rsbracket-terminal/src/hal/crossterm_be/scancode_helper.rs
🔇 Additional comments (33)
bracket-pathfinding/examples/astar/common.rs (1)
5-5: LGTM!bracket-algorithm-traits/src/algorithm3d.rs (1)
47-49: LGTM!bracket-terminal/src/hal/webgpu/quadrender.rs (1)
3-3: LGTM!bracket-geometry/src/angles.rs (1)
20-20: LGTM!bracket-terminal/src/hal/webgpu/backend.rs (1)
3-3: LGTM!bracket-color/Cargo.toml (1)
5-5: LGTM!bracket-noise/examples/value_fractal.rs (1)
7-7: LGTM!bracket-terminal/src/hal/crossterm_be/mod.rs (1)
3-3: ⚡ Quick winProvide the original <review_comment> (and any related diff)
Missing the review comment content to rewrite. Paste the exact text from the <review_comment> tags so I can update it appropriately.Cargo.toml (1)
5-5: LGTM!bracket-noise/examples/value.rs (1)
7-7: LGTM!bracket-terminal/Cargo.toml (1)
5-5: LGTM!bracket-terminal/examples/postprocess.rs (1)
49-53: LGTM!bracket-geometry/Cargo.toml (1)
5-5: LGTM!bracket-terminal/src/consoles/sparse_console.rs (1)
1-7: LGTM!bracket-terminal/examples/fontswitch.rs (1)
26-31: LGTM!bracket-terminal/examples/walking.rs (2)
96-96: Previously flagged: truncated inline comment is still incomplete.Line 96 still ends as “and applying movement vi”.
98-113: LGTM!bracket-random/benches/dice.rs (1)
5-5: LGTM!bracket-bevy/src/builder/loader_system.rs (1)
2-3: LGTM!bracket-random/Cargo.toml (1)
5-5: LGTM!bracket-terminal/src/bterm.rs (1)
4-8: LGTM!Also applies to: 13-13, 61-61, 289-289, 330-330
bracket-terminal/examples/no_cls.rs (1)
11-11: LGTM!bracket-terminal/src/input/event_queue.rs (1)
1-1: LGTM!Also applies to: 45-45
bracket-rex/Cargo.toml (1)
4-4: LGTM!bracket-terminal/src/lib.rs (1)
12-13: LGTM!Also applies to: 28-29, 33-35, 39-42, 47-54
bracket-color/src/palette.rs (1)
2-91: LGTM!bracket-terminal/examples/squishy.rs (1)
64-64: LGTM!bracket-terminal/examples/walking_with_is_pressed.rs (1)
45-56: LGTM!bracket-terminal/src/hal/gl_common/backing/simple_console_backing.rs (1)
1-3: LGTM!bracket-terminal/src/hal/gl_common/shader.rs (1)
1-1: LGTM!Also applies to: 63-64, 72-77, 84-85, 91-92, 98-105, 112-113, 119-120, 126-132
bracket-terminal/src/input/input_handler.rs (1)
17-17: 🏗️ Heavy liftUnable to rewrite review comment: missing <review_comment> content
No review comment text was provided to rewrite, so there’s nothing to update.bracket-terminal/src/hal/native/mainloop.rs (1)
279-289: ⚡ Quick winAdd a regression test for the new
PhysicalKey::Codepath.This changes native keyboard dispatch, but there’s still no colocated test locking down press/release propagation and the
event.textpressed-only gate. That makes the winit 0.30 migration here easy to regress.As per coding guidelines,
Add tests for new functionality in the relevant module; for split domains, prefer colocated tests.rs.bracket-terminal/src/hal/webgpu/init.rs (1)
121-152: 🏗️ Heavy liftUpdate
wgpuinit to 0.29 API-compatible surface/adapter/device setup
bracket-terminaldepends onwgpu = 29.0.3, butbracket-terminal/src/hal/webgpu/init.rs’s adapter/surface/device block still uses older signatures/fields (e.g.,Instance::new(wgpu::Backends::all()), surface creation returningResultthat must be handled, andSurfaceConfigurationmissing 0.29-required fields likealpha_mode,desired_maximum_frame_latency, andview_formats). As written, this path will not compile against the declaredwgpu29.0.3.
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 (2)
bracket-terminal/src/lib.rs (1)
46-53:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBoth
KeyCodeandVirtualKeyCodenow in the prelude — verify there's no naming clash and the path resolves.After this change the prelude re-exports
KeyCodedirectly fromwinit::keyboard(line 40) andVirtualKeyCodefromcrate::hal(line 53). Two concerns:
- The AI summary claims
VirtualKeyCodeis now an alias ofwinit::keyboard::KeyCode, but the code is stillpub use crate::hal::VirtualKeyCode;— that path must remain valid for every backend in thecfg(any(...))set. The root cause concern is flagged onhal/webgpu/mod.rs.- If
crate::hal::VirtualKeyCodeends up being defined aspub use winit::keyboard::KeyCode as VirtualKeyCode;in every backend, you'll have bothKeyCodeandVirtualKeyCodein the prelude pointing at the same type, which is benign but slightly redundant; consider documentingVirtualKeyCodeas a compatibility alias.🤖 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 `@bracket-terminal/src/lib.rs` around lines 46 - 53, Ensure the prelude re-export of VirtualKeyCode is valid across all conditional backends and unambiguous: verify that crate::hal::VirtualKeyCode is exported from every backend module (e.g., check hal/webgpu/mod.rs and other hal/* backends) and, if you intend it to be a compatibility alias, have each backend define VirtualKeyCode as pub use winit::keyboard::KeyCode as VirtualKeyCode; otherwise remove or change the prelude re-export to avoid duplicating KeyCode; update crate::hal exports so the path crate::hal::VirtualKeyCode always resolves and add a short doc comment on VirtualKeyCode explaining it is an alias for winit::keyboard::KeyCode when both appear in the prelude.bracket-terminal/src/hal/scaler.rs (1)
10-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix missing
default_gutter_sizeonwasm32when building thewebgpubackend
Onwasm32+feature="webgpu"(withopengloff),hal/mod.rscompileshal/webgpu, andhal/webgpu/platform.rsunconditionally imports/callsdefault_gutter_size, buthal/scaler.rsdefines it only forwindows/macosor for non-wasm32targets—so nodefault_gutter_sizeexists onwasm32and the build fails.🛠️ Alternative — keep the symbol defined for wasm too
-#[cfg(not(any(target_os = "windows", target_os = "macos", target_arch = "wasm32")))] +#[cfg(not(any(target_os = "windows", target_os = "macos")))] pub(crate) fn default_gutter_size() -> u32 { - // Testing showed that an 8-pixel gutter is enough to fix - // Big Sur and Windows 11. + // 0 on Linux and on wasm32 (no gutter needed). 0 }🤖 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 `@bracket-terminal/src/hal/scaler.rs` around lines 10 - 15, The symbol default_gutter_size is not compiled for wasm32, but hal/webgpu/platform.rs imports/calls it when building the webgpu backend on wasm, causing the build to fail; fix by ensuring default_gutter_size is defined for wasm32 as well (either remove the current restrictive #[cfg(not(any(target_os = "windows", target_os = "macos", target_arch = "wasm32")))] so the function is available on wasm, or add a dedicated #[cfg(target_arch = "wasm32")] implementation that returns the same value), updating the definition in hal/scaler.rs so hal/webgpu/platform.rs can call default_gutter_size without conditional compilation errors.
🧹 Nitpick comments (2)
bracket-terminal/src/hal/wasm/mainloop.rs (1)
71-76: 💤 Low value
GLOBAL_BUTTON = Noneis now redundant.After the
std::ptr::replace(..., None)on line 55,GLOBAL_BUTTONis alreadyNonefor the remainder of this frame, so the explicit reset on line 75 is dead. Safe to drop for clarity; leaving it does no harm.♻️ Proposed change
unsafe { GLOBAL_KEY = None; GLOBAL_MODIFIERS = (false, false, false); GLOBAL_LEFT_CLICK = false; - GLOBAL_BUTTON = None; }🤖 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 `@bracket-terminal/src/hal/wasm/mainloop.rs` around lines 71 - 76, Remove the redundant explicit reset of GLOBAL_BUTTON to None in the unsafe block inside mainloop.rs: since GLOBAL_BUTTON is already set to None via the earlier std::ptr::replace(..., None) call, delete the standalone assignment "GLOBAL_BUTTON = None" to avoid dead/redundant code while keeping the remaining resets for GLOBAL_KEY, GLOBAL_MODIFIERS, and GLOBAL_LEFT_CLICK intact.bracket-terminal/src/hal/wasm/events/external.rs (1)
1-4: 💤 Low valueModule docs are attached to a
usestatement.The
///outer doc comments on lines 2–3 attach to the following item (use wasm_bindgen::prelude::*;) and don't actually document the module. Since these read as module-level documentation, consider converting them to inner doc comments (//!) at the top of the file. The import reorder makes this pre-existing oddity more visible.♻️ Proposed change
-use wasm_bindgen::JsCast; -/// This module handles binding external web elements, for example movement buttons. -/// Based heavily on Zireael07's pull request, but modified to be significantly more generic. +//! This module handles binding external web elements, for example movement buttons. +//! Based heavily on Zireael07's pull request, but modified to be significantly more generic. + +use wasm_bindgen::JsCast; use wasm_bindgen::prelude::*;🤖 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 `@bracket-terminal/src/hal/wasm/events/external.rs` around lines 1 - 4, The module documentation comments are currently outer `///` comments attached to the following `use wasm_bindgen::prelude::*;` import; change them to inner doc comments by replacing the leading `///` with `//!` and placing them at the very top of the file (above the `use` statements) so they become true module-level docs; ensure the descriptive text currently on lines 2–3 is preserved and moved before `use wasm_bindgen::JsCast;`/`use wasm_bindgen::prelude::*;` so tools and rustdoc pick it up as module documentation.
🤖 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 `@bracket-terminal/src/hal/webgpu/mod.rs`:
- Line 15: The webgpu backend currently re-exports only KeyCode so
crate::hal::VirtualKeyCode is missing; update the webgpu mod export to also
re-export VirtualKeyCode (e.g., add VirtualKeyCode to the pub use from
winit::keyboard) so that the symbol VirtualKeyCode (and KeyCode) is available
for the cfg branch in lib.rs that re-exports crate::hal::VirtualKeyCode.
---
Outside diff comments:
In `@bracket-terminal/src/hal/scaler.rs`:
- Around line 10-15: The symbol default_gutter_size is not compiled for wasm32,
but hal/webgpu/platform.rs imports/calls it when building the webgpu backend on
wasm, causing the build to fail; fix by ensuring default_gutter_size is defined
for wasm32 as well (either remove the current restrictive
#[cfg(not(any(target_os = "windows", target_os = "macos", target_arch =
"wasm32")))] so the function is available on wasm, or add a dedicated
#[cfg(target_arch = "wasm32")] implementation that returns the same value),
updating the definition in hal/scaler.rs so hal/webgpu/platform.rs can call
default_gutter_size without conditional compilation errors.
In `@bracket-terminal/src/lib.rs`:
- Around line 46-53: Ensure the prelude re-export of VirtualKeyCode is valid
across all conditional backends and unambiguous: verify that
crate::hal::VirtualKeyCode is exported from every backend module (e.g., check
hal/webgpu/mod.rs and other hal/* backends) and, if you intend it to be a
compatibility alias, have each backend define VirtualKeyCode as pub use
winit::keyboard::KeyCode as VirtualKeyCode; otherwise remove or change the
prelude re-export to avoid duplicating KeyCode; update crate::hal exports so the
path crate::hal::VirtualKeyCode always resolves and add a short doc comment on
VirtualKeyCode explaining it is an alias for winit::keyboard::KeyCode when both
appear in the prelude.
---
Nitpick comments:
In `@bracket-terminal/src/hal/wasm/events/external.rs`:
- Around line 1-4: The module documentation comments are currently outer `///`
comments attached to the following `use wasm_bindgen::prelude::*;` import;
change them to inner doc comments by replacing the leading `///` with `//!` and
placing them at the very top of the file (above the `use` statements) so they
become true module-level docs; ensure the descriptive text currently on lines
2–3 is preserved and moved before `use wasm_bindgen::JsCast;`/`use
wasm_bindgen::prelude::*;` so tools and rustdoc pick it up as module
documentation.
In `@bracket-terminal/src/hal/wasm/mainloop.rs`:
- Around line 71-76: Remove the redundant explicit reset of GLOBAL_BUTTON to
None in the unsafe block inside mainloop.rs: since GLOBAL_BUTTON is already set
to None via the earlier std::ptr::replace(..., None) call, delete the standalone
assignment "GLOBAL_BUTTON = None" to avoid dead/redundant code while keeping the
remaining resets for GLOBAL_KEY, GLOBAL_MODIFIERS, and GLOBAL_LEFT_CLICK intact.
🪄 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: 8e7dcbcb-b7af-4813-b96f-fedc9c8f4059
📒 Files selected for processing (102)
bracket-algorithm-traits/src/algorithm2d.rsbracket-algorithm-traits/src/algorithm3d.rsbracket-algorithm-traits/src/lib.rsbracket-bevy/examples/bevy_roguelike_tutorial_4/map.rsbracket-bevy/src/builder/bterm_builder.rsbracket-bevy/src/builder/loader_system.rsbracket-bevy/src/consoles/common_draw.rsbracket-bevy/src/consoles/simple_console/back_end/simple_no_background.rsbracket-bevy/src/consoles/simple_console/back_end/simple_with_background.rsbracket-bevy/src/consoles/simple_console/front_end.rsbracket-bevy/src/consoles/sparse_console/back_end/sparse_no_background.rsbracket-bevy/src/consoles/sparse_console/back_end/sparse_with_background.rsbracket-bevy/src/consoles/sparse_console/front_end.rsbracket-bevy/src/consoles/virtual_console.rsbracket-bevy/src/context.rsbracket-bevy/src/lib.rsbracket-bevy/src/textblock.rsbracket-color/examples/colors.rsbracket-color/examples/lerp.rsbracket-color/examples/lerpit.rsbracket-color/examples/lerpit_hsv.rsbracket-color/examples/named_color.rsbracket-color/examples/shades_of_grey.rsbracket-color/src/palette.rsbracket-color/src/rgb.rsbracket-color/src/rgba.rsbracket-geometry/examples/bresenham_circle.rsbracket-geometry/examples/bresenham_line.rsbracket-geometry/examples/bresenham_sweep.rsbracket-geometry/examples/vector_line.rsbracket-geometry/src/angles.rsbracket-geometry/src/lines.rsbracket-noise/examples/cellular.rsbracket-noise/examples/perlin_fractal.rsbracket-noise/examples/simplex_billow_quintic.rsbracket-noise/examples/simplex_fractal.rsbracket-noise/examples/simplex_rigid_hermite.rsbracket-noise/examples/value.rsbracket-noise/examples/value_fractal.rsbracket-noise/examples/whitenoise.rsbracket-noise/src/fastnoise.rsbracket-pathfinding/benches/astar_benchmark.rsbracket-pathfinding/benches/fov_benchmark.rsbracket-pathfinding/examples/astar/common.rsbracket-pathfinding/examples/astar_manhattan/common.rsbracket-pathfinding/examples/dijkstra/common.rsbracket-pathfinding/examples/dijkstra/main.rsbracket-pathfinding/examples/dijkstra_weighted/common.rsbracket-pathfinding/examples/dijkstra_weighted/main.rsbracket-pathfinding/examples/fov/common.rsbracket-pathfinding/src/field_of_view/mod.rsbracket-pathfinding/src/lib.rsbracket-random/benches/dice.rsbracket-random/src/parsing.rsbracket-random/src/random.rsbracket-rex/src/rex.rsbracket-terminal/benches/batching_benchmark.rsbracket-terminal/examples/sparse_cls.rsbracket-terminal/src/bterm.rsbracket-terminal/src/consoles/command_buffer.rsbracket-terminal/src/consoles/flexible_console.rsbracket-terminal/src/consoles/simple_console.rsbracket-terminal/src/consoles/sparse_console.rsbracket-terminal/src/consoles/text/gui_helpers.rsbracket-terminal/src/consoles/text/multi_tile_sprite.rsbracket-terminal/src/consoles/text/textblock.rsbracket-terminal/src/consoles/virtual_console.rsbracket-terminal/src/hal/crossterm_be/init.rsbracket-terminal/src/hal/crossterm_be/main_loop.rsbracket-terminal/src/hal/curses/main_loop.rsbracket-terminal/src/hal/curses/mod.rsbracket-terminal/src/hal/gl_common/backing/fancy_console_backing.rsbracket-terminal/src/hal/gl_common/backing/shared_main_loop.rsbracket-terminal/src/hal/gl_common/backing/simple_console_backing.rsbracket-terminal/src/hal/gl_common/backing/sparse_console_backing.rsbracket-terminal/src/hal/gl_common/backing/sprite_console_backing.rsbracket-terminal/src/hal/gl_common/font.rsbracket-terminal/src/hal/gl_common/framebuffer.rsbracket-terminal/src/hal/gl_common/quadrender.rsbracket-terminal/src/hal/gl_common/shader.rsbracket-terminal/src/hal/gl_common/types_wasm.rsbracket-terminal/src/hal/native/init.rsbracket-terminal/src/hal/native/mainloop.rsbracket-terminal/src/hal/native/mod.rsbracket-terminal/src/hal/scaler.rsbracket-terminal/src/hal/wasm/events/external.rsbracket-terminal/src/hal/wasm/events/mod.rsbracket-terminal/src/hal/wasm/init.rsbracket-terminal/src/hal/wasm/mainloop.rsbracket-terminal/src/hal/wasm/mod.rsbracket-terminal/src/hal/webgpu/backend.rsbracket-terminal/src/hal/webgpu/backing/fancy_console_backing.rsbracket-terminal/src/hal/webgpu/backing/simple_console_backing.rsbracket-terminal/src/hal/webgpu/backing/sparse_console_backing.rsbracket-terminal/src/hal/webgpu/backing/sprite_console_backing.rsbracket-terminal/src/hal/webgpu/init.rsbracket-terminal/src/hal/webgpu/mainloop.rsbracket-terminal/src/hal/webgpu/mod.rsbracket-terminal/src/hal/webgpu/platform.rsbracket-terminal/src/hal/webgpu/quadrender.rsbracket-terminal/src/initializer.rsbracket-terminal/src/lib.rs
✅ Files skipped from review due to trivial changes (78)
- bracket-pathfinding/examples/fov/common.rs
- bracket-geometry/examples/bresenham_circle.rs
- bracket-color/examples/colors.rs
- bracket-color/examples/lerp.rs
- bracket-pathfinding/examples/astar_manhattan/common.rs
- bracket-random/src/parsing.rs
- bracket-terminal/src/hal/gl_common/font.rs
- bracket-pathfinding/examples/dijkstra/common.rs
- bracket-pathfinding/benches/astar_benchmark.rs
- bracket-pathfinding/benches/fov_benchmark.rs
- bracket-terminal/src/consoles/text/gui_helpers.rs
- bracket-bevy/src/consoles/common_draw.rs
- bracket-terminal/src/consoles/virtual_console.rs
- bracket-geometry/examples/vector_line.rs
- bracket-geometry/examples/bresenham_sweep.rs
- bracket-algorithm-traits/src/lib.rs
- bracket-bevy/src/consoles/virtual_console.rs
- bracket-terminal/src/hal/wasm/init.rs
- bracket-bevy/src/lib.rs
- bracket-noise/examples/simplex_billow_quintic.rs
- bracket-noise/examples/simplex_rigid_hermite.rs
- bracket-algorithm-traits/src/algorithm3d.rs
- bracket-geometry/examples/bresenham_line.rs
- bracket-bevy/src/builder/loader_system.rs
- bracket-noise/examples/simplex_fractal.rs
- bracket-terminal/src/consoles/text/textblock.rs
- bracket-terminal/src/consoles/text/multi_tile_sprite.rs
- bracket-terminal/src/hal/webgpu/quadrender.rs
- bracket-color/src/rgba.rs
- bracket-bevy/src/consoles/sparse_console/back_end/sparse_with_background.rs
- bracket-terminal/src/bterm.rs
- bracket-terminal/examples/sparse_cls.rs
- bracket-terminal/benches/batching_benchmark.rs
- bracket-terminal/src/hal/webgpu/backend.rs
- bracket-pathfinding/examples/dijkstra_weighted/common.rs
- bracket-color/src/rgb.rs
- bracket-bevy/src/builder/bterm_builder.rs
- bracket-terminal/src/hal/webgpu/backing/sprite_console_backing.rs
- bracket-terminal/src/hal/gl_common/backing/sparse_console_backing.rs
- bracket-noise/examples/perlin_fractal.rs
- bracket-noise/examples/whitenoise.rs
- bracket-pathfinding/examples/astar/common.rs
- bracket-terminal/src/hal/wasm/events/mod.rs
- bracket-terminal/src/hal/webgpu/backing/fancy_console_backing.rs
- bracket-bevy/src/context.rs
- bracket-terminal/src/hal/gl_common/backing/simple_console_backing.rs
- bracket-terminal/src/hal/curses/mod.rs
- bracket-pathfinding/src/field_of_view/mod.rs
- bracket-pathfinding/src/lib.rs
- bracket-geometry/src/lines.rs
- bracket-terminal/src/hal/curses/main_loop.rs
- bracket-terminal/src/hal/webgpu/mainloop.rs
- bracket-geometry/src/angles.rs
- bracket-color/examples/shades_of_grey.rs
- bracket-bevy/examples/bevy_roguelike_tutorial_4/map.rs
- bracket-terminal/src/hal/webgpu/backing/sparse_console_backing.rs
- bracket-algorithm-traits/src/algorithm2d.rs
- bracket-terminal/src/hal/gl_common/backing/shared_main_loop.rs
- bracket-bevy/src/consoles/sparse_console/front_end.rs
- bracket-pathfinding/examples/dijkstra_weighted/main.rs
- bracket-terminal/src/consoles/flexible_console.rs
- bracket-terminal/src/hal/native/mod.rs
- bracket-noise/examples/cellular.rs
- bracket-bevy/src/textblock.rs
- bracket-color/examples/lerpit_hsv.rs
- bracket-bevy/src/consoles/simple_console/back_end/simple_no_background.rs
- bracket-terminal/src/consoles/sparse_console.rs
- bracket-terminal/src/initializer.rs
- bracket-terminal/src/hal/crossterm_be/main_loop.rs
- bracket-terminal/src/hal/crossterm_be/init.rs
- bracket-color/examples/named_color.rs
- bracket-color/examples/lerpit.rs
- bracket-noise/examples/value_fractal.rs
- bracket-terminal/src/hal/native/mainloop.rs
- bracket-terminal/src/hal/wasm/mod.rs
- bracket-noise/examples/value.rs
- bracket-bevy/src/consoles/simple_console/back_end/simple_with_background.rs
- bracket-terminal/src/hal/gl_common/framebuffer.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format all Rust code usingcargo fmt --all
Fix allcargo clippywarnings — the CI enforces-D warnings
Add tests for new functionality in the relevant module; for split domains, prefer colocatedtests.rs
Files:
bracket-random/benches/dice.rsbracket-terminal/src/hal/native/init.rsbracket-bevy/src/consoles/sparse_console/back_end/sparse_no_background.rsbracket-random/src/random.rsbracket-terminal/src/consoles/command_buffer.rsbracket-terminal/src/hal/gl_common/types_wasm.rsbracket-bevy/src/consoles/simple_console/front_end.rsbracket-terminal/src/hal/gl_common/backing/fancy_console_backing.rsbracket-terminal/src/hal/webgpu/mod.rsbracket-terminal/src/hal/wasm/events/external.rsbracket-terminal/src/hal/gl_common/quadrender.rsbracket-rex/src/rex.rsbracket-terminal/src/hal/gl_common/backing/sprite_console_backing.rsbracket-terminal/src/hal/webgpu/backing/simple_console_backing.rsbracket-pathfinding/examples/dijkstra/main.rsbracket-noise/src/fastnoise.rsbracket-terminal/src/hal/webgpu/init.rsbracket-color/src/palette.rsbracket-terminal/src/hal/wasm/mainloop.rsbracket-terminal/src/lib.rsbracket-terminal/src/hal/scaler.rsbracket-terminal/src/hal/gl_common/shader.rsbracket-terminal/src/hal/webgpu/platform.rsbracket-terminal/src/consoles/simple_console.rs
🔇 Additional comments (20)
bracket-noise/src/fastnoise.rs (1)
1688-1688: LGTM!Also applies to: 3009-3009
bracket-pathfinding/examples/dijkstra/main.rs (1)
31-31: LGTM!bracket-terminal/src/hal/gl_common/backing/fancy_console_backing.rs (1)
1-3: LGTM!bracket-terminal/src/hal/gl_common/shader.rs (1)
1-1: LGTM!Also applies to: 72-77, 98-105, 126-132
bracket-color/src/palette.rs (1)
2-91: LGTM!bracket-bevy/src/consoles/simple_console/front_end.rs (1)
1-9: LGTM!bracket-bevy/src/consoles/sparse_console/back_end/sparse_no_background.rs (1)
2-2: LGTM!bracket-random/benches/dice.rs (1)
5-5: LGTM!bracket-random/src/random.rs (1)
2-3: LGTM!bracket-rex/src/rex.rs (1)
15-15: LGTM!bracket-terminal/src/consoles/command_buffer.rs (1)
6-6: LGTM!bracket-terminal/src/consoles/simple_console.rs (1)
2-3: LGTM!bracket-terminal/src/hal/gl_common/backing/sprite_console_backing.rs (1)
1-1: LGTM!bracket-terminal/src/hal/native/init.rs (1)
3-6: LGTM!bracket-terminal/src/hal/wasm/mainloop.rs (1)
55-55: LGTM!bracket-terminal/src/hal/webgpu/backing/simple_console_backing.rs (1)
5-9: LGTM!bracket-terminal/src/hal/webgpu/init.rs (1)
18-24: Window creation flow still incompatible with winit 0.30.
EventLoop::new()returnsResultin winit 0.30 andcreate_windowis only available onActiveEventLoop(insideApplicationHandler::resumed), not onEventLoop. PR description acknowledges the WebGPU backend remains broken and is out of scope, so this remains an outstanding migration task for a follow-up.bracket-terminal/src/hal/webgpu/platform.rs (1)
37-38:Send/Syncimpls still don't carry the'alifetime.
PlatformGLis nowPlatformGL<'a>(line 9) butunsafe impl Send for PlatformGL {}/unsafe impl Sync for PlatformGL {}reference the un-parameterized form. These need to becomeunsafe impl<'a> Send for PlatformGL<'a> {}/unsafe impl<'a> Sync for PlatformGL<'a> {}(or, preferably, fix the underlying lifetime coupling — see below).Additionally, the broader problem (already raised on a prior commit) remains:
WrappedContext { window: Window }is owned alongsideWgpuLink<'a> { surface: Surface<'a> }whereSurface<'a>is borrowed from thatWindow, and the whole thing is stored in astatic ref BACKEND: Mutex<PlatformGL>. With wgpu 29.0.3 the recommended pattern isArc<Window>intoInstance::create_surfaceso the surface is effectivelySurface<'static>. Until the lifetime coupling is resolved, this struct cannot soundly be a static.🛠️ Minimal patch to at least make the impls match the struct
-unsafe impl Send for PlatformGL {} -unsafe impl Sync for PlatformGL {} +unsafe impl<'a> Send for PlatformGL<'a> {} +unsafe impl<'a> Sync for PlatformGL<'a> {}bracket-terminal/src/hal/gl_common/types_wasm.rs (1)
5-6: ⚡ Quick winFix wasm cfg:
FramebufferIdbecomes unavailable on wasm32
bracket-terminal/src/hal/gl_common/mod.rscompilestypes_wasm.rsonly under#[cfg(target_arch = "wasm32")], sotypes_wasm.rs’s#[cfg(not(target_arch = "wasm32"))] pub type FramebufferId = glow::WebFramebufferKey;makesFramebufferIdeffectively undefined on wasm. If any wasm code expectsFramebufferId, it will fail to compile (or needs matching#[cfg(target_arch = "wasm32")]gating). Confirm/adjust the cfg soFramebufferIdis exported on the wasm target wheretypes_wasmis included.bracket-terminal/src/hal/gl_common/quadrender.rs (1)
53-53: ⚡ Quick winConfirm all
setup_quad_guttercall sites are also wasm-gated.Gating the definition with
#[cfg(not(target_arch = "wasm32"))]is fine as long as no caller still references it onwasm32; otherwise the wasm build will break with an unresolved name.#!/bin/bash # Find all callers of setup_quad_gutter and inspect their cfg gating rg -nP --type=rust -C5 '\bsetup_quad_gutter\b'
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 `@bracket-pathfinding/src/field_of_view/symmetric_shadowcasting.rs`:
- Around line 148-153: Add unit tests in a colocated tests.rs for
symmetric_shadowcasting.rs that exercise the scanline continuation path around
the prev_tile transition: construct small maps where a tile sequence goes opaque
→ transparent and also cases where the final tile is non-opaque so the branch
that creates next_line and does next_line.start_slope = current_start_slope and
stack.push(next_line) runs. Write assertions that the Field of View (or the
public API used to compute FOV in this module) returns the expected visible set
for both scenarios; target the functions/variables referenced in the diff
(prev_tile, is_opaque, scanline.next, next_line.start_slope, and stack.push) so
the tests lock the intended behavior and prevent regressions.
🪄 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: 40686798-9543-4992-bd48-fdc4bb768a48
📒 Files selected for processing (11)
bracket-bevy/src/consoles/update_system.rsbracket-pathfinding/src/astar.rsbracket-pathfinding/src/field_of_view/symmetric_shadowcasting.rsbracket-terminal/examples/dungeon_postman.rsbracket-terminal/examples/squishy.rsbracket-terminal/src/hal/gl_common/backing/simple_console_backing.rsbracket-terminal/src/hal/gl_common/backing/sparse_console_backing.rsbracket-terminal/src/hal/native/mainloop.rsbracket-terminal/src/hal/webgpu/init.rsbracket-terminal/src/hal/webgpu/mod.rsbracket-terminal/src/hal/webgpu/platform.rs
✅ Files skipped from review due to trivial changes (3)
- bracket-bevy/src/consoles/update_system.rs
- bracket-terminal/src/hal/native/mainloop.rs
- bracket-pathfinding/src/astar.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format all Rust code usingcargo fmt --all
Fix allcargo clippywarnings — the CI enforces-D warnings
Add tests for new functionality in the relevant module; for split domains, prefer colocatedtests.rs
Files:
bracket-terminal/src/hal/webgpu/mod.rsbracket-terminal/src/hal/gl_common/backing/simple_console_backing.rsbracket-terminal/src/hal/webgpu/platform.rsbracket-terminal/examples/dungeon_postman.rsbracket-terminal/src/hal/gl_common/backing/sparse_console_backing.rsbracket-terminal/src/hal/webgpu/init.rsbracket-terminal/examples/squishy.rsbracket-pathfinding/src/field_of_view/symmetric_shadowcasting.rs
🔇 Additional comments (7)
bracket-terminal/src/hal/webgpu/mod.rs (1)
16-16: LGTM!bracket-terminal/src/hal/gl_common/backing/simple_console_backing.rs (1)
91-97: LGTM!bracket-terminal/src/hal/webgpu/platform.rs (1)
37-38: LGTM!bracket-terminal/examples/dungeon_postman.rs (1)
550-558: LGTM!bracket-terminal/src/hal/gl_common/backing/sparse_console_backing.rs (1)
68-74: LGTM!bracket-terminal/src/hal/webgpu/init.rs (1)
18-24: LGTM!bracket-terminal/examples/squishy.rs (1)
63-67: LGTM!
Owner
|
@coderabbitai Can you review it again? |
|
✅ Actions performedFull review triggered. |
utilForever
approved these changes
May 21, 2026
Owner
utilForever
left a comment
utilForever
left a comment
There was a problem hiding this comment.
Thanks for your contribution. I looked over your code and made some changes because I thought you were heading in the wrong direction. Anyway, it’s fine now. Good to go! 🚀
Closed
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
VirtualKeyCodeusage to the newerKeyCodeAPI forwinit 0.30Testing
Tested successfully with:
Note:
--all-featurescurrently fails because the rendering backends are mutually exclusive by design.Summary by CodeRabbit
Chores
Refactor