[UUM-137643] Fix compilation errors and warnings

[mfe]

Purpose of this PR

Ticket/Jira #: UUM-137643

This PR address 3 issues:

1 - Compilation errors in the console

error CS0619: 'Object.GetInstanceID()' is obsolete: 'GetInstanceID is deprecated. Use GetEntityId instead.

2 - Warnings (that will become errors in a foreseeable futur):

warning CS0618: 'FindObjectsSortMode' is obsolete: 'FindObjectsSortMode has been deprecated. Use the FindObjectsByType overloads that do not take a FindObjectsSortMode parameter.'
⚠️ In previous versions of the code, a sorting by FindObjectsSortMode.instanceID was used to have the determinism needed for UpdateCaptureNodes. But that was an inconsistant way of sorting. Furthermore, with entityID, the small amount of creation-order semantics that were accidentally present in instanceID is removed. That's why this PR instroduces another way of sorting with determinism (cf SortComponentsByStableSceneHierarchy).

3- replace renderPipelineAsset by defaultRenderPipeline

defautRenderPipeline was introduced in 2019 stream so that was due.

A general mindset for this PR is the less we touch, the better.
The reason is that this code is very legacy and in basic support. We want to mitigate the risk of breaking the past versions and increase our support load.
That's why every changes needed were encapsulated in #if UNITY_6000_4_OR_NEWER...#endif even when the changes would be considered a better practice for precedent versions too (like not sorting with instanceIds).

Testing

Functional Testing status:

Performance Testing status:

Overall Product Risks

Complexity:

Halo Effect:

Documentation & UX Writing

User facing text to review		Details
User interface
Public API docs
Changelog

Documentation halo effect		Jira link
User manual
Other docs

Additional information

Note to reviewers:

Reminder:

• Add entry in CHANGELOG.md
• check defaultRenderPipeline availability - Was introduced as early as 2019.4 so before our minimal supported version (21.3)
• Wait for Recorder 5.1.6 to be released (see UUM-137641 - Cf Recorder tests
• Sanity check QA - test import + export both for local and streamed assets with depth

[LeoUnity]

shouldn't we also remove this and upgrade the minimum unity version to 6000.0?

Same with romotion_test_editors

[mfe]

It's the minimum version of the package, so we have to keep the job for promotion.

[LeoUnity]

I believe its allow to bump the minimum version of the package if said version is not supported anymore without being considered a new major release. But if that would increase the scope of this PR, we can do it separately

[LeoUnity]

Changes looks good to me, we do have a test failure on win trunk unfortunately.

[mfe]

The FindObjectsSortMode.InstanceID used in UNITY_2023_1_OR_NEWER code path is not needed as the result of GetTargets is only used with Any(), so I just removed the sorting mode here.

I was tempted to remove it in UNITY_2023_1_OR_NEWER code path too but on a very legacy package in basic support, the less we touch, the better.
Feel free to push back.

[mfe]

The native function called at the end of the line is aiContextCreate(int uid). So if we keep using a ulong here, we ultimately have to cast it, which can lead to some narrowing and rare but potential collisions.
But in the code where the id is used, there is no need for a correlation between the id and the abcTreeRoot object, we just need an id to destroy on dispose (cf aiContextDestroy(aiContext* ctx)).

Thus the introduction of an alternative surrogate id.

A question for reviewers: what is the bigger risk ?
1- Changing this code
2- Narrowing and collision
😅

[LeoUnity]

I think this approach you took is great!

[mfe]

Explaination about why we need this:
⚠️ In previous versions of the code, a sorting by FindObjectsSortMode.instanceID was used to have the determinism needed for UpdateCaptureNodes. But that was an inconsistant way of sorting. Furthermore, with entityID, the small amount of creation-order semantics that was accidentally present in instanceID is removed. That's why this PR instroduces another way of sorting with determinism,

[LeoUnity]

A concern that jumped out to me is performance.
When and how often is this sorting taking place? We are doing a lot more computations including string operations to get this determinism.

Also out of curiosity why is determinism important here? What would it mean not having determinism?

[LeoUnity]

Should/could this be uint? we are leaving half of its range on the table