From 943a5310a9419b16d57b92454ea50ece9f914adf Mon Sep 17 00:00:00 2001 From: Sergey Papushin Date: Fri, 19 Jan 2018 17:18:45 -0600 Subject: [PATCH 1/2] Fix small issues with the documentation --- src/component/blank_space.rs | 4 ++-- src/component/current_comparison.rs | 4 ++-- src/component/detailed_timer.rs | 8 ++++---- src/component/previous_segment.rs | 18 ++++++++---------- src/component/splits.rs | 8 ++++---- src/component/sum_of_best.rs | 4 ++-- src/component/title/mod.rs | 8 ++++---- src/layout/general_settings.rs | 4 ++-- src/run/editor/cleaning.rs | 11 ++++++----- src/run/editor/state.rs | 4 ++-- src/run/segment.rs | 2 +- src/settings/semantic_color.rs | 5 ++--- src/time/formatter/accuracy.rs | 4 ++-- 13 files changed, 41 insertions(+), 43 deletions(-) diff --git a/src/component/blank_space.rs b/src/component/blank_space.rs index a4ae1990..94d10438 100644 --- a/src/component/blank_space.rs +++ b/src/component/blank_space.rs @@ -1,5 +1,5 @@ //! Provides the Blank Space Component and relevant types for using it. The -//! Blank Space Component is simply an empty component, that doesn't show +//! Blank Space Component is simply an empty component that doesn't show //! anything other than a background. It mostly serves as padding between other //! components. @@ -9,7 +9,7 @@ use std::io::Write; use std::borrow::Cow; use settings::{Field, Gradient, SettingsDescription, Value}; -/// The Blank Space Component is simply an empty component, that doesn't show +/// The Blank Space Component is simply an empty component that doesn't show /// anything other than a background. It mostly serves as padding between other /// components. #[derive(Default, Clone)] diff --git a/src/component/current_comparison.rs b/src/component/current_comparison.rs index 3f83571a..00fbc8f9 100644 --- a/src/component/current_comparison.rs +++ b/src/component/current_comparison.rs @@ -1,6 +1,6 @@ //! Provides the Current Comparison Component and relevant types for using it. //! The Current Comparison Component is a component that shows the name of the -//! comparison that is currently selected for being compared against. +//! comparison that is currently selected to be compared against. use Timer; use serde_json::{to_writer, Result}; @@ -10,7 +10,7 @@ use settings::{Color, Field, Gradient, SettingsDescription, Value}; use super::DEFAULT_INFO_TEXT_GRADIENT; /// The Current Comparison Component is a component that shows the name of the -/// comparison that is currently selected for being compared against. +/// comparison that is currently selected to be compared against. #[derive(Default, Clone)] pub struct Component { settings: Settings, diff --git a/src/component/detailed_timer.rs b/src/component/detailed_timer.rs index 3fbd8c76..5e9d01ae 100644 --- a/src/component/detailed_timer.rs +++ b/src/component/detailed_timer.rs @@ -1,8 +1,8 @@ //! Provides the Detailed Timer Component and relevant types for using it. The //! Detailed Timer Component is a component that shows two timers, one for the //! total time of the current attempt and one showing the time of just the -//! current segment. Other information like segment times of up to two -//! comparisons, the segment icon and the segment's name can also be shown. +//! current segment. Other information, like segment times of up to two +//! comparisons, the segment icon, and the segment's name, can also be shown. use {GeneralLayoutSettings, TimeSpan, Timer, TimerPhase, TimingMethod}; use super::timer; @@ -16,8 +16,8 @@ use settings::{Field, Gradient, SemanticColor, SettingsDescription, Value}; /// The Detailed Timer Component is a component that shows two timers, one for /// the total time of the current attempt and one showing the time of just the -/// current segment. Other information like segment times of up to two -/// comparisons, the segment icon and the segment's name can also be shown. +/// current segment. Other information, like segment times of up to two +/// comparisons, the segment icon, and the segment's name, can also be shown. #[derive(Default, Clone)] pub struct Component { icon_id: usize, diff --git a/src/component/previous_segment.rs b/src/component/previous_segment.rs index 0b15ac84..acd17662 100644 --- a/src/component/previous_segment.rs +++ b/src/component/previous_segment.rs @@ -1,10 +1,9 @@ //! Provides the Previous Segment Component and relevant types for using it. The //! Previous Segment Component is a component that shows how much time was saved -//! or lost based in the last segment based on the chosen comparison. -//! Additionally the time save that would've been possible in the previous -//! segment can be displayed. This component switches to a `Live Segment` view -//! that shows active time loss, whenever the runner is losing time on the -//! current segment. +//! or lost during the previous segment based on the chosen comparison. +//! Additionally, the potential time save for the previous segment can be +//! displayed. This component switches to a `Live Segment` view that shows +//! active time loss whenever the runner is losing time on the current segment. use {analysis, comparison, GeneralLayoutSettings, Timer, TimerPhase}; use time::formatter::{Accuracy, Delta, PossibleTimeSave, TimeFormatter}; @@ -16,11 +15,10 @@ use settings::{Color, Field, Gradient, SemanticColor, SettingsDescription, Value use super::DEFAULT_INFO_TEXT_GRADIENT; /// The Previous Segment Component is a component that shows how much time was -/// saved or lost based in the last segment based on the chosen comparison. -/// Additionally the time save that would've been possible in the previous -/// segment can be displayed. This component switches to a `Live Segment` view -/// that shows active time loss, whenever the runner is losing time on the -/// current segment. +/// saved or lost during the previous segment based on the chosen comparison. +/// Additionally, the potential time save for the previous segment can be +/// displayed. This component switches to a `Live Segment` view that shows +/// active time loss whenever the runner is losing time on the current segment. #[derive(Default, Clone)] pub struct Component { settings: Settings, diff --git a/src/component/splits.rs b/src/component/splits.rs index f9ccedd1..dfac5e26 100644 --- a/src/component/splits.rs +++ b/src/component/splits.rs @@ -1,7 +1,7 @@ //! Provides the Splits Component and relevant types for using it. The Splits //! Component is the main component for visualizing all the split times. Each //! segment is shown in a tabular fashion showing the segment icon, segment -//! name, the delta compared to the chosen comparison and the split time. The +//! name, the delta compared to the chosen comparison, and the split time. The //! list provides scrolling functionality, so not every segment needs to be //! shown all the time. @@ -17,7 +17,7 @@ use settings::{Color, Field, Gradient, SemanticColor, SettingsDescription, Value /// The Splits Component is the main component for visualizing all the split /// times. Each segment is shown in a tabular fashion showing the segment icon, -/// segment name, the delta compared to the chosen comparison and the split +/// segment name, the delta compared to the chosen comparison, and the split /// time. The list provides scrolling functionality, so not every segment needs /// to be shown all the time. #[derive(Default, Clone)] @@ -39,8 +39,8 @@ pub struct Settings { pub visual_split_count: usize, /// If there's more segments than segments that are shown, the window /// showing the segments automatically scrolls up and down when the current - /// segment changes. This count determines how many future segments are at - /// least to be shown in this scrolling window, when it automatically + /// segment changes. This count determines the minimum number of future + /// segments to be shown in this scrolling window when it automatically /// scrolls. pub split_preview_count: usize, /// If not every segment is shown in the scrolling window of segments, then diff --git a/src/component/sum_of_best.rs b/src/component/sum_of_best.rs index 74f08dda..b6a25d8f 100644 --- a/src/component/sum_of_best.rs +++ b/src/component/sum_of_best.rs @@ -1,5 +1,5 @@ //! Provides the Sum of Best Segments Component. The Sum of Best Segments -//! Component shows the fastest time possible to complete a run of this +//! Component shows the fastest possible time to complete a run of this //! category, based on information collected from all the previous attempts. //! This often matches up with the sum of the best segment times of all the //! segments, but that may not always be the case, as skipped segments may @@ -16,7 +16,7 @@ use std::borrow::Cow; use settings::{Color, Field, Gradient, SettingsDescription, Value}; use super::DEFAULT_INFO_TEXT_GRADIENT; -/// The Sum of Best Segments Component shows the fastest time possible to +/// The Sum of Best Segments Component shows the fastest possible time to /// complete a run of this category, based on information collected from all the /// previous attempts. This often matches up with the sum of the best segment /// times of all the segments, but that may not always be the case, as skipped diff --git a/src/component/title/mod.rs b/src/component/title/mod.rs index ed02c0cb..591d79cf 100644 --- a/src/component/title/mod.rs +++ b/src/component/title/mod.rs @@ -1,7 +1,7 @@ //! Provides the Title Component and relevant types for using it. The Title //! Component is a component that shows the name of the game and the category -//! that is being run. Additionally the game's icon, the attempt count and the -//! amount of successfully finished attempts can be shown. +//! that is being run. Additionally, the game icon, the attempt count, and the +//! total number of finished runs can be shown. use {Timer, TimerPhase}; use serde_json::{to_writer, Result}; @@ -13,8 +13,8 @@ use settings::{Alignment, Color, Field, Gradient, SettingsDescription, Value}; mod tests; /// The Title Component is a component that shows the name of the game and the -/// category that is being run. Additionally the game's icon, the attempt count -/// and the amount of successfully finished attempts can be shown. +/// category that is being run. Additionally, the game icon, the attempt count, +/// and the total number of finished runs can be shown. #[derive(Default, Clone)] pub struct Component { icon_id: usize, diff --git a/src/layout/general_settings.rs b/src/layout/general_settings.rs index 47c738d8..adca8f9b 100644 --- a/src/layout/general_settings.rs +++ b/src/layout/general_settings.rs @@ -9,10 +9,10 @@ pub struct GeneralSettings { /// The color to use for when the runner achieved a best segment. pub best_segment_color: Color, /// The color to use for when the runner is ahead of the comparison and is - /// also gaining even more time. + /// gaining even more time. pub ahead_gaining_time_color: Color, /// The color to use for when the runner is ahead of the comparison, but is - /// losing time to the comparison. + /// losing time. pub ahead_losing_time_color: Color, /// The color to use for when the runner is behind the comparison, but is /// gaining back time. diff --git a/src/run/editor/cleaning.rs b/src/run/editor/cleaning.rs index 6efd76bc..6dc7d36e 100644 --- a/src/run/editor/cleaning.rs +++ b/src/run/editor/cleaning.rs @@ -1,9 +1,10 @@ //! The cleaning module provides the Sum of Best Cleaner which allows you to //! interactively remove potential issues in the Segment History that lead to an -//! inaccurate Sum of Best. If you skip a split, whenever you will do the next +//! inaccurate Sum of Best. If you skip a split, whenever you get to the next //! split, the combined segment time might be faster than the sum of the -//! individual best segments. The Sum of Best Cleaner will point out all of -//! these and allows you to delete them individually if any of them seem wrong. +//! individual best segments. The Sum of Best Cleaner will point out all +//! occurrences of this and allows you to delete them individually if any of +//! them seem wrong. use {Attempt, Run, Segment, TimeSpan, TimingMethod}; use analysis::sum_of_segments::{best, track_branch}; @@ -14,9 +15,9 @@ use chrono::Local; /// A Sum of Best Cleaner allows you to interactively remove potential issues in /// the Segment History that lead to an inaccurate Sum of Best. If you skip a -/// split, whenever you will do the next split, the combined segment time might +/// split, whenever you get to the next split, the combined segment time might /// be faster than the sum of the individual best segments. The Sum of Best -/// Cleaner will point out all of these and allows you to delete them +/// Cleaner will point out all occurrences of this and allows you to delete them /// individually if any of them seem wrong. pub struct SumOfBestCleaner<'r> { run: &'r mut Run, diff --git a/src/run/editor/state.rs b/src/run/editor/state.rs index f08791b7..019ede59 100644 --- a/src/run/editor/state.rs +++ b/src/run/editor/state.rs @@ -17,10 +17,10 @@ pub struct State { pub game: String, /// The name of the category the Run is for. pub category: String, - /// The timer offset specifies the time, the timer starts at when starting a + /// The timer offset specifies the time that the timer starts at when starting a /// new attempt. pub offset: String, - /// The amount of times this Run has been attempted for by the runner. This + /// The number of times this Run has been attempted by the runner. This /// is mostly just a visual number and has no effect on any history. pub attempts: u32, /// The timing method that is currently selected to be visualized and diff --git a/src/run/segment.rs b/src/run/segment.rs index c2ee6210..1ca28dbb 100644 --- a/src/run/segment.rs +++ b/src/run/segment.rs @@ -4,7 +4,7 @@ use comparison::personal_best; /// A Segment describes a point in a speedrun that is suitable for storing a /// split time. This stores the name of that segment, an icon, the split times -/// of different comparisons and a history of segment times. +/// of different comparisons, and a history of segment times. /// /// # Examples /// diff --git a/src/settings/semantic_color.rs b/src/settings/semantic_color.rs index a79c0a33..ca6c3384 100644 --- a/src/settings/semantic_color.rs +++ b/src/settings/semantic_color.rs @@ -8,11 +8,10 @@ use super::Color; pub enum SemanticColor { /// There's no meaningful information for this color. Default, - /// The runner is ahead of the comparison and is also gaining even more + /// The runner is ahead of the comparison and is gaining even more /// time. AheadGainingTime, - /// The runner is ahead of the comparison, but is losing time to the - /// comparison. + /// The runner is ahead of the comparison, but is losing time. AheadLosingTime, /// The runner is behind the comparison and is losing even more time. BehindLosingTime, diff --git a/src/time/formatter/accuracy.rs b/src/time/formatter/accuracy.rs index bb9a202a..78125422 100644 --- a/src/time/formatter/accuracy.rs +++ b/src/time/formatter/accuracy.rs @@ -4,9 +4,9 @@ pub enum Accuracy { /// Don't show any fractional part. Seconds, - /// Show the tenths of the times (12:34.5). + /// Show tenths of a second (12:34.5). Tenths, - /// Show the hundredths of the times (12:34.56). + /// Show hundredths of a second (12:34.56). Hundredths, } From 3cb670738e34aebdf6c010fe47a43ce7d6d0ea24 Mon Sep 17 00:00:00 2001 From: Sergey Papushin Date: Fri, 19 Jan 2018 18:17:44 -0600 Subject: [PATCH 2/2] Add comments to describe asserts in tests --- src/time/timer/tests.rs | 69 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/src/time/timer/tests.rs b/src/time/timer/tests.rs index 7147e919..d5839965 100644 --- a/src/time/timer/tests.rs +++ b/src/time/timer/tests.rs @@ -39,16 +39,21 @@ fn monotically_increasing_split_times_after_resetting() { let run = timer.into_run(true); + // The first segment's time should be unchanged. assert_eq!( run.segment(0).personal_best_split_time().game_time, Some(first) ); + // 15.0 is larger than 5.0, so the second segment's time should be + // unchanged. assert_eq!( run.segment(1).personal_best_split_time().game_time, Some(second) ); + // 10.0 is less than 15.0, and since we want split times to be monotonically + // increasing, the third segment's time should be adjusted to 15.0. assert_eq!( run.segment(2).personal_best_split_time().game_time, Some(second) @@ -82,6 +87,8 @@ fn deleting_best_segment_time_clears_segment_history() { editor.select_only(1); editor.active_segment().set_split_time(None); + // Clearing the second segment's split time should not affect the second + // segment's best segment time. assert_eq!( editor.run().segment(1).best_segment_time().game_time, Some(second - first) @@ -89,6 +96,9 @@ fn deleting_best_segment_time_clears_segment_history() { editor.active_segment().set_best_segment_time(None); + // Since the second segment's split time is already cleared, clearing its + // best segment time should clear the segment history and leave the best + // segment time as blank. assert_eq!( editor.run().segment(1).segment_history().iter().next(), None @@ -103,10 +113,14 @@ fn deleting_best_segment_time_clears_segment_history() { editor.select_only(1); editor.active_segment().set_best_segment_time(None); + // Clearing the second segment's best segment time without clearing its + // split time should still clear its segment history. assert_eq!( editor.run().segment(1).segment_history().iter().next(), None ); + // Since the second segment's split time was not cleared, the best segment + // time should be fixed based on the personal best split times. assert_eq!( editor.run().segment(1).best_segment_time().game_time, Some(second - first) @@ -140,6 +154,8 @@ fn modifying_best_segment_time_fixes_segment_history() { editor.select_only(1); editor.active_segment().set_split_time(None); + // Clearing the second segment's split time should not affect the second + // segment's best segment time. assert_eq!( editor.run().segment(1).best_segment_time().game_time, Some(second - first) @@ -149,6 +165,9 @@ fn modifying_best_segment_time_fixes_segment_history() { editor.active_segment().set_best_segment_time(new_best); + // Changing the second segment's best segment time from 5.0 to 7.0 after + // clearing the split time should change all times in the segment history so + // that none are shorter than 7.0. assert_eq!( editor .run() @@ -159,6 +178,7 @@ fn modifying_best_segment_time_fixes_segment_history() { .and_then(|&(_, t)| t.game_time), new_best ); + // The second segment's best segment time should have changed to 7.0. assert_eq!( editor.run().segment(1).best_segment_time().game_time, new_best @@ -172,6 +192,9 @@ fn modifying_best_segment_time_fixes_segment_history() { editor.select_only(1); editor.active_segment().set_best_segment_time(new_best); + // Changing the second segment's best segment time from 5.0 to 7.0 without + // clearing the split time first should keep the segment history unaffected. + // This is because the PB segment is still 5.0. assert_eq!( editor .run() @@ -182,6 +205,8 @@ fn modifying_best_segment_time_fixes_segment_history() { .and_then(|&(_, t)| t.game_time), Some(second - first) ); + // The second segment's best segment time should also be unaffected. This is + // because the PB segment is still 5.0. assert_eq!( editor.run().segment(1).best_segment_time().game_time, Some(second - first) @@ -228,6 +253,9 @@ fn import_pb_into_segment_history() { let run = timer.run(); + // The previous PB's first segment should be imported into the segment + // history with a non-positive index. A non-positive index means that it was + // not done during an actual run. assert_eq!( run.segment(0) .segment_history() @@ -235,6 +263,9 @@ fn import_pb_into_segment_history() { .and_then(|t| t.game_time), fake_first ); + // The new run's first segment should be imported into the segment history + // with a positive index. A positive index means that it was done during an + // actual run. assert_eq!( run.segment(0) .segment_history() @@ -243,7 +274,12 @@ fn import_pb_into_segment_history() { Some(real_first) ); + // The previous PB's second segment should not be imported into the segment + // history. This is because it is a duplicate of the new run's second + // segment. assert_eq!(run.segment(1).segment_history().get(-1), None); + // The new run's second segment should be imported into the segment history + // with a positive index. assert_eq!( run.segment(1) .segment_history() @@ -252,6 +288,8 @@ fn import_pb_into_segment_history() { Some(real_second - real_first) ); + // The previous PB's third segment should be imported into the segment + // history with a non-positive index. assert_eq!( run.segment(2) .segment_history() @@ -259,6 +297,8 @@ fn import_pb_into_segment_history() { .and_then(|t| t.game_time), catch! { fake_third? - fake_second? } ); + // The new run's third segment should be imported into the segment history + // with a positive index. assert_eq!( run.segment(2) .segment_history() @@ -302,6 +342,9 @@ fn import_pb_into_segment_history_and_remove_null_values() { let run = timer.run(); + // The previous PB's first segment should be imported into the segment + // history with a non-positive index. A non-positive index means that it was + // not done during an actual run. assert_eq!( run.segment(0) .segment_history() @@ -309,6 +352,9 @@ fn import_pb_into_segment_history_and_remove_null_values() { .and_then(|t| t.game_time), fake_first ); + // The new run's first segment should be imported into the segment history + // with a positive index. A positive index means that it was done during an + // actual run. assert_eq!( run.segment(0) .segment_history() @@ -317,13 +363,26 @@ fn import_pb_into_segment_history_and_remove_null_values() { Some(real_first) ); + // The previous PB's second segment should not be imported into the segment + // history. The second segment's split time was blank for the previous PB, + // so it is actually a part of a combined segment with the third segment. + // Since the third segment is a duplicate of the new run's third segment, + // neither the second nor the third segment should be imported. assert_eq!(run.segment(1).segment_history().get(-1), None); + // The new run's second segment has a blank split time, so a null time + // should be imported into the segment history with a positive index. This + // indicates that it is a part of a combined segment with the third segment. assert_eq!( run.segment(1).segment_history().get(1).map(|t| t.game_time), Some(None) ); + // The previous PB's third segment should not be imported into the segment + // history. This is because the third segment is a duplicate of the new + // run's third segment. assert_eq!(run.segment(2).segment_history().get(-1), None); + // The new run's third segment should be imported into the segment history + // with a positive index. assert_eq!( run.segment(2) .segment_history() @@ -353,11 +412,21 @@ fn import_best_segment_with_game_time_usage() { editor.insert_segment_above(); let history = editor.run().segment(0).segment_history(); + // The newly inserted segment's history should have a null time with a + // non-positive index. This represents a skipped split for the imported best + // segment with a time of 4.0. assert_eq!(history.get(0).map(|t| t.game_time), Some(None)); + // The newly inserted segment's history should have a null time with a + // positive index. This represents a skipped split for an actual run with a + // time of 5.0. assert_eq!(history.get(1).map(|t| t.game_time), Some(None)); let history = editor.run().segment(1).segment_history(); + // The second segment's history should have a time of 4.0 with a + // non-positive index, which represents the imported best segment. assert_eq!(history.get(0).and_then(|t| t.game_time), best); + // The second segment's history should have a time of 5.0 with a positive + // index, which represents an actual run. assert_eq!(history.get(1).and_then(|t| t.game_time), Some(first)); }