-
Notifications
You must be signed in to change notification settings - Fork 258
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 for bevy 0.10 #148
Update for bevy 0.10 #148
Conversation
Initially got the examples working with some string and bubblegum. Wondering if the instances of resources with |
@@ -171,9 +175,9 @@ pub struct EguiNode { | |||
|
|||
impl EguiNode { | |||
/// Constructs Egui render node. | |||
pub fn new(window_id: WindowId) -> Self { | |||
pub fn new(window_entity: Entity) -> Self { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to update to bevy main before seeing your PR ; and I did a type WindowEntity = Entity
, to help me understand where the various Entity
references were supposed to reference a window.
Would that be interesting ? It might be discussed in another issue, upgrading can already be a big enough scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That could potentially help make things a bit more clear, though I wonder if we should really be storing the entity all over the place vs turning most things into components on the window entity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like us to try the components approach. Not sure what that would look like exactly, but since windows are entities now, storing additional data as components sounds more idiomatic
Fix build error from missing add_systems_to_schedule
examples/render_to_image_widget.rs
Outdated
) { | ||
let cube_preview_texture_id = egui_ctx.image_id(&cube_preview_image).unwrap(); | ||
let preview_material_handle = preview_cube_query.single(); | ||
let preview_material = materials.get_mut(preview_material_handle).unwrap(); | ||
|
||
let ctx = egui_ctx.ctx_mut(); | ||
let ctx = egui_ctx.ctx_for_window_mut(windows.iter().next().unwrap()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's a loss of ergonomics indeed, I look forward to what can be done component-wise :) ; egui_context stored in a component attached to a window is my first not-much-informed thought
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now this looks like:
egui_ctx: Query<&EguiContext, With<Window>>,
[...]
let ctx = egui_ctx.iter().next().unwrap();
Thoughts?
The With<Window>
isn't really needed, but maybe clarifies things. idk if it makes sense or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With<PrimaryWindow>
might convey a better intention, that way you can use egui_ctx.single()
?
Bevy 0.10
Thanks a lot for the help! I merged the update as part of #159, so closing this one. |
Having a go at trying to update this for current bevy main. Tried to use the window Entity as the window id, but running into the issue that it is currently designed expecting that the id for the primary window will already exist before the window does.
Most relevant PRs to look at:
Windows as Entities #5589
Migrate engine to Schedule v3 #7267