Repository navigation
fix: classify sprite-sheet tilesets by their image source - #1483
Merged
AlmasB merged 1 commit intoSep 25, 2026
Merged
Conversation
Contributor
Author
|
Thanks for the merge @AlmasB! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change
Tileset.isSpriteSheetto derive its value fromimage.isNotEmpty(), documenting why tile metadata is independent of image ownership;imageis a non-null String, so a null check would be incorrect. Existing production consumers inTilesetLoader.ktalready 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 existingtileset_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 whentiles.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
isSpriteSheetremains true.Load the new fixture through
TMXLevelLoader.loadwith 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 withmvn -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