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

Added a dropdown to select loaded images in the inspector for Handle<Image>. #190

Merged
merged 4 commits into from
Mar 28, 2024

Conversation

eupraxia05
Copy link
Contributor

@eupraxia05 eupraxia05 commented Mar 17, 2024

Hiya! Took a crack at this as discussed in Discord.

Looks like a big change, but a large part of the diff is just moving some of the functionality from ui_readonly to a update_and_show_image function to make passing the world a little easier between it and the mutable ui function.

This likely isn't ready to merge yet, just wanted to get the pull request going and get eyes on the last issue I'm having with it - for some reason an error is generated when line 33 is uncommented - this works in the non-mutable ui_readonly, but for some reason in the mutable version the compiler complains about a self borrow escaping the scope of the function. Not sure why, still looking into that.

One more thing I wanted to ask about: for my purposes, I'm interested in showing all available assets, not just loaded ones. I'd probably go about implementing that using AssetReader to get all available asset paths instead of just iterating over Assets<Image>. It'd probably be a bit more complex but a lot more powerful as an editor tool and more generalizable to any Handle<Asset> type. Is that something you'd be interested in for a future PR?

Thanks! :)

@eupraxia05
Copy link
Contributor Author

eupraxia05 commented Mar 17, 2024

Looking at that compile error with self escaping the method body in the no_world_in_context(ui, self.reflect_short_type_path()) call, it seems like it breaks in the non-mutable version of the method because the passed &str is borrowed from self and passed over to egui without being owned. This works fine in the immutable version, but breaks in the mutable version because the mutable reference needs to be unique. I've pushed a hack that takes an immutable reference to self and passes it to that method call, but there's probably a better way to approach that?

@jakobhellermann
Copy link
Owner

Thanks for the PR, and sorry for not reviewing it for so long.

I've pushed a hack that takes an immutable reference to self and passes it to that method call, but there's probably a better way to approach that?

I'm fine merging the hack, although I have no idea what is going on. The no_world_in_context function just takes a &str with any lifetime so I don't know where the argument requires that '1 must outlive 'static comes from.

One more thing I wanted to ask about: for my purposes, I'm interested in showing all available assets, not just loaded ones. I'd probably go about implementing that using AssetReader to get all available asset paths instead of just iterating over Assets. It'd probably be a bit more complex but a lot more powerful as an editor tool and more generalizable to any Handle type. Is that something you'd be interested in for a future PR?

Sounds good to me. While reviewing this I thought to myself, actually why wouldn't this work for any handle, so yea that would be cool if that works. With the assetreader we probably will have to cache that data somehow to make sure that displaying it isn't immensely slow, but that can go into another PR.

@jakobhellermann
Copy link
Owner

I pushed a commit formatting this code and I'm gonna merge this since I plan on doing a release for the new bevy_egui version now anyways.

@jakobhellermann jakobhellermann merged commit 5596f18 into jakobhellermann:main Mar 28, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants