Bugfix/pbld 226 hidden face selection fix #615
Conversation
|
|
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## master #615 +/- ##
==========================================
+ Coverage 32.61% 33.05% +0.43%
==========================================
Files 277 277
Lines 34707 34736 +29
==========================================
+ Hits 11321 11483 +162
+ Misses 23386 23253 -133
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 4 files with indirect coverage changes 🚀 New features to boost your workflow:
|
| if (next != 0) | ||
| break; | ||
| // Update the newHash for the element actually picked after cycling/filtering | ||
| int newHash = selectedHit.hash; |
There was a problem hiding this comment.
nitpick: Couldn't you just combine this line with line 516?
| { | ||
| Vector3 centerLocal = Vector3.zero; | ||
| if (face.indexesInternal == null || face.indexesInternal.Length == 0) | ||
| return Vector3.zero; // Should not happen for valid ProBuilder faces |
There was a problem hiding this comment.
If this should never happen, is it worth making it into an assert instead?
There was a problem hiding this comment.
I will use Assume instead, as this is not what we want to test
| else | ||
| { | ||
| Debug.LogError($"Invalid vertex index {index} found in face indexes."); | ||
| return Vector3.zero; // Or handle error appropriately |
There was a problem hiding this comment.
Same here - make it in to an assert?
| } | ||
| else | ||
| { | ||
| return Mathf.Infinity; // Nothing at all hit | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we should probably clear s_PbHits at this point, shouldn't we? Otherwise references to meshes and faces may hang around longer that we want them to.
Purpose of this PR
This PR resolves a bug where single-click face picking on ProBuilder meshes failed if they were obscured by another GameObject, even when "Select Hidden" was enabled. The previous FaceRaycast implementation's internal next variable and looping logic caused it to often only consider the very first GameObject returned by the scene picker for a standard single click. This was problematic because if that initial GameObject was not a ProBuilder mesh, the ProBuilder picking routine would prematurely exit, preventing selection of the obscured ProBuilder face. The corrected approach now first collects all intersecting ProBuilder meshes and their hit faces under the mouse cursor. These ProBuilder hits are then internally sorted by distance, and the deep-click cycling logic is correctly applied exclusively to this filtered list of ProBuilder elements, ensuring reliable selection and improved interaction with obscured geometry.
Links
Jira:
Comments to Reviewers
Added unit tests