From 857fc5c978460f029c54b0c7e810b491846a777f Mon Sep 17 00:00:00 2001 From: Pravus Date: Wed, 17 Jan 2024 13:03:38 +0100 Subject: [PATCH] fix: sdk7 tween reset + tween transform pos update (#6057) * The TweenComponent DOTween tweener was being wrongly reset when it wasn't needed, specifically affecting cases of applying different kind of tweens on the same transform one after the other. * Transform component position was being updated wrongly for entities that were child of other displaced entities instead of the scene root transform. --- .../ECSComponents/Tween/ECSTweenHandler.cs | 15 ++----- .../Tween/Tests/ECSTweenHandlerShould.cs | 42 +++++++++++++++++++ .../Systems/TweenSystem/ECSTweenSystem.cs | 8 ++-- .../TweenSystem/Tests/ECSTweenSystemShould.cs | 14 +++++-- unity-renderer/Packages/manifest.json | 2 +- unity-renderer/Packages/packages-lock.json | 2 +- 6 files changed, 62 insertions(+), 21 deletions(-) diff --git a/unity-renderer/Assets/DCLPlugins/ECS7/ECSComponents/Tween/ECSTweenHandler.cs b/unity-renderer/Assets/DCLPlugins/ECS7/ECSComponents/Tween/ECSTweenHandler.cs index 758d4233a9..9023de795f 100644 --- a/unity-renderer/Assets/DCLPlugins/ECS7/ECSComponents/Tween/ECSTweenHandler.cs +++ b/unity-renderer/Assets/DCLPlugins/ECS7/ECSComponents/Tween/ECSTweenHandler.cs @@ -84,17 +84,10 @@ public void OnComponentModelUpdated(IParcelScene scene, IDCLEntity entity, PBTwe float durationInSeconds = model.Duration / 1000; currentTweener = internalComponentModel.tweener; - if (currentTweener == null) - { - // There may be a tween running for the entity transform, even though internalComponentModel.tweener - // is null, e.g: during preview mode hot-reload. - var transformTweens = DOTween.TweensByTarget(entityTransform, true); - transformTweens?[0].Rewind(false); - } - else - { - currentTweener.Rewind(false); - } + // There may be a tween running for the entity transform, even with internalComponentModel.tweener + // as null, e.g: during preview mode hot-reload. + var transformTweens = DOTween.TweensByTarget(entityTransform, true); + transformTweens?[0].Rewind(false); internalComponentModel.transform = entityTransform; internalComponentModel.currentTime = model.CurrentTime; diff --git a/unity-renderer/Assets/DCLPlugins/ECS7/ECSComponents/Tween/Tests/ECSTweenHandlerShould.cs b/unity-renderer/Assets/DCLPlugins/ECS7/ECSComponents/Tween/Tests/ECSTweenHandlerShould.cs index 7712445849..ae14edb212 100644 --- a/unity-renderer/Assets/DCLPlugins/ECS7/ECSComponents/Tween/Tests/ECSTweenHandlerShould.cs +++ b/unity-renderer/Assets/DCLPlugins/ECS7/ECSComponents/Tween/Tests/ECSTweenHandlerShould.cs @@ -465,5 +465,47 @@ public void KillTweenerImmediatelyOnComponentRemove() tweens = DOTween.PlayingTweens(); Assert.IsNull(tweens); } + + [Test] + public void NotResetPreviousTweenerOnComponentUpdate() + { + Vector3 startPosition = Vector3.zero; + Vector3 endPosition = new Vector3(57f, 41f, 5f); + float duration = 3000; + Transform entityTransform = entity.gameObject.transform; + + var model = new PBTween() + { + Duration = duration, + Move = new Move() + { + Start = new Decentraland.Common.Vector3() { X = startPosition.x, Y = startPosition.y, Z = startPosition.z }, + End = new Decentraland.Common.Vector3() { X = endPosition.x, Y = endPosition.y, Z = endPosition.z }, + FaceDirection = true + } + }; + + componentHandler.OnComponentModelUpdated(scene, entity, model); + + // Check it moved + var tweener = internalComponents.TweenComponent.GetFor(scene, entity).Value.model.tweener; + tweener.Goto(duration / 1000 / 2); + Assert.AreEqual(endPosition / 2, entityTransform.position); + + // Put new different component + model = new PBTween() + { + Duration = duration, + Rotate = new Rotate() + { + Start = new Decentraland.Common.Quaternion() { X = 1f, Y = 1f, Z = 1f, W = 1f }, + End = new Decentraland.Common.Quaternion() { X = 5f, Y = 5f, Z = 5f, W = 5f } + } + }; + componentHandler.OnComponentModelUpdated(scene, entity, model); + + // Check transform position hasn't been reset + Assert.AreEqual(endPosition / 2, entityTransform.position); + } } } diff --git a/unity-renderer/Assets/DCLPlugins/ECS7/Systems/TweenSystem/ECSTweenSystem.cs b/unity-renderer/Assets/DCLPlugins/ECS7/Systems/TweenSystem/ECSTweenSystem.cs index 1454b16f79..39e0c92e98 100644 --- a/unity-renderer/Assets/DCLPlugins/ECS7/Systems/TweenSystem/ECSTweenSystem.cs +++ b/unity-renderer/Assets/DCLPlugins/ECS7/Systems/TweenSystem/ECSTweenSystem.cs @@ -90,7 +90,7 @@ private void UpdateTweenComponentModel(KeyValueSetTriplet()` throw a 'System.Exception : Unexpected + // component value. was: DCL.ECSComponents.ECSTransform', That's because componentModel.position + // values are not the same as entityTransform.localPosition values. outgoingMessages.Put_Called( entity.entityId, ComponentID.TRANSFORM, componentModel => - componentModel.position == entityTransform.localPosition - && componentModel.parentId == entity.parentId + componentModel.position.Equals(entityTransform.localPosition) + && componentModel.parentId == entity.parentId ); Vector3 midPosition = entityTransform.localPosition; @@ -135,7 +143,7 @@ public void UpdateTransformComponentCorrectly() entity.entityId, ComponentID.TRANSFORM, componentModel => - componentModel.position == entityTransform.localPosition + componentModel.position.Equals(entityTransform.localPosition) && componentModel.parentId == entity.parentId ); Assert.AreNotEqual(midPosition, entityTransform.localPosition); diff --git a/unity-renderer/Packages/manifest.json b/unity-renderer/Packages/manifest.json index 8c89207e0c..bd5e51a2ad 100644 --- a/unity-renderer/Packages/manifest.json +++ b/unity-renderer/Packages/manifest.json @@ -16,7 +16,7 @@ "com.unity.ai.navigation": "1.1.4", "com.unity.build-report-inspector": "https://github.com/needle-mirror/com.unity.build-report-inspector.git", "com.unity.cinemachine": "2.9.5", - "com.unity.ide.rider": "3.0.26", + "com.unity.ide.rider": "3.0.27", "com.unity.ide.visualstudio": "2.0.18", "com.unity.ide.vscode": "1.2.5", "com.unity.memoryprofiler": "1.0.0", diff --git a/unity-renderer/Packages/packages-lock.json b/unity-renderer/Packages/packages-lock.json index 96d4b17fa1..54c5f42b11 100644 --- a/unity-renderer/Packages/packages-lock.json +++ b/unity-renderer/Packages/packages-lock.json @@ -155,7 +155,7 @@ "url": "https://packages.unity.com" }, "com.unity.ide.rider": { - "version": "3.0.26", + "version": "3.0.27", "depth": 0, "source": "registry", "dependencies": {