Fix obsolete warning when using instance ID in probuilder

[varinotmUnity]

DO NOT FORGET TO INCLUDE A CHANGELOG ENTRY

Purpose of this PR

Warning appear when opening probuilder projects. This was fixed by adding entityId instead of instanceID. to make sure old project worked as well, we used #IF UNITY_6000_4_Or_NEWWER

Links

https://jira.unity3d.com/browse/PBLD-284

Comments to Reviewers

N/A

[u-pr-agent]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ PBLD-284 - PR Code Verified Compliant requirements: Replaced obsolete 'instanceId' with 'entityId' for newer Unity versions. Implemented version-dependent logic using preprocessor directives. Requires further human verification: Verify if the warning appears in Unity versions between 6000.0 and 6000.4. If so, the current fix (which falls back to the old code for < 6000.4) will still show the warning on those versions unless suppressed.

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪ The PR is a targeted fix for an API deprecation with clear scope, though version handling requires verification.

🏅 Score: 85 The PR addresses the issue but might leave the warning active on intermediate Unity 6 versions if the define is too conservative and no suppression is added to the fallback path.

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Potential Unfixed Warning on Intermediate Versions If the warning exists in Unity 6000.0 but entityId is only available in 6000.4 (as implied by the define), the #else block will execute for versions 6000.0-6000.3. Since there is no #pragma warning disable in the #else block for data.instanceId, the warning will persist on those versions. If entityId is available in 6000.0, the define should be changed to UNITY_6000_0_OR_NEWER. #else GameObjectCreatedOrStructureModified(data.instanceId); #endif

Update review

---

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

[u-pr-agent]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Ensure custom version define exists in asmdef Verify that the UNITY_6000_4_OR_NEWER symbol is properly defined in the project's Assembly Definition (asmdef) "Version Defines". Unity does not automatically generate patch-level preprocessor directives (like 6000.0.4), so this symbol must be manually configured to ensure the new code path is active. Editor/EditorCore/HierarchyListener.cs [31-35] +// Requires 'UNITY_6000_4_OR_NEWER' in asmdef Version Defines #if UNITY_6000_4_OR_NEWER GameObjectCreatedOrStructureModified(data.entityId); #else GameObjectCreatedOrStructureModified(data.instanceId); #endif Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that UNITY_6000_4_OR_NEWER is likely a custom define requiring configuration in the Assembly Definition file, as Unity does not provide patch-level defines by default. Without this configuration, the fix for the obsolescence warning will not be active.	Medium
More suggestions

---

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

[codecov-github-com]

Codecov Report

Attention: Patch coverage is 71.42857% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
Editor/EditorCore/HierarchyListener.cs	71.42%	6 Missing ⚠️

@@            Coverage Diff             @@
##           master     #643      +/-   ##
==========================================
- Coverage   35.57%   35.57%   -0.01%     
==========================================
  Files         277      277              
  Lines       34892    34898       +6     
==========================================
+ Hits        12413    12415       +2     
- Misses      22479    22483       +4     

Flag	Coverage Δ
mac_trunk	35.34% <71.42%> (-0.01%)	⬇️
win_trunk	35.23% <71.42%> (-0.01%)	⬇️

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

Files with missing lines	Coverage Δ
Editor/EditorCore/HierarchyListener.cs	63.85% <71.42%> (-2.38%)	⬇️

🚀 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.

[jmcduff-unity]

Hmm it feels a bit of shame that we're checking again for the ObjectChangeKind here when that was already done in ObjectEventChangesPublished(). Instead of passing the stream, index and kind as params, couldn't you just pass the data itself? And have the variants of stream.Get[bla bla]Event() be called in the if checks inside ObjectEventChangesPublished()?

[varinotmUnity]

I considered extracting the event data inside the main loop to avoid the double switch. However, because the data structs (CreateGameObjectHierarchyEventArgs) have different property names in Unity 6000.4 (entityId vs instanceId), doing so would require peppering the main loop with #if UNITY_6000_4_OR_NEWER blocks for every single event type to access the correct ID field.

I chose to encapsulate the switch and the #if logic inside GetGameObjectFromEvent. It does check the kind twice, but it keeps the main logic loop readable and free of preprocessor directives. I felt readability outweighed the negligible cost of the extra switch statement here.

[jmcduff-unity]

But does the type of data itself differ? Or is it still CreateGameObjectHierarchyEventArgs in both cases? Because if that type doesn't change, can't you just do:

private static GameObject GetGameObjectFromEvent(CreateGameObjectHierarchyEventArgs data) 
{
#if UNITY_6000_4_OR_NEWER
     return UnityEditor.EditorUtility.EntityIdToObject(data.entityId) as GameObject;
#else
     var instanceId = data.instanceId
     #pragma warning disable CS0618 // Type or member is obsolete
				return UnityEditor.EditorUtility.InstanceIDToObject(instanceId) as GameObject;
	 #pragma warning restore CS0618
#endif
}

Although now that I think about it, I guess each event has a different args type... BUT if all these args types share a common parent/interface that has the entityId/instanceId than I think this would be a more concise approach?