Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ private void OnEquipOutfit(BackpackEquipOutfitCommand command, IReadOnlyCollecti
foreach (var f in command.ForceRender)
previewAvatarModel.ForceRenderCategories.Add(f);

ResetVerticalRotation();
OnModelUpdated();
}

Expand All @@ -141,6 +142,7 @@ private void OnWearableEquipped(IWearable i, bool isManuallyEquipped)
previewAvatarModel.BodyShape = i.GetUrn();
else previewAvatarModel.Wearables.Add(i.GetUrn());

ResetVerticalRotation();
OnModelUpdated();
}

Expand All @@ -165,6 +167,7 @@ private void OnColorChange(Color newColor, string category)
private void OnWearableUnequipped(IWearable i)
{
previewAvatarModel.Wearables.Remove(i.GetUrn());
ResetVerticalRotation();
OnModelUpdated();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ MonoBehaviour:
<rotationEnabled>k__BackingField: 1
<rotationModifier>k__BackingField: 1
<rotationInertia>k__BackingField: 1
<verticalRotationEnabled>k__BackingField: 1
<verticalRotationModifier>k__BackingField: 0.2
<maxVerticalAngle>k__BackingField: 40
<cursorSettings>k__BackingField:
- inputAction: 0
cursorSprite: {fileID: 21300000, guid: a0dd824ee2daadd46a4437a4e3145c9e, type: 3}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ MonoBehaviour:
<rotationEnabled>k__BackingField: 1
<rotationModifier>k__BackingField: 1
<rotationInertia>k__BackingField: 1
<verticalRotationEnabled>k__BackingField: 0
<verticalRotationModifier>k__BackingField: 0.2
<maxVerticalAngle>k__BackingField: 40
<cursorSettings>k__BackingField:
- inputAction: 1
cursorSprite: {fileID: 21300000, guid: d34aedda08e64344091b34deada8211e, type: 3}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ MonoBehaviour:
<rotationEnabled>k__BackingField: 1
<rotationModifier>k__BackingField: 1
<rotationInertia>k__BackingField: 1
<verticalRotationEnabled>k__BackingField: 1
<verticalRotationModifier>k__BackingField: 0.2
<maxVerticalAngle>k__BackingField: 40
<cursorSettings>k__BackingField:
- inputAction: 1
cursorSprite: {fileID: 21300000, guid: d34aedda08e64344091b34deada8211e, type: 3}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ MonoBehaviour:
<rotationEnabled>k__BackingField: 1
<rotationModifier>k__BackingField: 1
<rotationInertia>k__BackingField: 1
<verticalRotationEnabled>k__BackingField: 0
<verticalRotationModifier>k__BackingField: 0.2
<maxVerticalAngle>k__BackingField: 40
<cursorSettings>k__BackingField:
- inputAction: 1
cursorSprite: {fileID: 21300000, guid: d34aedda08e64344091b34deada8211e, type: 3}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,38 @@ Transform:
m_LocalScale: {x: 1, y: 1, z: 1}
m_ConstrainProportionsScale: 0
m_Children: []
m_Father: {fileID: 5642318870193425106}
m_LocalEulerAnglesHint: {x: 0, y: 0, z: 0}
--- !u!1 &5642318870193425107
GameObject:
m_ObjectHideFlags: 0
m_CorrespondingSourceObject: {fileID: 0}
m_PrefabInstance: {fileID: 0}
m_PrefabAsset: {fileID: 0}
serializedVersion: 6
m_Component:
- component: {fileID: 5642318870193425106}
m_Layer: 0
m_Name: CameraPivot
m_TagString: Untagged
m_Icon: {fileID: 0}
m_NavMeshLayer: 0
m_StaticEditorFlags: 0
m_IsActive: 1
--- !u!4 &5642318870193425106
Transform:
m_ObjectHideFlags: 0
m_CorrespondingSourceObject: {fileID: 0}
m_PrefabInstance: {fileID: 0}
m_PrefabAsset: {fileID: 0}
m_GameObject: {fileID: 5642318870193425107}
serializedVersion: 2
m_LocalRotation: {x: -0, y: -0, z: -0, w: 1}
m_LocalPosition: {x: 0, y: 0, z: 0}
m_LocalScale: {x: 1, y: 1, z: 1}
m_ConstrainProportionsScale: 0
m_Children:
- {fileID: 8263753172317840554}
m_Father: {fileID: 5035414441912325780}
m_LocalEulerAnglesHint: {x: 0, y: 0, z: 0}
--- !u!1 &629546620005323917
Expand Down Expand Up @@ -1046,7 +1078,7 @@ RectTransform:
m_ConstrainProportionsScale: 0
m_Children:
- {fileID: 7939420118482055398}
- {fileID: 8263753172317840554}
- {fileID: 5642318870193425106}
- {fileID: 2101287217013189836}
- {fileID: 618782347525725431}
- {fileID: 6757364373489025388}
Expand Down Expand Up @@ -1075,6 +1107,8 @@ MonoBehaviour:
<avatarParent>k__BackingField: {fileID: 616156006899896637}
<camera>k__BackingField: {fileID: 5366071342697410109}
<cameraTarget>k__BackingField: {fileID: 8263753172317840554}
<cameraPivot>k__BackingField: {fileID: 5642318870193425106}
<cameraFloorHeight>k__BackingField: -1
<rotationTarget>k__BackingField: {fileID: 618782347525725431}
<freeLookCamera>k__BackingField: {fileID: 1653081050115794389}
<previewPlatform>k__BackingField: {fileID: 38504064483759473}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,18 @@ public class CharacterPreviewAvatarContainer : MonoBehaviour, IDisposable
private float fovTransitionStartTime;
private float fovTransitionStartValue;
private bool isFOVTransitioning;
private float cameraPitch;

[field: SerializeField] internal Vector3 previewPositionInScene { get; private set; }
[field: SerializeField] internal Transform avatarParent { get; private set; }
[field: SerializeField] internal Camera camera { get; private set; }
[field: SerializeField] internal Transform cameraTarget { get; private set; }
[field: SerializeField] internal Transform cameraPivot { get; private set; }

/// <summary>
/// Height, in the container's local space, that the orbiting camera may not descend below.
/// </summary>
[field: SerializeField] internal float cameraFloorHeight { get; private set; }
[field: SerializeField] internal Transform rotationTarget { get; private set; }
[field: SerializeField] internal CinemachineFreeLook freeLookCamera { get; private set; }
[field: SerializeField] internal GameObject previewPlatform { get; private set; }
Expand All @@ -39,6 +46,9 @@ public class CharacterPreviewAvatarContainer : MonoBehaviour, IDisposable
internal bool IsDragging { get; set; }
internal float LastDragTime { get; set; }
internal float AngularVelocity { get; set; }
internal float VerticalRotationModifier { get; set; }
internal float MaxVerticalAngle { get; set; }
internal float VerticalAngularVelocity { get; set; }

public void Dispose()
{
Expand All @@ -54,6 +64,7 @@ public void Initialize(RenderTexture targetTexture, Vector3 position)
AngularVelocity = 0f;
IsDragging = false;
LastDragTime = 0f;
ResetVerticalRotation();

// FOV
TargetFOV = freeLookCamera.m_Lens.FieldOfView;
Expand Down Expand Up @@ -110,20 +121,15 @@ private void UpdateRotation()
if (RotationInertia <= 0f)
{
AngularVelocity = 0f;
VerticalAngularVelocity = 0f;
return;
}

// Deceleration, higher inertia = faster deceleration
float decelerationRate = RotationInertia * ANGULAR_VELOCITY_DECELERATION_COEFF * UnityEngine.Time.deltaTime;
float velocitySign = Mathf.Sign(AngularVelocity);
float velocityMagnitude = Mathf.Abs(AngularVelocity);

velocityMagnitude -= decelerationRate;

if (velocityMagnitude <= 0f)
AngularVelocity = 0f;
else
AngularVelocity = velocitySign * velocityMagnitude;
AngularVelocity = Decelerate(AngularVelocity, decelerationRate);
VerticalAngularVelocity = Decelerate(VerticalAngularVelocity, decelerationRate);
}

// Apply rotation if there's any angular velocity
Expand All @@ -136,6 +142,45 @@ private void UpdateRotation()
rotation.y += rotationAmount;
rotationTarget.rotation = Quaternion.Euler(rotation);
}

if (Mathf.Abs(VerticalAngularVelocity) > ANGULAR_VELOCITY_LOWER_THRES)
UpdateCameraPitch();
}

private static float Decelerate(float angularVelocity, float decelerationRate)
{
float velocityMagnitude = Mathf.Abs(angularVelocity) - decelerationRate;

return velocityMagnitude <= 0f ? 0f : Mathf.Sign(angularVelocity) * velocityMagnitude;
}

private void UpdateCameraPitch()
{
float tiltedPitch = cameraPitch + (VerticalAngularVelocity * VerticalRotationModifier * UnityEngine.Time.deltaTime);
cameraPitch = Mathf.Clamp(tiltedPitch, -MaxVerticalAngle, Mathf.Min(MaxVerticalAngle, FloorPitchLimit()));

// Drop the inertia at the limit, or a flick keeps "arriving" after the camera has visibly stopped.
if (!Mathf.Approximately(cameraPitch, tiltedPitch))
VerticalAngularVelocity = 0f;

ApplyCameraPitch();
}

/// <summary>
/// Downward pitch that sets the camera down on <see cref="cameraFloorHeight"/>. Descending past it
/// would shoot the avatar from under its platform.
/// </summary>
private float FloorPitchLimit()
{
// Undoing the pivot's rotation gives the camera's offset at rest, whatever the preset framed and
// wherever the pan left it. The rig turns rigidly about the pivot, so from there the camera's
// height traces radius * cos(pitch + phase); the floor is where that lands.
Vector3 restingOffset = cameraPivot.InverseTransformPoint(camera.transform.position);
float radius = new Vector2(restingOffset.y, restingOffset.z).magnitude;
float floorAbovePivot = cameraFloorHeight - cameraPivot.localPosition.y;
float phase = Mathf.Atan2(restingOffset.z, restingOffset.y) * Mathf.Rad2Deg;

return (Mathf.Acos(Mathf.Clamp(floorAbovePivot / radius, -1f, 1f)) * Mathf.Rad2Deg) - phase;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] R7 Nullability / Correctness — NaN propagation from division by zero in FloorPitchLimit().

When restingOffset.y and restingOffset.z are both zero (camera sits exactly at the pivot), radius is 0f. The expression floorAbovePivot / radius produces NaN (0/0) or ±Infinity. Mathf.Clamp(NaN, -1, 1) returns NaN on Mono/IL2CPP, Mathf.Acos(NaN) returns NaN, and Mathf.Min(MaxVerticalAngle, NaN) returns NaN. Once cameraPitch absorbs NaN through the Mathf.Clamp chain in UpdateCameraPitch(), every subsequent frame stays NaN and the camera rotation is permanently broken.

While the Cinemachine orbits (radius 2.2–6) make radius ≈ 0 unlikely during normal operation, the guard is cheap and prevents a permanent NaN corruption if the camera ever passes through the pivot (e.g. during a zoom snap or a prefab misconfiguration).

Suggested change
return (Mathf.Acos(Mathf.Clamp(floorAbovePivot / radius, -1f, 1f)) * Mathf.Rad2Deg) - phase;
Vector3 restingOffset = cameraPivot.InverseTransformPoint(camera.transform.position);
float radius = new Vector2(restingOffset.y, restingOffset.z).magnitude;
if (radius < 1e-4f)
return MaxVerticalAngle;
float floorAbovePivot = cameraFloorHeight - cameraPivot.localPosition.y;
float phase = Mathf.Atan2(restingOffset.z, restingOffset.y) * Mathf.Rad2Deg;
return (Mathf.Acos(Mathf.Clamp(floorAbovePivot / radius, -1f, 1f)) * Mathf.Rad2Deg) - phase;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checked by Claude and deliberately not applied. I traced it rather than taking it: the divisor can't reach zero (the camera hangs ~6 units off the pivot by rig construction, and zoom here drives FOV, not distance); at zero it would be -1f / 0f = -Infinity, which the existing Mathf.Clamp(…, -1f, 1f) already folds into a 40° fallback — the identical result to the proposed guard; and the claimed "permanent NaN corruption" can't occur because Mathf.Clamp returns value untouched when max is NaN. So it would be an unreachable branch, which CLAUDE.md names as a smell in its own right.

}

private void UpdateFOV()
Expand Down Expand Up @@ -176,20 +221,46 @@ private void UpdateFOV()
freeLookCamera.m_Lens.FieldOfView = newFOV;
}

/// <summary>
/// Levels the camera back to its default elevation, keeping the avatar's own rotation and the zoom.
/// </summary>
public void ResetVerticalRotation()
{
VerticalAngularVelocity = 0f;
cameraPitch = 0f;
ApplyCameraPitch();
}

public void ResetAvatarMovement()
{
// Reset rotation
rotationTarget.rotation = Quaternion.identity;
AngularVelocity = 0f;
IsDragging = false;
LastDragTime = 0f;
ResetVerticalRotation();

// Reset FOV
TargetFOV = freeLookCamera.m_Lens.FieldOfView;
fovTransitionStartTime = UnityEngine.Time.time;
fovTransitionStartValue = freeLookCamera.m_Lens.FieldOfView;
isFOVTransitioning = false;
}

/// <summary>
/// Turns the camera rig about the pivot the framing target hangs off, swinging the camera over
/// and under an avatar that stays upright: elevation changes, and the shot the screen was tuned
/// for holds.
/// </summary>
private void ApplyCameraPitch()
{
cameraPivot.localRotation = Quaternion.Euler(cameraPitch, 0f, 0f);

// Position and aim damping would leave the camera seconds behind the drag. The orbit is
// already smoothed by its own inertia, so the rig takes this rotation undamped and lands on
// the pose exactly. Damping stays in effect on every frame the orbit does not move.
freeLookCamera.PreviousStateIsValid = false;
Comment on lines +250 to +262

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] R23 — Comment narrates Cinemachine's external behavior.

The <summary> block describes what the Cinemachine rig does with the target's rotation ("The rig reads its target's rotation (LockToTargetNoRoll binding)…") and what would happen if a different approach were taken ("Pitching the target instead would orbit around the framing point…"). Per R23, a comment must state only what the annotated code itself does or guarantees — external behavior can change without this code changing, silently turning the comment into a lie.

The inline comment on PreviousStateIsValid is fine — it explains why this code sets that flag.

Suggested trim — keep the summary focused on what ApplyCameraPitch does:

Suggested change
/// <summary>
/// Swings the camera over and under the avatar, which stays upright on its platform.
/// The rig reads its target's rotation (LockToTargetNoRoll binding) and the target hangs off this
/// pivot, so camera and framing point rotate together: the elevation changes and the shot the
/// screen was tuned for survives. Pitching the target instead would orbit around the framing
/// point, which sits off the avatar's axis wherever the shot is angled.
/// </summary>
private void ApplyCameraPitch()
{
cameraPivot.localRotation = Quaternion.Euler(cameraPitch, 0f, 0f);
// Position and aim damping would leave the camera seconds behind the drag. The orbit is
// already smoothed by its own inertia, so the rig takes this rotation undamped and lands on
// the pose exactly. Damping stays in effect on every frame the orbit does not move.
freeLookCamera.PreviousStateIsValid = false;
/// <summary>
/// Applies the current pitch to the camera pivot, orbiting the camera vertically
/// while preserving each screen's tuned shot.
/// </summary>
private void ApplyCameraPitch()
{
cameraPivot.localRotation = Quaternion.Euler(cameraPitch, 0f, 0f);
// Position and aim damping would leave the camera seconds behind the drag. The orbit is
// already smoothed by its own inertia, so the rig takes this rotation undamped and lands on
// the pose exactly. Damping stays in effect on every frame the orbit does not move.
freeLookCamera.PreviousStateIsValid = false;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

}
}

[Serializable]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ public void Dispose()
public void ResetAvatarMovement() =>
characterPreviewAvatarContainer.ResetAvatarMovement();

public void ResetVerticalRotation() =>
characterPreviewAvatarContainer.ResetVerticalRotation();

private void OnChangePreviewCategory(AvatarWearableCategoryEnum categoryEnum)
{
int positions = cameraSettings.cameraPositions.Length;
Expand Down Expand Up @@ -142,8 +145,21 @@ private void CalculateAngularVelocity(PointerEventData pointerEventData)
characterPreviewAvatarContainer.IsDragging = true;
characterPreviewAvatarContainer.LastDragTime = UnityEngine.Time.time;

float angularVelocity = characterPreviewAvatarContainer.AngularVelocity;
float targetVelocity = -pointerEventData.delta.x / UnityEngine.Time.deltaTime;
characterPreviewAvatarContainer.AngularVelocity = Accelerate(characterPreviewAvatarContainer.AngularVelocity, -pointerEventData.delta.x);

if (!cameraSettings.verticalRotationEnabled) return;

characterPreviewAvatarContainer.VerticalRotationModifier = cameraSettings.verticalRotationModifier;
characterPreviewAvatarContainer.MaxVerticalAngle = cameraSettings.maxVerticalAngle;

// The avatar tracks the cursor on both axes: dragging up lowers the camera and brings the avatar
// up into the frame, the way dragging sideways carries its face across.
characterPreviewAvatarContainer.VerticalAngularVelocity = Accelerate(characterPreviewAvatarContainer.VerticalAngularVelocity, pointerEventData.delta.y);
}

private float Accelerate(float angularVelocity, float pointerDelta)
{
float targetVelocity = pointerDelta / UnityEngine.Time.deltaTime;

if (cameraSettings.rotationInertia <= 0f)
{
Expand All @@ -157,7 +173,7 @@ private void CalculateAngularVelocity(PointerEventData pointerEventData)
angularVelocity = Mathf.Lerp(angularVelocity, targetVelocity, accelerationRate);
}

characterPreviewAvatarContainer.AngularVelocity = Mathf.Clamp(angularVelocity, -MAX_ANGULAR_VELOCITY, MAX_ANGULAR_VELOCITY);
return Mathf.Clamp(angularVelocity, -MAX_ANGULAR_VELOCITY, MAX_ANGULAR_VELOCITY);
}

public void ResetZoom()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ public void SetCharacterPreviewAvatarContainerActive(bool isActive)
public void ResetAvatarMovement() =>
cameraController.ResetAvatarMovement();

public void ResetVerticalRotation() =>
cameraController.ResetVerticalRotation();

public void ResetZoom()
{
cameraController.ResetZoom();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,11 @@ public void ResetAvatarMovement()
previewController?.ResetAvatarMovement();
}

public void ResetVerticalRotation()
{
previewController?.ResetVerticalRotation();
}

public void ResetZoom()
{
previewController?.ResetZoom();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,20 @@ public struct CharacterPreviewCameraSettings
[field: SerializeField] public bool rotationEnabled { get; private set; }
[field: SerializeField] public float rotationModifier { get; private set; }
[field: SerializeField, Min(0f)] public float rotationInertia { get; private set; }

[field: Header("Vertical Rotation Settings")]
[field: SerializeField] internal bool verticalRotationEnabled { get; private set; }

/// <summary>
/// Degrees of camera pitch per pixel of vertical drag. A negative value inverts the orbit direction.
/// </summary>
[field: SerializeField] internal float verticalRotationModifier { get; private set; }

/// <summary>
/// Elevation the camera may reach above and below its default framing. Stays short of the
/// right angle where the rig's reference orientation degenerates.
/// </summary>
[field: SerializeField, Range(0f, 80f)] internal float maxVerticalAngle { get; private set; }
}

[Serializable]
Expand Down
Loading