Stop spamming Look Rotation log when holding shift

[jmcduff-unity]

Purpose of this PR

This PR fixes a log spam that happens when holding shift while using a create tool such as Create Sprite. Repro happens consistently on a freshly opened editor. The spam stops once you've done a shift + click (which will duplicate the last created mesh).

Before:

After:

Links

Jira: UUM-133861

Comments to Reviewers

The issue seems to be that until you've duplicated at least once, the m_PlaneForward and m_Plane fields have not yet been initialized. Holding shift however triggers down the line a call to Quaternion.LookRotation(m_PlaneForward, m_Plane.normal), which requires a vector with a non-zero magnitude. m_PlaneForward being uninitialized, its magnitude is 0 and LookRotation() will send out a log (this part is a bit weird; why a log? why not a warning or an error? ¯\ _ (ツ) _ /¯ ). However, it also defaults to returning a default value of Quaternion.identity.

The chosen fix was to simply add a check before calling LookRotation to see if m_PlaneFoward has a zero magnitude, and if so, to use Quaternion.identity. This allows to keep the current behaviour as it is except the log spam. There is probably a better solution, however when testing the feature, I have not found that the shift feature was working incorrectly (except for the log spam). As such, I would rather keep the risk of regression as low as possible by keeping the current behaviour largely unchanged.

[cla-assistant-unity]

All committers have signed the CLA.

[u-pr]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ UUM-133861 - Fully compliant Compliant requirements: Stop the "Look rotation viewing vector is zero" console log spam. Ensure the fix addresses the issue when using the ProBuilder Create Sprite tool and holding Shift in 2D view.

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪ The PR involves a localized fix in a single file with simple logic to handle uninitialized vectors, making it very straightforward to review.

🏅 Score: 95 The PR effectively resolves the reported issue with a safe and logical fix, reusing the calculated rotation to avoid redundant calls, though a minor optimization for the zero check is possible.

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Performance Optimization Calculating magnitude requires a square root operation. Since this check is performed inside a GUI/Repaint loop, it is more efficient to use m_PlaneForward.sqrMagnitude (or check against Vector3.zero) to avoid the square root calculation when checking for zero. m_PlaneRotation = m_PlaneForward.magnitude == 0 ? Quaternion.identity : Quaternion.LookRotation(m_PlaneForward, m_Plane.normal);

Update review

---

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr

[jmcduff-unity]

I'm not sure if I added the changelog entry in the right place, let me know if it needs to be moved 😅

[u-pr]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Use Vector3.zero for robust check Use m_PlaneForward == Vector3.zero instead of magnitude == 0. The Vector3 equality operator includes a small epsilon check, which is more robust for detecting near-zero vectors that can still trigger the LookRotation error. This approach also avoids the inefficient square root calculation associated with magnitude. Editor/EditorCore/DrawShapeTool.cs [871] -m_PlaneRotation = m_PlaneForward.magnitude == 0 ? Quaternion.identity : Quaternion.LookRotation(m_PlaneForward, m_Plane.normal); +m_PlaneRotation = m_PlaneForward == Vector3.zero ? Quaternion.identity : Quaternion.LookRotation(m_PlaneForward, m_Plane.normal); Suggestion importance[1-10]: 8 __ Why: Comparing magnitude directly to zero is inefficient (requires square root) and unsafe for floating-point arithmetic. Using m_PlaneForward == Vector3.zero leverages Unity's built-in epsilon check, which correctly handles near-zero vectors that could still trigger the LookRotation error, and is more performant.	Medium
More suggestions

---

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr

[codecov-github-com]

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
Editor/EditorCore/DrawShapeTool.cs	0.00%	2 Missing ⚠️

@@           Coverage Diff           @@
##           master     #653   +/-   ##
=======================================
  Coverage   36.06%   36.06%           
=======================================
  Files         277      277           
  Lines       34895    34906   +11     
=======================================
+ Hits        12584    12588    +4     
- Misses      22311    22318    +7     

Flag	Coverage Δ
probuilder_MacOS_6000.0	34.58% <0.00%> (-0.76%)	⬇️
probuilder_MacOS_6000.3	34.58% <0.00%> (-0.77%)	⬇️
probuilder_MacOS_6000.4	34.58% <0.00%> (-0.77%)	⬇️
probuilder_MacOS_6000.5	34.57% <0.00%> (-0.77%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
Editor/EditorCore/DrawShapeTool.cs	19.31% <0.00%> (ø)

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.