Skip to content

Conversation

@ThierryBerger
Copy link
Contributor

@ThierryBerger ThierryBerger commented Sep 24, 2024

Currently testing on bevy 0.15 rc3.

src/lib.rs Outdated
pub render_target_size: &'static mut RenderTargetSize,
/// [`Window`] component, when rendering to a window.
pub window: Option<&'static mut Window>,
/// Cursor for the
Copy link
Contributor

@Friz64 Friz64 Sep 27, 2024

Choose a reason for hiding this comment

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

Unfinished comment

Copy link
Contributor Author

@ThierryBerger ThierryBerger Sep 30, 2024

Choose a reason for hiding this comment

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

Thanks for pointing that out!

It's part of an actually a non trivial change: The handling of the cursor is changed in bevy 0.15 ; in this PR I moved the initialization of the cursor to the user through configure_cursor :
https://github.com/mvlabat/bevy_egui/blob/3be113b82f8b7122b33f92467d20d2560e0d7722/examples/ui.rs#L56-L60

But I think this code snippet should live with bevy_egui (or bevy_something 🤔 ; but in the meantime we should be able to add it if not present.)

@PPakalns
Copy link
Contributor

PPakalns commented Oct 3, 2024

Currently testing on bevy commit 9386bd0

* cursor handling changed, I think we should initialize it for all windows if not present (currently not implemented), see discussion: https://github.com/mvlabat/bevy_egui/pull/309/files#r1780569354

Just brainstorming cursor handling.
Maybe we could add for the window "EguiCursorIcon" component that bevy_egui sets.

We can provide a system that is added by default that correctly sets CursorIcon for the window from the egui EguiCursorIcon value.

But user could opt out:
User can handle manually EguiCursorIcon, it could be useful in cases where user would like to read the EguiCursorIcon state, but would want to have custom logic (For example user is utilizing a lot of custom cursor icons. He needs information from egui about cursor type, but wants to manually set their own cursor icons from icon set that do not map to SystemCursorIcon enum.

In this case it would be nice if egui would not set CursorIcon component directly.


Probably this functionality could be added in a PR at later date.

@vladbat00 vladbat00 force-pushed the main branch 5 times, most recently from 879a24b to 8956b27 Compare October 4, 2024 15:20
@khassar00
Copy link

bevy pushed version 0.15.0-rc.1

Something is rendering! but still mostly broken.
@ThierryBerger
Copy link
Contributor Author

ThierryBerger commented Oct 31, 2024

bevy pushed version 0.15.0-rc.1

@Friz64
Copy link
Contributor

Friz64 commented Oct 31, 2024

Also relevant: bevyengine/bevy#16093. As a workaround we can always just enable the custom_cursor feature and probably no one will notice or care, but still, it's relevant.

@ThierryBerger ThierryBerger changed the title Update to bevy main Update to bevy rc 2 Nov 5, 2024
@ThierryBerger
Copy link
Contributor Author

ThierryBerger commented Nov 5, 2024

Also relevant: bevyengine/bevy#16093. As a workaround we can always just enable the custom_cursor feature and probably no one will notice or care, but still, it's relevant.

I implemented your suggestion by always enabling custom_cursor bevy feature, this is closest to previous behaviour, a future PR could easily build on that to add custom cursor support for bevy_egui, but I'll consider this out of scope for this "upgrade bevy" PR.

@ThierryBerger ThierryBerger changed the title Update to bevy rc 2 Update to bevy rc 3 Nov 7, 2024
@ThierryBerger ThierryBerger changed the title Update to bevy rc 3 Update to bevy 0.15 Nov 7, 2024
@simon-an simon-an mentioned this pull request Nov 30, 2024
@vladbat00 vladbat00 merged commit 7af4d68 into vladbat00:main Nov 30, 2024
7 checks passed
@vladbat00
Copy link
Owner

@Vrixyz huge thanks, as always! I've just updated it to the actual 0.15 release and merged.

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.

5 participants