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

Allow obtaining the path of tilemaps that have been loaded #263

Open
LuckyTurtleDev opened this issue May 3, 2023 · 9 comments · Fixed by #303
Open

Allow obtaining the path of tilemaps that have been loaded #263

LuckyTurtleDev opened this issue May 3, 2023 · 9 comments · Fixed by #303
Labels

Comments

@LuckyTurtleDev
Copy link

I would like to get the path of the tileset used at the map.
But it does look like Tileset does not provide this option.

@aleokdev
Copy link
Contributor

aleokdev commented May 3, 2023

You're right, this isn't possible currently. But since the path depends on the reader/cache used, it maybe makes more sense to include this as a ResourceCache method. Nevermind, as this would limit obtaining it only to intermediate resources. Perhaps it is OK to just store the path in Tileset.

@aleokdev aleokdev changed the title get path of tilemaps used at a map Allow obtaining the path of tilemaps that have been loaded May 3, 2023
@bjorn
Copy link
Member

bjorn commented May 4, 2023

I don't really like the idea of adding the path to the Tileset struct, because I think it doesn't really belong there and for most use-cases it is enough to just be able to look up tilesets by their path in the ResourceCache. However, for those cases where you do need the path I have to admit that I don't know a nice alternative either and I have solved this issue the same way in Tiled itself.

@LuckyTurtleDev Just out of curiosity, what use-case do you have where you need to know the path?

@aleokdev
Copy link
Contributor

aleokdev commented May 4, 2023

BTW you theorically could create your own ResourceLoader and log the path of each tileset loaded that way.

@LuckyTurtleDev
Copy link
Author

@LuckyTurtleDev Just out of curiosity, what use-case do you have where you need to know the path?

@bjorn I have writen a proc macro, with does convert the tiled map to a more easy app internal map format and check some requirements at compile time.
This macro must rerun every time the map files has change. Regular this does not happen because the macro is cached.
But if you use the include_bytes!() macro inside your proc macro rust does automatically start watching the file and rerun the whole proc macro if the file has changed.

For the the main tmx map you can simple use the path from the input of the proc macro.

const _: &[u8] = ::core::include_bytes!(#path);

But this can not be done for the tileset, because the macro do not know where they are stored.
Currently I am using a workaround, to simple include all tileset of the workspace:

let mut tileset = quote!();
	for path in glob("./**/*.tsx").unwrap() {
		let path = path.unwrap().canonicalize().unwrap();
		let path = path.to_str().unwrap();
		tileset = quote! {
		#tileset;
		const _: &[u8] = ::core::include_bytes!(#path)
		}
	}

@aleokdev
Copy link
Contributor

aleokdev commented May 4, 2023

Have you considered using a build script insead to convert your maps? You can select when to rerun it, and you won't need any macro usage in your app code.

@LuckyTurtleDev
Copy link
Author

Have you considered using a build script insead to convert your maps?

I personal prefer using macros, because you can easier see, where the generated code is included. A build script instead does write in some "random" files and is run every time if I compile something.

However the current workaround does work for me.

@aleokdev
Copy link
Contributor

After refactoring my sokoban clone I had the same issue here. I've thought about it once again and I don't see much issue in adding a source member that just stores the path that the ResourceReader first used to load the tileset/map. While it is true it doesn't really belong there it's really convenient to store the source for later use.

@bjorn
Copy link
Member

bjorn commented Jun 20, 2024

@aleokdev Yeah, feel free to just add it!

@aleokdev
Copy link
Contributor

Done in #303. Will close on next release

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

Successfully merging a pull request may close this issue.

3 participants