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.9 dev #127

Merged
merged 4 commits into from
Nov 13, 2022
Merged

update to bevy 0.9 dev #127

merged 4 commits into from
Nov 13, 2022

Conversation

terhechte
Copy link
Contributor

One of my projects has a rendering bug that is fixed on Bevy/main. Due to breaking changes in 0.9 dev, I couldn't use bevy_egui anymore. This PR adopts all the bevy changes required to run bevy_egui with the current state of bevy 0.9 dev. Not sure if you already want to merge it, but I wanted to share it already in case anybody else is in my situation.

Comment on lines +121 to +127
impl Deref for EguiRenderOutputContainer {
type Target = HashMap<WindowId, EguiRenderOutput>;

fn deref(&self) -> &Self::Target {
&self.0
}
}
Copy link

Choose a reason for hiding this comment

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

You can actually just use Bevy's Deref and DerefMut derives to achieve this.

Comment on lines +482 to +484
world.insert_resource(EguiRenderInputContainer(
HashMap::<WindowId, EguiInput>::default(),
));
Copy link

Choose a reason for hiding this comment

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

I would just make all these new resources derive Default. Then you can just do EguiRenderInputContainer::default() (or even world.init_resource::<EguiRenderInputContainer>()).

Copy link
Owner

Choose a reason for hiding this comment

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

And will also make it possible to initialize resources if some users decide not to add the egui plugin, by adding the resources and systems manually (with an intention to replace some of them).

@@ -229,6 +233,7 @@ pub fn process_input(
if !event.char.is_control() {
input_resources
.egui_input
.0
Copy link

Choose a reason for hiding this comment

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

These .0's won't be needed if you derive DerefMut.

Copy link

@kerkmann kerkmann left a comment

Choose a reason for hiding this comment

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

Bevy 0.9 is now released. :)

Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@mvlabat
Copy link
Owner

mvlabat commented Nov 12, 2022

Will release the update tomorrow (it's midnight here in Ukraine 🙂).

@mvlabat
Copy link
Owner

mvlabat commented Nov 13, 2022

Merging the PRs, thanks!
I'm also going to address @MrGVSV's comments and do some cleaning up in a separate one.

@mvlabat mvlabat merged commit c906611 into mvlabat:main Nov 13, 2022
@@ -21,18 +21,18 @@ open_url = ["webbrowser"]
default_fonts = ["egui/default_fonts"]

[dependencies]
bevy = { version = "0.8", default-features = false, features = ["bevy_render", "bevy_core_pipeline", "bevy_asset"] }
bevy = { version = "0.9.0", default-features = false, features = ["bevy_render", "bevy_core_pipeline", "bevy_asset"] }

Choose a reason for hiding this comment

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

@mvlabat
Thanks for merging it! <3

btw; Is there a reason why the version is fixed to 0.9.0 instead of 0.9? :)

Copy link
Owner

Choose a reason for hiding this comment

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

Hi! No particular reason. In 0.9.0 behaves the same way as ^0.9.0 (and 0.9). If 0.9.1 is released, this won't prevent to install it.
https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-dependencies-from-cratesio

Choose a reason for hiding this comment

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

Thanks For Letting Me Know. :3

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