mirror of
https://github.com/storytold/effectcraft.git
synced 2026-11-12 21:32:52 +00:00
#3087: report the items a placeholder popup cuts
A popup of more than 1024 items kept only the first 1024 without a word, and a valid selection past them was reported as damaged data. The cut is now reported, and such a selection takes the first item and says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
b60f231fe9
commit
a44b042fce
2 files changed
+32
-4
No files matched your search
@@ -60,6 +60,9 @@ pub(crate) fn placeholder(
|
||||
ParamUi::Popup { options } => options.len(),
|
||||
_ => 0,
|
||||
};
|
||||
if p.kind == ParamKind::Popup && popup_len(p).is_some_and(|n| n > items) {
|
||||
col.lose(LossCategory::Effects, format!("{here}: parameter {label}: the items past the first {MAX_POPUP_ITEMS} not kept"));
|
||||
}
|
||||
let cost = col.placeholder_cost.saturating_add(items).saturating_add(1);
|
||||
if cost > MAX_COST {
|
||||
col.lose(LossCategory::Effects, format!("{here}: parameter {label} not kept (past the import's {MAX_COST} placeholder parameters)"));
|
||||
@@ -99,7 +102,7 @@ fn shape(p: &EffectParam<'_, '_>, size: (u32, u32)) -> Option<(Option<Kind>, Par
|
||||
// The file's items, separators (`(-`) included: After Effects counts them.
|
||||
let options: Vec<String> = match &p.items {
|
||||
Some(items) => items.iter().take(MAX_POPUP_ITEMS).cloned().collect(),
|
||||
None => (1..=usize::from(p.popup_item_count()?).min(MAX_POPUP_ITEMS)).map(|i| format!("Item {i}")).collect(),
|
||||
None => (1..=popup_len(p)?.min(MAX_POPUP_ITEMS)).map(|i| format!("Item {i}")).collect(),
|
||||
};
|
||||
let n = u32::try_from(options.len()).ok().filter(|n| *n > 0)?;
|
||||
(Kind::EffectPopup(n), ParamUi::Popup { options }, Value::Enum(0))
|
||||
@@ -109,6 +112,11 @@ fn shape(p: &EffectParam<'_, '_>, size: (u32, u32)) -> Option<(Option<Kind>, Par
|
||||
Some((Some(kind), ui, default))
|
||||
}
|
||||
|
||||
/// How many items a popup has: the file's names, else the count in its definition.
|
||||
fn popup_len(p: &EffectParam<'_, '_>) -> Option<usize> {
|
||||
p.items.as_ref().map(Vec::len).or_else(|| p.popup_item_count().map(usize::from))
|
||||
}
|
||||
|
||||
/// The property id of `p`: its match name when that is one segment of a property path (a
|
||||
/// third-party effect's `CC Cylinder-0003`), else `param<index>` (a pseudo effect's
|
||||
/// `Pseudo/0.25-0001`: `/` and `.` separate path segments). Expressions also find it by its name.
|
||||
@@ -159,8 +167,12 @@ fn fill(p: &EffectParam<'_, '_>, prop: &mut Property, kind: Kind, cx: &LayerCtx<
|
||||
None => return,
|
||||
},
|
||||
};
|
||||
match convert(value, kind) {
|
||||
Some(v) => prop.value = v,
|
||||
None => col.lose(LossCategory::DamagedData, format!("{here}: parameter {label} holds a value that is not valid ({value:?})")),
|
||||
match (convert(value, kind), value) {
|
||||
(Some(v), _) => prop.value = v,
|
||||
// An item of the file that is past the ones kept (reported with the cut): the first item.
|
||||
(None, ParamValue::Index(i)) if usize::try_from(i).is_ok_and(|i| i > MAX_POPUP_ITEMS && popup_len(p).is_some_and(|n| i <= n)) => {
|
||||
col.lose(LossCategory::Effects, format!("{here}: parameter {label}: item {i} is past the items kept, the first item used"));
|
||||
}
|
||||
(None, _) => col.lose(LossCategory::DamagedData, format!("{here}: parameter {label} holds a value that is not valid ({value:?})")),
|
||||
}
|
||||
}
|
||||
@@ -125,6 +125,22 @@ fn a_popup_default_of_no_item_takes_the_first_item() {
|
||||
assert!(losses(&r, LossCategory::DamagedData, "Some Fx").is_empty());
|
||||
}
|
||||
|
||||
/// A popup past the 1024 items kept: the cut is reported, and a selection among the cut items
|
||||
/// takes the first item and is reported as such, not as damaged data.
|
||||
#[test]
|
||||
fn a_popup_past_the_items_kept_is_reported() {
|
||||
let items: &'static [&'static str] = Box::leak((0..1100).map(|i| &*Box::leak(format!("I{i}").into_boxed_str())).collect::<Vec<_>>().into_boxed_slice());
|
||||
let fx = Fx { match_name: "Long Fx", name: "Long", defs: vec![popup("Pick", 1, items)], values: vec![(1, 1050.0)], data: vec![], keys: vec![] };
|
||||
let (g, r) = placeholder_of(&fx);
|
||||
let pick = g.prop("Long Fx-0001").unwrap();
|
||||
assert_eq!(pick.value, Value::Enum(0));
|
||||
assert!(matches!(&pick.ui, ParamUi::Popup { options } if options.len() == 1024));
|
||||
let found = losses(&r, LossCategory::Effects, "Long Fx");
|
||||
assert!(found.iter().any(|d| d.contains("past the first 1024")), "{found:?}");
|
||||
assert!(found.iter().any(|d| d.contains("item 1050 is past the items kept")), "{found:?}");
|
||||
assert!(losses(&r, LossCategory::DamagedData, "Long Fx").is_empty());
|
||||
}
|
||||
|
||||
/// Many large popups stay within the import's budget: what is beyond is reported, not kept.
|
||||
#[test]
|
||||
fn a_hostile_placeholder_stays_bounded() {
|
||||
|
||||
Reference in new issue
Block a user