Skip to content

fix: classify sprite-sheet tilesets by their image source - #1483

Merged
AlmasB merged 1 commit into
AlmasB:devfrom
mvanhorn:fix/1461-tileset-image-classification
Sep 25, 2026
Merged

AlmasB merged 1 commit into
AlmasB:devfrom
mvanhorn:fix/1461-tileset-image-classification

Conversation

@mvanhorn

Copy link
Copy Markdown
Contributor

Change Tileset.isSpriteSheet to derive its value from image.isNotEmpty(), documenting why tile metadata is independent of image ownership; image is a non-null String, so a null check would be incorrect. Existing production consumers in TilesetLoader.kt already read this getter for GID objects and orthogonal, hexagonal, and isometric tile layers, so the correction propagates through existing wiring without call-site edits. Add a compact embedded-tileset TMX fixture referencing the existing tileset_black.png, with metadata-only tile entries carrying boolean/string properties, an undecorated tile used by the same layer, and a GID object using the decorated tile.

Adding custom tile properties in Tiled creates metadata-only <tile> elements in an otherwise ordinary sprite-sheet tileset. FXGL currently classifies a tileset as a sprite sheet only when tiles.isEmpty(), so those entries incorrectly select collection-of-images rendering. The renderer then tries to load the tile's empty image source with zero dimensions, producing the reported exception. The complete cached thread supplies the diagnosis and a reproduction attachment; the owner requested a minimal example, and the reporter subsequently provided one.

Regression: parse a tileset containing both its top-level image and metadata-only tile entries; confirm the entries exist, their image sources remain empty, and isSpriteSheet remains true.
Load the new fixture through TMXLevelLoader.load with the existing test entity factory, checking level dimensions, entity count, and selected opaque/transparent pixels against the existing black tileset. Verify the GID object's image as well as the tile layer so a dummy-image fallback cannot satisfy the test.
Include a used tile with no metadata entry: all atlas tiles must remain addressable even when only some have custom properties.
Getter cases: a nonempty tileset image with zero or multiple tile entries is a sprite sheet; an empty tileset image with individual tile images is a collection. The getter must reflect subsequent mutation of image/tiles rather than cache its initial result.
Run existing separate-tile-images, flipped-tiles, isometric, hexagonal, incorrect-image-path, and malformed-map tests to cover rendering and error-path regressions without expanding their behavior.
Implementation validation on JDK 25: mvn -pl fxgl-core -am test -Dtest=TMXLevelLoaderTest -Dsurefire.failIfNoSpecifiedTests=false -Dgpg.skip=true -Dmaven.javadoc.skip=true -Dpmd.skip=true; then the core module suite with mvn -pl fxgl-core -am test -Dgpg.skip=true -Dmaven.javadoc.skip=true -Dpmd.skip=true. Use an available JavaFX display (CI uses xvfb). These commands are downstream instructions, not tests executed during this read-only planning task.

Pull Request (PR) prerequisites

None added, removed, or upgraded.

Fixes #1461

@AlmasB AlmasB left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks

@AlmasB
AlmasB merged commit 69e3151 into AlmasB:dev Sep 25, 2026
2 of 4 checks passed
@mvanhorn

Copy link
Copy Markdown
Contributor Author

Thanks for the merge @AlmasB!

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.

TilesetLoader.loadImage() fails with "Image dimensions must be positive" when tiles have custom properties

2 participants