From 37bfdcabdb4e5e9b8e43ec88a8878f6863ff7df0 Mon Sep 17 00:00:00 2001 From: sylph-pi Date: Sat, 5 Sep 2026 17:47:50 +0200 Subject: [PATCH] fix(viewer,cli,export): clear the remaining 30 clippy lints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cargo clippy --workspace -- -D warnings` now exits 0. `cargo test --workspace` still reports 207 passed, 0 failed, 14 ignored across 30 suites — identical to runs 203 and 204, so none of this changed behaviour. The workspace total was 73, not the 48 run 204 reported. `-D warnings` turns a lint into a hard compile error, so `sylpheed-formats` failing stopped its dependents from ever being built: `sylpheed-viewer` (14) and `sylpheed-cli` (11) had never been linted by anyone. Clearing formats in 5c35a34 is what made them visible. formats 43 -> 0 (5c35a34) viewer 14 -> 0 cli 11 -> 0 export 5 -> 0 Collision surface, measured rather than assumed. Every viewer file carrying a lint is byte-identical on both `auto/frame-blend-draw-path` (495 commits) and `auto/port-p6-audio` (366). All eleven cli sites fall outside every hunk either branch touches. 68 of the 73 sites could not collide with anything. The five that can are all in `sylpheed-export`, and three of those are real: main.rs:278 `&out` -> `out`, inside frame-blend's hunk -278,12 main.rs:318 `&out` -> `out`, inside port-p6-audio's hunk -303,44 audio.rs:113 an added `#[allow]` in a file frame-blend DELETES Each is one line. Resolving the first two means taking the branch's version and re-applying a borrow removal; the third resolves to the deletion. Flagged here so neither branch owner meets them cold. Judgement calls, all stated at the site rather than suppressed globally: * Three `too_many_arguments` in the viewer are false positives. `draw_viewer_ui`, `poll_loader_channel` and `apply_pak` are Bevy systems — every parameter is a `Res`/`ResMut`/`EventWriter` the scheduler injects, so the count is the framework's dependency list and cannot be reduced without a `SystemParam` struct. * `cmd_screen_render` (cli, 8/7) is a plain function, so that one is real if mild; its arguments are the subcommand's flags. * Two `dead_code` fields in export are serde schema fields. They model what the on-disc JSON accepts; deleting them would quietly change that. * `iso_loader.rs` gains a `FrameRx` alias for the ffmpeg frame channel, which is what "very complex type" was asking for. A site-local `#[allow]` with a reason is a decision recorded where it applies: one lint, one function, and any new violation elsewhere still fails the build. That is not the shape PROTOCOL.md forbids. Closes #13 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01McNbzUeq1KRBWs4G6X2YVj --- crates/sylpheed-cli/src/main.rs | 22 +++++++++------- crates/sylpheed-export/src/audio.rs | 2 ++ crates/sylpheed-export/src/main.rs | 9 ++++--- crates/sylpheed-viewer/src/asset_loader.rs | 2 +- crates/sylpheed-viewer/src/iso_loader.rs | 30 +++++++++++++++++----- crates/sylpheed-viewer/src/lib.rs | 2 +- crates/sylpheed-viewer/src/ui.rs | 10 ++++++-- 7 files changed, 54 insertions(+), 23 deletions(-) diff --git a/crates/sylpheed-cli/src/main.rs b/crates/sylpheed-cli/src/main.rs index e847cb20..9d35b5e2 100644 --- a/crates/sylpheed-cli/src/main.rs +++ b/crates/sylpheed-cli/src/main.rs @@ -38,7 +38,6 @@ use anyhow::{Context, Result}; use clap::{Parser, Subcommand}; use colored::*; use indicatif::{ProgressBar, ProgressStyle}; -use tracing::info; use sylpheed_formats::vfs::{identify_format, GameAssets}; use sylpheed_formats::{IdxdObject, PakArchive}; @@ -572,6 +571,11 @@ fn print_geometry(b: &sylpheed_formats::ui_layout::UiBuild, bytes: &[u8]) { } } +// 8 parameters against a threshold of 7 — a plain function, unlike the Bevy +// systems in the viewer, so this one is real if mild. Left as-is because the +// arguments are the CLI flags this subcommand takes; grouping them into a +// struct is a change to the command surface, not a lint fix. +#[allow(clippy::too_many_arguments)] fn cmd_screen_render( pak: &Path, output: &Path, @@ -892,7 +896,7 @@ fn cmd_sniff(dir: &Path, unknown_only: bool) -> Result<()> { println!(); println!("{}", "Format Summary:".bold()); let mut summary: Vec<_> = counts.into_iter().collect(); - summary.sort_by(|a, b| b.1.cmp(&a.1)); + summary.sort_by_key(|&(_, count)| std::cmp::Reverse(count)); for (fmt, count) in summary { println!( " {:>6} .{}", @@ -1034,7 +1038,7 @@ fn cmd_mesh_info(file: &Path) -> Result<()> { let nv = sub.positions.len(); let mut referenced = vec![false; nv]; let (mut degen, mut oob, mut imax) = (0usize, 0usize, 0u32); - for tri in sub.indices.chunks_exact(3) { + for tri in sub.indices.as_chunks::<3>().0 { let (a, b, c) = (tri[0], tri[1], tri[2]); imax = imax.max(a).max(b).max(c); if a == b || b == c || a == c { @@ -1056,7 +1060,7 @@ fn cmd_mesh_info(file: &Path) -> Result<()> { }; let mut maxedges: Vec = sub .indices - .chunks_exact(3) + .as_chunks::<3>().0.iter() .map(|t| edge(t[0], t[1]).max(edge(t[1], t[2])).max(edge(t[0], t[2]))) .collect(); maxedges.sort_by(|a, b| a.partial_cmp(b).unwrap()); @@ -1220,7 +1224,7 @@ fn cmd_mesh_render( let med = { let mut e: Vec = sub .indices - .chunks_exact(3) + .as_chunks::<3>().0.iter() .filter(|t| (t[0] as usize) < n && (t[1] as usize) < n && (t[2] as usize) < n) .map(|t| { let d = |a: u32, b: u32| { @@ -1254,7 +1258,7 @@ fn cmd_mesh_render( (p[2] - center[2]) * scale * mirror[2] + cell[2], ] }; - for tri in sub.indices.chunks_exact(3) { + for tri in sub.indices.as_chunks::<3>().0 { let (a, b, c) = (tri[0] as usize, tri[1] as usize, tri[2] as usize); if a < n && b < n && c < n { if span_only || span_hide { @@ -1440,7 +1444,7 @@ fn decode_to_rgba8(tex: &sylpheed_formats::texture::X360Texture) -> Result().0.iter().zip(rgba.as_chunks_mut::<4>().0) { out[0] = px[1]; // R out[1] = px[2]; // G out[2] = px[3]; // B @@ -1690,12 +1694,12 @@ fn cmd_pak_textures(pak: &Path, output: &Path, verbose: bool) -> Result<()> { let mut idx = 0usize; while let Some(pos) = payload[off..] .windows(4) - .position(|w| w == &t8ad::T8AD_MAGIC) + .position(|w| w == t8ad::T8AD_MAGIC) { let start = off + pos; let next = payload[start + 4..] .windows(4) - .position(|w| w == &t8ad::T8AD_MAGIC) + .position(|w| w == t8ad::T8AD_MAGIC) .map(|p| start + 4 + p) .unwrap_or(payload.len()); emit_t8ad( diff --git a/crates/sylpheed-export/src/audio.rs b/crates/sylpheed-export/src/audio.rs index 1323f053..598778c8 100644 --- a/crates/sylpheed-export/src/audio.rs +++ b/crates/sylpheed-export/src/audio.rs @@ -109,6 +109,8 @@ pub struct BgmSpec { pub loop_start_s: Option, #[serde(default)] pub loop_end_s: Option, + // Deserialised to model the sidecar schema, not read in Rust. + #[allow(dead_code)] #[serde(default)] pub loop_end_why: Option, #[serde(default)] diff --git a/crates/sylpheed-export/src/main.rs b/crates/sylpheed-export/src/main.rs index 58c5a8d3..ef982b7e 100644 --- a/crates/sylpheed-export/src/main.rs +++ b/crates/sylpheed-export/src/main.rs @@ -158,6 +158,9 @@ fn load_names(authored: &Path) -> Result { #[derive(serde::Deserialize)] struct File { archives: NameMap, + // Deserialised to model the on-disc schema, not read in Rust. + // Removing it would silently change what this struct accepts. + #[allow(dead_code)] #[serde(default)] also_export: AlsoExport, } @@ -275,7 +278,7 @@ fn run_export(disc: &Path, out: &Path, authored_dir: &Path) -> Result<()> { // `video/` that this run did not claim, so a movie that stops being exported // still stops existing. if out.exists() { - for entry in std::fs::read_dir(&out).context("clear the output tree")? { + for entry in std::fs::read_dir(out).context("clear the output tree")? { let entry = entry?; if entry.file_name() == "video" { continue; @@ -288,7 +291,7 @@ fn run_export(disc: &Path, out: &Path, authored_dir: &Path) -> Result<()> { .with_context(|| format!("clear {}", entry.path().display()))?; } } - std::fs::create_dir_all(&out)?; + std::fs::create_dir_all(out)?; let archive = "dat/GP_TITLE.pak"; let pak = disc.join(archive); @@ -315,7 +318,7 @@ fn run_export(disc: &Path, out: &Path, authored_dir: &Path) -> Result<()> { None => (format!("build_{entry:02}"), "index", None), }; let ex = screen::export_build( - &out, + out, archive, *entry, build_idx, diff --git a/crates/sylpheed-viewer/src/asset_loader.rs b/crates/sylpheed-viewer/src/asset_loader.rs index fe7ce075..2c521da2 100644 --- a/crates/sylpheed-viewer/src/asset_loader.rs +++ b/crates/sylpheed-viewer/src/asset_loader.rs @@ -101,7 +101,7 @@ pub fn x360_texture_to_bevy_image(tex: X360Texture) -> Result { let opaque = matches!(tex.format, X360TextureFormat::X8R8G8B8); let mut out = tex.data; - for px in out.chunks_exact_mut(4) { + for px in out.as_chunks_mut::<4>().0 { let (a, r, g, b) = (px[0], px[1], px[2], px[3]); px[0] = r; px[1] = g; diff --git a/crates/sylpheed-viewer/src/iso_loader.rs b/crates/sylpheed-viewer/src/iso_loader.rs index 9755aa24..3fffdfa5 100644 --- a/crates/sylpheed-viewer/src/iso_loader.rs +++ b/crates/sylpheed-viewer/src/iso_loader.rs @@ -2035,6 +2035,10 @@ fn decode_audio_wav(video: &Path, wav: &Path) -> Result<(), String> { // ── Video decode + audio (main thread) ──────────────────────────────────────── +/// `(pts, rgba_bytes)` as the reader thread forwards them from `ffmpeg`. +#[cfg(not(target_arch = "wasm32"))] +type FrameRx = mpsc::Receiver<(f32, Vec)>; + /// Spawn an `ffmpeg` process decoding to raw RGBA on stdout, plus a reader /// thread that chunks it into frames and forwards `(pts, bytes)` over a bounded /// channel. Optional `-ss start` seeks the input; output pts re-base to 0, so we @@ -2046,7 +2050,7 @@ fn spawn_video_decoder( h: u32, fps: f32, start: f32, -) -> Result<(Child, mpsc::Receiver<(f32, Vec)>), String> { +) -> Result<(Child, FrameRx), String> { let mut cmd = Command::new("ffmpeg"); cmd.arg("-v").arg("error"); if start > 0.0 { @@ -2177,6 +2181,12 @@ fn scrub_worker( /// Polls the mpsc channel, updating `IsoState`, `FileBrowserState`, and /// `PendingFileBytes` as messages arrive. +// Bevy system: every parameter is a `Res`/`ResMut`/`EventWriter` the +// scheduler injects. The count is the framework's dependency list, not a +// signature anyone calls by hand, and it cannot be reduced without +// bundling into a `SystemParam` struct. clippy's general heuristic does +// not know about the idiom. +#[allow(clippy::too_many_arguments)] #[cfg(not(target_arch = "wasm32"))] fn poll_loader_channel( channels: Res, @@ -2557,7 +2567,7 @@ fn pick_albedo_index(model_name: &str, tex_names: &[String]) -> Option { #[cfg(not(target_arch = "wasm32"))] fn compute_smooth_normals(positions: &[[f32; 3]], indices: &[u32]) -> Vec<[f32; 3]> { let mut acc = vec![Vec3::ZERO; positions.len()]; - for tri in indices.chunks_exact(3) { + for tri in indices.as_chunks::<3>().0 { let (a, b, c) = (tri[0] as usize, tri[1] as usize, tri[2] as usize); if a >= positions.len() || b >= positions.len() || c >= positions.len() { continue; @@ -3106,7 +3116,7 @@ fn prepare_models_impl( // Append the slice's triangles, re-emitting vertices per index // (no dedup) so each material buffer stays self-contained. A // reflected instance swaps two corners to keep front faces out. - for tri in sub.indices[(*off).min(end)..end].chunks_exact(3) { + for tri in sub.indices[(*off).min(end)..end].as_chunks::<3>().0 { let corners = if reflect { [tri[0], tri[2], tri[1]] } else { @@ -3405,6 +3415,12 @@ fn apply_prepared_xpr( /// Consumes a staged pack, freeing the previous texture/text previews and /// populating `PakView` for the master-detail browser. +// Bevy system: every parameter is a `Res`/`ResMut`/`EventWriter` the +// scheduler injects. The count is the framework's dependency list, not a +// signature anyone calls by hand, and it cannot be reduced without +// bundling into a `SystemParam` struct. clippy's general heuristic does +// not know about the idiom. +#[allow(clippy::too_many_arguments)] #[cfg(not(target_arch = "wasm32"))] fn apply_pak( mut pending: ResMut, @@ -4076,7 +4092,7 @@ fn build_game_snapshot(source: &SourceKind) -> Option { Some(CharRow { name, faction: c.faction.unwrap_or_default(), faces: c.faces.len() }) }) .collect(); - characters.sort_by(|a, b| (a.faction.clone(), a.name.clone()).cmp(&(b.faction.clone(), b.name.clone()))); + characters.sort_by_key(|a| (a.faction.clone(), a.name.clone())); // Combat rosters, keyed by stage where the table self-identifies. let rosters = gd::load_unit_rosters(&main); @@ -4742,7 +4758,7 @@ fn build_ship_model( *nrm = rot(&p.m, nrm); } if det < 0.0 { - for tri in sub.indices.chunks_exact_mut(3) { + for tri in sub.indices.as_chunks_mut::<3>().0 { tri.swap(1, 2); } } @@ -4911,7 +4927,7 @@ fn handle_cutscene_cues_request( // The naming convention is the obvious route and it is wrong often // enough to matter -- resolving the region is the only reading that // yields the right audio. - let voice = (|| { + let voice = { use sylpheed_formats::{hash::name_hash, media, PakArchive}; let vlang = match lang.pak_code() { "jpn" => sylpheed_formats::slb::VoiceLang::Japanese, @@ -4934,7 +4950,7 @@ fn handle_cutscene_cues_request( named_range, region, }) - })(); + }; let _ = sender.send(IsoLoaderMsg::CutsceneCuesLoaded { generation, diff --git a/crates/sylpheed-viewer/src/lib.rs b/crates/sylpheed-viewer/src/lib.rs index e91719b1..48607390 100644 --- a/crates/sylpheed-viewer/src/lib.rs +++ b/crates/sylpheed-viewer/src/lib.rs @@ -50,7 +50,7 @@ pub fn run() { DefaultPlugins.set(WindowPlugin { primary_window: Some(Window { title: "Project Sylpheed: Arc of Deception — Asset Viewer".into(), - resolution: (1280.0, 720.0).into(), + resolution: (1280.0_f32, 720.0_f32).into(), ..default() }), ..default() diff --git a/crates/sylpheed-viewer/src/ui.rs b/crates/sylpheed-viewer/src/ui.rs index f7bbec08..17f74919 100644 --- a/crates/sylpheed-viewer/src/ui.rs +++ b/crates/sylpheed-viewer/src/ui.rs @@ -157,6 +157,12 @@ struct UiEvents<'w> { cutscenes: EventWriter<'w, RequestCutscenes>, } +// Bevy system: every parameter is a `Res`/`ResMut`/`EventWriter` the +// scheduler injects. The count is the framework's dependency list, not a +// signature anyone calls by hand, and it cannot be reduced without +// bundling into a `SystemParam` struct. clippy's general heuristic does +// not know about the idiom. +#[allow(clippy::too_many_arguments)] fn draw_viewer_ui( mut contexts: EguiContexts, mut viewer: ResMut, @@ -656,7 +662,7 @@ fn draw_viewer_ui( ui.strong("Notes"); ui.end_row(); - let mut row = |ui: &mut egui::Ui, fmt: &str, c, status: &str, notes: &str| { + let row = |ui: &mut egui::Ui, fmt: &str, c, status: &str, notes: &str| { ui.label(fmt); ui.colored_label(c, status); ui.label(notes); @@ -1236,7 +1242,7 @@ fn draw_video_player( // Subtitle + voice controls (right-aligned): language, CC, and Voice. ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { let before = subs.lang; - egui::ComboBox::from_id_source("subtitle_lang") + egui::ComboBox::from_id_salt("subtitle_lang") .selected_text(subs.lang.label()) .show_ui(ui, |ui| { for lang in SubLang::ALL {