Skip to content
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

CMCL-0000: Camera selection bugfixes and various tweaks #1014

Merged
merged 9 commits into from
Jul 30, 2024

Conversation

glabute
Copy link
Collaborator

@glabute glabute commented Jul 5, 2024

Purpose of this PR

This is a bag with a bunch of small things.

A. Undo problem with child camera selectors in Sequencer and State Driven Camera:

  1. Add some child cameras and some instructions
  2. select the child cameras in the instructions
  3. delete the child cameras
  4. press undo/redo: the selectors don't update properly on redo

image

image

image

B. Add CinemachineVirtualCameraBaseEditor as default editors for CInemachineVirtualCameraBase, and make our custom inspectors inherit from it. This improves the quality of inspectors for client-authored cameras and managers (e.g. see the improved inspector for the custom CameraRig in the ThirdPersonWithAimMode sample).

image

C. Improved inspectors and user manual for CinemachineSplineRoll and LookAtTargetsOnSpline

D. Minor layout tweaks in inspectors: spacings and column alignment, mostly.

E. Added concept of State to SimplePlayerAnimator.cs in the sample code. This is not currently used, but is just future-proofing.

F: Minor tweaking of blend times (into and out of jumping) in the animation controller. This is part of the sample assets.

Testing status

  • Added an automated test
  • Passed all automated tests
  • Manually tested

Documentation status

  • Updated CHANGELOG
  • Updated README (if applicable)
  • Commented all public classes, properties, and methods
  • Updated user documentation

Technical risk

low

@glabute glabute requested a review from AntoineCharton July 5, 2024 17:16
@glabute glabute requested a review from sebastienduverne as a code owner July 5, 2024 17:16
@codecov-commenter
Copy link

codecov-commenter commented Jul 5, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 3.65854% with 79 lines in your changes missing coverage. Please review.

Project coverage is 26.96%. Comparing base (f38e9bd) to head (e5a39a9).
Report is 4 commits behind head on main.

Files Patch % Lines
...itor/Editors/CinemachineVirtualCameraBaseEditor.cs 0.00% 22 Missing ⚠️
...hine/Editor/Utility/CinemachineSceneToolHelpers.cs 0.00% 19 Missing ⚠️
...ditor/PropertyDrawers/ChildCameraPropertyDrawer.cs 0.00% 14 Missing ⚠️
...itor/Editors/CinemachineStateDrivenCameraEditor.cs 0.00% 6 Missing ⚠️
...Editor/Editors/CinemachineSequencerCameraEditor.cs 0.00% 4 Missing ⚠️
...itors/CinemachineSplineDollyLookAtTargetsEditor.cs 0.00% 4 Missing ⚠️
...hine/Editor/Editors/CinemachineSplineRollEditor.cs 0.00% 4 Missing ⚠️
...emachine/Editor/Editors/CinemachineCameraEditor.cs 0.00% 1 Missing ⚠️
...chine/Editor/Editors/CinemachineClearShotEditor.cs 0.00% 1 Missing ⚠️
...machine/Editor/Utility/CmCameraInspectorUtility.cs 0.00% 1 Missing ⚠️
... and 3 more

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1014   +/-   ##
=======================================
  Coverage   26.96%   26.96%           
=======================================
  Files         254      254           
  Lines       28419    28425    +6     
=======================================
+ Hits         7664     7666    +2     
- Misses      20755    20759    +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@glabute glabute requested a review from eakesy July 19, 2024 17:25
Copy link

@eakesy eakesy left a comment

Choose a reason for hiding this comment

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

A. Reproduced issue on main branch with Sequencer and State Driven components. Confirmed fix for issues on both components. No additional issues found.
B. Compared CameraRig component the Inspector. Changes confirmed. No issues found.
C. Compared CinemachineSplineRoll and LookAtTargetsOnSpline components in the Inspector. Changes confirmed. No issues found.
D. Compared Cinemachine Camera component in the Inspector. No visible change. No issues found.
E. Verified all 3D Samples. Work as expected. No regression found.
F. Compared jumping in place, walking & running animations. No noticeable change. Jump transition looks smooth to me.
Conclusion: no issues found, tested all 3D Samples. Approved!

@glabute glabute merged commit 09068c8 into main Jul 30, 2024
11 checks passed
@glabute glabute deleted the dev/add-CinemachineVirtualCameraBaseEditor branch July 30, 2024 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants