-
Notifications
You must be signed in to change notification settings - Fork 11
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
chore: adjust minimum camera distance for main avatar visibility #2468
base: dev
Are you sure you want to change the base?
chore: adjust minimum camera distance for main avatar visibility #2468
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.
LGTM! Just left a minor question
@@ -13,7 +13,7 @@ namespace DCL.AvatarRendering.AvatarShape.Systems | |||
[UpdateInGroup(typeof(CameraGroup))] | |||
public partial class AvatarShapeVisibilitySystem : BaseUnityLoopSystem | |||
{ | |||
private const float AVATAR_MINIMUM_CAMERA_DISTANCE_SQR = 1; | |||
private const float AVATAR_MINIMUM_CAMERA_DISTANCE_SQR = 3.5f * 3.5f; |
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.
I know it's a const
and will be calculated once, but is there a reason not to have the final value and have it written like this?
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.
For me it's easier to know we mean "3.5 meters" otherwise if I put 12.25 I have to go and make the calculation to know what actual distance we are talking about.
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.
I see, makes perfect sense! Thanks 😋
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.
🟢 Chore verified on Windows and Mac. Approved by QA. Unnable to reproduce the issue from the video.
2468.evidence.mp4
A smoke test was performed, covering:
- Backpack
- Emotes
- Multiplayer
- Social interaction
- Map navigation
- Scenes/worlds (Art Week/Metadynelabs).
WHY
Right now when performance degrades a lot and therea re hiccups in the main thread, the main camera following (in first person camera) can't keep up with the main player position, thus getting outside the minimum camera distance range that hides the main avatar in first person mode.
WHAT
Adjusted minimum camera distance for hiding the avatar by testing in a specific place where the problem is always reproducible.
TEST STEPS
Put the camera in first person mode, go to
51, 72
and move towards the center of the Art Week Plaza to reproduce the issue, as shown in the video:Screen.Recording.2024-10-16.at.8.47.14.PM.mp4
Do the same with the build from this PR and confirm that the problem doesn't happen anymore.