-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Provides motion to SDK avatars shapes when changing position #3297
feat: Provides motion to SDK avatars shapes when changing position #3297
Conversation
Windows and Mac build successfull in Unity Cloud! You can find a link to the downloadable artifact below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good for me. Some minor comments
Explorer/Assets/DCL/Multiplayer/Movement/Systems/SDKAvatarShapesMotionSystem.cs
Outdated
Show resolved
Hide resolved
Explorer/Assets/DCL/Multiplayer/Movement/Systems/SDKAvatarShapesMotionSystem.cs
Outdated
Show resolved
Hide resolved
Explorer/Assets/DCL/Multiplayer/Movement/Systems/SDKAvatarShapesMotionSystem.cs
Outdated
Show resolved
Hide resolved
Explorer/Assets/DCL/Multiplayer/Movement/Systems/SDKAvatarShapesMotionSystem.cs
Outdated
Show resolved
Hide resolved
Explorer/Assets/DCL/Multiplayer/Movement/Systems/SDKAvatarShapesMotionSystem.cs
Outdated
Show resolved
Hide resolved
Explorer/Assets/DCL/Multiplayer/Movement/Systems/SDKAvatarShapesMotionSystem.cs
Outdated
Show resolved
Hide resolved
Explorer/Assets/DCL/Multiplayer/Movement/Systems/SDKAvatarShapesMotionSystem.cs
Outdated
Show resolved
Hide resolved
Explorer/Assets/DCL/Multiplayer/Movement/Systems/SDKAvatarShapesMotionSystem.cs
Outdated
Show resolved
Hide resolved
Explorer/Assets/DCL/Multiplayer/Movement/Systems/SDKAvatarShapesMotionSystem.cs
Outdated
Show resolved
Hide resolved
Explorer/Assets/DCL/PluginSystem/Global/MultiplayerMovementPlugin.cs
Outdated
Show resolved
Hide resolved
Explorer/Assets/DCL/Multiplayer/Movement/Systems/SDKAvatarShapesMotionSystem.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! the needed solution is all contained in 1 concise system, looks super maintainable to me.
After Vitaly's suggestions and QA approval it will be ready for merging
Explorer/Assets/DCL/Character/CharacterMotion/Systems/SDKAvatarShapesMotionSystem.cs
Show resolved
Hide resolved
Explorer/Assets/DCL/SDKComponents/AvatarShape/Systems/UpdateAvatarShapeTargetPositionSystem.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✔️ PR reviewed and approved by QA on both platforms following instructions playing both happy and un-happy path
Regressions for this ticket had been performed in order to verify that the normal flow is working as expected:
- ✅ Log In/Log Out
- ✅ Backpack and wearables in world
- ✅ Emotes in world and in backpack
- ✅Teleport with map/coordinates/Jump In
- ✅ Chat and multiplayer
- ✅ Profile card
- ✅ Camera
- ✅ Skybox
- ✅ Settings
Evidence of all the points verified on this PR :
DAGON.mp4
Explorer/Assets/DCL/Character/CharacterObject/Components/CharacterTargetPositionComponent.cs
Outdated
Show resolved
Hide resolved
Explorer/Assets/DCL/Character/CharacterObject/Components/CharacterTargetPositionComponent.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some minor comments!
He's currently busy with his shape
What does this PR change?
Fix #3187
This PR adds motion to the avatar shapes that are created by the SDK so that if these avatars change its position in the scene, they will reproduce the corresponding movement interpolation, lookAt rotation and animation (walk, jog or run) instead of directly teleport the avatar object.
NOTE: If the AvatarShape position and/or rotation is already managed by a Tween, we let Tween system manage it as usual in order to not apply double interpolation and avoid conflicts.
How to test the changes?
santihisteria
world where I have a test scene prepared to test this PR).lookAt
rotation to the correct direction.9,-76
) and check that the NPCs that were floating before, now they are moving correctly with animations.Our Code Review Standards
https://github.com/decentraland/unity-renderer/blob/master/docs/code-review-standards.md