From fdc3eded7b48bb26d6d3d4803db5c12cb7c5c27b Mon Sep 17 00:00:00 2001
From: Jerome Humbert <djeedai@gmail.com>
Date: Tue, 27 Sep 2022 21:24:16 +0100
Subject: [PATCH] Forbid `Delay` with a zero duration (#56)

Make `Delay::new()` panic if a zero duration is passed as argument. Fix
the `menu` example to skip inserting a `Sequence<Transform>` containing
a zero-duration `Delay`.

Bug: #41
---
 CHANGELOG.md     |  1 +
 examples/menu.rs | 12 ++++++++----
 src/tweenable.rs | 15 +++++++++++++--
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3430915..c5d445f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Removed the `tweening_type` parameter from the signature of `Tween<T>::new()`; use `with_repeat_count()` and `with_repeat_strategy()` instead.
 - Animators now always have a tween (instead of it being optional). This means the default animator implementation was removed.
+- `Delay::new()` now panics if the `duration` is zero. This prevents creating no-op `Delay` objects, and avoids an internal edge case producing wrong results.
 
 ### Removed
 
diff --git a/examples/menu.rs b/examples/menu.rs
index fccc6f7..bd6cb01 100644
--- a/examples/menu.rs
+++ b/examples/menu.rs
@@ -45,8 +45,6 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
         .with_children(|container| {
             let mut start_time_ms = 0;
             for text in &["Continue", "New Game", "Settings", "Quit"] {
-                let delay = Delay::new(Duration::from_millis(start_time_ms));
-                start_time_ms += 500;
                 let tween_scale = Tween::new(
                     EaseFunction::BounceOut,
                     Duration::from_secs(2),
@@ -55,7 +53,13 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
                         end: Vec3::ONE,
                     },
                 );
-                let seq = delay.then(tween_scale);
+                let animator = if start_time_ms > 0 {
+                    let delay = Delay::new(Duration::from_millis(start_time_ms));
+                    Animator::new(delay.then(tween_scale))
+                } else {
+                    Animator::new(tween_scale)
+                };
+                start_time_ms += 500;
                 container
                     .spawn_bundle(NodeBundle {
                         node: Node {
@@ -76,7 +80,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
                         ..default()
                     })
                     .insert(Name::new(format!("button:{}", text)))
-                    .insert(Animator::new(seq))
+                    .insert(animator)
                     .with_children(|parent| {
                         parent.spawn_bundle(TextBundle {
                             text: Text::from_section(
diff --git a/src/tweenable.rs b/src/tweenable.rs
index c140df7..2bdd5b5 100644
--- a/src/tweenable.rs
+++ b/src/tweenable.rs
@@ -782,15 +782,20 @@ pub struct Delay {
 
 impl Delay {
     /// Create a new [`Delay`] with a given duration.
+    ///
+    /// # Panics
+    ///
+    /// Panics if the duration is zero.
     #[must_use]
     pub fn new(duration: Duration) -> Self {
+        assert!(!duration.is_zero());
         Self {
             timer: Timer::new(duration, false),
         }
     }
 
-    /// Chain another [`Tweenable`] after this tween, making a sequence with the
-    /// two.
+    /// Chain another [`Tweenable`] after this tween, making a [`Sequence`] with
+    /// the two.
     #[must_use]
     pub fn then<T>(self, tween: impl Tweenable<T> + Send + Sync + 'static) -> Sequence<T> {
         Sequence::with_capacity(2).then(self).then(tween)
@@ -1472,4 +1477,10 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    #[should_panic]
+    fn delay_zero_duration_panics() {
+        let _ = Delay::new(Duration::ZERO);
+    }
 }
-- 
GitLab