From 33ef93d516d4627572518ecb71f3058441e70779 Mon Sep 17 00:00:00 2001 From: Gregory Labute Date: Thu, 30 May 2024 17:05:27 -0400 Subject: [PATCH 1/4] CMCL-1590: statedrivencamera inspector bugs (#994) * wip * Fix SDC inspector, * Fix undo issue in SequencerCamera * Update CHANGELOG.md --- com.unity.cinemachine/CHANGELOG.md | 1 + .../CinemachineStateDrivenCameraEditor.cs | 14 +++++++------ .../Behaviours/CinemachineSequencerCamera.cs | 20 ++++--------------- .../CinemachineStateDrivenCamera.cs | 16 ++++++++------- .../Debug/CinemachinePanelSettings.asset | 3 +-- .../Runtime/Deprecated/AxisState.cs | 7 ++----- 6 files changed, 25 insertions(+), 36 deletions(-) diff --git a/com.unity.cinemachine/CHANGELOG.md b/com.unity.cinemachine/CHANGELOG.md index ff2dfa8b3..0035815b2 100644 --- a/com.unity.cinemachine/CHANGELOG.md +++ b/com.unity.cinemachine/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Bugfix: InputAxis.TriggerRecentering() function caused the axis to immediately snap to its recenter value. - Bugfix: When multiple CM Brains were present, FixedUpdte cameras were sometimes being updated too frequently, resulting in jittery motion. - SimplePlayerController no longer uses PlayerController.isGrounded because it's not reliable outside of FixedUpdate. +- Regression fix: StateDrivenCamera inspector was failing to correctly set the states in the instruction list. - Decollider ignores terrain layers when resolving obstacles. - Bugfix: The GroupAverage Rotation Mode in CinemachineTargetGroup was not calculated properly. - Bugfix: add missing null check in CinemachineTargetGroup.WeightedMemberBoundsForValidMember. diff --git a/com.unity.cinemachine/Editor/Editors/CinemachineStateDrivenCameraEditor.cs b/com.unity.cinemachine/Editor/Editors/CinemachineStateDrivenCameraEditor.cs index ef08d21f7..dc675f547 100644 --- a/com.unity.cinemachine/Editor/Editors/CinemachineStateDrivenCameraEditor.cs +++ b/com.unity.cinemachine/Editor/Editors/CinemachineStateDrivenCameraEditor.cs @@ -9,6 +9,7 @@ namespace Unity.Cinemachine.Editor { + [CanEditMultipleObjects] [CustomEditor(typeof(CinemachineStateDrivenCamera))] class CinemachineStateDrivenCameraEditor : UnityEditor.Editor { @@ -106,9 +107,12 @@ public override VisualElement CreateInspectorGUI() { if (evt.target == stateSel) { - var index = stateSel.index; - if (index >= 0 && index < m_TargetStates.Count) - stateSelProp.intValue = index; + var i = stateSel.index; + if (i >= 0 && i < m_TargetStates.Count) + { + stateSelProp.intValue = m_TargetStates[i]; + stateSelProp.serializedObject.ApplyModifiedProperties(); + } evt.StopPropagation(); } }); @@ -173,8 +177,6 @@ void UpdateCameraDropdowns() if (vcamSel != null) { vcamSel.choices.Clear(); -for (int i = 0; i < children.Count; ++i) - Debug.Log($"vcamSelector{index}: {children[i].name}"); for (int i = 0; i < children.Count; ++i) vcamSel.choices.Add(children[i]); } @@ -237,7 +239,7 @@ void UpdateTargetStates() while (iter.MoveNext()) parents.Add(new CinemachineStateDrivenCamera.ParentHash { Hash = iter.Current.Key, HashOfParent = iter.Current.Value }); - Target.HashOfParent = parents.ToArray(); + Target.SetParentHash(parents); } class StateCollector diff --git a/com.unity.cinemachine/Runtime/Behaviours/CinemachineSequencerCamera.cs b/com.unity.cinemachine/Runtime/Behaviours/CinemachineSequencerCamera.cs index 3ff353ce9..5a123a424 100644 --- a/com.unity.cinemachine/Runtime/Behaviours/CinemachineSequencerCamera.cs +++ b/com.unity.cinemachine/Runtime/Behaviours/CinemachineSequencerCamera.cs @@ -114,10 +114,8 @@ public override void OnTransitionFromCamera( protected override CinemachineVirtualCameraBase ChooseCurrentCamera(Vector3 worldUp, float deltaTime) { if (!PreviousStateIsValid) - { m_CurrentInstruction = -1; - ValidateInstructions(); - } + AdvanceCurrentInstruction(deltaTime); return (m_CurrentInstruction >= 0 && m_CurrentInstruction < Instructions.Count) ? Instructions[m_CurrentInstruction].Camera : null; @@ -130,21 +128,11 @@ protected override CinemachineVirtualCameraBase ChooseCurrentCamera(Vector3 worl protected override CinemachineBlendDefinition LookupBlend( ICinemachineCamera outgoing, ICinemachineCamera incoming) => Instructions[m_CurrentInstruction].Blend; - /// Internal API for the inspector editor. - /// // GML todo: make this private, part of UpdateCameraCache() - internal void ValidateInstructions() + /// + protected override bool UpdateCameraCache() { Instructions ??= new (); - for (var i = 0; i < Instructions.Count; ++i) - { - if (Instructions[i].Camera != null - && Instructions[i].Camera.transform.parent != transform) - { - var e = Instructions[i]; - e.Camera = null; - Instructions[i] = e; - } - } + return base.UpdateCameraCache(); } void AdvanceCurrentInstruction(float deltaTime) diff --git a/com.unity.cinemachine/Runtime/Behaviours/CinemachineStateDrivenCamera.cs b/com.unity.cinemachine/Runtime/Behaviours/CinemachineStateDrivenCamera.cs index d7e1eef4b..a02b84410 100644 --- a/com.unity.cinemachine/Runtime/Behaviours/CinemachineStateDrivenCamera.cs +++ b/com.unity.cinemachine/Runtime/Behaviours/CinemachineStateDrivenCamera.cs @@ -81,7 +81,14 @@ internal struct ParentHash public int HashOfParent; } /// Internal API for the Inspector editor - [HideInInspector][SerializeField] internal ParentHash[] HashOfParent = null; + [HideInInspector][SerializeField] private List HashOfParent = new(); + + /// Internal API for the Inspector editor + internal void SetParentHash(List list) + { + HashOfParent.Clear(); + HashOfParent.AddRange(list); + } [SerializeField, HideInInspector, FormerlySerializedAs("m_LookAt")] Transform m_LegacyLookAt; [SerializeField, HideInInspector, FormerlySerializedAs("m_Follow")] Transform m_LegacyFollow; @@ -166,11 +173,6 @@ internal void ValidateInstructions() m_InstructionDictionary = new Dictionary>(); for (int i = 0; i < Instructions.Length; ++i) { - if (Instructions[i].Camera != null - && Instructions[i].Camera.transform.parent != transform) - { - Instructions[i].Camera = null; - } if (!m_InstructionDictionary.TryGetValue(Instructions[i].FullHash, out var list)) { list = new List(); @@ -181,7 +183,7 @@ internal void ValidateInstructions() // Create the parent lookup m_StateParentLookup = new Dictionary(); - for (int i = 0; HashOfParent != null && i < HashOfParent.Length; ++i) + for (int i = 0; HashOfParent != null && i < HashOfParent.Count; ++i) m_StateParentLookup[HashOfParent[i].Hash] = HashOfParent[i].HashOfParent; // Zap the cached current instructions diff --git a/com.unity.cinemachine/Runtime/Debug/CinemachinePanelSettings.asset b/com.unity.cinemachine/Runtime/Debug/CinemachinePanelSettings.asset index bcff27b6f..e790b1f9f 100644 --- a/com.unity.cinemachine/Runtime/Debug/CinemachinePanelSettings.asset +++ b/com.unity.cinemachine/Runtime/Debug/CinemachinePanelSettings.asset @@ -12,8 +12,7 @@ MonoBehaviour: m_Script: {fileID: 19101, guid: 0000000000000000e000000000000000, type: 0} m_Name: CinemachinePanelSettings m_EditorClassIdentifier: - themeUss: {fileID: -4733365628477956816, guid: df9aec944f4474a04847b6d6375e4138, - type: 3} + themeUss: {fileID: -4733365628477956816, guid: df9aec944f4474a04847b6d6375e4138, type: 3} m_TargetTexture: {fileID: 0} m_RenderMode: 0 m_WorldSpaceLayer: 0 diff --git a/com.unity.cinemachine/Runtime/Deprecated/AxisState.cs b/com.unity.cinemachine/Runtime/Deprecated/AxisState.cs index 03341bb7d..ad583566b 100644 --- a/com.unity.cinemachine/Runtime/Deprecated/AxisState.cs +++ b/com.unity.cinemachine/Runtime/Deprecated/AxisState.cs @@ -427,10 +427,7 @@ public void CancelRecentering() } /// Skip the wait time and start recentering now (only if enabled). - public void RecenterNow() - { - mLastAxisInputTime = 0; - } + public void RecenterNow() => mLastAxisInputTime = -1; /// Bring the axis back to the centered state (only if enabled). /// The axis to recenter @@ -461,7 +458,7 @@ public void DoRecentering(ref AxisState axis, float deltaTime, float recenterTar if (delta == 0) return; - if (Time.realtimeSinceStartup < (mLastAxisInputTime + m_WaitTime)) + if (mLastAxisInputTime >= 0 && Time.realtimeSinceStartup < (mLastAxisInputTime + m_WaitTime)) return; // Determine the direction From 427f203106ab05b21ffc471fd48bc45df1c7563a Mon Sep 17 00:00:00 2001 From: Gregory Labute Date: Mon, 3 Jun 2024 10:04:43 -0400 Subject: [PATCH 2/4] cosmetic --- .../Editor/SaveDuringPlay/SaveDuringPlay.cs | 23 +++++++++---------- .../Shared Assets/Scripts/AimCameraRig.cs | 2 +- .../Scripts/CursorLockManager.cs | 2 +- .../Shared Assets/Scripts/FlyAround.cs | 2 +- .../Scripts/SimpleCarController.cs | 2 +- .../Scripts/SimplePlayerAimController.cs | 2 +- .../Scripts/SimplePlayerController.cs | 2 +- .../Scripts/SimplePlayerShoot.cs | 2 +- 8 files changed, 18 insertions(+), 19 deletions(-) diff --git a/com.unity.cinemachine/Editor/SaveDuringPlay/SaveDuringPlay.cs b/com.unity.cinemachine/Editor/SaveDuringPlay/SaveDuringPlay.cs index 93b222552..98d7fb9f1 100644 --- a/com.unity.cinemachine/Editor/SaveDuringPlay/SaveDuringPlay.cs +++ b/com.unity.cinemachine/Editor/SaveDuringPlay/SaveDuringPlay.cs @@ -303,11 +303,10 @@ public bool ScanFields(GameObject go, string prefix = null) /// class ObjectStateSaver { - string mObjectFullPath; + string m_ObjectFullPath; + readonly Dictionary m_Values = new (); - Dictionary mValues = new Dictionary(); - - public string ObjetFullPath => mObjectFullPath; + public string ObjetFullPath => m_ObjectFullPath; /// /// Recursively collect all the field values in the MonoBehaviours @@ -316,15 +315,15 @@ class ObjectStateSaver /// public void CollectFieldValues(GameObject go) { - mObjectFullPath = ObjectTreeUtil.GetFullName(go); + m_ObjectFullPath = ObjectTreeUtil.GetFullName(go); GameObjectFieldScanner scanner = new (); scanner.FilterField = FilterField; scanner.FilterComponent = HasSaveDuringPlay; scanner.OnLeafField = (string fullName, Type type, ref object value) => { // Save the value in the dictionary - mValues[fullName] = StringFromLeafObject(value); - //Debug.Log(mObjectFullPath + "." + fullName + " = " + mValues[fullName]); + m_Values[fullName] = StringFromLeafObject(value); + //Debug.Log(m_ObjectFullPath + "." + fullName + " = " + m_Values[fullName]); return false; }; scanner.ScanFields(go); @@ -332,7 +331,7 @@ public void CollectFieldValues(GameObject go) public GameObject FindSavedGameObject(List roots) { - return ObjectTreeUtil.FindObjectFromFullName(mObjectFullPath, roots); + return ObjectTreeUtil.FindObjectFromFullName(m_ObjectFullPath, roots); } /// @@ -350,11 +349,11 @@ public bool PutFieldValues(GameObject go, List roots) scanner.OnLeafField = (string fullName, Type type, ref object value) => { // Lookup the value in the dictionary - if (mValues.TryGetValue(fullName, out string savedValue) + if (m_Values.TryGetValue(fullName, out string savedValue) && StringFromLeafObject(value) != savedValue) { - //Debug.Log("Put " + mObjectFullPath + "." + fullName + " = " + mValues[fullName] + " --- was " + StringFromLeafObject(value)); - value = LeafObjectFromString(type, mValues[fullName].Trim(), roots); + //Debug.Log("Put " + m_ObjectFullPath + "." + fullName + " = " + m_Values[fullName] + " --- was " + StringFromLeafObject(value)); + value = LeafObjectFromString(type, m_Values[fullName].Trim(), roots); return true; // changed } return false; @@ -588,7 +587,7 @@ static void RestoreAllInterestingStates() { var name = saver.ObjetFullPath; if (name[0] == '/') - name = name.Substring(1); + name = name[1..]; savedObjects += name + "\n"; } EditorUtility.SetDirty(go); diff --git a/com.unity.cinemachine/Samples~/Shared Assets/Scripts/AimCameraRig.cs b/com.unity.cinemachine/Samples~/Shared Assets/Scripts/AimCameraRig.cs index 33490cfde..7c9f3e7a7 100644 --- a/com.unity.cinemachine/Samples~/Shared Assets/Scripts/AimCameraRig.cs +++ b/com.unity.cinemachine/Samples~/Shared Assets/Scripts/AimCameraRig.cs @@ -13,7 +13,7 @@ namespace Unity.Cinemachine.Samples /// player rotation. /// [ExecuteAlways] - public class AimCameraRig : CinemachineCameraManagerBase, IInputAxisOwner + public class AimCameraRig : CinemachineCameraManagerBase, Unity.Cinemachine.IInputAxisOwner { public InputAxis AimMode = InputAxis.DefaultMomentary; diff --git a/com.unity.cinemachine/Samples~/Shared Assets/Scripts/CursorLockManager.cs b/com.unity.cinemachine/Samples~/Shared Assets/Scripts/CursorLockManager.cs index 68f42d1e0..3fb839a7e 100644 --- a/com.unity.cinemachine/Samples~/Shared Assets/Scripts/CursorLockManager.cs +++ b/com.unity.cinemachine/Samples~/Shared Assets/Scripts/CursorLockManager.cs @@ -4,7 +4,7 @@ namespace Unity.Cinemachine.Samples { - public class CursorLockManager : MonoBehaviour, IInputAxisOwner + public class CursorLockManager : MonoBehaviour, Unity.Cinemachine.IInputAxisOwner { public InputAxis CursorLock = InputAxis.DefaultMomentary; diff --git a/com.unity.cinemachine/Samples~/Shared Assets/Scripts/FlyAround.cs b/com.unity.cinemachine/Samples~/Shared Assets/Scripts/FlyAround.cs index cdd8a0a13..37b9f8eb8 100644 --- a/com.unity.cinemachine/Samples~/Shared Assets/Scripts/FlyAround.cs +++ b/com.unity.cinemachine/Samples~/Shared Assets/Scripts/FlyAround.cs @@ -8,7 +8,7 @@ namespace Unity.Cinemachine.Samples /// Movement is relative to the GameObject's local axes. /// /// - public class FlyAround : MonoBehaviour, IInputAxisOwner + public class FlyAround : MonoBehaviour, Unity.Cinemachine.IInputAxisOwner { [Tooltip("Speed when moving")] public float Speed = 10; diff --git a/com.unity.cinemachine/Samples~/Shared Assets/Scripts/SimpleCarController.cs b/com.unity.cinemachine/Samples~/Shared Assets/Scripts/SimpleCarController.cs index d8832d218..37af39db5 100644 --- a/com.unity.cinemachine/Samples~/Shared Assets/Scripts/SimpleCarController.cs +++ b/com.unity.cinemachine/Samples~/Shared Assets/Scripts/SimpleCarController.cs @@ -3,7 +3,7 @@ namespace Unity.Cinemachine.Samples { - public class SimpleCarController : MonoBehaviour, IInputAxisOwner + public class SimpleCarController : MonoBehaviour, Unity.Cinemachine.IInputAxisOwner { public float MotorStrength = 2000; public float BrakeStrength = 5000; diff --git a/com.unity.cinemachine/Samples~/Shared Assets/Scripts/SimplePlayerAimController.cs b/com.unity.cinemachine/Samples~/Shared Assets/Scripts/SimplePlayerAimController.cs index 203dbc19b..ac6f99453 100644 --- a/com.unity.cinemachine/Samples~/Shared Assets/Scripts/SimplePlayerAimController.cs +++ b/com.unity.cinemachine/Samples~/Shared Assets/Scripts/SimplePlayerAimController.cs @@ -20,7 +20,7 @@ namespace Unity.Cinemachine.Samples /// /// To implement player shooting, add a SimplePlayerShoot behaviour to this GameObject. /// - public class SimplePlayerAimController : MonoBehaviour, IInputAxisOwner + public class SimplePlayerAimController : MonoBehaviour, Unity.Cinemachine.IInputAxisOwner { public enum CouplingMode { Coupled, CoupledWhenMoving, Decoupled } diff --git a/com.unity.cinemachine/Samples~/Shared Assets/Scripts/SimplePlayerController.cs b/com.unity.cinemachine/Samples~/Shared Assets/Scripts/SimplePlayerController.cs index 8e8f7331f..5191ebc9a 100644 --- a/com.unity.cinemachine/Samples~/Shared Assets/Scripts/SimplePlayerController.cs +++ b/com.unity.cinemachine/Samples~/Shared Assets/Scripts/SimplePlayerController.cs @@ -5,7 +5,7 @@ namespace Unity.Cinemachine.Samples { - public abstract class SimplePlayerControllerBase : MonoBehaviour, IInputAxisOwner + public abstract class SimplePlayerControllerBase : MonoBehaviour, Unity.Cinemachine.IInputAxisOwner { [Tooltip("Ground speed when walking")] public float Speed = 1f; diff --git a/com.unity.cinemachine/Samples~/Shared Assets/Scripts/SimplePlayerShoot.cs b/com.unity.cinemachine/Samples~/Shared Assets/Scripts/SimplePlayerShoot.cs index 798e6af9a..6acbb5542 100644 --- a/com.unity.cinemachine/Samples~/Shared Assets/Scripts/SimplePlayerShoot.cs +++ b/com.unity.cinemachine/Samples~/Shared Assets/Scripts/SimplePlayerShoot.cs @@ -13,7 +13,7 @@ namespace Unity.Cinemachine.Samples /// or of the SimplePlayerAimController object if it exists and is not decoupled /// from the player rotation. /// - class SimplePlayerShoot : MonoBehaviour, IInputAxisOwner + class SimplePlayerShoot : MonoBehaviour, Unity.Cinemachine.IInputAxisOwner { [Tooltip("The bullet prefab to instantiate when firing")] public GameObject BulletPrefab; From b1b718eeab139bb2ca0ae66ac497d44a30e2c459 Mon Sep 17 00:00:00 2001 From: Gregory Labute Date: Mon, 3 Jun 2024 11:29:33 -0400 Subject: [PATCH 3/4] More robust sample: don't lock the cursor when exiting play mode --- .../Samples~/Shared Assets/Scripts/CursorLockManager.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/com.unity.cinemachine/Samples~/Shared Assets/Scripts/CursorLockManager.cs b/com.unity.cinemachine/Samples~/Shared Assets/Scripts/CursorLockManager.cs index 3fb839a7e..b7225a61e 100644 --- a/com.unity.cinemachine/Samples~/Shared Assets/Scripts/CursorLockManager.cs +++ b/com.unity.cinemachine/Samples~/Shared Assets/Scripts/CursorLockManager.cs @@ -23,6 +23,7 @@ public void GetInputAxes(List axes) } void OnEnable() => UnlockCursor(); + void OnDisable() => UnlockCursor(); void Update() { @@ -40,8 +41,11 @@ void Update() public void LockCursor() { - Cursor.lockState = CursorLockMode.Locked; - OnCursorLocked.Invoke(); + if (enabled) + { + Cursor.lockState = CursorLockMode.Locked; + OnCursorLocked.Invoke(); + } } public void UnlockCursor() From 99eed588b0475e751ff1065e2b392675385d1cde Mon Sep 17 00:00:00 2001 From: Gregory Labute Date: Mon, 3 Jun 2024 12:48:19 -0400 Subject: [PATCH 4/4] fix broken icons --- com.unity.cinemachine/Editor/Editors/CinemachineBrainEditor.cs | 2 +- com.unity.cinemachine/Editor/Windows/CinemachineSettings.cs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/com.unity.cinemachine/Editor/Editors/CinemachineBrainEditor.cs b/com.unity.cinemachine/Editor/Editors/CinemachineBrainEditor.cs index a7062a8ee..5c3011061 100644 --- a/com.unity.cinemachine/Editor/Editors/CinemachineBrainEditor.cs +++ b/com.unity.cinemachine/Editor/Editors/CinemachineBrainEditor.cs @@ -65,7 +65,7 @@ static void DrawBrainGizmos(CinemachineBrain brain, GizmoType drawType) [DrawGizmo(GizmoType.Active | GizmoType.InSelectionHierarchy | GizmoType.Pickable, typeof(CinemachineVirtualCameraBase))] public static void DrawVirtualCameraBaseGizmos(CinemachineVirtualCameraBase vcam, GizmoType selectionType) { - const string kGizmoFileName = CinemachineCore.kPackageRoot + "/Editor/EditorResources/Icons/CmCamera@256.png"; + const string kGizmoFileName = CinemachineCore.kPackageRoot + "/Editor/EditorResources/Icons/Light/CmCamera@256.png"; // Don't draw gizmos on hidden stuff if ((vcam.gameObject.hideFlags & (HideFlags.HideInHierarchy | HideFlags.HideInInspector)) != 0) diff --git a/com.unity.cinemachine/Editor/Windows/CinemachineSettings.cs b/com.unity.cinemachine/Editor/Windows/CinemachineSettings.cs index 50813a83c..5e066f8ce 100644 --- a/com.unity.cinemachine/Editor/Windows/CinemachineSettings.cs +++ b/com.unity.cinemachine/Editor/Windows/CinemachineSettings.cs @@ -141,8 +141,7 @@ public static Texture2D CinemachineLogoTexture get { if (s_CinemachineLogoTexture == null) - s_CinemachineLogoTexture = AssetDatabase.LoadAssetAtPath( - $"{CinemachineCore.kPackageRoot}/Editor/EditorResources/Icons/CmCamera@256.png"); + s_CinemachineLogoTexture = AssetDatabase.LoadAssetAtPath($"{CinemachineSceneToolHelpers.IconPath}/CmCamera@256.png"); if (s_CinemachineLogoTexture != null) s_CinemachineLogoTexture.hideFlags = HideFlags.DontSaveInEditor; return s_CinemachineLogoTexture;