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

Prerequisite plugins and resources / dependency handling: .require_plugin() and .require_resource(). #14192

Open
colleen05 opened this issue Jul 6, 2024 · 1 comment
Labels
A-App Bevy apps and plugins C-Feature A new feature, making something new possible X-Contentious There are nontrivial implications that should be thought through

Comments

@colleen05
Copy link

What problem does this solve or what need does it fill?

Bevy does not currently provide an ergonomic way to handle prerequisite plugins / resources in the same fashion of similar initialisation / configuration concerns of apps.

Adding plugins and resources that may or may not already be added to the app requires condition checking outside of the usual chain of app.add_plugin(...).insert_resource(...). etc. that you would have in Plugin::build() or main().

Often this is done for setting up prerequisites, such as plugins which rely on egui checking to make sure EguiPlugin is added.

In these cases, a bit of semantics is lost in the code, which may obfuscate the code-writer's intent. It also unnecessarily separates initialisation logic into multiple parts of your code.

For example, the following patterns are very common:

fn build(app: &mut App) {
    app.add_systems(Update, ui_update);
    
    if !app.world.contains_resource::<UiSettings>() {
        panic!("UI Settings not present. Please provide a UiSettings resource!");
    }
    
    if !app.is_plugin_added::<EguiPlugin>() {
        app.add_plugins(EguiPlugin);
    }
}

What solution would you like?

I propose App::require_plugin::<T>(auto_init: bool) -> ? and App::require_resource::<T>(auto_init: bool) -> ?. The ? type here could be, for example, a Result for better error checking.

These functions would perform the same sort of checking in the code above, but making the code more concise and better preserving semantics / intent.

Example:

fn build(app: &mut App) {
    app.add_systems(Update, ui_update)
        .require_resource::<UiSettings>(false)
        .require_plugin::<EguiPlugin>(true);
}

Additionally, .require_* functions may be able to log that a panic was caused due to a dependency error, which may or may not happen when using the current method of manually checking - particularly if dependency checking is done many times throughout the code and the developer has forgotten to write sufficient logging logic.

Issues with this solution

  • Running additional code along side requirement checks. Example: Attempting to manually initialise a resource/plugin outside of its Default constructor, altering the app initialisation if a resource/plugin is not present, etc.
  • Does not work with Plugin sets. I'm not a pro Rust developer, but I'm pretty sure the following, although pretty, would not be possible: app.require_plugins::<(EguiPlugin, PlayerPlugin, ClientPlugin)>(). Maybe another solution than the one proposed would allow similarly ergonomic code.
  • Is this in the scope of Bevy's concerns? Although this is a very declarative approach, and does provide an ergonomic benefit to the developer, whether or not this is bloat or is better left to a third-party crate is questionable.

Additional context

Many other languages and frameworks such as PHP, NodeJS, Lua, Elixir, etc. have implemented requirement handling for conditional initialisation of external code.

This too alleviates the need for manual checking, and makes the intent of the code more clear.

Additional additional context

If this gets accepted and the behaviour / implementation details are decided upon, I'd be more than happy to create a PR with my implementation :)

@colleen05 colleen05 added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Jul 6, 2024
@mockersf
Copy link
Member

mockersf commented Jul 6, 2024

This would be making the bandaid we currently have the official solution.

There's work ongoing for a dependency graph for plugins (#11228) but it will take more time to finish

@mockersf mockersf added A-App Bevy apps and plugins X-Contentious There are nontrivial implications that should be thought through and removed S-Needs-Triage This issue needs to be labelled labels Jul 6, 2024
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 X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

No branches or pull requests

2 participants