Upgrade sixteen MMO placeholders with verified Blender-authored assets - #574
Conversation
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. |
![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: e44c1c6f88
ℹ️ 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".
| newmtl MMO_brass | ||
| Ns 462.399994 | ||
| Ka 0.720000 0.720000 0.720000 | ||
| Kd 0.620000 0.390000 0.100000 |
There was a problem hiding this comment.
When these replacements are displayed in the shipped MMO scenes, their new Kd palettes are not used: every referenced model still has a nonempty MeshRenderer.material (for example, Assets/Scenes/MMO/town_square.scene:65-68 and 81-84), while RenderWorldBasic only draws OBJ/MTL submeshes when materialPath is empty (WorldBasicRenderer.cpp:350-398). Consequently the forge, shop, stalls, fountain, and other upgraded props are rendered with one legacy stone/wood/etc. texture instead of their authored iron, brass, ember, cloth, and wood colors. Remove or adapt those scene overrides, or compose them with the OBJ material groups, so the runtime presentation matches the upgraded assets rather than only the Blender previews.
Useful? React with 👍 / 👎.
| * Exercises production MeshAsset::Load, not a replacement OBJ parser. Windows | ||
| * requires a real D3D11 device and is deliberately outside this CPU test scope. | ||
| * Material references and source/export provenance are checked separately; this | ||
| * loader does not expose OBJ material assignments. No rendering claim is made. |
There was a problem hiding this comment.
For non-Windows users who instantiate these self-contained models without an explicit JSON material, the live renderer still cannot use their authored colors: WorldMeshCache loads Mesh, but the OBJ branch in MeshLinux.cpp:338-405 discards materials and material_ids and never fills m_submeshes, which is required by WorldBasicRenderer.cpp:380-392 to issue per-Kd draws. These new tests instead call MeshAsset::Load and explicitly omit material assignments, so all 16 remain green while the advertised palette is lost in the actual Linux scene path. Preserve material ranges in the Linux Mesh importer and cover that path with the asset regression.
Useful? React with 👍 / 👎.
⏳ CodeQL Report PendingThe pull-request head changed. Results from earlier commits do not apply while the new head is being scanned. Current PR head: |
1 participant
Sixteen MMO props were primitive placeholders, several with invalid authored normals or missing UVs. This replaces them with distinct Blender-authored models, preserves filenames and original bounds, and prevents the legacy generator from overwriting them.
Includes editable Blender 4.0.2 source, reproducible authoring and CPU-render scripts, before/after previews, SHA-256 provenance, the complete 884-file baseline inspection, source-contract checks and actual production CPU importer regressions. Models use diffuse colors without external texture dependencies and stay below 5,000 triangles each. Regenerated MMO manifest, documentation, readiness handoff and build inventory; release gates remain open.
Reviewed candidate: e44c1c6
Independently reviewed tree: 63c930a15e51b47e47cf6ecaf9da6ab9823f2264
Stacked on PR #569; do not merge until required checks pass on the exact reviewed head.
Verification:
Scope limits: no full local suite pass claimed; sandbox Unix sockets remain unavailable. CPU Blender renders are not D3D11 captures. This does not qualify Windows 11, installed content, gameplay collision, animation, performance or stable release. The remaining 206 OBJ models, textures/audio and branding still need contextual quality review. The baseline audit flags 11 MMOFPS models with near-zero-area faces for the next investigation.
Authoring requires the recorded baseline Git object and supplied provenance; full-kit regeneration is documented. New original assets retain the root Spark Open License 1.0, with existing legacy attribution preserved.
Next quality pass: render all 222 current OBJ models through the same actual Blender CPU renderer for contextual visual review. Original-source inspection confirms collapsed geometry in the 11 flagged MMOFPS models; polygon and triangulation counts differ, so do not delete arbitrary fan triangles or re-export third-party materials blindly. Preserve Assets/MMOFPS attribution and original material semantics when preparing repairs.