feat(vat): slice 3 — Unity / Unreal / Godot targets + sidecars - #567
Conversation
Closes more of #371. Slice 3 of 4 — VAT now knows about per-engine axis / UV / sidecar conventions so a baked output drops straight into the target engine without tooling. ### Targets - **Agnostic** (default) — no swizzle, no sidecar. Engine-agnostic position texture + JSON. - **Unity** — Y-up (same as Ogre), but rows pre-flipped at write time so the runtime shader can use `row / frameCount` indexing without re-flipping V. Emits a `<base>_pos.png.meta` sidecar pre-configured for data textures (sRGB off, no compression, point filter, clamp wrap). - **Unreal** — `(x, y, z) → (x, z, y)` axis swizzle (Ogre Y-up → Unreal Z-up). Bounds + normals follow the same swizzle so the Niagara VAT module's U=vertex / V=frame layout works as-is. - **Godot** — Y-up + right-handed (same as Ogre). Emits a `<base>.gdshader` spatial-shader template with bounds + frame count + vertex count substituted from the bake. User drops the shader on a `MeshInstance3D` and animates `current_frame`. ### Files emitted per target | target | _pos.png | .json | .meta | .gdshader | |----------|----------|-------|-------|-----------| | agnostic | ✓ | ✓ | | | | unity | ✓ | ✓ | ✓ | | | unreal | ✓ | ✓ | | | | godot | ✓ | ✓ | | ✓ | ### Sidecar additions - `target` JSON field reflects the chosen target. - `BakeResult` carries `unityMetaPath` + `godotShaderPath` (empty when not applicable). CLI JSON output exposes the same. ### CLI `--target agnostic|unity|unreal|godot` (default `agnostic`). Sentry breadcrumb in `file.export` now records the target. ### Tests (6 new) - `SidecarTargetFieldReflectsOption` — sidecar carries the right target name for all four enum values. - `UnityTargetWritesPngMetaSidecar` — `.meta` exists, has expected YAML keys (`TextureImporter:`, `sRGBTexture: 0`, `filterMode: 0`). - `GodotTargetWritesShaderTemplate` — `.gdshader` exists, contains `shader_type spatial`, the right vertex entry, and the substituted frame_count + vertex_count uniforms. - `UnrealTargetSwizzlesPositionsXZY` — quantitative: the bounds computed for Unreal swap Y/Z versus the agnostic bake on the same entity. - `UnityRowsAreVerticallyFlipped` — pixel-level: Unity row 0 equals agnostic last-row. - Standalone fixture test on the sidecar JSON shape (above). ### Manual smoke (all 4 targets) ``` $ qtmesh vat robot.mesh --anim Idle --target unity -o /tmp/un position texture, sidecar, .meta $ qtmesh vat robot.mesh --anim Idle --target unreal -o /tmp/ue bounds Y↔Z swap visible in output $ qtmesh vat robot.mesh --anim Idle --target godot -o /tmp/gd .gdshader emitted with bounds substituted ``` Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughVAT baking pipeline now supports per-target output via configurable Target enum (Unity, Unreal, Godot). The CLI exposes --target flag selection, baking applies target-specific coordinate axis-swizzling and row-flipping for texture writes, and generates engine-specific sidecars (Unity .meta, Godot .gdshader). JSON output and BakeResult include target metadata and optional output paths. ChangesMulti-target VAT baking support
Sequence Diagram(s)sequenceDiagram
participant CLI as CLIPipeline
participant Baker as VATBaker::bake()
participant Swizzle as Axis transform
participant Textures as Texture write
participant Sidecars as Sidecar files
CLI->>Baker: Call bake(opts.target)
Baker->>Swizzle: Apply per-target axis swizzle to positions/normals
Swizzle->>Baker: Return swizzled coordinates
Baker->>Textures: Write position/normal textures with conditional row flip
Textures->>Baker: Textures written (flipRows per target)
Baker->>Sidecars: Write JSON + target-specific extras
Sidecars->>Sidecars: Unity writes .meta<br/>Godot writes .gdshader<br/>Unreal writes none
Sidecars->>Baker: Return BakeResult with output paths
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5b9008b90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { | ||
| return QStringLiteral( | ||
| "fileFormatVersion: 2\n" | ||
| "guid: 00000000000000000000000000000000\n" |
There was a problem hiding this comment.
The Unity sidecar template hard-codes guid: 00000000000000000000000000000000, so every VAT export gets the same asset GUID. When more than one exported VAT is imported into the same Unity project, this creates duplicate-GUID conflicts and Unity will reassign/ignore one of them, which breaks deterministic asset identity and can invalidate references. The exporter should either generate a unique GUID per file or omit the GUID and let Unity create it.
Useful? React with 👍 / 👎.
| // Per-engine extras — emitted alongside the JSON sidecar so a | ||
| // dropped-in asset folder is engine-ready without further tooling. | ||
| if (opts.target == Target::Unity) { | ||
| result.unityMetaPath = result.posTexPath + QStringLiteral(".meta"); |
There was a problem hiding this comment.
For --target unity --normals, the bake writes a normal texture but only emits a .meta sidecar for the position texture. That leaves the normal texture on Unity defaults (typically color-space/compression/filter settings for color textures), so decoded normals can be distorted compared with the intended data-texture settings used for positions. The Unity target path should emit matching import metadata for _nrm.png whenever normals are baked.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/CLIPipeline.cpp`:
- Around line 4785-4796: The code currently only handles "--target" when i+1 <
argc so a trailing "--target" is ignored; update the parsing so that when arg ==
"--target" you first check if i+1 >= argc and if so emit an error (use err() <<
"Error: --target requires a value" << Qt::endl; return 2;) otherwise read the
next token into QString tgt = QString(argv[++i]).toLower() and map it to
VATBaker::Target::Agnostic/Unity/Unreal/Godot as before; keep using the same
identifiers (arg, argc, argv, tgt, VATBaker::Target, err(), Qt::endl).
In `@src/VATBaker.cpp`:
- Around line 460-471: The buildUnityMeta function currently writes a constant
GUID ("guid: 000...") causing collisions; change it to generate a unique GUID
per file by creating a QUuid (e.g. QUuid::createUuid()), formatting it as a
32-character hex string without braces or hyphens and in lowercase (Unity
expects a 32-char hex guid), and substitute that value into the "guid: ..."
field of the returned metadata string (update buildUnityMeta to build the meta
string dynamically rather than using a fixed QStringLiteral).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4c305002-c0af-4354-bd5e-0a72519281d2
📒 Files selected for processing (4)
src/CLIPipeline.cppsrc/VATBaker.cppsrc/VATBaker.hsrc/VATBaker_test.cpp
Two findings from CodeRabbit on PR #567: 1. **Minor** — `--target` parsing silently fell through to the default when no value followed. Catch the missing value and emit a clear error message instead. 2. **Major** — the Unity `.meta` sidecar embedded a placeholder `00000000000000000000000000000000` GUID. Unity requires every asset GUID to be globally unique; sharing one causes Unity to silently remap references between assets at import time or refuse the duplicate. Generate a fresh random GUID per bake via `QUuid::createUuid()` and write it in the 32-hex-char shape Unity expects. Plus one new test: `UnityMetaGuidIsUniquePerBake` baking the same entity twice and asserting the two `.meta` files carry different 32-char GUIDs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
1 participant
Closes more of #371. Slice 3 of 4. VAT now knows about per-engine axis / UV / sidecar conventions so a baked output drops straight into the target engine without tooling.
Targets
The Unity row-pre-flip means the runtime shader uses plain `row / frameCount` indexing on every target. The Unreal swizzle puts Ogre's Y-up axis onto Unreal's Z-up; bounds + normals follow.
Files emitted
```
out/
Walk_pos.png # position texture (all targets)
Walk.json # sidecar (all targets)
Walk_pos.png.meta # only target=unity
Walk.gdshader # only target=godot
```
Sidecar JSON gains a `target` field; `BakeResult` carries `unityMetaPath` + `godotShaderPath` paths (empty when not applicable). CLI `--json` output exposes them.
CLI
`--target agnostic|unity|unreal|godot` (default `agnostic`). Sentry breadcrumb in `file.export` records the target alongside encoding + normals flag.
Tests (6 new)
Manual smoke (all 4 targets)
```
$ qtmesh vat robot.mesh --anim Idle --target unity -o /tmp/un # → pos, json, .meta
$ qtmesh vat robot.mesh --anim Idle --target unreal -o /tmp/ue # bounds Y↔Z swap visible
$ qtmesh vat robot.mesh --anim Idle --target godot -o /tmp/gd # → pos, json, .gdshader
```
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
--targetoption to VAT command supporting agnostic, Unity, Unreal, and Godot targets..meta, Godot shader templates).Tests