Repository: LiveSplit/livesplit-core
Pull request #448 (2021-09-30 20:10:23)

Use `SegmentTime` Formatter for Segment Time Columns
When we introduced `Possible Time Save` columns we also added its formatter. This formatter is the `SegmentTime` formatter. However the actual column types that show various kinds of segment times still kept using the `General` formatter that we use for split times. This changes all of them to use the `SegmentTime` formatter.


Patch:
diff --git a/src/component/splits/column.rs b/src/component/splits/column.rs
index ba81f39..3e35d85 100644
--- a/src/component/splits/column.rs
+++ b/src/component/splits/column.rs
@@ -138,7 +138,7 @@ impl Clear for ColumnState {
 enum ColumnFormatter {
     Time,
     Delta,
-    PossibleTimeSave,
+    SegmentTime,
 }
 
 pub fn update_state(
@@ -184,12 +184,12 @@ pub fn update_state(
                         method,
                     ),
                     SemanticColor::Default,
-                    ColumnFormatter::Time,
+                    ColumnFormatter::SegmentTime,
                 ),
                 ColumnStartWith::PossibleTimeSave => (
                     possible_time_save::calculate(timer, segment_index, comparison, false),
                     SemanticColor::Default,
-                    ColumnFormatter::PossibleTimeSave,
+                    ColumnFormatter::SegmentTime,
                 ),
             },
             false,
@@ -209,7 +209,7 @@ pub fn update_state(
                 "{}",
                 Delta::with_decimal_dropping().format(column_value)
             ),
-            ColumnFormatter::PossibleTimeSave => {
+            ColumnFormatter::SegmentTime => {
                 write!(state.value, "{}", SegmentTime::new().format(column_value))
             }
         };
@@ -302,12 +302,12 @@ fn column_update_value(
         (SegmentTime, false) => (
             analysis::previous_segment_time(timer, segment_index, method),
             SemanticColor::Default,
-            ColumnFormatter::Time,
+            ColumnFormatter::SegmentTime,
         ),
         (SegmentTime, true) => (
             analysis::live_segment_time(timer, segment_index, method),
             SemanticColor::Default,
-            ColumnFormatter::Time,
+            ColumnFormatter::SegmentTime,
         ),
 
         (SegmentDelta | SegmentDeltaWithFallback, false) => {
@@ -315,7 +315,7 @@ fn column_update_value(
             let (value, formatter) = if delta.is_none() && column.update_with.has_fallback() {
                 (
                     analysis::previous_segment_time(timer, segment_index, method),
-                    ColumnFormatter::Time,
+                    ColumnFormatter::SegmentTime,
                 )
             } else {
                 (delta, ColumnFormatter::Delta)


Test patch:
diff --git a/src/component/splits/tests/column.rs b/src/component/splits/tests/column.rs
index 8f750a1..36d5c10 100644
--- a/src/component/splits/tests/column.rs
+++ b/src/component/splits/tests/column.rs
@@ -2,12 +2,14 @@ use super::{
     ColumnSettings, ColumnStartWith, ColumnUpdateTrigger, ColumnUpdateWith, Component, Settings,
     State,
 };
-use crate::settings::SemanticColor::{
-    self, AheadGainingTime as AheadGaining, BehindLosingTime as BehindLosing, BestSegment as Best,
-    Default as Text,
+use crate::{
+    settings::SemanticColor::{
+        self, AheadGainingTime as AheadGaining, BehindLosingTime as BehindLosing,
+        BestSegment as Best, Default as Text,
+    },
+    tests_helper::{make_progress_run_with_splits_opt, run_with_splits_opt, start_run},
+    Run, Segment, TimeSpan, Timer,
 };
-use crate::tests_helper::{make_progress_run_with_splits_opt, run_with_splits_opt, start_run};
-use crate::{Run, Segment, TimeSpan, Timer};
 
 type Values = &'static [([&'static str; 6], [SemanticColor; 6])];
 
@@ -28,6 +30,7 @@ fn timer() -> Timer {
     Timer::new(run()).unwrap()
 }
 
+#[track_caller]
 fn check_column_state(state: &State, state_index: usize, expected_values: Values) {
     let actual_values = state
         .splits
@@ -182,27 +185,27 @@ fn column_empty_segment_time() {
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:08", "", "", "", "", ""],
+                ["8.50", "", "", "", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:08", "—", "", "", "", ""],
+                ["8.50", "—", "", "", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:08", "—", "0:01", "", "", ""],
+                ["8.50", "—", "1.50", "", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:08", "—", "0:01", "0:07", "", ""],
+                ["8.50", "—", "1.50", "7.50", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:08", "—", "0:01", "0:07", "—", ""],
+                ["8.50", "—", "1.50", "7.50", "—", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:08", "—", "0:01", "0:07", "—", "0:07"],
+                ["8.50", "—", "1.50", "7.50", "—", "7.50"],
                 [Text, Text, Text, Text, Text, Text],
             ),
         ],
@@ -254,31 +257,31 @@ fn column_comparison_segment_time_segment_time() {
         ColumnUpdateWith::SegmentTime,
         &[
             (
-                ["0:05", "—", "—", "0:10", "0:05", "1:05"],
+                ["5.00", "—", "—", "10.00", "5.00", "1:05.00"],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:08", "—", "—", "0:10", "0:05", "1:05"],
+                ["8.50", "—", "—", "10.00", "5.00", "1:05.00"],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:08", "—", "—", "0:10", "0:05", "1:05"],
+                ["8.50", "—", "—", "10.00", "5.00", "1:05.00"],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:08", "—", "0:01", "0:10", "0:05", "1:05"],
+                ["8.50", "—", "1.50", "10.00", "5.00", "1:05.00"],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:08", "—", "0:01", "0:07", "0:05", "1:05"],
+                ["8.50", "—", "1.50", "7.50", "5.00", "1:05.00"],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:08", "—", "0:01", "0:07", "—", "1:05"],
+                ["8.50", "—", "1.50", "7.50", "—", "1:05.00"],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:08", "—", "0:01", "0:07", "—", "0:07"],
+                ["8.50", "—", "1.50", "7.50", "—", "7.50"],
                 [Text, Text, Text, Text, Text, Text],
             ),
         ],
@@ -340,15 +343,15 @@ fn column_comparison_segment_time_segment_delta() {
         ColumnUpdateWith::SegmentDelta,
         &[
             (
-                ["0:05", "—", "—", "0:10", "0:05", "1:05"],
+                ["5.00", "—", "—", "10.00", "5.00", "1:05.00"],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["+3.5", "—", "—", "0:10", "0:05", "1:05"],
+                ["+3.5", "—", "—", "10.00", "5.00", "1:05.00"],
                 [BehindLosing, Text, Text, Text, Text, Text],
             ),
             (
-                ["+3.5", "—", "—", "0:10", "0:05", "1:05"],
+                ["+3.5", "—", "—", "10.00", "5.00", "1:05.00"],
                 [BehindLosing, Text, Text, Text, Text, Text],
             ),
             (
@@ -356,7 +359,7 @@ fn column_comparison_segment_time_segment_delta() {
                 // this column type if the comparison segment time was missing.
                 // Instead, we show a dash for the third segment rather than
                 // showing the segment time.
-                ["+3.5", "—", "—", "0:10", "0:05", "1:05"],
+                ["+3.5", "—", "—", "10.00", "5.00", "1:05.00"],
                 [BehindLosing, Text, Best, Text, Text, Text],
             ),
             (
@@ -366,11 +369,11 @@ fn column_comparison_segment_time_segment_delta() {
                 // combined segment time of 10 seconds. The third segment is
                 // empty in the best segments comparison because it is not a
                 // part of the shortest path in the Sum of Best calculation.
-                ["+3.5", "—", "—", "−1.0", "0:05", "1:05"],
+                ["+3.5", "—", "—", "−1.0", "5.00", "1:05.00"],
                 [BehindLosing, Text, Best, Best, Text, Text],
             ),
             (
-                ["+3.5", "—", "—", "−1.0", "—", "1:05"],
+                ["+3.5", "—", "—", "−1.0", "—", "1:05.00"],
                 [BehindLosing, Text, Best, Best, Text, Text],
             ),
             (
@@ -449,12 +452,13 @@ fn column_comparison_segment_time_dont_update() {
         ColumnStartWith::ComparisonSegmentTime,
         ColumnUpdateWith::DontUpdate,
         &[(
-            ["0:05", "—", "—", "0:10", "0:05", "1:05"],
+            ["5.00", "—", "—", "10.00", "5.00", "1:05.00"],
             [Text, Text, Text, Text, Text, Text],
         ); 7],
     )
 }
 
+#[track_caller]
 fn check_columns(
     start_with: ColumnStartWith,
     update_with: ColumnUpdateWith,
@@ -495,41 +499,41 @@ fn check_columns(
     });
 
     let state = component.state(&timer.snapshot(), &layout_settings);
-    check_column_state(&state, 0, &expected_values);
+    check_column_state(&state, 0, expected_values);
 
     timer.set_game_time(TimeSpan::from_seconds(8.5));
     timer.split();
 
     let state = component.state(&timer.snapshot(), &layout_settings);
-    check_column_state(&state, 1, &expected_values);
+    check_column_state(&state, 1, expected_values);
 
     timer.skip_split();
 
     let state = component.state(&timer.snapshot(), &layout_settings);
-    check_column_state(&state, 2, &expected_values);
+    check_column_state(&state, 2, expected_values);
 
     timer.set_game_time(TimeSpan::from_seconds(10.0));
     timer.split();
 
     let state = component.state(&timer.snapshot(), &layout_settings);
-    check_column_state(&state, 3, &expected_values);
+    check_column_state(&state, 3, expected_values);
 
     timer.set_game_time(TimeSpan::from_seconds(17.5));
     timer.split();
 
     let state = component.state(&timer.snapshot(), &layout_settings);
-    check_column_state(&state, 4, &expected_values);
+    check_column_state(&state, 4, expected_values);
 
     timer.skip_split();
 
     let state = component.state(&timer.snapshot(), &layout_settings);
-    check_column_state(&state, 5, &expected_values);
+    check_column_state(&state, 5, expected_values);
 
     timer.set_game_time(TimeSpan::from_seconds(25.0));
     timer.split();
 
     let state = component.state(&timer.snapshot(), &layout_settings);
-    check_column_state(&state, 6, &expected_values);
+    check_column_state(&state, 6, expected_values);
 }
 
 #[test]
@@ -681,19 +685,19 @@ fn column_segment_time_update_on_ending_segment() {
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:05", "—", "", "", "", ""],
+                ["5.50", "—", "", "", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:05", "—", "", "", "", ""],
+                ["5.50", "—", "", "", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:05", "—", "0:12", "—", "", ""],
+                ["5.50", "—", "12.50", "—", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:05", "—", "0:12", "—", "", ""],
+                ["5.50", "—", "12.50", "—", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
         ],
@@ -841,27 +845,27 @@ fn column_segment_time_update_contextual() {
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:03", "", "", "", "", ""],
+                ["3.50", "", "", "", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:05", "", "", "", "", ""],
+                ["5.50", "", "", "", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:05", "—", "", "", "", ""],
+                ["5.50", "—", "", "", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:05", "—", "0:07", "", "", ""],
+                ["5.50", "—", "7.50", "", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:05", "—", "0:12", "—", "", ""],
+                ["5.50", "—", "12.50", "—", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:05", "—", "0:12", "—", "", ""],
+                ["5.50", "—", "12.50", "—", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
         ],
@@ -1001,41 +1005,42 @@ fn column_segment_time_update_on_starting_segment() {
         ColumnUpdateTrigger::OnStartingSegment,
         &[
             (
-                ["0:00", "", "", "", "", ""],
+                ["0.00", "", "", "", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:02", "", "", "", "", ""],
+                ["2.00", "", "", "", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:03", "", "", "", "", ""],
+                ["3.50", "", "", "", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:05", "", "", "", "", ""],
+                ["5.50", "", "", "", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:05", "—", "0:05", "", "", ""],
+                ["5.50", "—", "5.50", "", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:05", "—", "0:07", "", "", ""],
+                ["5.50", "—", "7.50", "", "", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:05", "—", "0:12", "—", "0:11", ""],
+                ["5.50", "—", "12.50", "—", "11.00", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
             (
-                ["0:05", "—", "0:12", "—", "0:13", ""],
+                ["5.50", "—", "12.50", "—", "13.00", ""],
                 [Text, Text, Text, Text, Text, Text],
             ),
         ],
     )
 }
 
+#[track_caller]
 fn check_columns_update_trigger(
     update_with: ColumnUpdateWith,
     update_trigger: ColumnUpdateTrigger,
@@ -1085,25 +1090,25 @@ fn check_columns_update_trigger(
 
     let state = component.state(&timer.snapshot(), &layout_settings);
     // Timer at 0, contextual shouldn't show live times yet
-    check_column_state(&state, 0, &expected_values);
+    check_column_state(&state, 0, expected_values);
 
     timer.set_game_time(TimeSpan::from_seconds(2.0));
 
     let state = component.state(&timer.snapshot(), &layout_settings);
     // Shorter than the best segment, contextual shouldn't show live times yet
-    check_column_state(&state, 1, &expected_values);
+    check_column_state(&state, 1, expected_values);
 
     timer.set_game_time(TimeSpan::from_seconds(3.5));
 
     let state = component.state(&timer.snapshot(), &layout_settings);
     // Longer than the best segment, contextual should show live times
-    check_column_state(&state, 2, &expected_values);
+    check_column_state(&state, 2, expected_values);
 
     timer.set_game_time(TimeSpan::from_seconds(5.5));
 
     let state = component.state(&timer.snapshot(), &layout_settings);
     // Behind the personal best, contextual should show live times
-    check_column_state(&state, 3, &expected_values);
+    check_column_state(&state, 3, expected_values);
 
     timer.split();
     timer.skip_split();
@@ -1112,13 +1117,13 @@ fn check_columns_update_trigger(
     let state = component.state(&timer.snapshot(), &layout_settings);
     // Shorter than the combined best segment, contextual shouldn't show live
     // times yet
-    check_column_state(&state, 4, &expected_values);
+    check_column_state(&state, 4, expected_values);
 
     timer.set_game_time(TimeSpan::from_seconds(13.0));
 
     let state = component.state(&timer.snapshot(), &layout_settings);
     // Longer than the combined best segment, contextual should show live times
-    check_column_state(&state, 5, &expected_values);
+    check_column_state(&state, 5, expected_values);
 
     timer.set_game_time(TimeSpan::from_seconds(18.0));
     timer.split();
@@ -1127,14 +1132,14 @@ fn check_columns_update_trigger(
 
     let state = component.state(&timer.snapshot(), &layout_settings);
     // Not behind the personal best, contextual should not show live times yet
-    check_column_state(&state, 6, &expected_values);
+    check_column_state(&state, 6, expected_values);
 
     timer.set_game_time(TimeSpan::from_seconds(31.0));
 
     let state = component.state(&timer.snapshot(), &layout_settings);
     // Behind the personal best, contextual should show live times only if a
     // split time or delta column
-    check_column_state(&state, 7, &expected_values);
+    check_column_state(&state, 7, expected_values);
 }
 
 #[test]
diff --git a/src/component/splits/tests/mod.rs b/src/component/splits/tests/mod.rs
index 708066a..9a8e5c5 100644
--- a/src/component/splits/tests/mod.rs
+++ b/src/component/splits/tests/mod.rs
@@ -102,7 +102,7 @@ fn negative_segment_times() {
     timer.set_game_time(TimeSpan::from_seconds(-1.0));
 
     let state = component.state(&timer.snapshot(), &layout_settings);
-    assert_eq!(state.splits[0].columns[0].value, "−0:01");
+    assert_eq!(state.splits[0].columns[0].value, "−1.00");
 }
 
 #[test]