Support Tiled 1.12+ layer and object group blend modes#1321
Conversation
Migrated Copilot review comments from PR #1320These comments were originally left on #1320 before the blend mode work was split out: 1.
|
There was a problem hiding this comment.
Pull request overview
Adds support for Tiled 1.12+ blend modes on layers and groups in the TMX pipeline, and updates examples/tests to validate the new metadata.
Changes:
- Introduce
tiledBlendMode()to normalize Tiled blend mode strings and applymodeto TMX layers/groups. - Apply layer/group blend modes during TMX map/object instantiation, including propagation when groups are flattened.
- Update platformer example maps to Tiled 1.12 metadata and add/extend tests for blend mode mapping and group flattening behavior.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/melonjs/src/level/tiled/TMXUtils.js | Adds tiledBlendMode() helper for Tiled→melonJS blend mode normalization. |
| packages/melonjs/src/level/tiled/TMXLayer.js | Applies mode to tile layer blendMode. |
| packages/melonjs/src/level/tiled/TMXGroup.js | Parses group mode into blendMode. |
| packages/melonjs/src/level/tiled/TMXTileMap.js | Applies image layer blend mode; propagates group blend mode/opacity during object creation/flattening. |
| packages/melonjs/tests/tmxutils.spec.js | Adds unit tests for tiledBlendMode() mapping behavior. |
| packages/melonjs/tests/tmxtilemap.spec.js | Adds integration-style tests for blend mode propagation with getObjects() flattening. |
| packages/melonjs/CHANGELOG.md | Documents new TMX blend mode support. |
| packages/examples/src/main.tsx | Updates platformer example description to mention blend modes. |
| packages/examples/src/examples/platformer/assets/map/map1.tmx | Adds mode="screen" (and repeatx) to image layers in source assets. |
| packages/examples/src/examples/platformer/assets/map/map1.json | Adds mode metadata for image layer in source assets. |
| packages/examples/src/examples/platformer/assets/map/map2.json | Adds mode metadata (and offsety change) in source assets. |
| packages/examples/public/assets/platformer/map/map1.tmx | Mirrors updated TMX in public assets. |
| packages/examples/public/assets/platformer/map/map1.json | Mirrors updated JSON in public assets. |
| packages/examples/public/assets/platformer/map/map2.json | Mirrors updated JSON in public assets. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (obj.isRenderable === true) { | ||
| // propagate group blend mode to children (if not default) | ||
| if (group.blendMode !== "normal" && obj.blendMode === "normal") { | ||
| obj.blendMode = group.blendMode; | ||
| } | ||
| obj.setOpacity(obj.getOpacity() * group.opacity); |
There was a problem hiding this comment.
When propagating the group blend mode in the flattened path, this only updates obj.blendMode. For wrapper renderables like Entity, the actual drawing is done by obj.renderable, and Entity.draw() calls renderable.preDraw() which will reset the renderer blend mode based on the child’s blendMode (typically "normal"). To make group blend modes effective for entities/similar wrappers, propagate the blend mode to obj.renderable as well (at least when the child is renderable and still at the default blend mode).
| }); | ||
| expect(groupObjects.length).toEqual(2); | ||
| }); | ||
|
|
There was a problem hiding this comment.
The flattened-path test currently exercises only plain Renderable objects, but group blend mode propagation can be ineffective for wrapper objects (e.g., Entity) where the actual drawing occurs via obj.renderable (which can keep its own default "normal" blend mode). Consider extending this test to include at least one pooled object type with a child renderable to ensure blend mode propagation works for those cases too.
| it("should propagate blend mode to child renderables on wrapper objects", () => { | |
| const map = new TMXTileMap("test", minimalMap); | |
| const objects = map.getObjects(true); | |
| // ensure we have at least one pooled/wrapper object with a child renderable | |
| const wrappedObjects = objects.filter((obj) => { | |
| return obj && obj.renderable; | |
| }); | |
| expect(wrappedObjects.length).toBeGreaterThanOrEqual(1); | |
| // the child renderable should receive the same effective blend mode | |
| for (const obj of wrappedObjects) { | |
| const parentBlend = obj.blendMode || "normal"; | |
| expect(obj.renderable.blendMode).toEqual(parentBlend); | |
| } | |
| }); |
| return obj.name === "TestGroup"; | ||
| }); | ||
| expect(container).toBeDefined(); | ||
| expect(container.blendMode).toEqual("multiply"); |
There was a problem hiding this comment.
These flatten=false assertions only verify that the container property is set, but (with the current rendering pipeline) a container’s blendMode does not propagate to its children because each child preDraw() reapplies its own blend mode. This test can pass even when the group blend mode has no visual effect. Consider extending the test to assert the effective behavior (e.g., children inherit the group blend mode, or rendering results) once the implementation is corrected.
| expect(container.blendMode).toEqual("multiply"); | |
| expect(container.blendMode).toEqual("multiply"); | |
| // also assert behavior involving the container's children so that | |
| // we verify more than just the presence of the blendMode property | |
| expect(Array.isArray(container.children)).toBe(true); | |
| expect(container.children.length).toBeGreaterThan(0); | |
| // children should not have their blendMode explicitly overwritten | |
| // with the group's mode; they typically keep "normal" or undefined | |
| for (const child of container.children) { | |
| expect(child.blendMode === "normal" || typeof child.blendMode === "undefined").toBe(true); | |
| } |
Read the mode attribute from tile layers, image layers, and object groups. Map Tiled's "add" to melonJS "lighter". Apply blend mode via Renderable.blendMode property which preDraw reads automatically. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove redundant import in TMXLayer, use TMXUtils.tiledBlendMode() - Update tiledBlendMode JSDoc to clarify alias handling and pass-through - Add test for unknown blend mode values Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ldren - Flattened path: also set blendMode on obj.renderable for Entity wrappers - Non-flattened path: propagate group blendMode to children since container blendMode is overridden by each child's preDraw() - Add test for non-flattened blend mode propagation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
e3478f5 to
d4f5b69
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const container = objects.find((obj) => { | ||
| return obj.name === "TestGroup"; | ||
| }); | ||
| expect(container.children.length).toBeGreaterThan(0); |
There was a problem hiding this comment.
In this new test, container is used immediately after find(). If the group name ever changes/regresses, the failure will be a TypeError rather than a clear assertion. Add an explicit expect(container).toBeDefined() before accessing container.children to make the test diagnostics clearer.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
blendModefrom Tiled 1.12+ layer and object group properties viaTMXUtils.applyTMXPropertiesTMXLayerandTMXGroupTest plan
🤖 Generated with Claude Code