search

jakobhellermann / bevy-inspector-egui

Inspector plugin for the bevy game engine
Apache License 2.0
1.1k stars 164 forks source link

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

Closed eupraxia05 closed 3 months ago

eupraxia05 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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.