chore: upgrade to Unity 6000.5.4f1#9392
Conversation
decentraland-bot
left a comment
There was a problem hiding this comment.
PR Review — chore: upgrade to Unity 6000.5.2f1
STEP 2 — Root-cause check: ✅ PASS
The PR fixes two distinct breakages introduced by Unity 6000.5:
-
Physics.BakeMeshAPI change — Unity 6000.5 replaced theintinstance-ID parameter withEntityId. The diff migrates all three call sites (Utils.cs,BakeColliderMeshes.cs,CollideTerrainSystem.cs) to the newGetEntityId()API and updates theNativeArray<int>typing toNativeArray<EntityId>. This is a correct root-cause fix — the old APIs were removed, and the new ones are the documented replacements. -
MetricsRegistrystatic-constructor crash —IsAssignableFromcan force-load type definitions that 6000.5's Mono can't resolve, throwing from the static initializer and taking down the entire request-metrics subsystem. The fix wraps the call inIsAssignableSafewith a try/catch, mirroring the existingGetTypesSafelypattern in the same class. This is a correct defensive fix — the root cause is in Unity's Mono runtime (not controllable by this project), and the alternative (letting the static ctor throw) is strictly worse.
STEP 3 — Design & integration: ✅ PASS
No new long-lived units introduced. The only new code is IsAssignableSafe(Type), a private static helper method that extracts error handling from the existing LINQ predicate. No lifecycle management, no persistent state, no new subscriptions or connections.
The EntityId migration is consistent across all three call sites — Utils.AddMeshCollider, CollideTerrainSystem.Update, and BakeColliderMeshes.Execute all use the same API transition. EntityId is a blittable struct in Unity 6000.5, so the [BurstCompile] job and NativeArray<EntityId> remain compatible.
STEP 4 — Member audit: ✅ PASS
IsAssignableSafe(Type type) — private static, single consumer (the Where clause in the TYPES static field initializer). Justified as an error-handling extraction: it keeps the LINQ chain readable and makes the defensive try/catch visible. The existing GetTypesSafely in the same class follows the same "extract-for-safety" pattern.
STEP 5 — Line-level review
One P2 finding (see inline comment on MetricsRegistry.cs).
- Physics API migration (Utils.cs:44, BakeColliderMeshes.cs:11, CollideTerrainSystem.cs:140-144): Correct.
GetEntityId()is the 1:1 replacement for the removedGetInstanceID()in the physics mesh-baking context. TheNativeArray<EntityId>type change is consistent with theBakeColliderMeshesjob field. - LINQ in MetricsRegistry: Runs once in a static field initializer (not a hot path). No performance concern.
- No resource leaks: No new subscriptions, connections, or allocations without matching teardown.
- No nullability violations: No null assignments to non-nullable types.
Security review: ✅ PASS
- No secrets, credentials, or API keys in the diff
- No input validation changes
- No auth/authz modifications
- No sensitive data exposure
- The bare
catchinIsAssignableSafeis defensive (more restrictive, not less) — it cannot introduce a security vulnerability - The
EntityIdmigration is a type-system change with no security surface
STEP 6 — Complexity: SIMPLE
Mechanical API migration across 3 call sites + one defensive helper method. No ECS system logic, async flow, or architectural changes.
STEP 7 — QA: YES
Changes affect runtime physics collider baking (terrain + GLTF meshes) and the metrics registry. Author reports verified compilation and interactive play mode on 6000.5.2f1.
STEP 8 — Warnings
None. Main scene not modified.
STEP 9 — Verdict
REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: Mechanical Unity API migration (GetInstanceID → GetEntityId) across 3 call sites plus a defensive try/catch in a static initializer
QA_REQUIRED: YES
Reviewed by Jarvis 🤖 · Requested by mikhail-dcl via GitHub
| private static bool IsAssignableSafe(Type type) | ||
| { | ||
| try { return typeof(RequestMetricBase).IsAssignableFrom(type); } | ||
| catch { return false; } |
There was a problem hiding this comment.
[P2] Minor: bare catch could be narrowed for clarity. The bare catch silently swallows all exception types including non-CLS-compliant ones. While the behavior is correct here (any failure from IsAssignableFrom means the type is unusable, so false is the right answer), catch (Exception) is the conventional C# idiom and would be consistent with GetTypesSafely above which catches a specific exception type.
Tradeoff acknowledged: catching broadly is intentional since IsAssignableFrom can fail with various exception types (TypeLoadException, FileNotFoundException, FileLoadException, etc.), not just the one named in the XML doc. catch (Exception) is functionally equivalent for managed exceptions but signals intent more clearly.
| catch { return false; } | |
| catch (Exception) { return false; } |
|
Updated to Unity 6000.5.4f1 (was 6000.5.2f1) and verified in-editor: clean script compile (0 errors), clean shader compile, boots to the authentication screen with correctly shaded avatars and working UI Toolkit text. What 6000.5.3+ broke on top of .2f1 (all were warnings on .2f1, errors on .4f1):
A multi-agent audit of the full diff ran before the last commit; all confirmed findings are addressed (no silent package bumps, no editor churn in the diff, deterministic 6000.5 migration artifacts committed, test-only localization table kept out of shipped addressables — note Localization 1.5.12's sync may try to re-add it on editor open). Known cosmetic regression: Metal now logs Before merge: repoint the nine 🤖 Generated with Claude Code |
|
Second full audit + final fixes. A re-audit workflow (48 agents) reviewed the finalized upgrade; all confirmed findings addressed. Latest state: Since the last summary:
For-the-record notes from the audit (accepted, documenting so nothing is silent):
🤖 Generated with Claude Code |
46f0a60 to
6356baa
Compare
|
Landing sequence is now set up and cross-version verified:
Both dependency branches compile with zero errors on current dev (6000.4.0f1) and on this PR (6000.5.4f1) — they are safe to merge first without breaking anyone still on 6000.4. Two cross-version fixes were needed and are included: 🤖 Generated with Claude Code |
Full player-build verification (macOS)Went past editor compiles to real player builds, which compile shader variants (DOTS instancing) and run IL2CPP — neither of which an editor compile exercises. This surfaced several player-only breaks, now all fixed: 6000.5.4f1 — full IL2CPP player build ✅ SUCCEEDED, 0 script / 0 shader / 0 IL2CPP errors,
6000.4.0f1 — player build: dependency packages compile with 0 shader errors; the only failure was this repo's own pre-existing State
🤖 Generated with Claude Code |
|
✅ Windows/DirectX verified. Full 6000.5.4f1 StandaloneWindows64 player build on the Windows rig succeeded — 0 shader errors, 0 CS errors across 132k log lines, Explorer.exe produced. The DCL_Toon ForwardLit, DCL/Scene, and all DOTS-instancing variants compiled clean on d3d11. This closes the last platform gap: the upgrade's shader fixes now verified on both macOS/Metal and Windows/DirectX, across both 6000.4 and 6000.5 editors. |
Bump the editor and migrate the DCL code that hit APIs removed in 6000.5: - EntityId replaces the obsolete int instance-id APIs. Physics.BakeMesh now takes an EntityId (Utils, BakeColliderMeshes, CollideTerrainSystem), and the collider-bake job array is typed NativeArray<EntityId>. - MetricsRegistry guards its type scan: IsAssignableFrom can force-load a precompiled dependency type that 6000.5's Mono fails to resolve, which would otherwise throw from the static ctor and take request metrics down. Verified compiling and running on 6000.5.2f1 (editor boots, enters play mode, reaches interactive). Vendored/third-party packages (SoftMask, GPUI Pro, AVPro, UniTask, com.unity.localization, FileBrowserPro, renderfeatures) also need 6000.5 API fixes; those live outside this repo and are tracked separately. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVEaLqBhBbZTKnvveA48Rx
6000.5.3 escalated the InstanceID deprecations to hard errors and moved UnityEngine.XR types behind the XR module, which broke every third-party package. Changes on top of the 6000.5.2f1 migration: - ProjectVersion 6000.5.4f1; add com.unity.modules.xr (XR types are no longer reachable without it). - Point vendored packages (AVPro, GPUI Pro, RenderFeatures, FileBrowserPro) at unity-explorer-packages#fix/unity-6000.5.4-api-compat, which migrates them to the EntityId APIs. - Vendor AltTester 2.3.2, SerializeReferenceExtensions and AnimationSequencer on the same branch: no upstream Unity 6000.5 support exists yet. AltTester is now referenced by assembly name instead of the version-specific asmdef GUIDs. - SoftMask 3.5.0 -> 3.6.2 and UniTask lock bump; both fixed 6000.5 compatibility upstream. - Migrate remaining DCL GetInstanceID call sites: Physics.BakeMesh takes EntityId; object identity checks use reference equality. - Editor-migrated settings (PackageManager registry format, SoftMask shader registry, URP global settings, mppm version stamp). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Runtime fixes found by in-editor verification: - unity-shared-dependencies pinned to fix/unity-6000.5-shader-compat: URP 17.5's LitForwardPass.hlsl now self-includes LitInput.hlsl, which collided with DCL_Toon's own input file and broke the avatar toon shader (magenta avatars). Fixed upstream with guard-reuse. - Migrate the two UI Toolkit Inter font assets from static to dynamic atlas population (source .ttf restored): 6000.5's Advanced Text Generator cannot use static font assets, which blanked all UI Toolkit text. Baked atlases retained as the pre-populated starting set. - Point DebugUtilitiesStyle.uss at the real Inter Regular font asset; its previous reference targeted a theme asset that no longer exists. Audit cleanup (multi-agent review of the upgrade diff): - unity-explorer-packages branch rebuilt to vendor the previously pinned AltTester 2.3.0 / SerializeReferenceExtensions 1.3.1 / AnimationSequencer 0.5.4 (no silent version bumps) and to keep AltTester's int-based wire protocol on legacy instance-id values. - Revert the test-only localization table that the Localization 1.5.12 sync auto-added to the shipped addressables group. - Commit the deterministic 6000.5 migration artifacts (VFXManager version stamp, PhysicsCoreProjectSettings2D, ForwardRenderer hide flags, stopwatch meta normalization) so checkouts stay clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Asian fallback fonts (NotoSans SC/JP/KR) load asynchronously via
addressables in ChatPlugin. Any text generated before that load lands —
e.g. chat channel names with CJK characters during boot — resolved the
missing glyphs to spaces ('character \u6587 was not found in the
Inter-Regular SDF font asset or any potential fallbacks') and never
regenerated. Force active TMP texts to rebuild their meshes against the
completed fallback chain, so text created during the loading window
self-heals instead of staying blank.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The CJK fallback fonts were loaded fire-and-forget while the rest of chat setup raced ahead, so channel names and history rendered before the load landed resolved missing glyphs to spaces and logged 'character \u6587 was not found in the Inter-Regular SDF font asset or any potential fallbacks'. Await the load inside ChatPlugin.InitializeAsync so the fallback chain is complete before any chat text generates its mesh; the failure path is unchanged (fonts stay optional, chat init cannot be blocked by a load error). The live-text refresh remains as a safety net for non-chat text rendered during boot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ContextualImage on the Controls GameObject loads the same sprite (guid 4974685691c1) through its AssetReference on enable; the Image component also serialized it directly, hard-linking it into memory and tripping the '[UI] Image must not have a sprite' error on every ControlsPanelView preallocation. Clear the serialized sprite so only the contextual load provides it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The auto-translate indicator (文A / あA) and the language dropdown's 中文 are CJK glyphs serialized directly in UI prefabs and settings, so they render at boot — before the addressable CJK fallback fonts can load — and TMP replaced them with spaces while logging 'character 文 was not found' warnings. Add UIDesignGlyphs SDF: a 256x256 static font asset holding exactly those three glyphs (baked from NotoSansSC-SemiBold, source-font reference stripped so the 8.5MB OTF is not a build dependency), wired into Inter-Regular SDF's fallback table. Boot-time text now resolves the glyphs immediately; the runtime-loaded NotoSans fallbacks continue to cover arbitrary user content. Also re-pins unity-explorer-packages to 1293099, which drops seven Ocean material resaves (editor serialization churn) from the branch. Verified: full boot to the authentication screen logs zero missing-glyph warnings (previously eight). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Removes the 'The referenced script (Unknown) on this Behaviour is missing!' warning logged on every boot. The six dead components were corpses of deleted scripts, confirmed via git history: - DestinationMarker.prefab: pre-repo script, no surviving reference - FavoriteMarkerObject.prefab: navmap-era script dropped in the map discoverability refactor (decentraland#2734) - VoiceChatMicrophoneAudioSource.prefab: leftover from a local getFps prototype (hand-crafted guid a1b2c3d4...) - VoiceChatPlaybackAudioSource.prefab (x2): scripts removed in the communities voice chat audio-quality fix (decentraland#5402) - RoomIndicator.prefab: reference from the reverted UI Toolkit nametag refactor (decentraland#5657) Stripped with GameObjectUtility.RemoveMonoBehavioursWithMissingScript; a project-wide scan of every m_Script guid in Main.unity and all prefabs now resolves cleanly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ixes unity-explorer-packages 6bc4241: version-guard the AdvancedDropdownItem childList accessor (6000.5-only API) so the vendored packages compile on the 6000.4 line. unity-shared-dependencies a1a8ecc: inline the parameterless IsSurfaceTypeTransparent() under URP's own include guard instead of including Shaders/Utils/SurfaceType.hlsl, which 6000.4's URP does not ship. Verified: current dev (6000.4.0f1) with both fix branches compiles with zero errors, so the dependency branches can merge ahead of this PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…block The property is float4 everywhere (cbuffer, sampled cache, access macro) but the UNITY_DOTS_INSTANCED_PROP declaration said float. Unity 6000.4's untyped metadata symbols tolerated the mismatch; 6000.5 encodes the type into the generated metadata name, so the player-variant compile fails with 'undeclared identifier unity_DOTSInstancingF16_Metadata_TerrainBounds' (surfaces only in player builds — editor variants never hit it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
io.sentry.unity 4.0.0's repackaged System.Memory/System.Text.Json shims
crash Unity 6000.5's IL2CPP Span-intrinsic remapper ('Sequence contains
no matching element' in IntrinsicRemap.SpanGetItemArguments), failing
every player build with 9 conversion errors. 4.7.0 converts cleanly.
unity-shared-dependencies pinned to de4cce8: DOTS-instancing variant
fixes (legacy FROM_MACRO accessors, direct unity_ObjectToWorld uses,
missing instance-id transfer in the toon outline) that only surface in
player builds.
Verified: full macOS IL2CPP player build succeeds with zero script,
shader and IL2CPP errors; both dependency branches also compile clean
on 6000.4.0f1 dev for sequential landing.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Squashed the shader-compat branch to a single comment-free commit (5261933); no functional change from de4cce8. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
7344df8 to
38c0fde
Compare
Bump the editor and migrate the DCL code that hit APIs removed in 6000.5:
Verified compiling and running on 6000.5.2f1 (editor boots, enters play mode, reaches interactive).
This fixes the NPE on Unity Editor start