Merge pull request #2107 from gmrmad1/fix/2073-replace-footage-kind

engine: Replace Footage keeps a still a still (#2073)

crates/engine/src/cmd/file_more.rs

@@ -878,10 +878,34 @@ fn replace_footage(s: &mut Session, p: &Value) -> Result<Value> {
     let path = str_p(p, "path").ok_or_else(|| bad("file.replaceFootage", "missing `path`"))?.to_string();
     let item = *items_p(s, p).first().ok_or_else(|| bad("file.replaceFootage", "select footage"))?;
     let importer = s.importer.clone().ok_or_else(|| EngineError::Other("media import is not available in this build".into()))?;
-    // A numbered still brings its image sequence, as on import (`sequence: false` for one frame).
-    let opts = SequenceOptions::from_params(p);
+    // Replacing a still keeps it a still; replacing a sequence keeps bringing the new file's run.
+    // `sequence`/`alphabetical` override that (#2073).
+    let previous = match s.project.item(item).map(|it| &it.kind) {
+        Some(ItemKind::Footage(f)) if f.kind == FootageKind::Sequence => (true, true, f.alphabetical),
+        _ => (false, false, false),
+    };
+    // Only `sequence` decides whether the item keeps its kind; `alphabetical` is a sequence
+    // modifier, not a request to make one.
+    let inherited = b_p(p, "sequence").is_none();
+    let mut opts = SequenceOptions::from_params(p);
+    if inherited {
+        (opts.sequence, opts.folders) = (previous.0, previous.1);
+        if previous.0 && b_p(p, "alphabetical").is_none() {
+            opts.alphabetical = previous.2;
+        }
+    }
     let src = crate::sequence::source_of(s.services.as_ref(), &path, opts);
-    let f = crate::sequence::probe(importer.as_ref(), &src, s.sequence_rate()).map_err(|e| EngineError::Other(format!("{path}: {e}")))?;
+    let mut f = crate::sequence::probe(importer.as_ref(), &src, s.sequence_rate()).map_err(|e| EngineError::Other(format!("{path}: {e}")))?;
+    // A single-file source comes back as a still; an inherited sequence kind keeps it a
+    // one-frame sequence, so a later default replacement still brings numbered runs (#2073).
+    if inherited && previous.0 && matches!(src, crate::sequence::Source::File(_)) && f.kind == FootageKind::Still {
+        f.kind = FootageKind::Sequence;
+        f.sequence = vec![f.path.clone()];
+        f.alphabetical = opts.alphabetical;
+        f.frame_rate = s.sequence_rate();
+        f.native_rate = None;
+        f.sync_sequence_duration();
+    }
     let name = src.name();
     // Replacing a missing file relinks the other missing files that moved with it (as After Effects
     // does when you locate one).
@@ -1093,7 +1117,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "File...",
             ["File", "Replace Footage"],
             Some("Cmd+H"),
-            "{path, item?, sequence?: bool (default true: a numbered still brings its image sequence), alphabetical?: bool}",
+            "{path, item?, sequence?: bool (a numbered still brings its image sequence; default: the item keeps its kind, so a still stays a still and a sequence stays a sequence), alphabetical?: bool}",
             has_footage_selection,
             replace_footage
         ),

crates/engine/src/tests/sequence.rs

@@ -182,14 +182,58 @@ fn interpret_footage_start_timecode_relabels_source_time() {
 }
 
 #[test]
-fn replace_footage_and_reload_keep_sequences() {
+fn replace_footage_keeps_a_still_a_still() {
     let dir = folder(&RUN);
     let mut s = session();
     let item = s.execute("file.import", json!({"paths": [at(&dir, "shot_0001.png")], "sequence": false})).unwrap()["items"][0].clone();
     s.state.project_selection = vec![ItemId(item.as_u64().unwrap())];
+    // Replacing with a numbered still does not silently turn the item into a sequence (#2073).
     s.execute("file.replaceFootage", json!({"path": at(&dir, "shot_0004.png")})).unwrap();
     let (name, f) = footage(&s, &item);
-    assert_eq!((name.as_str(), f.sequence.len()), ("shot_[0001-0005].png", 4));
+    assert_eq!((name.as_str(), f.kind), ("shot_0004.png", FootageKind::Still));
+    // JSON null is unspecified, not "on": the still stays a still.
+    s.execute("file.replaceFootage", json!({"path": at(&dir, "shot_0004.png"), "sequence": null})).unwrap();
+    let (name, f) = footage(&s, &item);
+    assert_eq!((name.as_str(), f.kind), ("shot_0004.png", FootageKind::Still));
+    // Alphabetical is a sequence modifier, not a request to make one.
+    s.execute("file.replaceFootage", json!({"path": at(&dir, "shot_0004.png"), "alphabetical": true})).unwrap();
+    let (name, f) = footage(&s, &item);
+    assert_eq!((name.as_str(), f.kind), ("shot_0004.png", FootageKind::Still));
+    // Asking for the run still brings it.
+    s.execute("file.replaceFootage", json!({"path": at(&dir, "shot_0004.png"), "sequence": true})).unwrap();
+    let (name, f) = footage(&s, &item);
+    assert_eq!((name.as_str(), f.kind, f.sequence.len()), ("shot_[0001-0005].png", FootageKind::Sequence, 4));
+}
+
+#[test]
+fn replace_footage_keeps_a_one_frame_run_a_sequence() {
+    let dir = folder(&["shot_0001.png", "shot_0002.png", "solo_0007.png", "cover.png"]);
+    let mut s = session();
+    let item = s.execute("file.import", json!({"paths": [at(&dir, "shot_0001.png")], "sequence": true})).unwrap()["items"][0].clone();
+    s.state.project_selection = vec![ItemId(item.as_u64().unwrap())];
+    // A numbered file with no other frame in its run is still the replacement for a sequence.
+    s.execute("file.replaceFootage", json!({"path": at(&dir, "solo_0007.png")})).unwrap();
+    let (name, f) = footage(&s, &item);
+    assert_eq!((name.as_str(), f.kind, f.sequence.len()), ("solo_0007.png", FootageKind::Sequence, 1));
+    // A single-file source keeps the sequence kind too.
+    s.execute("file.replaceFootage", json!({"path": at(&dir, "cover.png")})).unwrap();
+    let (name, f) = footage(&s, &item);
+    assert_eq!((name.as_str(), f.kind, f.sequence.len()), ("cover.png", FootageKind::Sequence, 1));
+    // An explicit `sequence: false` still asks for one frame.
+    s.execute("file.replaceFootage", json!({"path": at(&dir, "cover.png"), "sequence": false})).unwrap();
+    let (_, f) = footage(&s, &item);
+    assert_eq!((f.kind, f.sequence.len()), (FootageKind::Still, 0));
+}
+
+#[test]
+fn replace_footage_and_reload_keep_sequences() {
+    let dir = folder(&RUN);
+    let mut s = session();
+    let item = s.execute("file.import", json!({"paths": [at(&dir, "shot_0001.png")], "sequence": true})).unwrap()["items"][0].clone();
+    s.state.project_selection = vec![ItemId(item.as_u64().unwrap())];
+    s.execute("file.replaceFootage", json!({"path": at(&dir, "shot_0004.png")})).unwrap();
+    let (name, f) = footage(&s, &item);
+    assert_eq!((name.as_str(), f.kind, f.sequence.len()), ("shot_[0001-0005].png", FootageKind::Sequence, 4));
     // Reload finds a new frame and keeps the interpretation.
     s.execute("file.interpretFootage", json!({"items": [item], "frameRate": 12, "startTimecode": 2})).unwrap();
     std::fs::write(at(&dir, "shot_0003.png"), b"").unwrap();

docs/footage.md

@@ -129,9 +129,10 @@ including files overwritten at the same path and byte size.
   and 3D models are not sources, and a composition can't be nested in itself. Alt-dragging onto a
   layer is one layer at a time, as in After Effects.
   Agents: `layer.replaceSource {"item": id, "layers": [ids]}`.
-- **File ▸ Replace Footage ▸ File…** (Ctrl+H) points the item at another file. Pick a numbered
-  file to replace it with that file's whole sequence (`file.replaceFootage {"path": …,
-  "sequence": false}` for one frame).
+- **File ▸ Replace Footage ▸ File…** (Ctrl+H) points the item at another file. A still stays a
+  still, and a sequence item keeps bringing the new numbered file's run; pass
+  `sequence: true` to bring a numbered still's whole run (`file.replaceFootage {"path": …,
+  "sequence": true}`), or `sequence: false` for one frame.
 - **Replace with Precomposition (used N times)**, right-clicking a footage item or solid in the
   Project panel, puts the item in a new composition of its size,
   pixel aspect and frame rate, in the same folder, and switches every layer in every composition