Merge pull request #863 from itrycodinganditsbad/fix-748

Save the active comp and open comp tabs in project files (#748)

crates/engine/src/commands/app_more.rs

@@ -128,7 +128,7 @@ fn clear_recent_presets(s: &mut Session, _: &Value) -> Result<Value> {
 
 fn save_copy_xml(s: &mut Session, p: &Value) -> Result<Value> {
     let path = str_p(p, "path").ok_or_else(|| bad("file.saveCopyAsXml", "missing `path` (.ecprojx)"))?;
-    let xml = crate::xml_project::to_xml(&s.project.to_file_json()?).map_err(EngineError::Other)?;
+    let xml = crate::xml_project::to_xml(&s.project.to_file_json_with_view(&s.saved_view())?).map_err(EngineError::Other)?;
     s.services.write_file(path, xml.as_bytes()).map_err(|e| EngineError::Other(format!("cannot write {path}: {e}")))?;
     s.toast(format!("Saved an XML copy to {path}"));
     Ok(json!({"path": path, "bytes": xml.len()}))

crates/engine/src/commands/file.rs

@@ -47,6 +47,7 @@ pub(crate) fn open(s: &mut Session, p: &Value) -> Result<Value> {
         Project::from_json_repaired(&text).map_err(|e| EngineError::Other(format!("{path} is not a project EffectCraft can open: {e}")))?;
     resolve_relative_footage(&mut proj, path, s.services.as_ref());
     s.replace_project(proj, Some(path.to_string()));
+    s.restore_view(&effectcraft_project::saved_view(&text));
     s.note_project_path(path);
     // Values past the limits (a corrupt or hostile file) were brought within them (#788).
     if !repaired.is_empty() {
@@ -97,7 +98,7 @@ fn resolve_relative_footage(p: &mut Project, project_path: &str, services: &dyn
 /// project opened from an XML copy saves as XML again). Fails, writing nothing, when the project
 /// wouldn't read back.
 pub(crate) fn file_text(s: &Session, path: &str) -> Result<String> {
-    let json = s.project.to_file_json()?;
+    let json = s.project.to_file_json_with_view(&s.saved_view())?;
     if crate::xml_project::is_xml_path(path) { crate::xml_project::to_xml(&json).map_err(EngineError::Other) } else { Ok(json) }
 }
 

crates/engine/src/lib.rs

@@ -859,6 +859,26 @@ impl Session {
         self.events.push(Event::OpenComp(id));
     }
 
+    /// The open compositions, as a project file records them ([`effectcraft_project::SavedView`]).
+    pub fn saved_view(&self) -> effectcraft_project::SavedView {
+        effectcraft_project::SavedView { active_comp: self.state.active_comp, open_comps: self.state.open_comps.clone() }
+    }
+
+    /// Reopen the compositions a project file was saved with (#748). Ids that no longer name a
+    /// comp are skipped; with none left the project stays on the comp [`Session::replace_project`]
+    /// opened (its first), as files from before the view was saved do.
+    pub fn restore_view(&mut self, view: &effectcraft_project::SavedView) {
+        let mut open: Vec<ItemId> = Vec::new();
+        for id in view.open_comps.iter().copied().filter(|id| self.project.comp(*id).is_some()) {
+            if !open.contains(&id) {
+                open.push(id);
+            }
+        }
+        let Some(active) = view.active_comp.filter(|id| self.project.comp(*id).is_some()).or_else(|| open.first().copied()) else { return };
+        self.state.open_comps = open;
+        self.open_comp(active);
+    }
+
     pub fn toast(&mut self, msg: impl Into<String>) {
         self.events.push(Event::Toast { message: msg.into(), error: false });
     }

crates/engine/src/tests.rs

@@ -628,3 +628,61 @@ fn new_text_rejects_malformed_positions_without_creating_a_layer() {
     s.execute("layer.newText", json!({"text": "No explicit position"})).unwrap();
     assert_eq!(s.active_comp().unwrap().layers.len(), 2);
 }
+
+/// The active comp and the open comp tabs are saved with the project and restored on open (#748).
+#[test]
+fn reopening_a_project_restores_its_active_comp() {
+    use effectcraft_project::ItemId;
+    let dir = std::env::temp_dir().join(format!("ec-test-748-{}", std::process::id()));
+    std::fs::create_dir_all(&dir).unwrap();
+    let path = dir.join("view.ecproj").to_string_lossy().to_string();
+    let mut s = Session::default();
+    let a = s.execute("comp.new", json!({"name": "CompA"})).unwrap()["comp"].as_u64().unwrap();
+    let b = s.execute("comp.new", json!({"name": "CompB"})).unwrap()["comp"].as_u64().unwrap();
+    let c = s.execute("comp.new", json!({"name": "CompC"})).unwrap()["comp"].as_u64().unwrap();
+    s.execute("comp.open", json!({"comp": "CompB"})).unwrap();
+    s.state.open_comps.retain(|id| id.0 != c);
+    s.execute("file.saveAs", json!({"path": path})).unwrap();
+    let mut s2 = Session::default();
+    s2.execute("file.open", json!({"path": path})).unwrap();
+    assert_eq!(s2.state.active_comp, Some(ItemId(b)));
+    assert_eq!(s2.state.open_comps, vec![ItemId(a), ItemId(b)]);
+    assert!(!s2.is_dirty());
+
+    // A file from before the view was saved opens on its first comp, as it always did.
+    let text = std::fs::read_to_string(&path).unwrap();
+    let mut old: serde_json::Value = serde_json::from_str(&text).unwrap();
+    let obj = old.as_object_mut().unwrap();
+    obj.remove("activeComp");
+    obj.remove("openComps");
+    std::fs::write(&path, serde_json::to_string_pretty(&old).unwrap()).unwrap();
+    let mut s3 = Session::default();
+    s3.execute("file.open", json!({"path": path})).unwrap();
+    assert_eq!((s3.state.active_comp, s3.state.open_comps.clone()), (Some(ItemId(a)), vec![ItemId(a)]));
+
+    // Ids that no longer name a comp (edited by hand, or written by another tool) fall back.
+    obj_set(&mut old, json!(999), json!([999, b, "x"]));
+    std::fs::write(&path, serde_json::to_string_pretty(&old).unwrap()).unwrap();
+    let mut s4 = Session::default();
+    s4.execute("file.open", json!({"path": path})).unwrap();
+    assert_eq!((s4.state.active_comp, s4.state.open_comps.clone()), (Some(ItemId(b)), vec![ItemId(b)]));
+    obj_set(&mut old, json!(999), json!([]));
+    std::fs::write(&path, serde_json::to_string_pretty(&old).unwrap()).unwrap();
+    let mut s5 = Session::default();
+    s5.execute("file.open", json!({"path": path})).unwrap();
+    assert_eq!((s5.state.active_comp, s5.state.open_comps.clone()), (Some(ItemId(a)), vec![ItemId(a)]));
+
+    // XML copies carry it too.
+    let xml = dir.join("view.ecprojx").to_string_lossy().to_string();
+    s.execute("file.saveAs", json!({"path": xml})).unwrap();
+    let mut s6 = Session::default();
+    s6.execute("file.open", json!({"path": xml})).unwrap();
+    assert_eq!(s6.state.active_comp, Some(ItemId(b)));
+    let _ = std::fs::remove_dir_all(dir);
+
+    fn obj_set(v: &mut serde_json::Value, active: serde_json::Value, open: serde_json::Value) {
+        let o = v.as_object_mut().unwrap();
+        o.insert("activeComp".into(), active);
+        o.insert("openComps".into(), open);
+    }
+}

crates/project/src/lib.rs

@@ -49,6 +49,42 @@ pub fn saved_by(text: &str) -> Option<String> {
     serde_json::from_str::<Meta>(text).ok()?.saved_by
 }
 
+/// The compositions that were open when a project file was saved (`activeComp` and `openComps`
+/// next to `savedBy`), so it reopens where it was left (#748). Editor state rather than project
+/// content: opening a comp doesn't make the project unsaved, and it isn't part of
+/// [`Project`]. Older files have none; older versions ignore it.
+#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct SavedView {
+    /// The comp shown in the viewer and the Timeline.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub active_comp: Option<ItemId>,
+    /// The open comps (Timeline tabs), in tab order.
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    pub open_comps: Vec<ItemId>,
+}
+
+/// The view a project file was saved with ([`SavedView`]); empty for older files. Values that
+/// aren't ids are ignored (whether an id still names a comp is up to the caller).
+pub fn saved_view(text: &str) -> SavedView {
+    #[derive(Deserialize)]
+    struct Meta {
+        #[serde(rename = "activeComp", default)]
+        active_comp: Option<serde_json::Value>,
+        #[serde(rename = "openComps", default)]
+        open_comps: Option<serde_json::Value>,
+    }
+    let Ok(m) = serde_json::from_str::<Meta>(text) else { return SavedView::default() };
+    SavedView {
+        active_comp: m.active_comp.as_ref().and_then(serde_json::Value::as_u64).map(ItemId),
+        open_comps: m
+            .open_comps
+            .as_ref()
+            .and_then(serde_json::Value::as_array)
+            .map_or_else(Vec::new, |a| a.iter().filter_map(serde_json::Value::as_u64).map(ItemId).collect()),
+    }
+}
+
 /// Whether version `a` (`major.minor.patch`, pre-release suffixes ignored) is newer than `b`.
 pub fn is_newer_version(a: &str, b: &str) -> bool {
     let parts = |v: &str| -> [u64; 3] {
@@ -1318,14 +1354,20 @@ impl Project {
     /// to read back, so a save never writes a file that can't be opened (JSON has no NaN or
     /// infinity: such a value would be written as `null`).
     pub fn to_file_json(&self) -> Result<String, ProjectError> {
+        self.to_file_json_with_view(&SavedView::default())
+    }
+    /// [`Project::to_file_json`] that also records the open compositions ([`SavedView`]).
+    pub fn to_file_json_with_view(&self, view: &SavedView) -> Result<String, ProjectError> {
         #[derive(Serialize)]
         struct File<'a> {
             #[serde(rename = "savedBy")]
             saved_by: &'a str,
             #[serde(flatten)]
+            view: &'a SavedView,
+            #[serde(flatten)]
             project: &'a Project,
         }
-        let text = serde_json::to_string_pretty(&File { saved_by: APP_VERSION, project: self })
+        let text = serde_json::to_string_pretty(&File { saved_by: APP_VERSION, view, project: self })
             .map_err(|e| ProjectError::Invalid(format!("the project can't be saved: {e}")))?;
         Project::from_json(&text)
             .map_err(|e| ProjectError::Invalid(format!("the project can't be saved: it holds a value a project file can't store ({e})")))?;

crates/project/src/tests.rs

@@ -121,3 +121,16 @@ fn project_files_name_their_version_and_always_read_back() {
     assert!(crate::is_newer_version("0.2.0", "0.1.9") && crate::is_newer_version("1.0.0-rc.1", "0.9.9"));
     assert!(!crate::is_newer_version("0.1.1", "0.1.1") && !crate::is_newer_version("0.1.0", "0.1.1"));
 }
+
+#[test]
+fn project_files_remember_the_open_comps() {
+    let (p, cid) = project_with_layer();
+    let view = crate::SavedView { active_comp: Some(cid), open_comps: vec![cid] };
+    let text = p.to_file_json_with_view(&view).unwrap();
+    assert_eq!(crate::saved_view(&text), view);
+    assert_eq!(Project::from_json(&text).unwrap(), p, "the view is not part of the project");
+    // Files from before #748 have no view; a garbled one is ignored, not an error.
+    assert_eq!(crate::saved_view(&p.to_file_json().unwrap()), crate::SavedView::default());
+    let odd = text.replacen(&format!("\"activeComp\": {}", cid.0), "\"activeComp\": \"Main\"", 1);
+    assert_eq!(crate::saved_view(&odd), crate::SavedView { active_comp: None, open_comps: vec![cid] });
+}

docs/architecture.md

@@ -95,7 +95,10 @@ XML). Saving goes through `Project::to_file_json`, which checks that the text re
 anything is written (JSON can't hold NaN or infinity), and files are written atomically. Opening
 a file that a newer EffectCraft saved shows a warning: fields this version doesn't know are
 dropped if it saves the project. Save, Save As, Save a Copy and Increment and Save keep the
-file's format (`.ecprojx` stays XML).
+file's format (`.ecprojx` stays XML). Next to `savedBy` the file records the active comp and the
+open comp tabs (`activeComp`, `openComps`), which opening restores; they are editor state, not
+part of `Project`, so older files (and ids that no longer name a comp) open on the first comp and
+older versions ignore them.
 
 `schema` changes when a saved value changes meaning; a project from an older schema is converted
 when it is opened (`effectcraft_engine::upgrade_effects`, which also refreshes effect instances