Skip to content

fix: guard TransformHandle against reflection serialization (destroyed-object NRE)#1274

Open
agudmund wants to merge 1 commit into
CoplayDev:betafrom
agudmund:fix/transform-handle-serialization
Open

fix: guard TransformHandle against reflection serialization (destroyed-object NRE)#1274
agudmund wants to merge 1 commit into
CoplayDev:betafrom
agudmund:fix/transform-handle-serialization

Conversation

@agudmund

@agudmund agudmund commented Jul 16, 2026

Copy link
Copy Markdown

Why

GameObjectSerializer reflects over component properties and hands the values to Json.NET. On Unity 6, a component property returning UnityEngine.TransformHandle makes the serializer enumerate the handle's direct children (TransformHandle.DirectChildrenEnumerator). Two problems fall out of that:

  • A play-mode race: the enumeration throws NullReferenceException from TransformHandle.AssertHandleIsValid the moment the underlying object is destroyed. Reading the gameobject/{id}/components resource while a game script Destroy()s objects (a spawner destroying and respawning pickups makes it reliable) logs the [GameObjectSerializer] Unexpected error serializing value of type UnityEngine.TransformHandle warning on every read that races a destroy.
  • Even on live objects, serializing a handle walks child hierarchies of further handles rather than producing a meaningful value.

What

A write-only TransformHandleConverter that emits null for the type. It matches by full type name (UnityEngine.TransformHandle) so the package still compiles on every supported Unity version (floor 2021.3, where the type does not exist) without version defines, per the Unity*Compat guidance in CONTRIBUTING. It is internal for the same third-party converter-scanner reason as UnityEngineObjectConverter (#1138), and registered in GameObjectSerializer's output settings.

EditMode tests resolve the type by reflection and self-ignore on Unity versions without it; the key regression test obtains a handle, destroys its object, and asserts serialization no longer throws.

Observed in the field on Unity 6000.4.10f1 while polling component resources during play mode.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved Unity 6 compatibility when serializing TransformHandle values.
    • Prevented serialization errors for handles linked to destroyed objects.
    • Transform handles are safely represented as null in serialized output.
  • Tests

    • Added coverage for supported, unsupported, live, and destroyed TransformHandle scenarios.

GameObjectSerializer reflects over component properties and hands values
to Json.NET. On Unity 6, a property returning UnityEngine.TransformHandle
makes the serializer enumerate the handle's direct children - which throws
NullReferenceException (AssertHandleIsValid) the moment the underlying
object is destroyed, a common race when component data is read during
play mode while scripts Destroy() objects. On live objects it would walk
entire child hierarchies of handles instead of a value.

Add a write-only TransformHandleConverter that emits null for the type,
matched by full name so the package still compiles on Unity versions
without the type (floor is 2021.3), avoiding version defines per the
Unity*Compat guidance. Internal for the same converter-scanner reason as
UnityEngineObjectConverter (CoplayDev#1138).

Repro: read mcpforunity://scene/gameobject/{id}/components repeatedly in
play mode while a spawner destroys and respawns tagged objects.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2ffa1c9c-1be3-4593-8a47-be0b57d63920

📥 Commits

Reviewing files that changed from the base of the PR and between bd72241 and c50646e.

📒 Files selected for processing (4)
  • MCPForUnity/Editor/Helpers/GameObjectSerializer.cs
  • MCPForUnity/Runtime/Serialization/UnityTypeConverters.cs
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/TransformHandleConverterTests.cs
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/TransformHandleConverterTests.cs.meta

📝 Walkthrough

Walkthrough

Adds a write-only Newtonsoft converter for Unity 6 TransformHandle values, registers it in GameObjectSerializer, and adds EditMode tests covering type matching, live handles, and handles from destroyed objects.

Changes

TransformHandle serialization

Layer / File(s) Summary
Converter and serializer integration
MCPForUnity/Runtime/Serialization/UnityTypeConverters.cs, MCPForUnity/Editor/Helpers/GameObjectSerializer.cs
TransformHandleConverter matches Unity TransformHandle values and writes JSON null; GameObjectSerializer registers the converter.
Converter validation
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/TransformHandleConverterTests.cs, TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/TransformHandleConverterTests.cs.meta
EditMode tests cover unrelated types, Unity-version availability, live handles, destroyed-object handles, and Unity asset metadata.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description covers the why and what, but it omits most required template sections like type, testing, compatibility, and related issues. Fill in the template sections for type of change, changes made, compatibility/package source, testing, docs updates, related issues, and additional notes.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: adding a TransformHandle guard for reflection serialization crashes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant