Merge pull request #10856 from bdach/reset-transforms-on-return-to-pool

Clear DrawableHitObject transforms on return to pool

osu.Game.Tests/Visual/Gameplay/TestScenePoolingRuleset.cs

@@ -57,6 +57,43 @@ namespace osu.Game.Tests.Visual.Gameplay
             AddAssert("DHO reused", () => this.ChildrenOfType<DrawableTestHitObject>().Single() == firstObject);
         }
 
+        [Test]
+        public void TestCustomTransformsClearedBetweenReuses()
+        {
+            ManualClock clock = null;
+
+            createTest(new Beatmap
+            {
+                HitObjects =
+                {
+                    new HitObject(),
+                    new HitObject { StartTime = 2000 }
+                }
+            }, 1, () => new FramedClock(clock = new ManualClock()));
+
+            DrawableTestHitObject firstObject = null;
+            Vector2 position = default;
+
+            AddUntilStep("first object shown", () => this.ChildrenOfType<DrawableTestHitObject>().SingleOrDefault()?.HitObject == drawableRuleset.Beatmap.HitObjects[0]);
+            AddStep("get DHO", () => firstObject = this.ChildrenOfType<DrawableTestHitObject>().Single());
+            AddStep("store position", () => position = firstObject.Position);
+            AddStep("add custom transform", () => firstObject.ApplyCustomUpdateState += onStateUpdate);
+
+            AddStep("fast forward past first object", () => clock.CurrentTime = 1500);
+            AddStep("unapply custom transform", () => firstObject.ApplyCustomUpdateState -= onStateUpdate);
+
+            AddStep("fast forward to second object", () => clock.CurrentTime = drawableRuleset.Beatmap.HitObjects[1].StartTime);
+            AddUntilStep("second object shown", () => this.ChildrenOfType<DrawableTestHitObject>().SingleOrDefault()?.HitObject == drawableRuleset.Beatmap.HitObjects[1]);
+            AddAssert("DHO reused", () => this.ChildrenOfType<DrawableTestHitObject>().Single() == firstObject);
+            AddAssert("object in new position", () => firstObject.Position != position);
+
+            void onStateUpdate(DrawableHitObject hitObject, ArmedState state)
+            {
+                using (hitObject.BeginAbsoluteSequence(hitObject.StateUpdateTime))
+                    hitObject.MoveToOffset(new Vector2(-100, 0));
+            }
+        }
+
         [Test]
         public void TestNotReusedWithHitObjectsSpacedClose()
         {
@@ -210,7 +247,6 @@ namespace osu.Game.Tests.Visual.Gameplay
                 Anchor = Anchor.Centre;
                 Origin = Anchor.Centre;
 
-                Position = new Vector2(RNG.Next(-200, 200), RNG.Next(-200, 200));
                 Size = new Vector2(50, 50);
 
                 Colour = new Color4(RNG.NextSingle(), RNG.NextSingle(), RNG.NextSingle(), 1f);
@@ -225,6 +261,12 @@ namespace osu.Game.Tests.Visual.Gameplay
                 });
             }
 
+            protected override void OnApply(HitObject hitObject)
+            {
+                base.OnApply(hitObject);
+                Position = new Vector2(RNG.Next(-200, 200), RNG.Next(-200, 200));
+            }
+
             protected override void CheckForResult(bool userTriggered, double timeOffset)
             {
                 if (timeOffset > HitObject.Duration)

osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs

@@ -287,6 +287,8 @@ namespace osu.Game.Rulesets.Objects.Drawables
             HitObject = null;
             lifetimeEntry = null;
 
+            clearExistingStateTransforms();
+
             hasHitObjectApplied = false;
         }
 
@@ -403,8 +405,7 @@ namespace osu.Game.Rulesets.Objects.Drawables
 
             double transformTime = HitObject.StartTime - InitialLifetimeOffset;
 
-            base.ApplyTransformsAt(double.MinValue, true);
+            clearExistingStateTransforms();
-            base.ClearTransformsAfter(double.MinValue, true);
 
             using (BeginAbsoluteSequence(transformTime, true))
                 UpdateInitialTransforms();
@@ -432,6 +433,12 @@ namespace osu.Game.Rulesets.Objects.Drawables
                 PlaySamples();
         }
 
+        private void clearExistingStateTransforms()
+        {
+            base.ApplyTransformsAt(double.MinValue, true);
+            base.ClearTransformsAfter(double.MinValue, true);
+        }
+
         /// <summary>
         /// Apply (generally fade-in) transforms leading into the <see cref="HitObject"/> start time.
         /// The local drawable hierarchy is recursively delayed to <see cref="LifetimeStart"/> for convenience.