fabi/Sylpheed
1
0
Fork 0
Code Issues 9 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(viewer,cli,export): clear the remaining 30 clippy lints

Browse Source 
`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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01McNbzUeq1KRBWs4G6X2YVj
This commit is contained in:
sylph-decoder
2026-09-05 17:47:50 +02:00
parent 5c35a34e36
commit e7907aa604
7 changed files with 54 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
crates/sylpheed-cli/src/main.rs
View File

@@ -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<f32> = 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<f32> = 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<Vec<u
// [A,R,G,B] byte order (verified against the retail Acheron backdrop).
// Emit RGBA. X8 has no meaningful alpha.
let opaque = matches!(tex.format, F::X8R8G8B8);
for (px, out) in tex.data.chunks_exact(4).zip(rgba.chunks_exact_mut(4)) {
for (px, out) in tex.data.as_chunks::<4>().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(

2
crates/sylpheed-export/src/audio.rs
View File

@@ -109,6 +109,8 @@ pub struct BgmSpec {
pub loop_start_s: Option<f64>,
#[serde(default)]
pub loop_end_s: Option<f64>,
// Deserialised to model the sidecar schema, not read in Rust.
#[allow(dead_code)]
#[serde(default)]
pub loop_end_why: Option<serde_json::Value>,
#[serde(default)]

9
crates/sylpheed-export/src/main.rs
View File

@@ -158,6 +158,9 @@ fn load_names(authored: &Path) -> Result<NameMap> {
#[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,

2
crates/sylpheed-viewer/src/asset_loader.rs
View File

@@ -101,7 +101,7 @@ pub fn x360_texture_to_bevy_image(tex: X360Texture) -> Result<Image, Xpr2LoadErr
X360TextureFormat::A8R8G8B8 | X360TextureFormat::X8R8G8B8 => {
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;

30
crates/sylpheed-viewer/src/iso_loader.rs
View File

@@ -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<u8>)>;
/// 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<u8>)>), 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<IsoChannels>,
@@ -2557,7 +2567,7 @@ fn pick_albedo_index(model_name: &str, tex_names: &[String]) -> Option<usize> {
#[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<PendingPak>,
@@ -4076,7 +4092,7 @@ fn build_game_snapshot(source: &SourceKind) -> Option<GameSnapshot> {
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,

2
crates/sylpheed-viewer/src/lib.rs
View File

@@ -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()

10
crates/sylpheed-viewer/src/ui.rs
View File

@@ -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<ViewerState>,
@@ -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 {