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

Update to bevy 0.11 #141

Merged

Conversation

KMaheshBhat
Copy link
Contributor

Updates on top of #137

  • upgrade to bevy 0.11 crate

@KMaheshBhat KMaheshBhat mentioned this pull request Jul 10, 2023
Copy link

@NotAFile NotAFile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beat me to it! These changes are mostly the same as mine, but I've commented with some minor differences and explanations.

crates/bevy-inspector-egui/Cargo.toml Show resolved Hide resolved
crates/bevy-inspector-egui/Cargo.toml Outdated Show resolved Hide resolved
crates/bevy-inspector-egui/src/bevy_inspector/mod.rs Outdated Show resolved Hide resolved
crates/bevy-inspector-egui/src/quick.rs Outdated Show resolved Hide resolved
@@ -366,13 +366,6 @@ impl InspectorUi<'_, '_> {
}
TypeInfo::Enum(info) => self.ui_for_enum_many(info, ui, id, options, values, projector),
TypeInfo::Value(info) => self.ui_for_value_many(info, ui, id, options),
TypeInfo::Dynamic(_) => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, this should technically be replaced with an is_dynamic check, but I could not figure out whether this is actually relevant anymore.

Copy link
Contributor Author

@KMaheshBhat KMaheshBhat Jul 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unsure. (inherited from #137 )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too familiar with Reflect specifics, thanks for pointing that out

@@ -894,7 +887,10 @@ impl InspectorUi<'_, '_> {
id: egui::Id,
options: &dyn Any,
) -> bool {
let type_info = value.get_type_info();
let Some(type_info) = value.get_represented_type_info() else {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I panicked with a message similar to the unreachable call below, as this can only happen when you create a dynamic enum that does not represent any type, which AIUI should not be able to happen with a well-formed reflect impl.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am leaving it as is from #137 as left by @Vrixyz

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed get_type_info to get_represented_type_info because I thought get_type_info was removed as I had a compile error leading me that way, but I might have been mistaken as I see it in there: https://docs.rs/bevy/0.11.0/bevy/reflect/struct.TypeRegistryInternal.html#method.get_type_info ; maybe I was just missing a trait/derive or import ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Vrixyz nope that part's correct, I just highlighted the wrong line... I'm referring to the error handling below

crates/bevy-inspector-egui/src/reflect_inspector/mod.rs Outdated Show resolved Hide resolved
This required further upgrades to other related dependencies:

- bevy_egui 0.20 > 0.21
- egui 0.21 > 0.22
- egui_dock 0.4 > 0.6
- egui-gizmo 0.10 > 0.11
@Vrixyz Vrixyz mentioned this pull request Jul 11, 2023
@jakobhellermann jakobhellermann merged commit a181bdf into jakobhellermann:main Jul 12, 2023
@jakobhellermann
Copy link
Owner

Thanks for the PR!

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.

4 participants