mapeditor / rs-tiled

Reads files from the Tiled editor into Rust
https://crates.io/crates/tiled
MIT License
270 stars 103 forks source link

More convenient way to load embedded maps #266

Closed raimundomartins closed 1 year ago

raimundomartins commented 1 year ago

Hi! I'm trying out Bevy and (similar Amethyst) it passes a &[u8] to load the file. As of version 0.11, I have no idea how can load maps as implementing ResourceReader for loading from byte arrays would have to be intricate to then be able to load the remaining files from disk.

Is there any chance to add back this super useful method? If not, how can I load the main tmx from bytes and the rest of the files using FilesystemResourceReader?

bjorn commented 1 year ago

In the bevy_ecs_tilemap crate, somebody introduced a BytesResourceReader, see https://github.com/StarArawn/bevy_ecs_tilemap/pull/425. Does that help or would you still suggest we should add back load_tmx_map_from?

raimundomartins commented 1 year ago

Huh, I guess I should've gone to the main branch before posting this. That along with what's in #265 is a good enough solution to me. I still don't see why the helper method had to go, but at least there's a reasonable alternative. Thanks!

aleokdev commented 1 year ago

I still don't see why the helper method had to go, but at least there's a reasonable alternative. Thanks!

load_..._from was a hacky method to only load completely embedded maps from some source. That is still possible in 0.11, it's just a bit more manual work. Perhaps we could add it back as the BytesResourceReader solution.

raimundomartins commented 1 year ago

Personally, it wouldn't do much good unless it would support supplying another ResourceReader for the remainder of files. I know I asked how I could use it with the FilesystemResourceReader, but since I can easily use a more portable solution from the beginning, I prefer portability.

aleokdev commented 1 year ago

Personally, it wouldn't do much good unless it would support supplying another ResourceReader for the remainder of files.

Yeah, that's something load_tmx_map_from could not do and it may be useful to have.

I know I asked how I could use it with the FilesystemResourceReader, but since I can easily use a more portable solution from the beginning, I prefer portability.

What do you mean by this? I don't understand.

Now that I think about it, we could implement ResourceReader for anything that implements for<'a> Fn(&'a Path) -> Result<R, E> where R: Read, E: Error + Send + Sync + 'static, which would allow easily defining a ResourceReader inline in the Loader declaration, removing a lot of boilerplate and simplifying the process altogether, something like this:

let mut loader = Loader::with_cache_and_reader(
    DefaultResourceCache,
    |path| { if path == "my-map.tmx" { Ok(that_map) } else { Err(MapLoadingError) }
);

What do you think about this?

raimundomartins commented 1 year ago

What do you mean by this? I don't understand.

I was just explaining why I wouldn't use it. It's not important.

What's important is your last suggestion which is actually pretty cool! That I would definitely use!

aleokdev commented 1 year ago

Thanks :) I'll try to PR it whenever possible. I'm currently really busy with exams so it probably won't be done until ~2weeks from now.

aleokdev commented 1 year ago

This has been merged for a while now, so we can safely close this issue.