From c3ce1d2aa58da142989909f8b64af38ed20a0a7d Mon Sep 17 00:00:00 2001 From: Alice Date: Wed, 15 Sep 2021 02:26:49 -0400 Subject: [PATCH 01/20] First draft --- rfcs/template.md | 227 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 227 insertions(+) create mode 100644 rfcs/template.md diff --git a/rfcs/template.md b/rfcs/template.md new file mode 100644 index 00000000..0263b812 --- /dev/null +++ b/rfcs/template.md @@ -0,0 +1,227 @@ +# Feature Name: `apps_own_scheduling` + +## Summary + +All scheduling information is ultimately owned and configurable by the central app (or hand-written `bevy_ecs` equivalent). +Plugins specify local-only invariants about the ordering of their systems which must be respected. + +## Motivation + +The current plugin model works well in simple cases, but completely breaks down when the central consumer (typically, an `App`) needs to configure when and if their systems run. See [Bevy #2160](https://github.com/bevyengine/bevy/issues/2160) for more background on this problem. + +By moving the ultimate responsibility for scheduling configuration to the central app, users can comfortably combine plugins that were not originally designed to work together correctly and fit the logic into their particular control flow. + +## User-facing explanation + +**Plugins** are cohesive, drop-in units of functionality that you can add to your Bevy app. +Plugins can add **system sets** to your app or initialize **resources**. + +You can add plugins to your app using the `App::add_plugin` and `App::add_plugins` methods. +If you're new to Bevy, plugins added in this way should immediately work without further configuration. + +```rust +use bevy::prelude::*; + +fn main(){ + App::minimal().add_plugin(HelloWorldPlugin).run(); +} +``` + +You can write your own plugins by exporting a function that returns a `Plugin` struct with the desired system sets and resources. +This can be provided as a standalone function or, more commonly, as a method on a simple `MyPlugin` struct: + +```rust +struct CombatPlugin; + +impl IntoPlugin for CombatPlugin { + fn into() -> Plugin { + Plugin::default() + .add_systems("Combat", SystemSet::new().with(attack_system).with(death_system)); + .add_system("PlayerMovement", player_movement_system) + .init_resouce::() + } +} + +fn main(){ + App::default().add_plugin(CombatPlugin).run(); +} + +``` + +Under the hood, plugins are very simple, and their systems and resources can be created and configured directly. + +The new `Plugin` struct stores data in a straightforward way: + +```rust +pub struct Plugin { + pub systems: HashMap, SystemSet>, + pub resources: HashSet>; +} +``` + +When you're creating Bevy apps, you can read, add, remove and configure both systems and resources stored within a plugin that you're using, allowing you to fit it into your app's needs using the standard `HashMap` and `HashSet` APIs. +The methods shown above (such as `.add_system`) are simply convenience methods that manipulate these public fields. + +Note that each system set that you include requires its own label to allow other plugins to control their order relative to that system set, and allows the central `App` to further configure them as a unit if needed. + +### Standalone `bevy_ecs` usage + +Even if you're just using `bevy_ecs`, plugins can be a useful tool for enabling code reuse and managing dependencies. + +Instead of adding plugins to your app, independently add their system sets to your schedule and their resources to your world. + +### Hard system ordering rules + +In addition to `.before` and `.after`, `.hard_before` and `.hard_after` can be used to specify that two systems must be separated by a hard sync point in the schedule (generally the end of the stage), in addition to following the standard ordering constraint. + +This is critical when `Commands` from the first system must complete (e.g. spawning or despawning entities) before the second system can validly run. + +Note that hard system ordering rules have no special schedule-altering powers! +They are merely an assertion to be checked at the time of schedule construction. +Their purpose is to ensure that you (or the users of your plugin) do not accidentally break the required logic by shuffling when systems run relative to each other. + +### Configuring plugins + +System sets don't store simple `Systems`: instead they store fully configured `SystemDescriptor` objects which store information about when and if the system can run. This includes: + +- which stage a system runs in +- any system ordering dependencies a system may have +- any run criteria attached to the system +- which states the system is executed in +- the labels a system has + +Without any configuration, these systems are designed to operate as intended when the Bevy app is configured in the standard fashion (using `App::default()`). For many users, this should just work out of the box. + +When creating your `App`, you can add additional systems and resources directly to the app, or choose to remove system sets or resources from the plugins you are using by directly using the appropriate `HashSet` and `HashMap` methods. + +More commonly though, you'll be configuring the system sets using the labels that they were given by their plugin. +When system sets are added to a `Plugin`, the provided label is applied to all systems within that set, allowing other systems to specify their order relative to that system set. Note that *all* systems that share a label must complete before the scheduler allows their dependencies to proceed. + +The `App` can also configure systems provided by a plugin using the `App::configure_label` method. +This method allows the user to change any of the system configuration listed above by acting on the system set as a group. + +There are two important exceptions to this: + +1. `configure_label` cannot remove labels from systems. +2. `configure_label` cannot remove ordering dependencies, only add them. + +If you change the configuration of the systems in such a way that ordering dependencies cannot be satisfied (by creating an impossible cycle or breaking a hard system ordering rule by changing the stage in which a system runs), the app's schedule will panic when it is run. + +These limitations are designed to enable plugin authors to carefully control how their systems can be configured in order to avoid subtly breaking internal guarantees. + +### Writing plugins to be configured + +When writing plugins (especially those designed for other people to use), group systems that should be thought about in one coherent "unit of function" together into a single system set. + +Be sure to use hard system ordering dependencies when needed to ensure that downstream consumers cannot accidentally break your system sets which require command processing by moving systems into the same stage. +**Systems which must live in separate stages due to hard system ordering rules must be in separate system sets in order to allow users to add them to their own stages freely.** + +## Implementation strategy + +### Code organization + +As plugins no longer depend on `App` information at all, they should be moved into `bevy_ecs` directly. + +### System sets + +The changes proposed are simple ergonomic renaming to reflect the increased prominence of system sets. +`App::add_system_set` should be changed to `App::add_systems`, and `SystemSet::with_system` should be shortened to `SystemSet::with`. + +### Resources + +All resources are added to the app before the schedule begins. The app should panic if duplicate resources are detected at this stage; app users can manually remove them from one or more plugins in a transparent fashion to remove the conflict if needed. + +### Schedule overhaul prerequisites + +1. Stage boundaries must be weakened. This API assumes that e.g. system sets can span hard sync points to allow them to be inserted into a given state and that ordering dependencies operate across hard sync points. +2. All system scheduling information must be stored in the system descriptor (which are then stored within system sets) in order to enable a unified approach to configuration. See [Bevy #2736](https://github.com/bevyengine/bevy/pull/2736) for a nearly complete PR on this point. +3. Users must be able to configure systems by reference to a particular label. This proposal has previously been called **label properties**. + +### Default stages and runner + +If plugins can no longer configure the `App` in arbitrary ways, we need a new, ergonomic way to set up the default schedule and runner for standard Bevy games. + +The best way to do this is to create two new builder methods on `App`: `App::minimal` and `App::default`, which sets up the default stages, sets the runner and adds the required plugins. + +### Plugin groups + +Plugin groups now require the simple `IntoPluginGroup` trait with a `into` method that returns a `Vec`. + +Alternatively, these could just be completely deprecated due to the move towards `App::default`, which was overwhelmingly their most common use. + +## Drawbacks + +1. Plugins are no longer quite as powerful as they were. This is both good and bad: it reduces their expressivity, but reduces the risk that they mysteriously break your app. +2. Apps can directly manipulate the system ordering of their plugins. This is essential for ensuring that interoperability and configurability issues are resolved, but risks breaking internal guarantees. +3. Users can accidentally break the requirement for a hard sync point to pass between systems within a plugin as encouraged by the plugin configuration by moving system sets into the same stage. Hard ordering dependencies briefly discussed in the *Future Work* section of this RFC could ensure that this is impossible. +4. Replacing existing resource values is more verbose, requiring either a startup system or explicitly removing the resource from the pre-existing plugin. This is much more explicit, but could slow down some common use cases such as setting window size. + +## Rationale and alternatives + +### Don't plugins need to be able to mutate the app in arbitrary ways? + +After extensive experience with Bevy and review of community-made plugins, adding systems and adding resources seem to be the only functionality commonly used in plugins. + +Direct world access is the other pattern commonly used. These usages can be directly replaced by startup systems which perform the same task in more explicit and controllable fashions. + +If schedule and runner manipulation is ultimately required, this functionality can and should be added to exclusive systems instead. + +### Why should we use a structured approach to plugin definition? + +Taking a structured approach improves things by: + +1. Enforcing principled rules about label exporting. +2. Clearly communicating to users how plugins should be used. +3. Decoupling the app and plugin API to reflect the fact that the things that an app can reasonably do are a strict superset of the things that a plugin can reasonably do due to its access to more information. +4. Avoids surprising and non-local effects caused by plugins, improving transparency and reducing the risk involved in trying out a new 3rd-party plugin. + +### Why are system sets the correct granularity to expose? + +If we accept that apps must be allowed to configure when and if the systems in their plugins run, there are four possible options for how we could expose this: + +1. Individual systems. These are too granular, and risk ballooning complexity and further breaking of plugin-internal guarantees. +2. System sets. The most flexible of options: allowing plugin authors to obscure or expose details as desired in order to encourage shared configuration. Configuration applied to a system set can only be applied to all systems in the set at once. +3. Stages. This heavily discourages system parallelism, and forces a global and linear view of the schedule in order to enforce the desired ordering constraints. +4. All systems. This is far too coarse, and will prevent users from changing which stage systems runs in as any reasonable configuration API will apply the same operation to every system in a group. + +System sets are also the most flexible of these options: they can be lumped or split to fit the particular needs of the users and invariants required by the plugin's logic. + +### Why are plugins forced to export a unique label for each system set? + +One of the driving principles behind this design is that plugin authors cannot and should not predict every possible user need for their plugins. +This leads to either limited configurability, which forces dependency "vendoring" (maintaining a custom fork of the dependency) or internally developing the functionality needed, or excessive configurability, which bloats API surface in ways that are irrelevant to almost all users. + +Exported system sets are the atomic unit of plugin configurability, and configuration cannot occur effectively without labels. +By forcing a one-to-one relationship between exported labels and system sets plugin authors have a clear standard to build towards that simplifies their decisions about what to make public. + +### Why should we allow users to add dependencies to external plugin systems? + +From a design perspective, this is essential to ensure that two third-party plugins can co-exist safely without needing to know about each other's existence in any way. + +More humorously, we could *already* induce dependencies between labelled third-party systems. +Suppose we have two labelled systems: `A` and `C`, and want `A` to run before `C`. +The existing model is designed to prevent this level of meddling, and provides no way to configure the ordering of either system by any direct means. +However, if we insert a system `B` (which may do literally nothing!), and state that `B` runs after `A` and before `C`, we induce the desired dependency as system ordering is necessarily transitive. + +There is no reasonable or desirable way to prevent this: instead we should embrace this by designing a real API to do so. + +## Unresolved questions + +1. Should we change the `AppBuilder` API to be structured for consistency, or keep the existing builder pattern? +2. Should an automatic or simple coercion from a single system into a system set be added to improve ergonomics? +3. Should we rename `SystemDescriptor` to something more user-friendly like `ConfiguredSystem`? +4. Are system labels and resource types exported within the `Plugin` trait forced to be `pub`? Should they be? +5. What should `hard_before` and `hard_after` be called? + +## Future possibilities + +This is a simple building block that would help with the broader scheduler plans intended in [Bevy #2801](https://github.com/bevyengine/bevy/discussions/2801). + +Automatic hard sync point insertion is by far the most controversial and ambitious part of this proposal. +This could be extended and improved in the future with: + +1. Enhanced system visualization tools. +2. Hints to manually specify which hard sync points various systems run between. +3. Stages could (optionally?) be replaced with automatically inferred hard sync points on the basis of hard system ordering dependencies. +4. Provide a more global, staged analysis of system and resource initialization to reduce or eliminate order-dependence of app initialization, as raised in [Bevy #1255](https://github.com/bevyengine/bevy/issues/1255). +5. `as_if_before` functionality will improve the ability to control ordering between plugin system sets without inducing excessive blocking. From 4155fd67359adad1e32844dea5c388d6e0b80c51 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 15 Sep 2021 13:39:51 -0400 Subject: [PATCH 02/20] Editing pass --- ...{template.md => 33-apps_own_scheduling.md} | 81 +++++++++++-------- 1 file changed, 47 insertions(+), 34 deletions(-) rename rfcs/{template.md => 33-apps_own_scheduling.md} (77%) diff --git a/rfcs/template.md b/rfcs/33-apps_own_scheduling.md similarity index 77% rename from rfcs/template.md rename to rfcs/33-apps_own_scheduling.md index 0263b812..b34c39fd 100644 --- a/rfcs/template.md +++ b/rfcs/33-apps_own_scheduling.md @@ -2,28 +2,39 @@ ## Summary -All scheduling information is ultimately owned and configurable by the central app (or hand-written `bevy_ecs` equivalent). -Plugins specify local-only invariants about the ordering of their systems which must be respected. +All scheduling information is attached directly to systems, and ultimately owned and configurable by the central app (or hand-written `bevy_ecs` equivalent). +To ensure that this can be done safely, plugins specify invariants about the ordering of their systems which must be respected. +Hard ordering constraints (where systems must be both ordered and separated by a hard sync point) are introduced as an assert-style protection to guard against a serious and common form of plugin misconfiguration. ## Motivation -The current plugin model works well in simple cases, but completely breaks down when the central consumer (typically, an `App`) needs to configure when and if their systems run. See [Bevy #2160](https://github.com/bevyengine/bevy/issues/2160) for more background on this problem. +The current plugin model works well in simple cases, but completely breaks down when the central consumer (typically, an `App`) needs to configure when and if their systems run. The main difficulties are: + +- 3rd party plugins can have "unfixable" ordering ambiguities or interop issues with other 3rd party plugins +- plugins can not be added to states, and run criteria cannot be added +- plugins can not be incorporated into apps and games that use a non-standard control flow architecture +- end users who care about granular scheduling control for performance optimization have no way to change when systems run +- worse, plugins can freely insert their own stages, causing sudden and uncontrollable bottlenecks +- plugins cannot be used by standalone `bevy_ecs` users + +See [Bevy #2160](https://github.com/bevyengine/bevy/issues/2160) for more background on this problem. By moving the ultimate responsibility for scheduling configuration to the central app, users can comfortably combine plugins that were not originally designed to work together correctly and fit the logic into their particular control flow. ## User-facing explanation **Plugins** are cohesive, drop-in units of functionality that you can add to your Bevy app. -Plugins can add **system sets** to your app or initialize **resources**. +Plugins can initialize **resources** to store data to, or add **system sets**, used to add coherent batches of logic to your app. -You can add plugins to your app using the `App::add_plugin` and `App::add_plugins` methods. -If you're new to Bevy, plugins added in this way should immediately work without further configuration. +Plugins can be added to your app using the `App::add_plugin` method. +If you're new to Bevy, third-party plugins added in this way should immediately work without further configuration. ```rust use bevy::prelude::*; +use bevy_hypothetical_tui::prelude::*; fn main(){ - App::minimal().add_plugin(HelloWorldPlugin).run(); + App::minimal().add_plugin(TuiPlugin).run(); } ``` @@ -50,12 +61,12 @@ fn main(){ Under the hood, plugins are very simple, and their systems and resources can be created and configured directly. -The new `Plugin` struct stores data in a straightforward way: +The `Plugin` struct stores data in a straightforward way: ```rust pub struct Plugin { pub systems: HashMap, SystemSet>, - pub resources: HashSet>; + pub resources: HashSet>, } ``` @@ -64,12 +75,6 @@ The methods shown above (such as `.add_system`) are simply convenience methods t Note that each system set that you include requires its own label to allow other plugins to control their order relative to that system set, and allows the central `App` to further configure them as a unit if needed. -### Standalone `bevy_ecs` usage - -Even if you're just using `bevy_ecs`, plugins can be a useful tool for enabling code reuse and managing dependencies. - -Instead of adding plugins to your app, independently add their system sets to your schedule and their resources to your world. - ### Hard system ordering rules In addition to `.before` and `.after`, `.hard_before` and `.hard_after` can be used to specify that two systems must be separated by a hard sync point in the schedule (generally the end of the stage), in addition to following the standard ordering constraint. @@ -90,24 +95,26 @@ System sets don't store simple `Systems`: instead they store fully configured `S - which states the system is executed in - the labels a system has -Without any configuration, these systems are designed to operate as intended when the Bevy app is configured in the standard fashion (using `App::default()`). For many users, this should just work out of the box. +Without any configuration, these systems are designed to operate as intended when the Bevy app is configured in the standard fashion (using `App::default()`). Each system will belong to the standard stage that the plugin author felt fit best, and ordering between other members of the plugins and Bevy's default systems should already be set up for you. +For most plugins and most users, this strategy should just work out of the box. When creating your `App`, you can add additional systems and resources directly to the app, or choose to remove system sets or resources from the plugins you are using by directly using the appropriate `HashSet` and `HashMap` methods. +Removing resources in this way can be helpful when you want to override provided initial values, or if you need to resolve a conflict where two of your dependencies attempt to set a resource's value in conflicting way. More commonly though, you'll be configuring the system sets using the labels that they were given by their plugin. When system sets are added to a `Plugin`, the provided label is applied to all systems within that set, allowing other systems to specify their order relative to that system set. Note that *all* systems that share a label must complete before the scheduler allows their dependencies to proceed. -The `App` can also configure systems provided by a plugin using the `App::configure_label` method. -This method allows the user to change any of the system configuration listed above by acting on the system set as a group. - -There are two important exceptions to this: +The `App` can also configure systems provided by its plugins using the `App::configure_label` method. +There are four important limitations to this configurability: -1. `configure_label` cannot remove labels from systems. -2. `configure_label` cannot remove ordering dependencies, only add them. - -If you change the configuration of the systems in such a way that ordering dependencies cannot be satisfied (by creating an impossible cycle or breaking a hard system ordering rule by changing the stage in which a system runs), the app's schedule will panic when it is run. +1. Systems can only be configured as a group, on the basis of their public labels. +2. Systems can only be removed from plugins as a complete system set. +3. `configure_label` cannot remove labels from systems. +4. `configure_label` cannot remove ordering dependencies, only add them. These limitations are designed to enable plugin authors to carefully control how their systems can be configured in order to avoid subtly breaking internal guarantees. +If you change the configuration of the systems in such a way that ordering dependencies cannot be satisfied (by creating an impossible cycle or breaking a hard system ordering rule by changing the stage in which a system runs), the app's schedule will panic when it is run. +Duplicate and missing resources will also cause a panic when the app is run. ### Writing plugins to be configured @@ -116,6 +123,12 @@ When writing plugins (especially those designed for other people to use), group Be sure to use hard system ordering dependencies when needed to ensure that downstream consumers cannot accidentally break your system sets which require command processing by moving systems into the same stage. **Systems which must live in separate stages due to hard system ordering rules must be in separate system sets in order to allow users to add them to their own stages freely.** +### Standalone `bevy_ecs` usage + +Even if you're just using `bevy_ecs`, plugins can be a useful tool for enabling code reuse and managing dependencies. + +Instead of adding plugins to your app, independently add their system sets to your schedule and their resources to your world. + ## Implementation strategy ### Code organization @@ -172,8 +185,9 @@ Taking a structured approach improves things by: 1. Enforcing principled rules about label exporting. 2. Clearly communicating to users how plugins should be used. -3. Decoupling the app and plugin API to reflect the fact that the things that an app can reasonably do are a strict superset of the things that a plugin can reasonably do due to its access to more information. -4. Avoids surprising and non-local effects caused by plugins, improving transparency and reducing the risk involved in trying out a new 3rd-party plugin. +3. Decoupling the app and plugin API. +4. Avoids surprising and non-local effects caused by plugins. +5. Moves all top-level app-configuration into the main app-building logic, to encourage discoverable, well-organized code. ### Why are system sets the correct granularity to expose? @@ -207,21 +221,20 @@ There is no reasonable or desirable way to prevent this: instead we should embra ## Unresolved questions -1. Should we change the `AppBuilder` API to be structured for consistency, or keep the existing builder pattern? -2. Should an automatic or simple coercion from a single system into a system set be added to improve ergonomics? -3. Should we rename `SystemDescriptor` to something more user-friendly like `ConfiguredSystem`? -4. Are system labels and resource types exported within the `Plugin` trait forced to be `pub`? Should they be? -5. What should `hard_before` and `hard_after` be called? +1. Should we automatically label single systems added to plugins to improve ergonomics? +2. Should we rename `SystemDescriptor` to something more user-friendly like `ConfiguredSystem`? +3. Are system labels and resource types exported within the `Plugin` trait forced to be `pub`? Should they be? +4. What should `hard_before` and `hard_after` be called? +5. Should `PluginGroup` and `App::add_plugins` be deprecated? ## Future possibilities -This is a simple building block that would help with the broader scheduler plans intended in [Bevy #2801](https://github.com/bevyengine/bevy/discussions/2801). +This is a simple building block that advances the broader scheduler plans being explored in [Bevy #2801](https://github.com/bevyengine/bevy/discussions/2801). -Automatic hard sync point insertion is by far the most controversial and ambitious part of this proposal. This could be extended and improved in the future with: 1. Enhanced system visualization tools. 2. Hints to manually specify which hard sync points various systems run between. 3. Stages could (optionally?) be replaced with automatically inferred hard sync points on the basis of hard system ordering dependencies. 4. Provide a more global, staged analysis of system and resource initialization to reduce or eliminate order-dependence of app initialization, as raised in [Bevy #1255](https://github.com/bevyengine/bevy/issues/1255). -5. `as_if_before` functionality will improve the ability to control ordering between plugin system sets without inducing excessive blocking. +5. If-needed ordering constraints ([Bevy #2747](https://github.com/bevyengine/bevy/discussions/2747)) will significantly improve the ability to control ordering between plugin system sets without inducing excessive blocking. From 2cebb68bf987e76b04fc7861f903309d44f6e776 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 15 Sep 2021 13:56:21 -0400 Subject: [PATCH 03/20] Thumbs-down to automatic system labelling " Imo, it's fine to saddle plugin authors with boilerplate if it makes it easier for the users downstream" - Ratys --- rfcs/33-apps_own_scheduling.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index b34c39fd..0524e733 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -221,11 +221,10 @@ There is no reasonable or desirable way to prevent this: instead we should embra ## Unresolved questions -1. Should we automatically label single systems added to plugins to improve ergonomics? -2. Should we rename `SystemDescriptor` to something more user-friendly like `ConfiguredSystem`? -3. Are system labels and resource types exported within the `Plugin` trait forced to be `pub`? Should they be? -4. What should `hard_before` and `hard_after` be called? -5. Should `PluginGroup` and `App::add_plugins` be deprecated? +1. Should we rename `SystemDescriptor` to something more user-friendly like `ConfiguredSystem`? +2. Are system labels and resource types exported within the `Plugin` trait forced to be `pub`? Should they be? +3. What should `hard_before` and `hard_after` be called? +4. Should `PluginGroup` and `App::add_plugins` be deprecated? ## Future possibilities From b6f17764179e287080d4b49afd8d88ba0409c287 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 16 Sep 2021 14:03:51 -0400 Subject: [PATCH 04/20] Labels cannot be forced to be pub --- rfcs/33-apps_own_scheduling.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index 0524e733..20e55d82 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -222,9 +222,8 @@ There is no reasonable or desirable way to prevent this: instead we should embra ## Unresolved questions 1. Should we rename `SystemDescriptor` to something more user-friendly like `ConfiguredSystem`? -2. Are system labels and resource types exported within the `Plugin` trait forced to be `pub`? Should they be? -3. What should `hard_before` and `hard_after` be called? -4. Should `PluginGroup` and `App::add_plugins` be deprecated? +2. What should `hard_before` and `hard_after` be called? +3. Should `PluginGroup` and `App::add_plugins` be deprecated? ## Future possibilities From d851b0cadb9fc3e3d021a4e5c8ffe8b257986e7d Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 16 Sep 2021 14:08:09 -0400 Subject: [PATCH 05/20] Clarity fix for plugin vs states interaction --- rfcs/33-apps_own_scheduling.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index 20e55d82..49feec8a 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -11,7 +11,7 @@ Hard ordering constraints (where systems must be both ordered and separated by a The current plugin model works well in simple cases, but completely breaks down when the central consumer (typically, an `App`) needs to configure when and if their systems run. The main difficulties are: - 3rd party plugins can have "unfixable" ordering ambiguities or interop issues with other 3rd party plugins -- plugins can not be added to states, and run criteria cannot be added +- the systems from plugins can not be added to states, and run criteria cannot be added to them - plugins can not be incorporated into apps and games that use a non-standard control flow architecture - end users who care about granular scheduling control for performance optimization have no way to change when systems run - worse, plugins can freely insert their own stages, causing sudden and uncontrollable bottlenecks From c1cc3d786349f10e6a593eb6ccb8b01b3e5998a0 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 16 Sep 2021 15:04:01 -0400 Subject: [PATCH 06/20] Options for plugin type semantics --- rfcs/33-apps_own_scheduling.md | 112 +++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index 49feec8a..502a9df4 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -225,6 +225,118 @@ There is no reasonable or desirable way to prevent this: instead we should embra 2. What should `hard_before` and `hard_after` be called? 3. Should `PluginGroup` and `App::add_plugins` be deprecated? +### Plugin type semantics + +There are several decent options for how plugins should be defined. +Let's review two different sorts of plugins, one with no config and one with config, and see how the ergonomics compare. + +All of these assume a standard `Plugin` struct, allowing us to have structured fields. + +### Crates define functions that return `Plugin` + +This is the simplest option, but looks quite a bit different from our current API. + +```rust +mod third_party_crate { + pub fn simple_plugin() -> Plugin { + Plugin::new() + } + + pub struct MyConfig(pub bool); + + pub fn configured_plugin(my_config: MyConfig) -> Plugin { + Plugin::new().insert_resource(MyConfig(my_config)) + } +} + +fn main(){ + App::new() + .add_pluging(simple_plugin()) + .add_plugin(configured_plugin(MyConfig(true))) + .run(); +} +``` + +### Crates define structs that implement `Into` + +Slightly more convoluted, but lets us pass in structs rather than calling functions. + +If a user cares about allowing reuse of a particular plugin-defining struct, they can `impl Into for &ConfiguredPlugin`. + +```rust +mod third_party_crate { + pub struct SimplePlugin; + + impl Into for SimplePlugin { + fn into(self) -> Plugin { + Plugin::new() + } + } + + pub struct MyConfig(pub bool); + + pub struct ConfiguredPlugin { + pub my_config: MyConfig, + } + + impl Into for ConfiguredPlugin { + fn into(self) -> Plugin { + Plugin::new().insert_resource(self.my_config) + } + } + + fn third_party_plugin() -> Plugin { + Plugin::new() + } +} + +fn main(){ + App::new() + .add_plugin(third_party_plugin()) + .run(); +} +``` + +### Crates define structs that implement `MakePlugin` + +Compared to the `Into` solution above, this would enable us to write our own derive macro for the common case of "turn all my fields into inserted resources". + +On the other hand, this is less idiomatic and more elaborate. + +```rust +mod third_party_crate { + pub struct SimplePlugin; + + impl MakePlugin for SimplePlugin { + fn make(self) -> Plugin { + Plugin::new() + } + } + + pub struct MyConfig(pub bool); + + pub struct ConfiguredPlugin { + pub my_config: MyConfig, + } + + impl MakePlugin for ConfiguredPlugin { + fn make(self) -> Plugin { + Plugin::new().insert_resource(self.my_config) + } + } + + fn third_party_plugin() -> Plugin { + Plugin::new() + } +} + +fn main(){ + App::new() + .add_plugin(third_party_plugin()) + .run(); +} +``` + ## Future possibilities This is a simple building block that advances the broader scheduler plans being explored in [Bevy #2801](https://github.com/bevyengine/bevy/discussions/2801). From e27e5a598e7ff3237c044a8a0e5762a22f3d4844 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 16 Sep 2021 15:12:56 -0400 Subject: [PATCH 07/20] Swapped to simpler functions-that-return-Plugin semantics for example --- rfcs/33-apps_own_scheduling.md | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index 502a9df4..a51fe412 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -34,7 +34,7 @@ use bevy::prelude::*; use bevy_hypothetical_tui::prelude::*; fn main(){ - App::minimal().add_plugin(TuiPlugin).run(); + App::minimal().add_plugin(tui_plugin()).run(); } ``` @@ -42,21 +42,16 @@ You can write your own plugins by exporting a function that returns a `Plugin` s This can be provided as a standalone function or, more commonly, as a method on a simple `MyPlugin` struct: ```rust -struct CombatPlugin; - -impl IntoPlugin for CombatPlugin { - fn into() -> Plugin { +fn combat_plugin() { Plugin::default() .add_systems("Combat", SystemSet::new().with(attack_system).with(death_system)); .add_system("PlayerMovement", player_movement_system) .init_resouce::() - } } fn main(){ - App::default().add_plugin(CombatPlugin).run(); + App::default().add_plugin(combat_plugin()).run(); } - ``` Under the hood, plugins are very simple, and their systems and resources can be created and configured directly. From c4dc2320ef65bc8487deeefbd84986b3613d7637 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 16 Sep 2021 15:42:05 -0400 Subject: [PATCH 08/20] Replaced Plugin::new with Plugin::default --- rfcs/33-apps_own_scheduling.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index a51fe412..db0e97a1 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -234,13 +234,13 @@ This is the simplest option, but looks quite a bit different from our current AP ```rust mod third_party_crate { pub fn simple_plugin() -> Plugin { - Plugin::new() + Plugin::default() } pub struct MyConfig(pub bool); pub fn configured_plugin(my_config: MyConfig) -> Plugin { - Plugin::new().insert_resource(MyConfig(my_config)) + Plugin::default().insert_resource(MyConfig(my_config)) } } @@ -264,7 +264,7 @@ mod third_party_crate { impl Into for SimplePlugin { fn into(self) -> Plugin { - Plugin::new() + Plugin::default() } } @@ -276,12 +276,12 @@ mod third_party_crate { impl Into for ConfiguredPlugin { fn into(self) -> Plugin { - Plugin::new().insert_resource(self.my_config) + Plugin::default().insert_resource(self.my_config) } } fn third_party_plugin() -> Plugin { - Plugin::new() + Plugin::default() } } @@ -304,7 +304,7 @@ mod third_party_crate { impl MakePlugin for SimplePlugin { fn make(self) -> Plugin { - Plugin::new() + Plugin::default() } } @@ -316,12 +316,12 @@ mod third_party_crate { impl MakePlugin for ConfiguredPlugin { fn make(self) -> Plugin { - Plugin::new().insert_resource(self.my_config) + Plugin::default().insert_resource(self.my_config) } } fn third_party_plugin() -> Plugin { - Plugin::new() + Plugin::default() } } From a0e52fb03ebf27bbb35434a842cf59045e123b72 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 16 Sep 2021 15:56:18 -0400 Subject: [PATCH 09/20] Cleaned up plugins 101 section --- rfcs/33-apps_own_scheduling.md | 65 ++++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 19 deletions(-) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index db0e97a1..cae25e35 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -23,11 +23,23 @@ By moving the ultimate responsibility for scheduling configuration to the centra ## User-facing explanation +### Plugins 101 + **Plugins** are cohesive, drop-in units of functionality that you can add to your Bevy app. -Plugins can initialize **resources** to store data to, or add **system sets**, used to add coherent batches of logic to your app. +Most of what plugins do is the result of **systems** that they add to your app. +Plugins can also initialize **resources**, which store data outside of the entity-component data store in a way that can be accessed by other systems. + +The `Plugin` struct stores this data in a straightforward way: + +```rust +pub struct Plugin { + pub systems: HashMap, SystemSet>, + pub resources: HashSet>, + // Other plugin-configuration settings ommitted here +} +``` Plugins can be added to your app using the `App::add_plugin` method. -If you're new to Bevy, third-party plugins added in this way should immediately work without further configuration. ```rust use bevy::prelude::*; @@ -38,14 +50,13 @@ fn main(){ } ``` -You can write your own plugins by exporting a function that returns a `Plugin` struct with the desired system sets and resources. -This can be provided as a standalone function or, more commonly, as a method on a simple `MyPlugin` struct: +The best way to pass `Plugins` to the apps that consume them is by exporting a function that returns a `Plugin` struct with the desired system sets and resources from your crate or module. ```rust -fn combat_plugin() { +fn combat_plugin() -> Plugin { Plugin::default() - .add_systems("Combat", SystemSet::new().with(attack_system).with(death_system)); - .add_system("PlayerMovement", player_movement_system) + .add_systems("Combat", SystemSet::new().with(attack).with(death)); + .add_system("PlayerMovement", player_movement) .init_resouce::() } @@ -54,21 +65,32 @@ fn main(){ } ``` -Under the hood, plugins are very simple, and their systems and resources can be created and configured directly. - -The `Plugin` struct stores data in a straightforward way: +When designing plugins for a larger audience, you may want to make them configurable in some way to accommodate variations in logic that users might need. +Doing so is quite straightforward: you can simply add parameters to the plugin-creating function. +If you want the types that your plugin operates on to be configurable as well, add a generic type parameter to these functions. ```rust -pub struct Plugin { - pub systems: HashMap, SystemSet>, - pub resources: HashSet>, -} -``` +// We may want our physics plugin to work under both f64 and f32 coordinate systems +trait Floatlike; -When you're creating Bevy apps, you can read, add, remove and configure both systems and resources stored within a plugin that you're using, allowing you to fit it into your app's needs using the standard `HashMap` and `HashSet` APIs. -The methods shown above (such as `.add_system`) are simply convenience methods that manipulate these public fields. +fn physics_plugin(realistic: bool) -> Plugin { + let mut plugin = Plugin::default().add_system(collision_detection::); -Note that each system set that you include requires its own label to allow other plugins to control their order relative to that system set, and allows the central `App` to further configure them as a unit if needed. + if realistic { + plugin.add_system(realistic_gravity); + } else { + plugin.add_system(platformer_gravity); + } + + plugin +} + +fn main(){ + App::default() + .add_plugin(physics_plugin::(true)) + .run(); +} +``` ### Hard system ordering rules @@ -82,6 +104,11 @@ Their purpose is to ensure that you (or the users of your plugin) do not acciden ### Configuring plugins +When you're creating Bevy apps, you can read, add, remove and configure both systems and resources stored within a plugin that you're using, allowing you to fit it into your app's needs using the standard `HashMap` and `HashSet` APIs. +The methods shown above (such as `.add_system`) are simply convenience methods that manipulate these public fields. + +Note that each system set that you include requires its own label to allow other plugins to control their order relative to that system set, and allows the central `App` to further configure them as a unit if needed. + System sets don't store simple `Systems`: instead they store fully configured `SystemDescriptor` objects which store information about when and if the system can run. This includes: - which stage a system runs in @@ -246,7 +273,7 @@ mod third_party_crate { fn main(){ App::new() - .add_pluging(simple_plugin()) + .add_plugin(simple_plugin()) .add_plugin(configured_plugin(MyConfig(true))) .run(); } From 587714a33609c66f848489069a3950b50bdedb8d Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 16 Sep 2021 19:14:26 -0400 Subject: [PATCH 10/20] Partially revised system ordering design --- rfcs/33-apps_own_scheduling.md | 107 +++++++++++++++++++++------------ 1 file changed, 69 insertions(+), 38 deletions(-) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index cae25e35..f68cccb9 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -92,15 +92,34 @@ fn main(){ } ``` -### Hard system ordering rules - -In addition to `.before` and `.after`, `.hard_before` and `.hard_after` can be used to specify that two systems must be separated by a hard sync point in the schedule (generally the end of the stage), in addition to following the standard ordering constraint. - -This is critical when `Commands` from the first system must complete (e.g. spawning or despawning entities) before the second system can validly run. - -Note that hard system ordering rules have no special schedule-altering powers! -They are merely an assertion to be checked at the time of schedule construction. -Their purpose is to ensure that you (or the users of your plugin) do not accidentally break the required logic by shuffling when systems run relative to each other. +### System ordering in Bevy + +Bevy schedules are composed of stages, which have two phases: parallel and sequential. +The parallel phase of a stage always runs first, then is followed by a sequential phase which applies any commands generated in the parallel phase. + +During **sequential phases**, each system can access the entire world freely but only one can run at a time. +Exclusive systems can only be added to the sequential phase while parallel systems can be added to either phase. + +During **parallel phases**, systems are allowed to operate in parallel, carefully dividing up access to the `World` according to the data accesses requested by their system parameters to avoid undefined behavior. +By default, the only rule that their order follows is that a **waiting** system cannot be started if any **active** (currently running) systems are **incompatible** (cannot be scheduled at the same time) if they have conflicting data access. +This is helpful for performance reasons, but, as the precise order in which systems start and complete is nondeterministic, can result in logic bugs or inconsistencies. + +To help you eliminate these sort of difficulties, Bevy lets you specify several types of **system ordering constraints**. +Ordering constraints can be applied directly to systems, on system sets, or to labels, but are ultimately stored and worked with on a per-system basis. +However, ordering constraints can only be applied *relative* to labels, allowing users to replace core parts of the engine (or other dependencies) by substituting new systems with the same public label. + +- **Strict ordering:** Systems from set `A` cannot be started while any system from set `B` are waiting or active. + - Simple and explicit. + - Use the `before(label: impl SystemLabel)` or `after(label: impl SystemLabel)` methods to set this behavior. +- **If-needed ordering:** A given system in set `A` cannot be started while any incompatible system from set `B` is waiting or active. + - Usually, if-needed ordering is the correct tool for ordering groups of systems as it avoids unnecessary blocking. + - If systems in `A` use interior mutability, if-needed ordering may result in non-deterministic outcomes. + - Use `before_if_needed(label: impl SystemLabel)` or `after_if_needed(label: impl SystemLabel)`. +- **At-least-once separation:** Systems in set `A` cannot be started if a system in set `B` has been started until at least one system with the label `S` has completed. Systems from `A` and `B` are incompatible with each other. + - This is most commonly used when commands created by systems in `A` must be processed before systems in `B` can run correctly. `CoreLabels::ApplyCommands` is the label used for the exclusive system that applies queued commands that is typically added to the beginning of each sequential stage. + - Use the `between(before: impl SystemLabel, after: impl SystemLabel)` method. + - The order of arguments in `between` matters; if + - This methods do not automatically insert systems to enforce this separation: instead, the schedule will panic upon initialization as no valid system execution strategy exists. ### Configuring plugins @@ -140,10 +159,23 @@ Duplicate and missing resources will also cause a panic when the app is run. ### Writing plugins to be configured -When writing plugins (especially those designed for other people to use), group systems that should be thought about in one coherent "unit of function" together into a single system set. +When writing plugins (especially those designed for other people to use), try to define constraints on your plugins logic so that your users could not break it if they tried: + +- if some collection of systems will not work properly without each other, make sure they are in the same system. This prevents them from being split by either schedule location or run criteria. +- if one system must run before the other, use granular strict system ordering constraints +- if a broad control flow must be follow, use if-needed system ordering constraints between labels +- if commands must be processed before a follow-up system can be run correctly, use at-least-once separation ordering constraints +- if a secondary source of truth (such as an index) must be updated in between two systems, use at-least-once separation ordering constraints + +Exposing system labels via `pub` allows anyone with access to: + +- set the order of their systems relative to the exposed label +- add new constraints to systems with that label +- change which states systems with that label will run in +- add new run criteria to systems with that label +- apply the labels to new systems -Be sure to use hard system ordering dependencies when needed to ensure that downstream consumers cannot accidentally break your system sets which require command processing by moving systems into the same stage. -**Systems which must live in separate stages due to hard system ordering rules must be in separate system sets in order to allow users to add them to their own stages freely.** +As such, only expose labels that apply to systems ### Standalone `bevy_ecs` usage @@ -153,43 +185,45 @@ Instead of adding plugins to your app, independently add their system sets to yo ## Implementation strategy -### Code organization +Small changes: -As plugins no longer depend on `App` information at all, they should be moved into `bevy_ecs` directly. +1. As plugins no longer depend on `App` information at all, they should be moved into `bevy_ecs` directly. +2. To improve ergonomics, `App::add_system_set` should be changed to `App::add_systems`, and `SystemSet::with_system` should be shortened to `SystemSet::with`. +3. If plugins can no longer configure the `App` in arbitrary ways, we need a new, ergonomic way to set up the default schedule and runner for standard Bevy games. The best way to do this is to create two new builder methods on `App`: `App::minimal` and `App::default`, which sets up the default stages, sets the runner and adds the required plugins. +4. Plugin groups are replaced with simple `Vec>` objects when needed. -### System sets +### Stageless architecture -The changes proposed are simple ergonomic renaming to reflect the increased prominence of system sets. -`App::add_system_set` should be changed to `App::add_systems`, and `SystemSet::with_system` should be shortened to `SystemSet::with`. - -### Resources - -All resources are added to the app before the schedule begins. The app should panic if duplicate resources are detected at this stage; app users can manually remove them from one or more plugins in a transparent fashion to remove the conflict if needed. +1. System ordering between exclusive and parallel systems is defined, as is system ordering between systems that run in different stages. +2. All system scheduling information must be stored in the system descriptor (which are then stored within system sets) in order to enable a unified approach to configuration. See [Bevy #2736](https://github.com/bevyengine/bevy/pull/2736) for a nearly complete PR on this point. +3. `Commands` application is re-architected, and is now handled explicitly in an exclusive system. +This is by default the first system in each sequential stage. +This system is now labeled so systems can be before or after command application. +Users can freely add more copies of this system to their schedule. +4. Systems cannot run "between stages" anymore, and the current "should an exclusive system run at the start of the stage, before commands, or after commands" is removed in favor of simple ordering. -### Schedule overhaul prerequisites +### System ordering mechanics -1. Stage boundaries must be weakened. This API assumes that e.g. system sets can span hard sync points to allow them to be inserted into a given state and that ordering dependencies operate across hard sync points. -2. All system scheduling information must be stored in the system descriptor (which are then stored within system sets) in order to enable a unified approach to configuration. See [Bevy #2736](https://github.com/bevyengine/bevy/pull/2736) for a nearly complete PR on this point. -3. Users must be able to configure systems by reference to a particular label. This proposal has previously been called **label properties**. +Under the hood, all three of the system ordering constraints collapse to the standard `.before`-style edges that we know and love in Bevy 0.5. -### Default stages and runner +If-needed edges are evaluated and inserted on the basis of the existing entities at the start of each stage. -If plugins can no longer configure the `App` in arbitrary ways, we need a new, ergonomic way to set up the default schedule and runner for standard Bevy games. +At-least-once-separation forces the `before` label to be before the intervening system, and the `after` system to be after the intervening system. +However, instead of counting the number of instances of that system with that label, only a single dependency is added to the [`dependency_count`.](https://github.com/bevyengine/bevy/blob/9eb1aeee488684ed607d249f883676f3e711a1d2/crates/bevy_ecs/src/schedule/executor_parallel.rs). -The best way to do this is to create two new builder methods on `App`: `App::minimal` and `App::default`, which sets up the default stages, sets the runner and adds the required plugins. +### Resource initialization -### Plugin groups +All resources are added to the app before the schedule begins. The app should panic if duplicate resources are detected at this stage; app users can manually remove them from one or more plugins in a transparent fashion to remove the conflict if needed. -Plugin groups now require the simple `IntoPluginGroup` trait with a `into` method that returns a `Vec`. +### Label properties -Alternatively, these could just be completely deprecated due to the move towards `App::default`, which was overwhelmingly their most common use. +Users must be able to configure systems by reference to a particular label. This proposal has previously been called **label properties**. ## Drawbacks 1. Plugins are no longer quite as powerful as they were. This is both good and bad: it reduces their expressivity, but reduces the risk that they mysteriously break your app. 2. Apps can directly manipulate the system ordering of their plugins. This is essential for ensuring that interoperability and configurability issues are resolved, but risks breaking internal guarantees. -3. Users can accidentally break the requirement for a hard sync point to pass between systems within a plugin as encouraged by the plugin configuration by moving system sets into the same stage. Hard ordering dependencies briefly discussed in the *Future Work* section of this RFC could ensure that this is impossible. -4. Replacing existing resource values is more verbose, requiring either a startup system or explicitly removing the resource from the pre-existing plugin. This is much more explicit, but could slow down some common use cases such as setting window size. +3. Replacing existing resource values is more verbose, requiring either a startup system or explicitly removing the resource from the pre-existing plugin. This is much more explicit, but could slow down some common use cases such as setting window size. ## Rationale and alternatives @@ -244,8 +278,6 @@ There is no reasonable or desirable way to prevent this: instead we should embra ## Unresolved questions 1. Should we rename `SystemDescriptor` to something more user-friendly like `ConfiguredSystem`? -2. What should `hard_before` and `hard_after` be called? -3. Should `PluginGroup` and `App::add_plugins` be deprecated? ### Plugin type semantics @@ -367,6 +399,5 @@ This could be extended and improved in the future with: 1. Enhanced system visualization tools. 2. Hints to manually specify which hard sync points various systems run between. -3. Stages could (optionally?) be replaced with automatically inferred hard sync points on the basis of hard system ordering dependencies. -4. Provide a more global, staged analysis of system and resource initialization to reduce or eliminate order-dependence of app initialization, as raised in [Bevy #1255](https://github.com/bevyengine/bevy/issues/1255). -5. If-needed ordering constraints ([Bevy #2747](https://github.com/bevyengine/bevy/discussions/2747)) will significantly improve the ability to control ordering between plugin system sets without inducing excessive blocking. +3. A scheduler / app option could be added to automatically infer and insert systems (including command-processing systems) on the basis of at-least-once separation constraints. +4. At-least-once separation constraints can be used to solve the cache invalidation issues that are blocking the implementation of indexes, as discussed in [Bevy #1205](https://github.com/bevyengine/bevy/discussions/1205). From dd80e25700f8e405203ee150c601904af6e9334b Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 17 Sep 2021 13:20:53 -0400 Subject: [PATCH 11/20] Plugin trait API option --- rfcs/33-apps_own_scheduling.md | 66 ++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 3 deletions(-) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index f68cccb9..20b166e1 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -378,15 +378,75 @@ mod third_party_crate { Plugin::default().insert_resource(self.my_config) } } +} - fn third_party_plugin() -> Plugin { - Plugin::default() +fn main(){ + App::new() + .add_plugin(SimplePlugin) + .add_plugin(ConfiguredPlugin(MyConfig(true))) + .run(); +} +``` + +### Crates export structs that impl `Plugin` + +This is the closest to our existing model. +Critically, it also [allows us to force public labels](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=8f7ca85fb85ef163dc152b40eca743f9) by making the label type an associated type. See [this comment](https://github.com/bevyengine/rfcs/pull/33#issuecomment-921380669) for why this is important. + +This is the most verbose and indirect of the options, although the derive macro would help dramatically. + +```rust +mod third_party_crate { + #[derive(Default)] + pub struct SimplePlugin{ + label_type: PhantomData, + systems: HashMap, + resources: HashSet>, + } + + pub enum ThirdPartyLabel { + A, + B, + } + + // This is shown for demonstration purposes only; + // a derive macro would be used instead + impl Plugin for SimplePlugin { + type Label: ThirdPartyLabel; + + fn systems(&mut self) -> &mut HashMap { + &mut self.systems + } + + fn resources(&mut self) -> &mut HashSet> { + &mut self.resources + } + + // Other convenience methods elided here + // as the derive macro will virtually always be used + } + + #[derive(Default)] + pub struct MyConfig(pub bool); + + #[derive(Plugin, Default)] + pub struct ConfiguredPlugin { + label_type: PhantomData, + systems: HashMap, + resources: HashSet>, + } + + impl ConfiguredPlugin { + fn new(my_config: MyConfig) -> Self { + Self::default().insert_resource(my_config) + } } } fn main(){ App::new() - .add_plugin(third_party_plugin()) + .add_plugin(SimplePlugin::default()) + .add_plugin(ConfiguredPlugin::new(MyConfig(true))) .run(); } ``` From f057f21f834e8a19556db60de5af4a1ec4cf346a Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 17 Sep 2021 13:22:54 -0400 Subject: [PATCH 12/20] Finish sentences --- rfcs/33-apps_own_scheduling.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index 20b166e1..04a90ec0 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -118,7 +118,7 @@ However, ordering constraints can only be applied *relative* to labels, allowing - **At-least-once separation:** Systems in set `A` cannot be started if a system in set `B` has been started until at least one system with the label `S` has completed. Systems from `A` and `B` are incompatible with each other. - This is most commonly used when commands created by systems in `A` must be processed before systems in `B` can run correctly. `CoreLabels::ApplyCommands` is the label used for the exclusive system that applies queued commands that is typically added to the beginning of each sequential stage. - Use the `between(before: impl SystemLabel, after: impl SystemLabel)` method. - - The order of arguments in `between` matters; if + - The order of arguments in `between` matters; the labels must only be separated by a cleanup system in one direction, rather than both. - This methods do not automatically insert systems to enforce this separation: instead, the schedule will panic upon initialization as no valid system execution strategy exists. ### Configuring plugins @@ -175,7 +175,7 @@ Exposing system labels via `pub` allows anyone with access to: - add new run criteria to systems with that label - apply the labels to new systems -As such, only expose labels that apply to systems +As such, try to only expose labels that apply to systems that can safely be enabled or disabled as a group. ### Standalone `bevy_ecs` usage From 3adc3acd17b953b283ab7a00b84bdccd5982f46c Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 17 Sep 2021 14:50:20 -0400 Subject: [PATCH 13/20] Tentatively settled on Into type semantics --- rfcs/33-apps_own_scheduling.md | 73 +++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 23 deletions(-) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index 04a90ec0..c8ed6b3c 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -33,9 +33,10 @@ The `Plugin` struct stores this data in a straightforward way: ```rust pub struct Plugin { - pub systems: HashMap, SystemSet>, - pub resources: HashSet>, - // Other plugin-configuration settings ommitted here + /// SystemDescriptors are systems which store both the function to be run + /// and information about scheduling constraints + pub systems: Vec, + pub resources: Vec>, } ``` @@ -46,22 +47,27 @@ use bevy::prelude::*; use bevy_hypothetical_tui::prelude::*; fn main(){ - App::minimal().add_plugin(tui_plugin()).run(); + App::minimal().add_plugin(TuiPlugin).run(); } ``` -The best way to pass `Plugins` to the apps that consume them is by exporting a function that returns a `Plugin` struct with the desired system sets and resources from your crate or module. +The standard pattern for defining plugins is to export a `MyCrateNamePlugin` struct that implements the `Into` trait. +In the `into()` method for that trait, create and return a new `Plugin` struct. ```rust -fn combat_plugin() -> Plugin { +struct CombatPlugin; + +impl Into for CombatPlugin { + fn into(self) -> Plugin { Plugin::default() - .add_systems("Combat", SystemSet::new().with(attack).with(death)); - .add_system("PlayerMovement", player_movement) + .add_systems(SystemSet::new().with(attack).with(death)); + .add_system(player_movement) .init_resouce::() + } } fn main(){ - App::default().add_plugin(combat_plugin()).run(); + App::default().add_plugin(CombatPlugin).run(); } ``` @@ -73,21 +79,27 @@ If you want the types that your plugin operates on to be configurable as well, a // We may want our physics plugin to work under both f64 and f32 coordinate systems trait Floatlike; -fn physics_plugin(realistic: bool) -> Plugin { - let mut plugin = Plugin::default().add_system(collision_detection::); +struct PhysicsPlugin { + realistic: bool +}; - if realistic { - plugin.add_system(realistic_gravity); - } else { - plugin.add_system(platformer_gravity); - } +impl Into for PhysicsPlugin { + fn into(self) -> Plugin { + let mut plugin = Plugin::default().add_system(collision_detection::); + + if realistic { + plugin.add_system(realistic_gravity); + } else { + plugin.add_system(platformer_gravity); + } - plugin + plugin + } } fn main(){ App::default() - .add_plugin(physics_plugin::(true)) + .add_plugin(PhysicsPlugin::{realistic: true}) .run(); } ``` @@ -275,17 +287,27 @@ However, if we insert a system `B` (which may do literally nothing!), and state There is no reasonable or desirable way to prevent this: instead we should embrace this by designing a real API to do so. -## Unresolved questions - -1. Should we rename `SystemDescriptor` to something more user-friendly like `ConfiguredSystem`? - ### Plugin type semantics -There are several decent options for how plugins should be defined. +There are several potential options for how plugins should be defined. Let's review two different sorts of plugins, one with no config and one with config, and see how the ergonomics compare. All of these assume a standard `Plugin` struct, allowing us to have structured fields. +**TL;DR:** + +1. We would like to export a type, rather than use bare functions. + 1. This is more discoverable, standardized and prettier than exporting a bare function. + 2. However, it comes with slightly increased boilerplate for end users. +2. The `Plugin` trait approach plays very poorly with the desire for a structured data storage. + 1. We can't both have a nice derive macro for boilerplate and allow users to use a method to define what systems / resources a plugin should include. + 2. We could bypass this with two traits, one of which is derived and the other is manually implemented... + 3. Without using two traits, users are forced to call `.add_plugin(ExamplePlugin::default())` each time rather than just adding the bare struct. +3. Having a unified single type for plugins is convenient to work with internally and easy to teach. + 1. This approach allows us to use fields, rather than getters and setters, reducing indirection and boilerplate. + +As a result, the rest of this RFC uses a standard `Plugin` struct, with crates creating their own types that implement the `Into` trait. + ### Crates define functions that return `Plugin` This is the simplest option, but looks quite a bit different from our current API. @@ -394,6 +416,7 @@ This is the closest to our existing model. Critically, it also [allows us to force public labels](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=8f7ca85fb85ef163dc152b40eca743f9) by making the label type an associated type. See [this comment](https://github.com/bevyengine/rfcs/pull/33#issuecomment-921380669) for why this is important. This is the most verbose and indirect of the options, although the derive macro would help dramatically. +Unfortunately, we can't both use a derive macro *and* use a method impl on the same trait to define the data that's in the plugin. ```rust mod third_party_crate { @@ -451,6 +474,10 @@ fn main(){ } ``` +## Unresolved questions + +1. Should we rename `SystemDescriptor` to something more user-friendly like `ConfiguredSystem`? + ## Future possibilities This is a simple building block that advances the broader scheduler plans being explored in [Bevy #2801](https://github.com/bevyengine/bevy/discussions/2801). From 5d8398816d6acf7a65a5f7ff3ad201969f1d26dc Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 17 Sep 2021 15:02:38 -0400 Subject: [PATCH 14/20] Updated unresolved questions --- rfcs/33-apps_own_scheduling.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index c8ed6b3c..533dca92 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -477,6 +477,8 @@ fn main(){ ## Unresolved questions 1. Should we rename `SystemDescriptor` to something more user-friendly like `ConfiguredSystem`? +2. How do we handle [plugins that need to extend other resources at initialization](https://github.com/bevyengine/rfcs/pull/33#discussion_r709654419)? +3. How do we ensure that [adding run criteria won't break invariants](https://github.com/bevyengine/rfcs/pull/33#discussion_r709655879)? ## Future possibilities From a9cdf54ce4ae564bdad7450712c1b84d3963bdfe Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 17 Sep 2021 15:59:08 -0400 Subject: [PATCH 15/20] Proof of concept of label configuration --- rfcs/33-apps_own_scheduling.md | 99 +++++++++++++++++++++++++++------- 1 file changed, 80 insertions(+), 19 deletions(-) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index 533dca92..d1848688 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -32,11 +32,11 @@ Plugins can also initialize **resources**, which store data outside of the entit The `Plugin` struct stores this data in a straightforward way: ```rust -pub struct Plugin { +struct Plugin { /// SystemDescriptors are systems which store both the function to be run /// and information about scheduling constraints - pub systems: Vec, - pub resources: Vec>, + systems: Vec, + resources: Vec>, } ``` @@ -133,6 +133,43 @@ However, ordering constraints can only be applied *relative* to labels, allowing - The order of arguments in `between` matters; the labels must only be separated by a cleanup system in one direction, rather than both. - This methods do not automatically insert systems to enforce this separation: instead, the schedule will panic upon initialization as no valid system execution strategy exists. +### Configuring systems by their label + +Systems do not exist in a vacuum: in order to perform useful computations, they must follow certain **constraints**. +The `SystemDescriptor` type captures this information, storing the underlying function and any labels, ordering constraints, run criteria and state information that may be attached to a particular system. + +Note that a single function can be reused, and turned into multiple distinct systems, each with their own `SystemDescriptor` object which is owned by a `Schedule`. + +In many cases, systems will be configured locally, either on their own or as part of a system set. + +```rust +struct CombatPlugin; + +impl Into for CombatPlugin { + fn into(self) -> Plugin { + Plugin::default() + .add_systems(SystemSet::new().with(attack).with(death).label("Damage")); + .add_system(player_movement.after("Damage")) + .init_resouce::() + } +} +``` + +However, the `App` can also configure systems by their label, applying the same configuration to all systems that share that label. +In addition to explicitly given labels, each plugin automatically assigns a shared label to every system that it includes. + +```rust +fn main(){ + App::default() + .add_plugin(CombatPlugin) + // This uses the SystemLabel trait to define operations on the manually specified label + .configure_label("Damage".after(CoreSystem::Time)) + // This label is automatically applied based on the type_id of the Plugin struct provided to add_plugin + .configure_label(CombatPlugin.get_label().set_run_criteria(FixedTimeStep::(0.5))) + .run(); +} +``` + ### Configuring plugins When you're creating Bevy apps, you can read, add, remove and configure both systems and resources stored within a plugin that you're using, allowing you to fit it into your app's needs using the standard `HashMap` and `HashSet` APIs. @@ -257,26 +294,51 @@ Taking a structured approach improves things by: 4. Avoids surprising and non-local effects caused by plugins. 5. Moves all top-level app-configuration into the main app-building logic, to encourage discoverable, well-organized code. -### Why are system sets the correct granularity to expose? +### Why do we need to automatically assign a shared label to systems added by a plugin? + +The basic principle here is that **each system must always have at least one publicly visible label.** + +If a system does not have any labels that are visible to the `App`, **unresolvable system ordering** ambiguities can occur. +The path to this is straightforward: + +1. A third-party plugin defines some component or resource that they make `pub`. +2. They operate on this data in one of their systems, that they do not make `pub` and do not assign a `pub` label to. +3. The user defines another system that operates on the data. + +In this realistic scenario, the user has now created a gameplay-relevant system order ambiguity that they have no way of fixing (short of vendoring the dependency). +The plugin author could not have foreseen this *particular* interaction. + +In order to fix gameplay bugs (and ensure determinism for the use cases that care about it), this ambiguity must be resolved. +In a stage-centric architecture, this could be done by ensuring that the stage that the user's system is in isn't the same as the stage that the plugin's system is in. + +However, this has some serious drawbacks: + +1. The stage separation is not actually necessary, pointlessly limiting parallelism. +2. The fact that a system must live in a particular stage to avoid bugs must be manually maintained through comments and tests. +3. There may not be a suitable existing stage to satisfy all of the existing constraints, forcing the user to create a special stage *just* to satisfy this requirement. + +In an inferred-hard-sync-points architecture, even this escape hatch is gone. +By providing at least one public label per system, we can avoid this problem without giving the end user unfettered access to the plugin internals. + +As a nice benefit, this also allows us to use labels as our primary tool for customizing plugins, without having to use system sets as yet another top-level tool for grouping systems. + +### Why is only the `App` allowed to configure systems by their labels? -If we accept that apps must be allowed to configure when and if the systems in their plugins run, there are four possible options for how we could expose this: +The principle here is similar to that of orphan rules in Rust (and the reason why a structured `Plugin` API is a good idea): if you allow arbitrary dependencies to change unrelated things in complex ways, things start to break in horrible fashions once your dependency tree grows. +This is particularly bad under the working model where constraints are write-only. -1. Individual systems. These are too granular, and risk ballooning complexity and further breaking of plugin-internal guarantees. -2. System sets. The most flexible of options: allowing plugin authors to obscure or expose details as desired in order to encourage shared configuration. Configuration applied to a system set can only be applied to all systems in the set at once. -3. Stages. This heavily discourages system parallelism, and forces a global and linear view of the schedule in order to enforce the desired ordering constraints. -4. All systems. This is far too coarse, and will prevent users from changing which stage systems runs in as any reasonable configuration API will apply the same operation to every system in a group. +By contrast, the `App` has context on both the global control flow and is end-user controlled, allowing you to centralize your rules in a single place. -System sets are also the most flexible of these options: they can be lumped or split to fit the particular needs of the users and invariants required by the plugin's logic. +Plugins should almost always be able to configure their own systems correctly without this tool, as they own the `SystemDescriptors` directly. +In the very rare cases where they require app-level permissions to set some global rule, they can instruct the user to do so in their README or examples. -### Why are plugins forced to export a unique label for each system set? +### Why should we let the `App` remove systems by their label? -One of the driving principles behind this design is that plugin authors cannot and should not predict every possible user need for their plugins. -This leads to either limited configurability, which forces dependency "vendoring" (maintaining a custom fork of the dependency) or internally developing the functionality needed, or excessive configurability, which bloats API surface in ways that are irrelevant to almost all users. +If the `App` is allowed to set arbitrary run criteria on systems, they could just add a run criteria that always return `ShouldRun::No`. -Exported system sets are the atomic unit of plugin configurability, and configuration cannot occur effectively without labels. -By forcing a one-to-one relationship between exported labels and system sets plugin authors have a clear standard to build towards that simplifies their decisions about what to make public. +This is dumb, so we should support it properly (but with warnings about how there's a high chance that this breaks things). -### Why should we allow users to add dependencies to external plugin systems? +### Why should we allow users to add dependencies between external plugin systems? From a design perspective, this is essential to ensure that two third-party plugins can co-exist safely without needing to know about each other's existence in any way. @@ -487,6 +549,5 @@ This is a simple building block that advances the broader scheduler plans being This could be extended and improved in the future with: 1. Enhanced system visualization tools. -2. Hints to manually specify which hard sync points various systems run between. -3. A scheduler / app option could be added to automatically infer and insert systems (including command-processing systems) on the basis of at-least-once separation constraints. -4. At-least-once separation constraints can be used to solve the cache invalidation issues that are blocking the implementation of indexes, as discussed in [Bevy #1205](https://github.com/bevyengine/bevy/discussions/1205). +2. A scheduler / app option could be added to automatically infer and insert systems (including command-processing systems) on the basis of at-least-once separation constraints. +3. At-least-once separation constraints can be used to solve the cache invalidation issues that are blocking the implementation of indexes, as discussed in [Bevy #1205](https://github.com/bevyengine/bevy/discussions/1205). From 5b4822e6a304d5d65d431d07d85709240945b751 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 17 Sep 2021 19:11:45 -0400 Subject: [PATCH 16/20] Fancy plugin trait type semantics Credit to @cart for the design, and @therawmeatball for significant feedback --- rfcs/33-apps_own_scheduling.md | 344 ++++++++++++--------------------- 1 file changed, 127 insertions(+), 217 deletions(-) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index d1848688..e226e454 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -29,17 +29,6 @@ By moving the ultimate responsibility for scheduling configuration to the centra Most of what plugins do is the result of **systems** that they add to your app. Plugins can also initialize **resources**, which store data outside of the entity-component data store in a way that can be accessed by other systems. -The `Plugin` struct stores this data in a straightforward way: - -```rust -struct Plugin { - /// SystemDescriptors are systems which store both the function to be run - /// and information about scheduling constraints - systems: Vec, - resources: Vec>, -} -``` - Plugins can be added to your app using the `App::add_plugin` method. ```rust @@ -57,9 +46,9 @@ In the `into()` method for that trait, create and return a new `Plugin` struct. ```rust struct CombatPlugin; -impl Into for CombatPlugin { - fn into(self) -> Plugin { - Plugin::default() +impl Plugin for CombatPlugin { + fn build(plugin: &mut PluginState) { + plugin .add_systems(SystemSet::new().with(attack).with(death)); .add_system(player_movement) .init_resouce::() @@ -72,34 +61,52 @@ fn main(){ ``` When designing plugins for a larger audience, you may want to make them configurable in some way to accommodate variations in logic that users might need. -Doing so is quite straightforward: you can simply add parameters to the plugin-creating function. -If you want the types that your plugin operates on to be configurable as well, add a generic type parameter to these functions. +To ensure that plugins can be properly controlled by the editor and customized by the users, all plugin configuration must be done using resources. ```rust // We may want our physics plugin to work under both f64 and f32 coordinate systems trait Floatlike; -struct PhysicsPlugin { - realistic: bool +// We can make our plugins generic and specify trait bounds +pub struct PhysicsPlugin { + // Remember to add a phantom data field to satisfy the compiler + _phantom: PhantomData, }; -impl Into for PhysicsPlugin { - fn into(self) -> Plugin { - let mut plugin = Plugin::default().add_system(collision_detection::); +struct RealisticGravity(bool); - if realistic { - plugin.add_system(realistic_gravity); - } else { - plugin.add_system(platformer_gravity); - } +fn realistic_gravity_criteria(realistic_gravity_setting: Res) -> ShouldRun { + if RealisticGravity.0 { + ShouldRun::Yes + } else { + ShouldRun::No + } +} + +fn platformer_gravity_criteria(realistic_gravity_setting: Res) -> ShouldRun { + if RealisticGravity.0 { + ShouldRun::No + } else { + ShouldRun::Yes + } +} +impl Plugin for PhysicsPlugin { + fn build(plugin: &mut PluginState) { + plugin + .insert_resource(RealisticCollisionDetection) + .add_system(collision_detection::.label("Collision detection")) + // These systems share a label to ensure that other systems will be scheduled correctly + // regardless of the configuration chosen + .add_system(realistic_gravity::.set_run_criteria(realistic_gravity_criteria).label("Gravity")) + .add_system(platformer_gravity::.set_run_criteria(platformer_gravity_criteria).label("Gravity")); plugin } } fn main(){ App::default() - .add_plugin(PhysicsPlugin::{realistic: true}) + .add_plugin(PhysicsPlugin::) .run(); } ``` @@ -145,9 +152,9 @@ In many cases, systems will be configured locally, either on their own or as par ```rust struct CombatPlugin; -impl Into for CombatPlugin { - fn into(self) -> Plugin { - Plugin::default() +impl Plugin for CombatPlugin { + fn build(plugin: &mut PluginState) -> Plugin { + plugin .add_systems(SystemSet::new().with(attack).with(death).label("Damage")); .add_system(player_movement.after("Damage")) .init_resouce::() @@ -237,9 +244,90 @@ Instead of adding plugins to your app, independently add their system sets to yo Small changes: 1. As plugins no longer depend on `App` information at all, they should be moved into `bevy_ecs` directly. -2. To improve ergonomics, `App::add_system_set` should be changed to `App::add_systems`, and `SystemSet::with_system` should be shortened to `SystemSet::with`. -3. If plugins can no longer configure the `App` in arbitrary ways, we need a new, ergonomic way to set up the default schedule and runner for standard Bevy games. The best way to do this is to create two new builder methods on `App`: `App::minimal` and `App::default`, which sets up the default stages, sets the runner and adds the required plugins. -4. Plugin groups are replaced with simple `Vec>` objects when needed. +2. If plugins can no longer configure the `App` in arbitrary ways, we need a new, ergonomic way to set up the default schedule and runner for standard Bevy games. The best way to do this is to create two new builder methods on `App`: `App::minimal` and `App::default`, which sets up the default stages, sets the runner and adds the required plugins. +3. Plugin groups are replaced with simple `Vec>` objects when needed. + +### Plugin architecture + +The ergonomics and semantics here are quite constrained. The following design: + +- exports a type for users to consume +- allows users to pass in the struct to `.add_plugin` +- ensures each system has at least one label +- creates a structured API that limits the power of plugins +- allows us to later add more required fields to plugins (mostly without breaking the existing ecosystem) + +Machinery: + +```rust +// This trait must be implemented for all Plugins +pub trait Plugin: 'static { + // This method is automatically called when plugins are added to the app + pub fn build(plugin: &mut PluginState); + + /// Automatically generates a unique label based on the type implementing Plugin + pub fn get_label() -> PluginLabel { + PluginLabel(TypeId::of::()) + } +} + +/// A label corresponding to the type of the Plugin is automatically added +/// to each system added by that plugin when `App::add_plugin` is called +#[derive(SystemLabel, Debug, Clone, PartialEq, Eq, Hash)] +pub struct PluginLabel(TypeId); + +#[derive(Default)] +pub struct PluginState { + systems: Vec, + system_set: Vec, + resources: Vec>, + /* other plugin config fields here */ +} + +impl PluginState { + pub fn new() -> Self { + Self { + system_sets: Vec::new(), + systems: Vec::new(), + label: PluginLabel::of::

(), + } + } + pub fn apply(self, app: &mut App) { + for system in self.systems { + app.add_system(system.label(self.label.clone())) + } + + for system_set in self.system_sets { + app.add_system_set(system_set.label(self.label.clone())) + } + } + + /* other plugin builder functions here */ +} +``` + +Example: + +```rust +pub struct CustomPlugin; + +impl Plugin for CustomPlugin { + // Most existing Bevy code should be easily portable; + // only the function signature of `build` will change + fn build(plugin: &mut PluginState) { + plugin + .add_system(hello_world) + .add_system_set(SystemSet::new().with_system(velocity).with_system(movement)) + .init_resource::() + } +} + +fn main (){ + App::default() + .add_plugin(CustomPlugin) + .configure_label(CustomPlugin.get_label().after(CoreSystem::Time)) +} +``` ### Stageless architecture @@ -349,192 +437,12 @@ However, if we insert a system `B` (which may do literally nothing!), and state There is no reasonable or desirable way to prevent this: instead we should embrace this by designing a real API to do so. -### Plugin type semantics - -There are several potential options for how plugins should be defined. -Let's review two different sorts of plugins, one with no config and one with config, and see how the ergonomics compare. - -All of these assume a standard `Plugin` struct, allowing us to have structured fields. - -**TL;DR:** - -1. We would like to export a type, rather than use bare functions. - 1. This is more discoverable, standardized and prettier than exporting a bare function. - 2. However, it comes with slightly increased boilerplate for end users. -2. The `Plugin` trait approach plays very poorly with the desire for a structured data storage. - 1. We can't both have a nice derive macro for boilerplate and allow users to use a method to define what systems / resources a plugin should include. - 2. We could bypass this with two traits, one of which is derived and the other is manually implemented... - 3. Without using two traits, users are forced to call `.add_plugin(ExamplePlugin::default())` each time rather than just adding the bare struct. -3. Having a unified single type for plugins is convenient to work with internally and easy to teach. - 1. This approach allows us to use fields, rather than getters and setters, reducing indirection and boilerplate. - -As a result, the rest of this RFC uses a standard `Plugin` struct, with crates creating their own types that implement the `Into` trait. - -### Crates define functions that return `Plugin` - -This is the simplest option, but looks quite a bit different from our current API. - -```rust -mod third_party_crate { - pub fn simple_plugin() -> Plugin { - Plugin::default() - } - - pub struct MyConfig(pub bool); - - pub fn configured_plugin(my_config: MyConfig) -> Plugin { - Plugin::default().insert_resource(MyConfig(my_config)) - } -} - -fn main(){ - App::new() - .add_plugin(simple_plugin()) - .add_plugin(configured_plugin(MyConfig(true))) - .run(); -} -``` - -### Crates define structs that implement `Into` - -Slightly more convoluted, but lets us pass in structs rather than calling functions. - -If a user cares about allowing reuse of a particular plugin-defining struct, they can `impl Into for &ConfiguredPlugin`. - -```rust -mod third_party_crate { - pub struct SimplePlugin; - - impl Into for SimplePlugin { - fn into(self) -> Plugin { - Plugin::default() - } - } - - pub struct MyConfig(pub bool); - - pub struct ConfiguredPlugin { - pub my_config: MyConfig, - } - - impl Into for ConfiguredPlugin { - fn into(self) -> Plugin { - Plugin::default().insert_resource(self.my_config) - } - } - - fn third_party_plugin() -> Plugin { - Plugin::default() - } -} - -fn main(){ - App::new() - .add_plugin(third_party_plugin()) - .run(); -} -``` - -### Crates define structs that implement `MakePlugin` - -Compared to the `Into` solution above, this would enable us to write our own derive macro for the common case of "turn all my fields into inserted resources". - -On the other hand, this is less idiomatic and more elaborate. - -```rust -mod third_party_crate { - pub struct SimplePlugin; - - impl MakePlugin for SimplePlugin { - fn make(self) -> Plugin { - Plugin::default() - } - } - - pub struct MyConfig(pub bool); - - pub struct ConfiguredPlugin { - pub my_config: MyConfig, - } - - impl MakePlugin for ConfiguredPlugin { - fn make(self) -> Plugin { - Plugin::default().insert_resource(self.my_config) - } - } -} - -fn main(){ - App::new() - .add_plugin(SimplePlugin) - .add_plugin(ConfiguredPlugin(MyConfig(true))) - .run(); -} -``` - -### Crates export structs that impl `Plugin` - -This is the closest to our existing model. -Critically, it also [allows us to force public labels](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=8f7ca85fb85ef163dc152b40eca743f9) by making the label type an associated type. See [this comment](https://github.com/bevyengine/rfcs/pull/33#issuecomment-921380669) for why this is important. +## Why should plugins be configured by setting a resource? -This is the most verbose and indirect of the options, although the derive macro would help dramatically. -Unfortunately, we can't both use a derive macro *and* use a method impl on the same trait to define the data that's in the plugin. - -```rust -mod third_party_crate { - #[derive(Default)] - pub struct SimplePlugin{ - label_type: PhantomData, - systems: HashMap, - resources: HashSet>, - } - - pub enum ThirdPartyLabel { - A, - B, - } - - // This is shown for demonstration purposes only; - // a derive macro would be used instead - impl Plugin for SimplePlugin { - type Label: ThirdPartyLabel; - - fn systems(&mut self) -> &mut HashMap { - &mut self.systems - } +Using resources to configure plugins provides a clear, unified tool for setting plugin configuration. +This will be particularly valuable when working on editor support, or when building complex trees of plugin dependencies. - fn resources(&mut self) -> &mut HashSet> { - &mut self.resources - } - - // Other convenience methods elided here - // as the derive macro will virtually always be used - } - - #[derive(Default)] - pub struct MyConfig(pub bool); - - #[derive(Plugin, Default)] - pub struct ConfiguredPlugin { - label_type: PhantomData, - systems: HashMap, - resources: HashSet>, - } - - impl ConfiguredPlugin { - fn new(my_config: MyConfig) -> Self { - Self::default().insert_resource(my_config) - } - } -} - -fn main(){ - App::new() - .add_plugin(SimplePlugin::default()) - .add_plugin(ConfiguredPlugin::new(MyConfig(true))) - .run(); -} -``` +It also cleans up the ergonomics of the `Plugin` machinery substantially. ## Unresolved questions @@ -551,3 +459,5 @@ This could be extended and improved in the future with: 1. Enhanced system visualization tools. 2. A scheduler / app option could be added to automatically infer and insert systems (including command-processing systems) on the basis of at-least-once separation constraints. 3. At-least-once separation constraints can be used to solve the cache invalidation issues that are blocking the implementation of indexes, as discussed in [Bevy #1205](https://github.com/bevyengine/bevy/discussions/1205). +4. `ShouldRun` could be cleaned up and moved back to simple binary logic. More helpers could be added for common plugin use cases. +5. Systems could be fully added and removed from the schedule based on run-time information, keeping the schedule clean when configuring plugins with resources. From 71a8637e641fb6783d0e8e4336788a731306d2b1 Mon Sep 17 00:00:00 2001 From: Alice Date: Fri, 17 Sep 2021 22:28:27 -0400 Subject: [PATCH 17/20] What do we do about plugin groups? --- rfcs/33-apps_own_scheduling.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index e226e454..3be38a7c 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -245,7 +245,6 @@ Small changes: 1. As plugins no longer depend on `App` information at all, they should be moved into `bevy_ecs` directly. 2. If plugins can no longer configure the `App` in arbitrary ways, we need a new, ergonomic way to set up the default schedule and runner for standard Bevy games. The best way to do this is to create two new builder methods on `App`: `App::minimal` and `App::default`, which sets up the default stages, sets the runner and adds the required plugins. -3. Plugin groups are replaced with simple `Vec>` objects when needed. ### Plugin architecture @@ -449,6 +448,7 @@ It also cleans up the ergonomics of the `Plugin` machinery substantially. 1. Should we rename `SystemDescriptor` to something more user-friendly like `ConfiguredSystem`? 2. How do we handle [plugins that need to extend other resources at initialization](https://github.com/bevyengine/rfcs/pull/33#discussion_r709654419)? 3. How do we ensure that [adding run criteria won't break invariants](https://github.com/bevyengine/rfcs/pull/33#discussion_r709655879)? +4. Do we still need plugin groups at all? Can we replace them with a simple standard collection? The ergonomics matter much less with `App::default()` initialization. ## Future possibilities From d0bb5bc59a304b19ea69eb45ffa80189ad318899 Mon Sep 17 00:00:00 2001 From: Alice Date: Fri, 17 Sep 2021 23:37:04 -0400 Subject: [PATCH 18/20] Plugin privacy philosophy --- rfcs/33-apps_own_scheduling.md | 55 ++++++++++++++++------------------ 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index 3be38a7c..d03748d2 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -177,41 +177,42 @@ fn main(){ } ``` -### Configuring plugins +### Configuring systems added by plugins -When you're creating Bevy apps, you can read, add, remove and configure both systems and resources stored within a plugin that you're using, allowing you to fit it into your app's needs using the standard `HashMap` and `HashSet` APIs. -The methods shown above (such as `.add_system`) are simply convenience methods that manipulate these public fields. +Carefully configuring the systems added by your plugins can be critical for fitting the logic defined by third-party crates to fit the unique needs and control flow of your app. You might want to: -Note that each system set that you include requires its own label to allow other plugins to control their order relative to that system set, and allows the central `App` to further configure them as a unit if needed. +1. Make sure that one of your systems run before a third-party system. +2. Control the relative order of two conflicting third-party systems. +3. Add a run criteria to disable a plugin's system when certain criteria are met. +4. Change which state(s) a plugins' systems runs in. +5. Add a plugin's systems to a nested schedule that runs repeatedly each frame. +6. Optimize the schedule strategy of a large schedule containing both first and third-party plugins. -System sets don't store simple `Systems`: instead they store fully configured `SystemDescriptor` objects which store information about when and if the system can run. This includes: +However, Bevy enforces **plugin privacy rules**, and does not allow you to freely dissect and reconfigure every systems provided by third-party plugins. These rules are: -- which stage a system runs in -- any system ordering dependencies a system may have -- any run criteria attached to the system -- which states the system is executed in -- the labels a system has +1. System ordering constraints and labels cannot be removed, although you can add new ones. Schedules will panic if ordering constraints cannot be satisfied. +2. The app can only configure plugins' systems by applying configuration changes to every system in that label. +3. Not all labels are public: you can only apply a label to new systems, or configure systems by that label, if the label is publicly exported. -Without any configuration, these systems are designed to operate as intended when the Bevy app is configured in the standard fashion (using `App::default()`). Each system will belong to the standard stage that the plugin author felt fit best, and ordering between other members of the plugins and Bevy's default systems should already be set up for you. -For most plugins and most users, this strategy should just work out of the box. +You can think of these rules as an extra layer of safety-through-constraints provided by the creators of the plugins you are using, much like Rust's type system. +If you're coming to Bevy from a traditional game engine, these constraints may feel dangerous and limiting to you. +However, there are very good reasons that they exist: -When creating your `App`, you can add additional systems and resources directly to the app, or choose to remove system sets or resources from the plugins you are using by directly using the appropriate `HashSet` and `HashMap` methods. -Removing resources in this way can be helpful when you want to override provided initial values, or if you need to resolve a conflict where two of your dependencies attempt to set a resource's value in conflicting way. +1. System ordering constraints are powerful, flexible and minimal. If these constraints are violated, you are likely to get serious, hard-to-detect, non-deterministic bugs in your code that you will not discover until the night before launch. +2. Tying your design to individual systems, rather than carefully exposed sets of systems, makes it much harder to upgrade your code base as the plugin you are depending on evolves. This is problematic for plugin authors too, as this tight coupling makes re-architecting their code (even to split, merge or rename systems) expensive as they need to worry about breaking their users' apps. +3. Systems in Bevy are generally rather small and granular due to the benefits for code modularity and parallelizability that it offers. Separating closely-interlocked systems (with distinct run criteria, or by moving them into different states or schedules) will tend to result in complete but bizarre failure. +4. Manually specifying the precise order and stage arrangement increases the maintenance costs of your code base in a quadratic fashion. Each time you organize your schedule manually (for performance or to fix a bug), you risk creating new problems and create an undocumented, unenforced dependency that future maintainers cannot break. +5. Rust's own privacy rules will not (sanely) allow Bevy to force all system label types to be public, even if we tried. -More commonly though, you'll be configuring the system sets using the labels that they were given by their plugin. -When system sets are added to a `Plugin`, the provided label is applied to all systems within that set, allowing other systems to specify their order relative to that system set. Note that *all* systems that share a label must complete before the scheduler allows their dependencies to proceed. +There are also some powerful but subtle tools to work around these limitations: -The `App` can also configure systems provided by its plugins using the `App::configure_label` method. -There are four important limitations to this configurability: +1. You can remove an existing group of systems by their public label, and then replace them with a new set of your own systems that perform the same job. +2. System ordering constraints (usually) cease to have any effect if no systems with that label exist in the schedule. You can remove (or disable) any system by its label. +3. Every system has at least one label (other than those added directly to the app that you control), which is automatically assigned to it by its `Plugin` when it is added to the app. -1. Systems can only be configured as a group, on the basis of their public labels. -2. Systems can only be removed from plugins as a complete system set. -3. `configure_label` cannot remove labels from systems. -4. `configure_label` cannot remove ordering dependencies, only add them. +Now, let's take a look at how we can use `App::configure_label` to tackle those use cases we mentioned at the top of this section: -These limitations are designed to enable plugin authors to carefully control how their systems can be configured in order to avoid subtly breaking internal guarantees. -If you change the configuration of the systems in such a way that ordering dependencies cannot be satisfied (by creating an impossible cycle or breaking a hard system ordering rule by changing the stage in which a system runs), the app's schedule will panic when it is run. -Duplicate and missing resources will also cause a panic when the app is run. +TODO: Demonstrate how these examples work. ### Writing plugins to be configured @@ -351,10 +352,6 @@ However, instead of counting the number of instances of that system with that la All resources are added to the app before the schedule begins. The app should panic if duplicate resources are detected at this stage; app users can manually remove them from one or more plugins in a transparent fashion to remove the conflict if needed. -### Label properties - -Users must be able to configure systems by reference to a particular label. This proposal has previously been called **label properties**. - ## Drawbacks 1. Plugins are no longer quite as powerful as they were. This is both good and bad: it reduces their expressivity, but reduces the risk that they mysteriously break your app. From 47d3b1172e6c8b3775bec9b15ac382c03940c0a1 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sat, 18 Sep 2021 13:40:17 -0400 Subject: [PATCH 19/20] Remove outdated description Thanks @IceSentry --- rfcs/33-apps_own_scheduling.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index d03748d2..5a461365 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -40,8 +40,7 @@ fn main(){ } ``` -The standard pattern for defining plugins is to export a `MyCrateNamePlugin` struct that implements the `Into` trait. -In the `into()` method for that trait, create and return a new `Plugin` struct. +The standard pattern for defining plugins is to export a `MyCrateNamePlugin` struct that implements the `Plugin` trait. ```rust struct CombatPlugin; From c355609d4b2721ec9be85a4e616cd5a0cc663acb Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sat, 18 Sep 2021 21:17:04 -0400 Subject: [PATCH 20/20] Configuration examples --- rfcs/33-apps_own_scheduling.md | 212 ++++++++++++++++++++++++++++----- 1 file changed, 180 insertions(+), 32 deletions(-) diff --git a/rfcs/33-apps_own_scheduling.md b/rfcs/33-apps_own_scheduling.md index 5a461365..d0903661 100644 --- a/rfcs/33-apps_own_scheduling.md +++ b/rfcs/33-apps_own_scheduling.md @@ -82,14 +82,6 @@ fn realistic_gravity_criteria(realistic_gravity_setting: Res) } } -fn platformer_gravity_criteria(realistic_gravity_setting: Res) -> ShouldRun { - if RealisticGravity.0 { - ShouldRun::No - } else { - ShouldRun::Yes - } -} - impl Plugin for PhysicsPlugin { fn build(plugin: &mut PluginState) { plugin @@ -98,7 +90,7 @@ impl Plugin for PhysicsPlugin { // These systems share a label to ensure that other systems will be scheduled correctly // regardless of the configuration chosen .add_system(realistic_gravity::.set_run_criteria(realistic_gravity_criteria).label("Gravity")) - .add_system(platformer_gravity::.set_run_criteria(platformer_gravity_criteria).label("Gravity")); + .add_system(platformer_gravity::.set_run_criteria(!realistic_gravity_criteria).label("Gravity")); plugin } } @@ -110,6 +102,10 @@ fn main(){ } ``` +Even if you're just using `bevy_ecs`, plugins can be a useful tool for enabling code reuse and managing dependencies. + +Instead of adding plugins to your app, independently add their system sets to your schedule and their resources to your world. + ### System ordering in Bevy Bevy schedules are composed of stages, which have two phases: parallel and sequential. @@ -134,8 +130,9 @@ However, ordering constraints can only be applied *relative* to labels, allowing - If systems in `A` use interior mutability, if-needed ordering may result in non-deterministic outcomes. - Use `before_if_needed(label: impl SystemLabel)` or `after_if_needed(label: impl SystemLabel)`. - **At-least-once separation:** Systems in set `A` cannot be started if a system in set `B` has been started until at least one system with the label `S` has completed. Systems from `A` and `B` are incompatible with each other. - - This is most commonly used when commands created by systems in `A` must be processed before systems in `B` can run correctly. `CoreLabels::ApplyCommands` is the label used for the exclusive system that applies queued commands that is typically added to the beginning of each sequential stage. - - Use the `between(before: impl SystemLabel, after: impl SystemLabel)` method. + - This is most commonly used when commands created by systems in `A` must be processed before systems in `B` can run correctly. + - Use the `between(before: impl SystemLabel, after: impl SystemLabel)`, `before_and_seperated_by(before: impl SystemLabel, seperated_by: impl SystemLabel)` method or its `after` analogue. + - `hard_before` and `hard_after` are syntactic sugar for the common case where the separating system label is `CoreSystem::ApplyCommands`, the label used for the exclusive system that applies queued commands that is typically added to the beginning of each sequential stage. - The order of arguments in `between` matters; the labels must only be separated by a cleanup system in one direction, rather than both. - This methods do not automatically insert systems to enforce this separation: instead, the schedule will panic upon initialization as no valid system execution strategy exists. @@ -209,35 +206,176 @@ There are also some powerful but subtle tools to work around these limitations: 2. System ordering constraints (usually) cease to have any effect if no systems with that label exist in the schedule. You can remove (or disable) any system by its label. 3. Every system has at least one label (other than those added directly to the app that you control), which is automatically assigned to it by its `Plugin` when it is added to the app. -Now, let's take a look at how we can use `App::configure_label` to tackle those use cases we mentioned at the top of this section: +Now, let's take a look at how we can use `App::configure_label` to tackle those use cases we mentioned at the top of this section. -TODO: Demonstrate how these examples work. +#### Making sure one of your systems runs before a third-party system -### Writing plugins to be configured +```rust +fn main(){ + App::default() + .add_plugin(ThirdPartyPlugin) + // We could also use a public label exposed by this plugin to control the timing more granularly + .add_system(hello_world.before(ThirdPartyPlugin.get_label())) + .run(); +} +``` -When writing plugins (especially those designed for other people to use), try to define constraints on your plugins logic so that your users could not break it if they tried: +This `before` ordering operates across stage boundaries: placing systems in a later stage than the system they are before will cause a panic at schedule initialization. -- if some collection of systems will not work properly without each other, make sure they are in the same system. This prevents them from being split by either schedule location or run criteria. -- if one system must run before the other, use granular strict system ordering constraints -- if a broad control flow must be follow, use if-needed system ordering constraints between labels -- if commands must be processed before a follow-up system can be run correctly, use at-least-once separation ordering constraints -- if a secondary source of truth (such as an index) must be updated in between two systems, use at-least-once separation ordering constraints +#### Control the relative order of two conflicting third-party systems -Exposing system labels via `pub` allows anyone with access to: +```rust +fn main(){ + App::default() + .add_plugin(ThirdPartyPlugin) + .add_plugin(FourthPartyPlugin) + .configure_label( + ThirdPartyPlugin.get_label() + .before(FourthPartyPlugin.get_label()) + ) + .run(); +} +``` -- set the order of their systems relative to the exposed label -- add new constraints to systems with that label -- change which states systems with that label will run in -- add new run criteria to systems with that label -- apply the labels to new systems +You are not restricted to simple strict ordering constraints here: if-needed ordering and at-least-once separation can also be used. +For example, you may commonly need to ensure that commands are processed between the execution of the plugins from one system and the start of a second one. -As such, try to only expose labels that apply to systems that can safely be enabled or disabled as a group. +```rust +fn main(){ + App::default() + .add_plugin(ThirdPartyPlugin) + .add_plugin(FourthPartyPlugin) + .configure_label( + ThirdPartySystem::Spawning + .before_and_separated_by(FourthPartyPlugin.get_label(), CoreSystem::ApplyCommands) + ) + // This ordering constraint has the same effect as the one above + .configure_label(CoreSystem::ApplyCommands.between( + ThirdPartySystem::Spawning, + FourthPartyPlugin.get_label() + )) + .run(); +} +``` -### Standalone `bevy_ecs` usage +With this constraint, the schedule will panic on initialization if any system from `FourthPartyPlugin` occurs after a `ThirdPartySystem::Spawning` system and no `CoreSystem::ApplyCommands` can be scheduled in between it. +Using the default schedule, this means that they must occur in a later stage (or during the sequential phase of a single stage, with extra command-applying systems inserted in the middle). -Even if you're just using `bevy_ecs`, plugins can be a useful tool for enabling code reuse and managing dependencies. +However, like with standard `before` constraints, there is no *requirement* that a `FourthPartyPlugin` label exist later in the schedule than `ThirdPartySystem::Spawning`. +Constraints are always ultimately imposed on the later system in the schedule, and have no effect if no later system exists. -Instead of adding plugins to your app, independently add their system sets to your schedule and their resources to your world. +#### Add a run criteria to disable a plugin's system when certain criteria are met + +```rust +fn main(){ + App::default() + .add_plugin(ThirdPartyPlugin) + .configure_label( + ThirdPartySystem::TrickyMechanic + // This additional run criteria will be added to any existing run criteria, + // chaining together in an AND fashion, + // ensuring that existing criteria continue to restrict when the system runs + .add_run_criteria(hard_mode_only_criteria) + ) + .configure_label( + ThirdPartySystem::Tooltips + // This run criteria will be chained in an OR fashion, + // allowing you to add behavior at additional, unexpected times + .add_enabling_run_criteria(tutorial_segment_criteria) + ) + .configure_label( + ThirdPartySystem::Physics + // This removes all existing run criteria + .clear_run_criteria() + // New run criteria can then be added later + .add_run_criteria(FixedTimeStep::(0.1)) + ) + .run(); +} +``` + +#### Change which state(s) a plugins' systems runs in + +```rust +fn main(){ + App::default() + .add_plugin(PhysicsPlugin) + .configure_label( + PhysicsPlugin.get_label() + .move_to_state(GameState::Running(State::Update)) + ) + .run(); +} +``` + +**NOTE:** this particular example is very WIP, as states are due for a complete overhaul. +It is not clear how you would ensure that the systems added by a plugin run in multiple states (or e.g. on both exit and enter), if the schedule owns systems. + +#### Add a plugin's systems to a nested schedule that runs repeatedly each frame + +```rust +enum GameSchedule { + Main, + Actions +} + +fn main(){ + App::default() + .add_schedule( + // .set_main_schedule causes this schedule to replace + // the standard schedule provided by App::default + Schedule::new(GameSchedule::Main).set_main_schedule() + ) + .add_schedule( + Schedule::new(GameSchedule::Actions) + // This adds the Action schedule as a nested schedule within the Main schedule + .nest(GameSchedule::Main) + // The Action schedule will repeat until the criteria returns ShouldRun::No + .repeat(action_looping_criteria) + ) + // The systems added by this plugin will be added to the Actions schedule + // rather than the main schedule + .add_plugin_to_schedule(ActionResolutionPlugin, GameSchedule::Actions) + .run(); +} +``` + +**NOTE:** this example is very speculative. +Currently, we don't have great `App`-level tools for reasoning about and configuring nested schedules like this so the API here is invented whole-cloth. + +#### Schedule strategy optimization + +```rust +fn main(){ + App::default() + // Both of these plugins are very expensive, + // and through careful profiling we've determined that + // frame time is minimized when they live in seperate stages + .add_plugin(PathfindingPlugin) + .add_plugin(PhysicsPlugin) + .configure_label( + PathfindingPlugin.get_label() + // This is the default anchor for all plugins, + // but we can change that by using app::set_default_stage + .anchor_in_stage(CoreStage::Update) + ) + .configure_label( + PhysicsPlugin.get_label() + .anchor_in_stage(CoreStage::PostUpdate) + ) + .configure_label( + Physics::CollisionDetection + .anchor_in_stage(CoreStage::PreUpdate) + ) + .run(); +} +``` + +If all systems that share a label can coexist within a single stage, using `.anchor_in_stage` will move them all to that stage. +However, if "hard system ordering constraints" exist (due to the need to run an exclusive system such as commands application) between the systems, +constrained systems will be placed into adjacent stages if they exist (repeating as needed). + +If not enough stages exist in the schedule to satisfy these constraints, the schedule will panic upon initialization. ## Implementation strategy @@ -245,6 +383,7 @@ Small changes: 1. As plugins no longer depend on `App` information at all, they should be moved into `bevy_ecs` directly. 2. If plugins can no longer configure the `App` in arbitrary ways, we need a new, ergonomic way to set up the default schedule and runner for standard Bevy games. The best way to do this is to create two new builder methods on `App`: `App::minimal` and `App::default`, which sets up the default stages, sets the runner and adds the required plugins. +3. `ShouldRun` is changed to a simple Boolean logic to enable sane composition. ### Plugin architecture @@ -439,12 +578,21 @@ This will be particularly valuable when working on editor support, or when build It also cleans up the ergonomics of the `Plugin` machinery substantially. +## Why do we need to change `ShouldRun`? + +The existing `ShouldRun` construct, which combines whether a system should be run with whether it should be checked again is very complicated, hard to compose and reason about, and causes serious user confusion. +This looping logic should be handled with other tools, rather than being added to run criteria. + +The increased use (and particularly composition of) run criteria in this design mean that we should solve this (working out the details in a separate RFC), rather than contorting plugin configuration to work around it. + ## Unresolved questions 1. Should we rename `SystemDescriptor` to something more user-friendly like `ConfiguredSystem`? 2. How do we handle [plugins that need to extend other resources at initialization](https://github.com/bevyengine/rfcs/pull/33#discussion_r709654419)? 3. How do we ensure that [adding run criteria won't break invariants](https://github.com/bevyengine/rfcs/pull/33#discussion_r709655879)? 4. Do we still need plugin groups at all? Can we replace them with a simple standard collection? The ergonomics matter much less with `App::default()` initialization. +5. What tools do we need to add in order to be able to safely simplify `ShouldRun` ? +6. What does the new `State` paradigm look like, and how does it interact with this design? ## Future possibilities @@ -455,5 +603,5 @@ This could be extended and improved in the future with: 1. Enhanced system visualization tools. 2. A scheduler / app option could be added to automatically infer and insert systems (including command-processing systems) on the basis of at-least-once separation constraints. 3. At-least-once separation constraints can be used to solve the cache invalidation issues that are blocking the implementation of indexes, as discussed in [Bevy #1205](https://github.com/bevyengine/bevy/discussions/1205). -4. `ShouldRun` could be cleaned up and moved back to simple binary logic. More helpers could be added for common plugin use cases. -5. Systems could be fully added and removed from the schedule based on run-time information, keeping the schedule clean when configuring plugins with resources. +4. More run criteria helpers could be added, to make enabling and disabling systems based on resource state more ergonomic. +5. Systems could be fully added and removed from the schedule based on run-time information, keeping the schedule clean when configuring plugins with resources. The mostly likely tool for this is to extend (or supplement) `Commands`, as seen in [Bevy #2507](https://github.com/bevyengine/bevy/pull/2507).