jakobhellermann / bevy_editor_pls

In-App editor tools for bevy applications
Other
758 stars 78 forks source link

Upgrade to Bevy 0.14 & upgrade egui-gizmo dependency #110

Closed zhaop closed 3 months ago

zhaop commented 4 months ago

I am no longer working on this PR due to lack of time. I'm keeping it open for now for visibility -- feel free to make your own changes and make it your own PR.

It basically works, except for one problem where certain windows would keep growing in height.


This PR allows bevy_editor_pls to be used with Bevy 0.14, and is based on top of #106 by @ActuallyHappening .

Before anything happens to this, we should probably first:


Concretely, this implements the following migration steps:

and also fixes an example:

zhaop commented 4 months ago

@jakobhellermann The changes for bevy 0.14 are done here -- this is just waiting on a few dependencies to get published to crates.io (& PR #106).

FYI commit e9bec52dce7e480612b3fbdba376c02f25dfed7b also bumps the version number to 0.9.0 & adds corresponding entries to README, CHANGELOG -- but let me know if you want me to revert that.

jakobhellermann commented 4 months ago

Thanks a lot. I'll update bevy_mod_debugdump and then we can wait for https://github.com/urholaukkarinen/transform-gizmo/pull/69 and merge + release

jakobhellermann commented 4 months ago

bevy_mod_debugdump is updated

zhaop commented 4 months ago

Hmm just noticed a few more things: forgot about the examples 😅 but more seriously, opening any editor window results in:

thread 'main' panicked at /Users/patrick/.cargo/registry/src/index.crates.io-6f17d22bba15001f/egui-0.28.1/src/placer.rs:109:9:
assertion failed: child_size.is_finite() && child_size.x >= 0.0 && child_size.y >= 0.0
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Encountered a panic in system `bevy_editor_pls_core::editor::Editor::system`!
Encountered a panic in system `bevy_app::main_schedule::Main::run_main`!

which is caused by bevy_editor_pls/crates/bevy_editor_pls_core/src/editor.rs:460:

ui.allocate_space(ui.available_size() - (5.0, 5.0).into());

It sometimes requests space with negative width, which was "tolerated" before egui 0.28, but since egui #4478 is now an assert that panics. I'm not so familiar with how this crate handles sizes, so I'm not sure how to deal with this yet. Any ideas?

jakobhellermann commented 4 months ago

I'd try replacing the -5 with something like a saturating_sub or an if to make sure it never goes below zero.

zhaop commented 4 months ago

Hmm, changing it to

let desired_size = (ui.available_size() - (5.0, 5.0).into()).max((0.0, 0.0).into());
ui.allocate_space(desired_size);

where desired_size.y used to be -5 and is now 0, leads to an ever growing window height (here on the "empty" example):

https://github.com/jakobhellermann/bevy_editor_pls/assets/6090897/f22dad64-cee8-47c4-a0e2-76d07b2b7008

For context, here is the surrounding for loop:

        for (i, floating_window) in floating_windows.into_iter().enumerate() {
            let id = egui::Id::new(floating_window.id);
            let title = self.windows[&floating_window.window].name;

            let mut open = true;
            let default_size = self.windows[&floating_window.window].default_size;
            let mut window = egui::Window::new(title)
                .id(id)
                .open(&mut open)
                .resizable(true)
                .default_size(default_size);
            if let Some(initial_position) = floating_window.initial_position {
                window = window.default_pos(initial_position - egui::Vec2::new(10.0, 10.0))
            }
            window.show(ctx, |ui| {
                self.editor_window_inner(world, internal_state, floating_window.window, ui);
                let desired_size = (ui.available_size() - (5.0, 5.0).into()).max((0.0, 0.0).into());
                ui.allocate_space(desired_size);
            });

            if !open {
                close_floating_windows.push(i);
            }
        }
zhaop commented 4 months ago

The examples are now migrated, cargo test no longer shows errors, everything compiles.

Some examples are a bit broken, although this was already the case on main:

Caused by: In a RenderPass note: encoder = main_opaque_pass_3d_command_encoder In a set_viewport command Viewport has invalid rect Rect { x: 1501.0, y: 92.0, w: 2560.0, h: 1440.0 }; origin and/or size is less than or equal to 0, and/or is not contained in the render target (2560, 1440, 1)

note: run with RUST_BACKTRACE=1 environment variable to display a backtrace thread 'main' panicked at /Users/patrick/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_tasks-0.14.0/src/single_threaded_task_pool.rs:144:54: called Option::unwrap() on a None value Encountered a panic in system bevy_render::renderer::render_system!



- `ui` is missing an asset at `bevy_editor_pls/crates/bevy_editor_pls/assets/branding/bevy_logo_dark_big.png`
zhaop commented 4 months ago

Hmm, I'm not sure I have time to debug the growing-window-height issue above. Does anyone else with more experience with egui have an idea how to resolve this?

Aceeri commented 4 months ago

I can't seem to repro the issue, you said using the Open Window would cause it right?

zhaop commented 4 months ago

@Aceeri Could you give these steps a try:

git clone git@github.com:zhaop/bevy_editor_pls.git --branch bevy-0.14
cd bevy_editor_pls/crates/bevy_editor_pls
cargo run --example empty

Then, click "Open window", then click "Hierarchy".

What I see: the Hierarchy window shows up, and its height continuously grows until filling the whole window height.

What I expected to see: the Hierarchy window shows up at a certain size and stays the same size.

Aceeri commented 4 months ago

@Aceeri Could you give these steps a try:

git clone git@github.com:zhaop/bevy_editor_pls.git --branch bevy-0.14
cd bevy_editor_pls/crates/bevy_editor_pls
cargo run --example empty

Then, click "Open window", then click "Hierarchy".

What I see: the Hierarchy window shows up, and its height continuously grows until filling the whole window height.

What I expected to see: the Hierarchy window shows up at a certain size and stays the same size.

Gotcha, I can see it now

XX commented 3 months ago

@zhaop What is the status of this PR? Is help needed?

zhaop commented 3 months ago

@XX I'd be happy if someone could take over this PR -- I don't think I'll have time to work on this anymore (and have moved away from using this crate in my project).

As I understand, it basically works, except for that one problem where the height of certain windows keeps growing when it shouldn't.

jakobhellermann commented 3 months ago

I think I figured out a fix for that, the windows where this was a problem had .auto_shrink([false, false]), removing that make them not grow automatically anymore: https://github.com/jakobhellermann/bevy_editor_pls/pull/110/commits/8ae7170f4a5851bb5459530510053efa3b95e45e

jakobhellermann commented 3 months ago

I think this is good to merge now, thank you very much for the PR @zhaop and everyone for the patience, I'm gonna merge this now and release a new version

zhaop commented 3 months ago

Thanks a lot @jakobhellermann for the fix :)