Skip to content

Conversation

@Wcubed
Copy link
Contributor

@Wcubed Wcubed commented Sep 17, 2024

Added a HeadlessPlugins plugin group, that adds more default functionality (like logging) than the MinimumPlugins. Fixes #15203
Changed the headless example to use the new plugin group.

I am not entirely sure if the list of plugins is correct. Are there ones that should be added / removed?


The TerminalCtrlCHandlerPlugin has interesting effects in the headless example: Installing it a second time it will give a log message about skipping installation, because it is already installed. Ctrl+C will terminate the application in that case. However, not installing it the second time (so only on the app that runs once) has the effect that the app that runs continuously cannot be stopped using Ctrl+C.
This implies that, even though the second app did not install the Ctrl+C handler, it did something because it was keeping the one from the first app alive.
Not sure if this is a problem or issue, or can be labeled a wierd quirk of having multiple Apps in one executable.

@Wcubed
Copy link
Contributor Author

Wcubed commented Sep 17, 2024

Update because of typo

@mnmaita mnmaita added C-Feature A new feature, making something new possible A-App Bevy apps and plugins S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 17, 2024
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Including audio, input and dev tools in this set feels quite odd to me. The value for this plugin set is overwhelmingly on servers, which shouldn't benefit from any of those.

@alice-i-cecile alice-i-cecile added the M-Release-Note Work that should be called out in the blog due to impact label Sep 17, 2024
@Wcubed
Copy link
Contributor Author

Wcubed commented Sep 17, 2024

Good point, I have removed them from the list.

bevy_transform:::TransformPlugin,
bevy_hierarchy:::HierarchyPlugin,
bevy_diagnostic:::DiagnosticsPlugin,
bevy_a11y:::AccessibilityPlugin,
Copy link
Contributor

Choose a reason for hiding this comment

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

Accessibility might be less common than the rest, I wonder if it's worth including by default

Copy link
Member

Choose a reason for hiding this comment

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

IMO this should also be excluded: it's very much tied to input handling and UI.

@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 17, 2024
@alice-i-cecile
Copy link
Member

@Wcubed once we cut AccessibilityPlugin from this set this LGTM and I'll merge.

/// This plugin is included within `DefaultPlugins` and `MinimalPlugins` when the `bevy_ci_testing`
/// feature is enabled. It is recommended to only used this plugin during testing (manual or
/// This plugin is included within `DefaultPlugins`, `HeadlessPlugins` and `MinimalPlugins`
/// when the `bevy_ci_testing`feature is enabled.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// when the `bevy_ci_testing`feature is enabled.
/// when the `bevy_ci_testing` feature is enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, fixed

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Sep 19, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Sep 19, 2024
Merged via the queue into bevyengine:main with commit 55c84cc Sep 19, 2024
@Wcubed Wcubed deleted the headless_plugins branch September 19, 2024 18:02
@alice-i-cecile
Copy link
Member

Thank you to everyone involved with the authoring or reviewing of this PR! This work is relatively important and needs release notes! Head over to bevyengine/bevy-website#1689 if you'd like to help out.

@mockersf
Copy link
Member

mockersf commented Oct 29, 2024

I'm not sure this is a good idea to have in Bevy. "Headless" meaning will change depending on the use case, and controlling it with a plugin rather than features means longer compile time and a lot of code that can't be used, events not registered, component not reflected, but still available when developing which will mean a less good developer experience.

@Luminoth
Copy link
Contributor

Luminoth commented Dec 26, 2024

Oh my gosh, why is this only behind a feature gate now? Is it still correct that to do a runtime headless you have to do this long mess of stuff?

        app.add_plugins((
            MinimalPlugins.set(bevy::app::ScheduleRunnerPlugin::run_loop(
                bevy::utils::Duration::from_secs_f64(1.0 / SERVER_TICK_RATE as f64),
            )),
            bevy::app::PanicHandlerPlugin,
            bevy::log::LogPlugin::default(),
            bevy::transform::TransformPlugin,
            bevy::hierarchy::HierarchyPlugin,
            bevy::diagnostic::DiagnosticsPlugin,
            bevy::asset::AssetPlugin::default(),
            bevy::scene::ScenePlugin,
            bevy::animation::AnimationPlugin,
            bevy::state::app::StatesPlugin,
        ));

I was looking forward to not having to maintain the entire set of "everything that isn't rendering" just to have a switch for this.

I guess this example is implying using DefaultPlugin with the ScheduleRunnerPlugin will get the same result?

@mockersf
Copy link
Member

Is it still correct that to do a runtime headless you have to do this long mess of stuff?

No, headless is just

        app.add_plugins((
            MinimalPlugins.set(bevy::app::ScheduleRunnerPlugin::run_loop(
                bevy::utils::Duration::from_secs_f64(1.0 / SERVER_TICK_RATE as f64),
            )),
        ));

All the others are entirely dependent on what you're doing. In your list you've only selected some of the Bevy default plugins.

I guess this example is implying using DefaultPlugin with the ScheduleRunnerPlugin will get the same result?

And no primary window. You can also decide to disable the winit and window plugins

@Luminoth
Copy link
Contributor

You're saying this is what I should do?

        app.add_plugins(
            DefaultPlugins
                .set(WindowPlugin {
                    primary_window: None,
                    ..default()
                })
                .disable::<bevy::winit::WinitPlugin>(),
        )
        .add_plugins(bevy::app::ScheduleRunnerPlugin::run_loop(
            bevy::utils::Duration::from_secs_f64(1.0 / SERVER_TICK_RATE as f64),
        ));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-App Bevy apps and plugins C-Feature A new feature, making something new possible M-Release-Note Work that should be called out in the blog due to impact S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HeadlessPlugins for headless apps, rather than MinimalPlugins

7 participants