Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clippy.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<<<<<<< SEARCH
=======
>>>>>>> REPLACE
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
30 changes: 30 additions & 0 deletions patch_main.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import re

with open("src/main.rs", "r") as f:
content = f.read()

content = content.replace(
""" if event::poll(Duration::from_millis(16))? {
if let Event::Key(key) = event::read()? {
if key.code == KeyCode::Esc { break; }""",
""" if event::poll(Duration::from_millis(16))?
&& let Event::Key(key) = event::read()? {
if key.code == KeyCode::Esc { break; }""")

content = content.replace(
""" }
}
}
}""",
""" }
}
}""")

content = content.replace(
""" let mut start_y = if py >= view_h/2 { py - view_h/2 } else { 0 };
let mut start_x = if px >= view_w/2 { px - view_w/2 } else { 0 };""",
""" let mut start_y = py.saturating_sub(view_h / 2);
let mut start_x = px.saturating_sub(view_w / 2);""")

with open("src/main.rs", "w") as f:
f.write(content)
67 changes: 67 additions & 0 deletions patch_net.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import re

with open("src/net.rs", "r") as f:
content = f.read()

content = content.replace(
""" if let Ok(s) = serde_json::to_string(&snap) {
if tx.send(warp::ws::Message::text(s)).await.is_err() { break; }
}""",
""" if let Ok(s) = serde_json::to_string(&snap)
&& tx.send(warp::ws::Message::text(s)).await.is_err() { break; }""")

content = content.replace(
""" if let Ok(text) = msg.to_str() {
if let Ok(cmd) = serde_json::from_str::<ClientCommand>(text) {
let mut lock = state_for_recv.lock().unwrap();
if cmd.action == "move" {
if let Some(direction) = cmd.direction.as_deref() {
match direction {
"up" => { let _ = move_player(&mut lock, 0, -1); }
"down" => { let _ = move_player(&mut lock, 0, 1); }
"left" => { let _ = move_player(&mut lock, -1, 0); }
"right" => { let _ = move_player(&mut lock, 1, 0); }
_ => {}
}
}
} else if cmd.action == "fire" {
if let Some(direction) = cmd.direction.as_deref() {
match direction {
"up" => { let _ = fire_at_direction(&mut lock, 0, -1); }
"down" => { let _ = fire_at_direction(&mut lock, 0, 1); }
"left" => { let _ = fire_at_direction(&mut lock, -1, 0); }
"right" => { let _ = fire_at_direction(&mut lock, 1, 0); }
_ => {}
}
}
}
}
}""",
""" if let Ok(text) = msg.to_str()
&& let Ok(cmd) = serde_json::from_str::<ClientCommand>(text) {
let mut lock = state_for_recv.lock().unwrap();
if cmd.action == "move" {
if let Some(direction) = cmd.direction.as_deref() {
match direction {
"up" => { let _ = move_player(&mut lock, 0, -1); }
"down" => { let _ = move_player(&mut lock, 0, 1); }
"left" => { let _ = move_player(&mut lock, -1, 0); }
"right" => { let _ = move_player(&mut lock, 1, 0); }
_ => {}
}
}
} else if cmd.action == "fire"
&& let Some(direction) = cmd.direction.as_deref() {
match direction {
"up" => { let _ = fire_at_direction(&mut lock, 0, -1); }
"down" => { let _ = fire_at_direction(&mut lock, 0, 1); }
"left" => { let _ = fire_at_direction(&mut lock, -1, 0); }
"right" => { let _ = fire_at_direction(&mut lock, 1, 0); }
_ => {}
}
}
}""")


with open("src/net.rs", "w") as f:
f.write(content)
31 changes: 31 additions & 0 deletions patch_serial.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import re

with open("src/serial_daemon.rs", "r") as f:
content = f.read()

# collapse nested ifs
# if rx_port.read_exact(&mut sync_byte).is_ok() {
# if sync_byte[0] == 0xAA {
# if rx_port.read_exact(payload.as_mut_slice()).is_ok() {

content = content.replace(
""" if rx_port.read_exact(&mut sync_byte).is_ok() {
if sync_byte[0] == 0xAA {
if rx_port.read_exact(payload.as_mut_slice()).is_ok() {""",
""" if rx_port.read_exact(&mut sync_byte).is_ok()
&& sync_byte[0] == 0xAA
&& rx_port.read_exact(payload.as_mut_slice()).is_ok() {""")
content = content.replace(
""" if lock.stats.health == 0 && lock.phase == AppPhase::Playing {
lock.phase = AppPhase::GameOver;
}
}
}
}""",
""" if lock.stats.health == 0 && lock.phase == AppPhase::Playing {
lock.phase = AppPhase::GameOver;
}
}""")

with open("src/serial_daemon.rs", "w") as f:
f.write(content)
11 changes: 11 additions & 0 deletions patch_state.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import re

with open("src/state.rs", "r") as f:
content = f.read()

# remove apply_pickup
apply_pickup_re = re.compile(r'/// Apply a pickup/drop effect at the given index in the map for the provided state\.\n/// Returns true if a pickup was consumed and applied\.\npub fn apply_pickup.*?^}\n', re.MULTILINE | re.DOTALL)
content = apply_pickup_re.sub('', content)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Make this rewrite fail-fast instead of silently no-op.

Right now, a pattern drift leaves content unchanged and still writes success output. Please validate match counts (subn/replace accounting) and abort when expected replacements are not applied; the same pattern should be used across the other patch scripts too.

Suggested patch pattern
-apply_pickup_re = re.compile(r'/// Apply a pickup/drop effect at the given index in the map for the provided state\.\n/// Returns true if a pickup was consumed and applied\.\npub fn apply_pickup.*?^}\n', re.MULTILINE | re.DOTALL)
-content = apply_pickup_re.sub('', content)
+apply_pickup_re = re.compile(
+    r'/// Apply a pickup/drop effect at the given index in the map for the provided state\.\n'
+    r'/// Returns true if a pickup was consumed and applied\.\n'
+    r'pub fn apply_pickup.*?^}\n',
+    re.MULTILINE | re.DOTALL,
+)
+content, replaced = apply_pickup_re.subn('', content, count=1)
+if replaced != 1:
+    raise RuntimeError(f"Expected to remove exactly one apply_pickup block, got {replaced}")
🤖 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 `@patch_state.py` around lines 7 - 8, Change the apply_pickup_re pattern
matching from using sub() to using subn() to capture both the modified content
and the count of replacements made. Store the result in a tuple (new_content,
count) instead of just content, then add validation logic to check that count is
greater than zero; if the pattern doesn't match as expected (count == 0), raise
an exception or abort to fail-fast rather than silently continuing with
unchanged content. This same validation pattern should be applied consistently
to all other regex substitutions in the patch scripts to prevent pattern drift
from going undetected.


with open("src/state.rs", "w") as f:
f.write(content)
11 changes: 11 additions & 0 deletions patch_state2.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import re

with open("src/state.rs", "r") as f:
content = f.read()

# I also need to update the test to use consume_tile_effect instead of apply_pickup
test_re = re.compile(r'let applied = apply_pickup\(&mut gs, 0\);')
content = test_re.sub('let applied = consume_tile_effect(&mut gs, Tile::Health as u8); gs.map_matrix[0] = Tile::Empty as u8;', content)

with open("src/state.rs", "w") as f:
f.write(content)
17 changes: 17 additions & 0 deletions patch_state_clippy.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import re

with open("src/state.rs", "r") as f:
content = f.read()

# remove unused code `let mut start_y = if py >= view_h/2 { py - view_h/2 } else { 0 };`

# in src/state.rs, change `for i in 0..size` to `for item in map.iter_mut().take(size)`
content = re.sub(r'for i in 0\.\.size \{\n\s*if rng\.gen_bool\(0\.12\) \{\n\s*map\[i\] = Tile::Wall as u8;\n\s*\}\n\s*\}', r'for item in map.iter_mut().take(size) {\n if rng.gen_bool(0.12) {\n *item = Tile::Wall as u8;\n }\n }', content)

content = re.sub(r'for i in 0\.\.size \{\n\s*if map\[i\] == Tile::Empty as u8 \{\n\s*let roll: f64 = rng\.gen_range\(0\.0\.\.1\.0\);\n\s*if roll < 0\.03 \{\n\s*map\[i\] = Tile::Resource as u8;\n\s*\} else if roll < 0\.07 \{\n\s*// health or other pickups\n\s*map\[i\] = if rng\.gen_bool\(0\.5\) \{ Tile::Health as u8 \} else \{ Tile::Smoke as u8 \};\n\s*\}\n\s*\}\n\s*\}', r'for item in map.iter_mut().take(size) {\n if *item == Tile::Empty as u8 {\n let roll: f64 = rng.gen_range(0.0..1.0);\n if roll < 0.03 {\n *item = Tile::Resource as u8;\n } else if roll < 0.07 {\n // health or other pickups\n *item = if rng.gen_bool(0.5) { Tile::Health as u8 } else { Tile::Smoke as u8 };\n }\n }\n }', content)

content = content.replace("let idx = rng.gen_range(0..size) as usize;", "let idx = rng.gen_range(0..size);")


with open("src/state.rs", "w") as f:
f.write(content)
6 changes: 6 additions & 0 deletions plan.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
1. I found unused `apply_pickup` in `src/state.rs`. The code uses `consume_tile_effect` for pickups when moving onto them, which makes `apply_pickup` dead code. So I can remove `apply_pickup` from `src/state.rs`.
2. I found some warnings from `cargo clippy`. I should fix the warnings:
- In `src/state.rs`, the `for i in 0..size` loops over `map` can be replaced with iterators. Or I can keep the loop and use `map[i] = ...`. `clippy` suggests `for item in map.iter_mut().take(size)`. I can rewrite this.
- In `src/state.rs`, `rng.gen_range(0..size) as usize` casts a `usize` to `usize`. I can fix this by removing `as usize`.
- In `src/serial_daemon.rs`, `src/net.rs`, `src/main.rs`, I can collapse nested `if` statements as suggested by clippy.
- In `src/main.rs`, there are manual arithmetic checks (`if px >= view_w/2 { px - view_w/2 } else { 0 }`) which can be replaced by `px.saturating_sub(view_w / 2)`.
Loading
Loading