Repository: LiveSplit/livesplit-core (rust)

Issue:
Implement a history fixing routine for unattached segment times
When you have segment time history elements that don't correspond to attempt history elements, through possibly manual modification of the split file, then the original LiveSplit just outright crashes in various ways, while livesplit-core seems to just overwrite the existing segment history elements when doing new attempts. Both seem suboptimal. There should probably be a fixing step that modifies the attempt index of the segment history elements with a new negative index.

This should be backported to the original LiveSplit.

https://splits.io/4of2


Pull request description:
Reattach unattached Segment History Elements
In `Run::fix_splits` we now check for segment history elements that are considered achieved by the runner, but don't correspond to an element in the attempt history. We move them over into the synthetic segment history elements (elements with index <= 0).

Fixes #249 

Patch:
diff --git a/src/run/mod.rs b/src/run/mod.rs
index c855771..c9682d6 100644
--- a/src/run/mod.rs
+++ b/src/run/mod.rs
@@ -583,6 +583,7 @@ impl Run {
         }
         self.remove_duplicates();
         self.remove_none_values();
+        self.reattach_unattached_segment_history_elements();
     }
 
     /// Clears out the Attempt History and the Segment Histories of all the segments.
@@ -809,6 +810,30 @@ impl Run {
             Ok(())
         }
     }
+
+    fn reattach_unattached_segment_history_elements(&mut self) {
+        let max_id = self.max_attempt_history_index().unwrap_or_default();
+        let mut min_id = self.min_segment_history_index().unwrap_or_default();
+
+        while let Some(unattached_id) = self
+            .segments
+            .iter()
+            .filter_map(|s| s.segment_history().try_get_max_index())
+            .filter(|&i| i > max_id)
+            .max()
+        {
+            let reassign_id = min_id - 1;
+
+            for segment in self.segments_mut() {
+                let history = segment.segment_history_mut();
+                if let Some(time) = history.remove(unattached_id) {
+                    history.insert(reassign_id, time);
+                }
+            }
+
+            min_id = reassign_id;
+        }
+    }
 }
 
 impl Default for Run {
diff --git a/src/run/segment_history.rs b/src/run/segment_history.rs
index cdaf086..0e14a2d 100644
--- a/src/run/segment_history.rs
+++ b/src/run/segment_history.rs
@@ -18,8 +18,9 @@ impl SegmentHistory {
         Some(self.0.first()?.0)
     }
 
-    /// Returns the minimum index of all the segment times. If there's no
-    /// segment times or the minimum is less than 1, 1 is returned instead.
+    /// Returns the minimum index of all the segment times. If there are no
+    /// segment times or there are only indices above 1, then 1 is returned
+    /// instead.
     pub fn min_index(&self) -> i32 {
         self.try_get_min_index().map_or(1, |m| min(m, 1))
     }
@@ -65,10 +66,9 @@ impl SegmentHistory {
     /// Removes the segment time with the given index. If it doesn't exist,
     /// nothing is done.
     #[inline]
-    pub fn remove(&mut self, index: i32) {
-        if let Ok(pos) = self.get_pos(index) {
-            self.0.remove(pos);
-        }
+    pub fn remove(&mut self, index: i32) -> Option<Time> {
+        let pos = self.get_pos(index).ok()?;
+        Some(self.0.remove(pos).1)
     }
 
     /// Removes all the segment times from the Segment History.


Test patch:
diff --git a/src/run/tests/fixing.rs b/src/run/tests/fixing.rs
new file mode 100644
index 0000000..1e00197
--- /dev/null
+++ b/src/run/tests/fixing.rs
@@ -0,0 +1,23 @@
+use crate::tests_helper::{create_timer, run_with_splits};
+
+#[test]
+fn reattaches_unattached_segment_history_elements_by_using_negative_ids() {
+    let mut timer = create_timer(&["A", "B"]);
+    run_with_splits(&mut timer, &[3.0, 6.0]);
+    run_with_splits(&mut timer, &[2.0, 4.0]);
+    let mut run = timer.into_run(true);
+
+    // We pop the last attempt from the history, but keep it in the segment
+    // history, which makes the segment history elements unattached.
+    run.attempt_history.pop().unwrap();
+
+    run.fix_splits();
+
+    let segments = run.segments();
+
+    assert_eq!(segments[0].segment_history().try_get_min_index(), Some(0));
+    assert_eq!(segments[0].segment_history().try_get_max_index(), Some(1));
+
+    assert_eq!(segments[1].segment_history().try_get_min_index(), Some(0));
+    assert_eq!(segments[1].segment_history().try_get_max_index(), Some(1));
+}
diff --git a/src/run/tests/mod.rs b/src/run/tests/mod.rs
index 7881900..327d2a1 100644
--- a/src/run/tests/mod.rs
+++ b/src/run/tests/mod.rs
@@ -1,3 +1,4 @@
 mod comparison;
 mod empty_run;
+mod fixing;
 mod metadata;