Repository navigation
Collision at the root (sidecar): place nodes whatever Read/Write is - #397
Conversation
Every import refused the listed nodes as "unreadable" on a model whose Read/Write is off (Unity's default), and PlaceInSidecar then dropped them from the sidecar. In OnPostprocessModel a mesh's isReadable follows that setting, yet Unity lets the import change the mesh: BakeMatrix takes an `importing` flag that skips the readability guard, and ApplyOnImport no longer refuses an unreadable mesh. BakeMatrix is split into its normals, tangents and winding parts (same behaviour), bringing its cognitive complexity under Sonar's limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9SukSbP376zkY7oEgVTpH
ModelReplacementInvalidatesOnlyUvSidecarBeforeFailingFirstImport set a default ToolSettings directly, which since ToolSettings.saved counts as no settings: the UV reset deleted the then empty sidecar and the test's import hook hit a null sidecar. It now marks them saved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9SukSbP376zkY7oEgVTpH
|
Warning Review limit reachedYour organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Next included review available in 1 minute. View limit details
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
No issues found by static checks — no coupling regressions or blocking issues in the code graph. That is not the same as safe to merge: see what was not checked.
Not checked: tests were not run; no formal proof of the changed code.
Graphify review — findings
Fixes Place Collision at Root (Sidecar) on models with Read/Write off (Unity's default): the import no longer refuses the listed nodes as "unreadable" and drops them from the sidecar. The UnreadableMesh check is gone, and placement now bakes with a new importing: true flag on MeshTransform.BakeMatrix, which skips the isReadable guard because Unity allows mesh changes inside OnPostprocessModel regardless of that setting. Outside an import, BakeMatrix still returns early on unreadable meshes. Also splits BakeMatrix into BakeNormals, BakeTangents and FlipWinding helpers without changing the transform math, and adjusts a sidecar save test to set saved = true on its ToolSettings, matching what SaveSettings leaves. A changelog entry covers the fix.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 364 functions depend on the 239 functions this change touches.
Health — grade B; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
Source()— 22 callers, 5 callees (high)ExportBench()— 9 callers, 3 callees (medium)ExportDocumentGroups()— 1 callers, 25 callees (high)ExportHierarchyGroupPrepared()— 1 callers, 17 callees (high)PrepareExportHierarchy()— 1 callers, 16 callees (high)ExportFile()— 2 callers, 6 callees (medium)ExportOffsetBench()— 3 callers, 4 callees (medium)SidecarRetainsSharedMeshInstancesAndRebuildUsesEachHierarchyFrame()— 0 callers, 18 callees (high)- …and 2 more
Verification — 364 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 321 function(s) in the blast radius were not formally verified this run
The main coverage run after #396 ended at 1631 passed, 2 failed. This PR fixes both.
Place Collision at Root (Sidecar) did nothing on a model with Read/Write off
The test log #396 now prints showed the cause:
OnPostprocessModel, a mesh'sisReadablefollows the model's Read/Write setting, and Read/Write is off by default.PlaceInSidecarthen dropped them from the sidecar.Uv2AssetPostprocessoralready relies on this when it rebuilds meshes.MeshTransform.BakeMatrixtakes animportingflag that skips the readability guard.CollisionRootPlacement.ApplyOnImportpasses that flag and no longer refuses an unreadable mesh.BakeMatrixis split into its normals, tangents and winding parts. Behaviour is the same, and its cognitive complexity drops below Sonar's limit; the S3776 it carried before is gone.ToolAssetSaveTests.ModelReplacementInvalidatesOnlyUvSidecarBeforeFailingFirstImportToolSettingsinto the sidecar directly, and sinceToolSettings.savedthat counts as no settings. The UV reset then deleted the sidecar, which was empty, and the test's import hook hit a null sidecar.SidecarStore.SaveSettingsdoes.Checks
python3 Tools~/compile_check.py: 0 errors with and withoutLIGHTMAP_UV_TOOL_FBX_EXPORTER.PlaceInSidecar…(instance, unpacked copy, clear) runs for the first time once the node is placed; the coverage script prints its log if anything there fails.Generated by Claude Code