diff --git a/crates/aep/src/import/effects.rs b/crates/aep/src/import/effects.rs index e53d6a9aa..1ab4b208a 100644 --- a/crates/aep/src/import/effects.rs +++ b/crates/aep/src/import/effects.rs @@ -83,11 +83,11 @@ pub(crate) fn import_effects<'f, 'a>( } let shown = fx.display_name().unwrap_or_else(|| fx.match_name().to_string()); let Some(map) = EFFECTS.iter().find(|m| m.match_name == fx.match_name()) else { - col.lose(LossCategory::Effects, format!("{}: '{}' has the effect '{shown}' ({})", cx.comp, cx.layer, fx.match_name())); + col.lose_unmapped_effect(fx.match_name(), format!("{}: '{}' has the effect '{shown}' ({})", cx.comp, cx.layer, fx.match_name())); continue; }; let Some(spec) = effectcraft_effects::find(map.effect) else { - col.lose(LossCategory::Effects, format!("{}: '{}' has the effect '{shown}' ({})", cx.comp, cx.layer, fx.match_name())); + col.lose_unmapped_effect(fx.match_name(), format!("{}: '{}' has the effect '{shown}' ({})", cx.comp, cx.layer, fx.match_name())); continue; }; if defs.source(&fx) == DefinitionSource::Missing { diff --git a/crates/aep/src/import/report.rs b/crates/aep/src/import/report.rs index 129fcf143..2825b61b3 100644 --- a/crates/aep/src/import/report.rs +++ b/crates/aep/src/import/report.rs @@ -269,8 +269,15 @@ pub(crate) struct Collector { losses: BTreeMap, pub missing_footage: Vec, pub missing_fonts: Vec, + /// The effects with no counterpart, by match name: the first instance's report line and how + /// many instances there were. They get a line each in [`Collector::finish`], so one that + /// comes after the example lines are used up is still named (#3365). + unmapped_effects: BTreeMap, } +/// How many different unmapped effects get a line of their own (a file names at most this many). +const MAX_UNMAPPED_EFFECTS: usize = 100; + impl Collector { /// Records one loss. pub fn lose(&mut self, category: LossCategory, detail: impl Into) { @@ -281,7 +288,28 @@ impl Collector { } } - pub fn finish(self) -> ImportReport { + /// Records one instance of an effect with no counterpart. The loss is counted now; its line is + /// made in [`Collector::finish`], one per effect with the number of instances, so that many + /// other losses cannot hide which effects were dropped. + pub fn lose_unmapped_effect(&mut self, match_name: &str, detail: String) { + let loss = self.losses.entry(LossCategory::Effects).or_insert_with(|| Loss { category: LossCategory::Effects, count: 0, details: Vec::new() }); + loss.count = loss.count.saturating_add(1); + if let Some(seen) = self.unmapped_effects.get_mut(match_name) { + seen.1 = seen.1.saturating_add(1); + } else if self.unmapped_effects.len() < MAX_UNMAPPED_EFFECTS { + self.unmapped_effects.insert(match_name.to_string(), (detail, 1)); + } + // (Past the limit an effect is only counted: its name is not worth a line of its own.) + } + + pub fn finish(mut self) -> ImportReport { + if let Some(loss) = self.losses.get_mut(&LossCategory::Effects) { + let named = std::mem::take(&mut self.unmapped_effects).into_values().map(|(first, n)| match n { + 1 => first, + _ => format!("{first}, and {} more like it", n - 1), + }); + loss.details.splice(0..0, named); + } ImportReport { imported: self.imported, losses: self.losses.into_values().collect(), @@ -301,3 +329,45 @@ pub const LIMITATIONS: &[&str] = &[ "Text: per-character 3D, text on a path and several style slots of unknown meaning are not decoded (stored values that differ from the default are reported as text layer features).", "Footage that is not a solid is linked by its stored path with its stored frame rate and duration; the importer never opens the files (use Reload Footage after the files are found).", ]; + +#[cfg(test)] +mod tests { + use super::*; + + /// #3365: with other losses filling the 20 example lines, an effect with no counterpart still + /// gets its own line, with how many instances were lost. The category's count is unchanged. + #[test] + fn every_unmapped_effect_is_named_however_many_other_losses_there_are() { + let mut col = Collector::default(); + for i in 0..30 { + col.lose(LossCategory::Effects, format!("C: 'L{i}': parameter P not imported")); + } + for i in 0..25 { + col.lose_unmapped_effect("CC Cylinder", format!("C: 'Cyl{i}' has the effect 'CC Cylinder' (CC Cylinder)")); + } + col.lose_unmapped_effect("Pseudo/", "C: 'D' has the effect 'Dropdown' (Pseudo/)".to_string()); + let report = col.finish(); + let effects = report.losses.iter().find(|l| l.category == LossCategory::Effects).unwrap(); + assert_eq!(effects.count, 56, "30 parameters, 25 cylinders and 1 dropdown"); + let named = |what: &str| effects.details.iter().filter(|d| d.contains(what)).collect::>(); + assert_eq!(named("(CC Cylinder)").len(), 1, "{:?}", effects.details); + assert!(named("(CC Cylinder)")[0].contains("24 more"), "{:?}", named("(CC Cylinder)")); + // One instance stays the single line it was. + assert_eq!(named("(Pseudo/)"), ["C: 'D' has the effect 'Dropdown' (Pseudo/)"]); + assert!(effects.details.len() <= MAX_DETAILS + 2, "the examples stay bounded"); + } + + /// A file with more than [`MAX_UNMAPPED_EFFECTS`] different unmapped effects names that many + /// and counts the rest, whatever room the example lines have. + #[test] + fn at_most_the_limit_of_unmapped_effects_are_named() { + let mut col = Collector::default(); + for i in 0..=MAX_UNMAPPED_EFFECTS { + col.lose_unmapped_effect(&format!("Effect {i}"), format!("C: 'L' has the effect 'Effect {i}' (Effect {i})")); + } + let report = col.finish(); + let effects = report.losses.iter().find(|l| l.category == LossCategory::Effects).unwrap(); + assert_eq!(effects.count, MAX_UNMAPPED_EFFECTS + 1, "every instance is counted"); + assert_eq!(effects.details.len(), MAX_UNMAPPED_EFFECTS, "{} lines", effects.details.len()); + } +}