Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[proposal] Omit empty TiledImage from TiledMap.tiledImages #67

Closed
Hwan-seok opened this issue Mar 28, 2023 · 2 comments · Fixed by #68
Closed

[proposal] Omit empty TiledImage from TiledMap.tiledImages #67

Hwan-seok opened this issue Mar 28, 2023 · 2 comments · Fixed by #68
Assignees
Labels
enhancement New feature or request

Comments

@Hwan-seok
Copy link
Contributor

What could be improved

Note: I am not sure if this is a bug or a proposal.

Change the TiledMap.tiledImages to return only tilesets that have an actual source.

final imageSet = <TiledImage>{};
for (var i = 0; i < tilesets.length; ++i) {
final image = tilesets[i].image;
if (image != null) {
imageSet.add(image);
}
for (var j = 0; j < tilesets[i].tiles.length; ++j) {
final image = tilesets[i].tiles[j].image;
if (image != null) {
imageSet.add(image);
}

As-is
if (image != null) {
  imageSet.add(image);
}

imageSet.addAll(layers.whereType<ImageLayer>().map((e) => e.image));

To-be
if (image?.source != null) {
  imageSet.add(image!);
}

imageSet.addAll(layers.whereType<ImageLayer>().map((e) => e.image)).where((e) => e.source != null);

Why should this be improved

At present, TiledMap.tiledImages collects all images in the map including those without the source.
I think the TiledImages without source is not actually the "image".

For example, the following is a bare image layer without the source:

 <imagelayer id="4" name="Image Layer 2"/>

I think it is a layer but doesn't include any image.

The following is another example:

 <tileset firstgid="1810" name="tileset 1" tilewidth="1" tileheight="1" tilecount="0" columns="0"/>

Also, this is a tileset but not includes any "image".

Any risks?

This cannot be true but there might be some users that use the empty Tiledimage.

More information

@Hwan-seok Hwan-seok added the enhancement New feature or request label Mar 28, 2023
@spydon
Copy link
Member

spydon commented Mar 28, 2023

Sounds like a good improvement, should I assign you to the issue?

@Hwan-seok
Copy link
Contributor Author

It would be great :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants