FeatureRequest: Add support for multiple tilesets for tilemaps imported from Tiled.

[tliron]

Description

I cannot get bevy_entitles to load my tmx. Note that this is a very simple tmx with no custom objects.

The log isn't helping me much, but maybe you can help me understand where the problem is?

2024-04-12T20:54:27.963207Z  WARN bevy_entitiles::tiled::resources: Object rotation is not recommended! It will results in wrong collision shapes. But if you just want to use this object as sort of static image it will be ok.
thread 'main' panicked at /home/emblemparade/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_entitiles-0.7.0/src/tiled/xml/layer.rs:308:35:
attempt to subtract with overflow
stack backtrace:
   0: rust_begin_unwind
             at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/std/src/panicking.rs:647:5
   1: core::panicking::panic_fmt
             at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/panicking.rs:72:14
   2: core::panicking::panic
             at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/panicking.rs:144:5
   3: bevy_entitiles::tiled::xml::layer::Tiles::iter_decoded::{{closure}}
   4: bevy_entitiles::tiled::load_layer
   5: core::ops::function::impls::<impl core::ops::function::FnMut<A> for &mut F>::call_mut
   6: <bevy_ecs::system::function_system::FunctionSystem<Marker,F> as bevy_ecs::system::system::System>::run_unsafe
   7: <futures_lite::future::CatchUnwind<F> as core::future::future::Future>::poll
   8: std::panicking::try
   9: async_task::raw::RawTask<F,T,S,M>::run
  10: bevy_tasks::thread_executor::ThreadExecutorTicker::tick::{{closure}}
  11: futures_lite::future::block_on
  12: bevy_tasks::task_pool::TaskPool::scope_with_executor
  13: <bevy_ecs::schedule::executor::multi_threaded::MultiThreadedExecutor as bevy_ecs::schedule::executor::SystemExecutor>::run
  14: bevy_ecs::schedule::schedule::Schedule::run
  15: bevy_ecs::world::World::try_schedule_scope
  16: bevy_ecs::world::World::resource_scope
  17: <bevy_ecs::system::exclusive_function_system::ExclusiveFunctionSystem<Marker,F> as bevy_ecs::system::system::System>::run
  18: <bevy_ecs::schedule::executor::single_threaded::SingleThreadedExecutor as bevy_ecs::schedule::executor::SystemExecutor>::run
  19: bevy_ecs::schedule::schedule::Schedule::run
  20: bevy_ecs::world::World::schedule_scope
  21: bevy_app::app::App::update
  22: bevy_winit::run_app_update_if_should
  23: bevy_winit::winit_runner::{{closure}}
  24: winit::platform_impl::platform::EventLoop<T>::run
  25: bevy_winit::winit_runner
  26: core::ops::function::FnOnce::call_once{{vtable.shim}}
  27: bevy_app::app::App::run
  28: great_game::main
  29: core::ops::function::FnOnce::call_once
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
Encountered a panic in system `bevy_entitiles::tiled::load_tiled_xml`!
Encountered a panic in system `bevy_app::main_schedule::Main::run_main`!

Versions

• OS: Linux
• Crate and other dependencies: entitiles: 0.4.0

[443eb9]

The log indicates that the error happens in a closure, but it didn't say which line is it. There're three subtract operations in this closure, and I'm not sure which one caused this panic.

Could you make a minimal reproduction? Or send me the following information:

• All tilesets like <tileset firstgid="1" source="../tilesets/Tileset1.tsx"/>
• All layers like
  \ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,62,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,67,0,62,0,0,0,0,0,217,0,0,0,0,0,217,0,0,0,0,217,0,0,0, 0,13,14,14,14,14,2147483661,79,217,0,0,0,62,0,0,0,0,217,0,0,0,0,217,0,0,0,0,0,0,0, 0,25,1,1,26,26,2147483673,79,0,0,0,0,217,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,25,26,3,1,2,2147483673,79,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,25,2,26,1,26,2147483673,79,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,37,38,38,38,38,2147483685,79,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,66,0,79,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,62,0,0,78,0,79,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,79,0,0,0,0,0,0,0,0,13,14,14,2147483661,0,0,0,0,0,0,0,0,0,0, 0,0,0,217,0,16,17,17,17,2147483664,2147483668,19,19,19,19,20,25,5,1,2147483673,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,28,5,1,5,2147483676,0,0,0,0,0,0,25,98,1,2147483673,0,0,0,0,0,0,0,0,0,0, 0,217,0,0,0,40,41,41,41,2147483688,0,0,0,0,0,0,37,98,38,2147483685,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,98,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,98,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,217,0,0,0,0,0,0,0,0,0,0,0,0,109,110,111,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,121,122,123,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 \

[tliron]

Here are my tmx and tsx files, sorry if the directory structure is wrong.

I'm very happy to help debug this and thank you so much for maintaining this library.

Caves.zip

[443eb9]

Ok I think I found the source of this issue. In chunk 32,0 you're using multiple tilesets in one layer, which is not supported.

A layer is equals to a tilemap and a tiled tileset is equals to a TilemapTexture, but there's only one texture for each tilemap. It is impossible to have multiple textures on one tilemap.

In fact, I had thought of this special case and wrote a assert right below the code that panics in your case. But obviously, it panics before my assertion.

So thanks for your feedback! This assertion will be brought up in advance in next commit.

[tliron]

Hm, thank you. Could you suggest a way to avoid this problem? Is it as easy as just dividing into more layers?

[443eb9]

Yes, and that seems to be the only way to avoid this.

[tliron]

OK, so this is ending up being a big problem for me. I have a lot of levels designed with multiple tilesets, and indeed Tiled seems to be designed to allow you to do this, so I suspect many users would do this, too.

Would you at all be interested in adding support for this Tiled feature? I could try to write a PR. My thought is that we can automatically split to separate layers internally when multiple tilesets are detected.

[443eb9]

If you're willing to submit a pull request to implement this feature, you're very welcome to do so. I've been quite busy with other projects lately and don't currently have the bandwidth to develop new features for this library...

[tliron]

Thanks for the honest response! We took a look, and it's not so simple. :/ You make the assumption of 1-layer-to-1-tileset pretty deep in the library and it would have to be a major refactor. It seems it would be easier for us to do a total rewrite.

If you could find time just to do the community a favor: please make it clear at the top of the README that this library does not support Tiled levels with mixed tilesets, it would save potential users some time. It is a very basic (and useful) feature in Tiled.

[443eb9]

I'm really sorry for the inconvenience... I'll pause my current project and realize this feature in this weekend, and you'll be soon able to load these tilemaps. So maybe I don't need to mark that out now.

[tliron]

Thank you, that's very kind and I think it would make your library better. Even without Tiled, it's a useful feature to have one-to-many layer-to-tileset.

[443eb9]

I'm sorry, but I may not be able to complete this feature by this weekend as the refactoring work is far beyond my expectations. I'm trying to implement support for multiple tilesets on one tilemap when the atlas feature is enabled, and provide options for separation and non-separation when loading tilemaps from Tiled.

The reason for not implementing it without enabling the atlas feature is that wgpu restricts the maximum depth (or length of the array) to 2048 in a texture array. In other words, without enabling the atlas feature, a tilemap can only support up to 2048 different tiles at most, which I think is too few, so I don't plan to implement it.

I'm not sure if I can finish them before May, but I will try my best...

[tliron]

I appreciate your effort. It does seem like a major refactor for your code base.

Meanwhile, I am writing a new solution from scratch. I have a running async parser based on a streaming XML over Tokio. I will release this as a general-purpose Tiled parser for Rust, independent of Bevy, though it also works well with Bevy's async asset loader. I'm aiming for "correctness" there on par with Tiled's spec. I will then attempt to "optimize" the parsed result for Bevy. Not all Tiled features will be supported but they will all be parsed, so users still have access.

I like your idea of tile = entity and hope to experiment with it soon.

[443eb9]

Ok.. Finally finished. You can now use the master brach if you can't wait to try. The new version will be released soon.