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

WindowDescriptor added through a startup_system command always uses default WindowDescriptor from default #278

Closed
scriptandcompile opened this issue Aug 21, 2020 · 11 comments
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Bug An unexpected or incorrect behavior C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@scriptandcompile
Copy link

The code below demonstrates the bug. This should create a window with the title of 'test' but instead will create one with the title of 'bevy', at least on Linux. I haven't tested this on any other platform.

use bevy::prelude::*;

fn main() {
    App::build()
        .add_startup_system(ui_loader.system())
        .add_default_plugins()
        .run();
}

fn ui_loader(mut commands: Commands) {
    let window = WindowDescriptor {
        title: "test".to_string(),
        width: 200,
        height: 300,
        vsync: true,
    };
    commands.insert_resource(window);
}
@scriptandcompile scriptandcompile changed the title WindowDescriptor added through a startup_system command always has default title WindowDescriptor added through a startup_system command always uses default WindowDescriptor from default Aug 21, 2020
@karroffel karroffel added C-Bug An unexpected or incorrect behavior A-Windowing Platform-agnostic interface layer to run your app in labels Aug 22, 2020
@coltontcrowe
Copy link

I checked and saw this on Windows 10 as well.

It's also worth noting that I needed to add a ..Default::default() line after the "vsync: true" line to get it to compile.

@ropewalker
Copy link

ropewalker commented Aug 23, 2020

It's also worth noting that I needed to add a ..Default::default() line after the "vsync: true" line to get it to compile.

WindowDescriptor was updated there: #149 — hence the need for ..Default::default().

@Kabie
Copy link

Kabie commented Aug 23, 2020

Try using add_resource before add_default_plugins to set the window title.

@scriptandcompile
Copy link
Author

scriptandcompile commented Aug 23, 2020

If I do that it works, yes. The point was that I was trying to setup a startup resource in a startup system. If I have to initialize things outside the App builder section, what is the point of them? It's an obvious problem. I'm not looking for a way to work around the problem (though, thank you for that and it's good to note), but pointing out the issue with the current default behaviour.

It seems like a case of "let's make it easy for the tutorial at the cost of functionality" since all that is needed is to not add a default window when the default plugins are added.

@Kabie
Copy link

Kabie commented Aug 23, 2020

Then I think you have to:

  • copy the build steps from add_default_plugins
  • set add_primary_window: false for WindowPlugin
  • and send the CreateWindow event by yourself

@scriptandcompile
Copy link
Author

scriptandcompile commented Aug 24, 2020

That fails with a panic when the render plugin tries to do a swap chain event without a window.

use bevy::prelude::*;
use bevy::window::WindowMode;
use bevy::type_registry::TypeRegistryPlugin;
use bevy::core::CorePlugin;
use bevy::transform::TransformPlugin;
use bevy::diagnostic::DiagnosticsPlugin;
use bevy::input::InputPlugin;
use bevy::window::WindowPlugin;
use bevy::asset::AssetPlugin;
use bevy::scene::ScenePlugin;
use bevy::render::RenderPlugin;
use bevy::sprite::SpritePlugin;
use bevy::pbr::PbrPlugin;
use bevy::ui::UiPlugin;
use bevy::text::TextPlugin;
use bevy::audio::AudioPlugin;
use bevy::gltf::GltfPlugin;
use bevy::winit::WinitPlugin;
use bevy::wgpu::WgpuPlugin;

fn main() {
    App::build()
        .add_startup_system(ui_loader.system())
        .add_plugin(TypeRegistryPlugin::default())
        .add_plugin(CorePlugin::default())
        .add_plugin(TransformPlugin::default())
        .add_plugin(DiagnosticsPlugin::default())
        .add_plugin(InputPlugin::default())
        .add_plugin(WindowPlugin {
            add_primary_window: false,
            exit_on_close: true,
        })
        .add_plugin(AssetPlugin::default())
        .add_plugin(ScenePlugin::default())
        .add_plugin(RenderPlugin::default())
        .add_plugin(SpritePlugin::default())
        .add_plugin(PbrPlugin::default())
        .add_plugin(UiPlugin::default())
        .add_plugin(TextPlugin::default())
        .add_plugin(AudioPlugin::default())
        .add_plugin(GltfPlugin::default())
        .add_plugin(WinitPlugin::default())
        .add_plugin(WgpuPlugin::default())
        .run();
}


fn ui_loader(mut commands: Commands) {

    // https://github.com/bevyengine/bevy/issues/278
    // WindowDescriptor added as a resource always uses default window.
    let window = WindowDescriptor {
        title: "test".to_string(),
        width: 200,
        height: 300,
        vsync: true,
        mode: WindowMode::Windowed,
        resizable: true,
        ..Default::default()
    };

    commands.insert_resource(window);
}

Which is also an issue, but a different one.

This is a usability nightmare. Moving the building of a component like this shouldn't cause this many issues.

@cart
Copy link
Member

cart commented Aug 24, 2020

Yeah re-registering all of the default plugins shouldn't be required for something like this. Currently the WindowDescriptor resource is just used as a "configuration item" that determines the default primary window configuration. I agree that we need more control over window settings at runtime. Right now its very "static".

What this looks like (dynamically adapting to Res<WindowDescriptor> changes, sending events, etc) is TBD. Lets scope this issue to discussing what the ideal api looks like.

@scriptandcompile
Copy link
Author

Well, my suggestion off the top of my head is not to create a default window. If someone doesn't want a window, they don't want a window. This should be distinctly different than 'headless' mode. A 'headless' mode means I want to pretend I've got a window...but not actually do any drawing. This is very useful in some contexts. The tutorial should definitely cover adding a primary window (and likely should cover adding a second window, perhaps to send diagnostics or use as a dual windowed program).

Further, if not creating a window crashes the render system, then the render system depends on the window. This is important! We either make the render window independent of the windowing system or make it explicit how one plugin/system depends on another.

Reordering the default plugin's inclusion shouldn't cause a panic. disabling any of those plugins shouldn't cause a panic. Setting them with valid options shouldn't cause a panic.

@coltontcrowe
Copy link

I want to weigh in that #65 and #69 are related to this, but I don't believe anyone has worked on those yet.

@alice-i-cecile
Copy link
Member

Another bug caused by #1255.

@alice-i-cecile alice-i-cecile added the C-Usability A targeted quality-of-life change that makes Bevy easier to use label Feb 17, 2021
@mockersf
Copy link
Member

mockersf commented Nov 1, 2022

it's no longer possible to add WindowDescriptor as a resource. Initial window setup is now done when adding the plugin

@mockersf mockersf closed this as completed Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Bug An unexpected or incorrect behavior C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

No branches or pull requests

8 participants