From 2583ff2ef697bd5345a09f904cc80a29c2cbaafc Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 12:00:20 -0400 Subject: [PATCH 01/50] Boilerplate stub Co-authored-by: Mateusz Wachowiak --- rfcs/77-dev-tools-abstraction.md | 82 ++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 rfcs/77-dev-tools-abstraction.md diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md new file mode 100644 index 00000000..32093959 --- /dev/null +++ b/rfcs/77-dev-tools-abstraction.md @@ -0,0 +1,82 @@ +# Feature Name: `dev-tools-abstraction` + +## Summary + +One paragraph explanation of the feature. + +## Motivation + +Why are we doing this? What use cases does it support? + +## User-facing explanation + +Explain the proposal as if it was already included in the engine and you were teaching it to another Bevy user. That generally means: + +- Introducing new named concepts. +- Explaining the feature, ideally through simple examples of solutions to concrete problems. +- Explaining how Bevy users should *think* about the feature, and how it should impact the way they use Bevy. It should explain the impact as concretely as possible. +- If applicable, provide sample error messages, deprecation warnings, or migration guidance. +- If applicable, explain how this feature compares to similar existing features, and in what situations the user would use each one. + +## Implementation strategy + +This is the technical portion of the RFC. +Try to capture the broad implementation strategy, +and then focus in on the tricky details so that: + +- Its interaction with other features is clear. +- It is reasonably clear how the feature would be implemented. +- Corner cases are dissected by example. + +When necessary, this section should return to the examples given in the previous section and explain the implementation details that make them work. + +When writing this section be mindful of the following [repo guidelines](https://github.com/bevyengine/rfcs): + +- **RFCs should be scoped:** Try to avoid creating RFCs for huge design spaces that span many features. Try to pick a specific feature slice and describe it in as much detail as possible. Feel free to create multiple RFCs if you need multiple features. +- **RFCs should avoid ambiguity:** Two developers implementing the same RFC should come up with nearly identical implementations. +- **RFCs should be "implementable":** Merged RFCs should only depend on features from other merged RFCs and existing Bevy features. It is ok to create multiple dependent RFCs, but they should either be merged at the same time or have a clear merge order that ensures the "implementable" rule is respected. + +## Drawbacks + +Why should we *not* do this? + +## Rationale and alternatives + +- Why is this design the best in the space of possible designs? +- What other designs have been considered and what is the rationale for not choosing them? +- What objections immediately spring to mind? How have you addressed them? +- What is the impact of not doing this? +- Why is this important to implement as a feature of Bevy itself, rather than an ecosystem crate? + +## \[Optional\] Prior art + +Discuss prior art, both the good and the bad, in relation to this proposal. +This can include: + +- Does this feature exist in other libraries and what experiences have their community had? +- Papers: Are there any published papers or great posts that discuss this? + +This section is intended to encourage you as an author to think about the lessons from other tools and provide readers of your RFC with a fuller picture. + +Note that while precedent set by other engines is some motivation, it does not on its own motivate an RFC. + +## Unresolved questions + +- What parts of the design do you expect to resolve through the RFC process before this gets merged? +- What parts of the design do you expect to resolve through the implementation of this feature before the feature PR is merged? +- What related issues do you consider out of scope for this RFC that could be addressed in the future independently of the solution that comes out of this RFC? + +## \[Optional\] Future possibilities + +Think about what the natural extension and evolution of your proposal would +be and how it would affect Bevy as a whole in a holistic way. +Try to use this section as a tool to more fully consider other possible +interactions with the engine in your proposal. + +This is also a good place to "dump ideas", if they are out of scope for the +RFC you are writing but otherwise related. + +Note that having something written down in the future-possibilities section +is not a reason to accept the current or a future RFC; such notes should be +in the section on motivation or rationale in this or subsequent RFCs. +If a feature or change has no direct value on its own, expand your RFC to include the first valuable feature that would build on it. From 4ebb77efc00ba96eef9b80dd332120152b703505 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 12:25:49 -0400 Subject: [PATCH 02/50] Initial notes --- rfcs/77-dev-tools-abstraction.md | 57 ++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 32093959..0ab7012d 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -1,5 +1,62 @@ # Feature Name: `dev-tools-abstraction` +Key questions: + +- what are the defining features of a dev tool? + - eases Bevy development experience, typically be presenting information about the app or its function + - not intended for end-user use + - must be able to be disabled at compile time + - contextually useful based on encountered weirdness in the app + - must be able to be toggled on and off at runtime without a reboot + - should not interfere with normal operation of the app + - can be customized to meet the needs of both the app and the developer +- what are clear examples of dev tools that we might want? + - system graph visualizer + - bevy_inspector_egui + - fly camera + - UI node visualizer + - FPS display + - resetting a level + - toggling god mode +- who creates dev tools? + - Bevy itself: FPS display + - third-party crates: bevy_rapier collider overlay + - end users: toggle god mode +- who creates dev tool consumers? + - Bevy itself: scene editor + - third-party crates: dev console + - end users: custom level editors +- how might dev tools be toggled and consumed? + - Quake-style dev console + - unified set of hot keys + - menu bar from scene editor +- how might dev tools be displayed? + - screen overlay + - pop-out window + - embedded panel widget + - cli +- how might dev tools be configured? + - color palettes + - fonts + - font size + - screen location + - dev tool specific features: camera speed, entities to ignore etc +- why do dev tools need to be aware of each other? + - overlays can clash: toggle off current overlay before enabling next +- how can we make it easy for third-party consumers and producers to play nice together? + - Cart's suggestion: let each consumer decide which tools it wants to use + - creates a quadratic problem: must wire up plumbing separately for each tool and consumer + - define a standard API for available dev tools and essential operations + - existing tools will have to conform to this in some way + +Out of scope: + +- [where should dev tools live](https://github.com/bevyengine/bevy/pull/12354)? + - where the primitives used are defined? e.g. bevy_gizmos + - where they're used for debugging? e.g. bevy_sprite + - in bevy_dev_tools? + - in dedicated crates, saving bevy_dev_tools for higher level abstractions? + ## Summary One paragraph explanation of the feature. From a8869e2e0001fa63758900f112262392a187bea0 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 12:37:47 -0400 Subject: [PATCH 03/50] Motivation --- rfcs/77-dev-tools-abstraction.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 0ab7012d..64df34c4 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -63,7 +63,17 @@ One paragraph explanation of the feature. ## Motivation -Why are we doing this? What use cases does it support? +[`bevy_dev_tools`](gh link) was recently added to Bevy, giving us a home for first-party tools to ease the developer experience. +However, since its inception, it has been plagued by debate over "what is a dev tool?" +and every **tool** within both Bevy and its ecosystem follows its own ad hoc conventions about how it might be enabled and configured. + +This is frustrating to navigate as an end user, but more critically, +makes the creation of **toolboxes**, interfaces designed to collect multiple dev tools in a single place (such as a Quake-style dev console) +needlessly painful, requiring manual glue work for every new tool that's added to it. + +In some cases, tools can actively interfere with the function of other tools. +A prime example of this are text-based overlays, like you might see in an FPS meter. +Toggling one tool might completely cover (or distort the layout) of another, and so some level of coordination is required for a smooth user experience. ## User-facing explanation From f0f0cba17df9b05c64d7553f7e22bc4a426c001d Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 13:13:29 -0400 Subject: [PATCH 04/50] Add background to the user facing explanation --- rfcs/77-dev-tools-abstraction.md | 64 +++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 9 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 64df34c4..e319c769 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -56,6 +56,7 @@ Out of scope: - where they're used for debugging? e.g. bevy_sprite - in bevy_dev_tools? - in dedicated crates, saving bevy_dev_tools for higher level abstractions? +- should dev tools be embedded in each application or should testing be done through an editor which controls these dev tools? ## Summary @@ -67,8 +68,8 @@ One paragraph explanation of the feature. However, since its inception, it has been plagued by debate over "what is a dev tool?" and every **tool** within both Bevy and its ecosystem follows its own ad hoc conventions about how it might be enabled and configured. -This is frustrating to navigate as an end user, but more critically, -makes the creation of **toolboxes**, interfaces designed to collect multiple dev tools in a single place (such as a Quake-style dev console) +This is frustrating to navigate as an end user: every tool has its own way to configure it, with no rhyme, reason or way to set the same property to the same value across tools. +It also makes the creation of **toolboxes**, interfaces designed to collect multiple dev tools in a single place (such as a Quake-style dev console) needlessly painful, requiring manual glue work for every new tool that's added to it. In some cases, tools can actively interfere with the function of other tools. @@ -77,13 +78,58 @@ Toggling one tool might completely cover (or distort the layout) of another, and ## User-facing explanation -Explain the proposal as if it was already included in the engine and you were teaching it to another Bevy user. That generally means: - -- Introducing new named concepts. -- Explaining the feature, ideally through simple examples of solutions to concrete problems. -- Explaining how Bevy users should *think* about the feature, and how it should impact the way they use Bevy. It should explain the impact as concretely as possible. -- If applicable, provide sample error messages, deprecation warnings, or migration guidance. -- If applicable, explain how this feature compares to similar existing features, and in what situations the user would use each one. +**Dev tools**, or simply **tools**, are an eclectic collection of features used by developers to inspect and manipulate their game at runtime. +These arise naturally to aid debugging and testing: setting up test scenarios. +Some examples include: + +- displaying the FPS or other performance characteristics +- an entity inspector like `bevy_inspector_egui`, to understand the state of the `World` +- a UI node visualizer, to debug issues with layout +- a fly camera, to examine the environment around the player avatar +- system stepping, to walk through the evaluation of system logic +- adding items to the player's inventory +- toggling god mode + +As you can see, these tools are currently and will continue to be created by the creators of indidvidual games, third-party crates in Bevy's ecosystem, and Bevy itself. + +As a result of this usage pattern, dev tools are: + +- not intended to be shipped to end users + - as a result, they can be enabled or disabled at compile time, generally behind a single `dev` flag in the application +- highly application specific: the tools needed for a 3D FPS, a 2D tilemap game, or a puzzle game are all very different + - adding these to your game should be done granularly at compile time +- enabled or operated one at a time, on an as-needed basis, when problems arise + - toggling dev tools needs to be done at runtime: without recompiling or restarting the game +- intended to avoid interfering with the games ordinary operation, especially when disabled + - keybindings must be configurable to avoid conflicting with actual gameplay + - while off, performance costs should be low or zero +- expansive and ad hoc: it's common to end up with dozens of assorted dev tools in a finished game, added to help solve specific problems as they occur + - some structure is generally needed to present these options to the developer, and avoid ovewhelming them with dozens of chorded hotkeys + +Some tools, while useful for development, *don't* match this pattern! +For example: + +- a system graph visualizer like [`bevy_mod_debugdump`](https://github.com/jakobhellermann/bevy_mod_debugdump): this doesn't rely on information about a running game +- the [`perfetto`](https://ui.perfetto.dev/) performance profiler or a time travel debugger like [rerun-io's Revy](https://github.com/rerun-io/revy/tree/main): these relies on logs created, although in-process equivalents could be built +- a scene editor, pixel art tool or audio workstation: these are used to create assets, and do not need to be accessible at runtime +- gizmos: while these are commonly used for *making* dev tools, they don't inherently provide a way to inspect or manipulate the game world + +In order to manage the complexity of an eclectic collection of one-off debugging tools, dev tools are commonly organized into a **toolbox**: an abstraction used to provide a unified interface over the available dev tools. +This might be: + +- an in-game dev console (like in Quake) +- special developer commands entered into the chat box, commonly with a `/` to denote their non-textual effect +- cheat codes +- game editor widgets + +Regardless of the exact interface used, toolboxes have a few shared needs: + +- turn on and off various modes +- ensure that the active modes don't interfere with each other +- execute one-time commands (like spawning enemies or setting the player's gold) +- list the various options for the user, ideally with help text + +### A shared abstraction ## Implementation strategy From 0955fa260ea134b42cc27d8ffcfe4e3ed0d85b76 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 14:42:25 -0400 Subject: [PATCH 05/50] Immediate and modal dev tools --- rfcs/77-dev-tools-abstraction.md | 202 ++++++++++++++++++++++++++----- 1 file changed, 171 insertions(+), 31 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index e319c769..356797dd 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -64,13 +64,20 @@ One paragraph explanation of the feature. ## Motivation -[`bevy_dev_tools`](gh link) was recently added to Bevy, giving us a home for first-party tools to ease the developer experience. -However, since its inception, it has been plagued by debate over "what is a dev tool?" -and every **tool** within both Bevy and its ecosystem follows its own ad hoc conventions about how it might be enabled and configured. +[`bevy_dev_tools`](https://github.com/bevyengine/bevy/tree/main/crates/bevy_dev_tools) was recently added to Bevy, giving us a home for first-party tools to ease the developer experience. +However, since its inception, it has been plagued by debate over ["what is a dev tool?"](https://github.com/bevyengine/bevy/issues/12358) +and every **tool** within both Bevy and its ecosystem follows its own ad hoc conventions about [how it might be enabled and configured](https://github.com/bevyengine/bevy/issues/12368). This is frustrating to navigate as an end user: every tool has its own way to configure it, with no rhyme, reason or way to set the same property to the same value across tools. It also makes the creation of **toolboxes**, interfaces designed to collect multiple dev tools in a single place (such as a Quake-style dev console) -needlessly painful, requiring manual glue work for every new tool that's added to it. +needlessly painful, requiring manual glue work by either the toolbox creator or application developer for every new tool that's added to it. + +This problem is particularly nasty when combined with the orphan rule: Rust's restriction that traits can only be implemented in crates that own either the type or the trait. +Suppose that `bevy_xr_dev_console` (a hypothetical promising new toolbox crate!) requires its own `XrConsoleCommand` trait on the config for each of the dev tools that they want to add to their XR game. +If we want to integrate this with beloved existing tools like `bevy_inspector_egui`, we need to special-case either the tool or the toolbox in the third-party crates. +There's no path forward for the end users who wants to create the simple unified interface they want. +Instead, for every new tool, they have to newtype the existing configuration, and then add systems to carefully synchronize the settings. +By providing a standard interface in Bevy itself, we can ensure the ecosystem plays nicely together. In some cases, tools can actively interfere with the function of other tools. A prime example of this are text-based overlays, like you might see in an FPS meter. @@ -78,7 +85,7 @@ Toggling one tool might completely cover (or distort the layout) of another, and ## User-facing explanation -**Dev tools**, or simply **tools**, are an eclectic collection of features used by developers to inspect and manipulate their game at runtime. +**Dev tools** are an eclectic collection of features used by developers to inspect and manipulate their game at runtime. These arise naturally to aid debugging and testing: setting up test scenarios. Some examples include: @@ -88,11 +95,26 @@ Some examples include: - a fly camera, to examine the environment around the player avatar - system stepping, to walk through the evaluation of system logic - adding items to the player's inventory +- warping to a given level - toggling god mode +Some tools, while useful for development, *don't* match this pattern! +For example: + +- a system graph visualizer like [`bevy_mod_debugdump`](https://github.com/jakobhellermann/bevy_mod_debugdump): this doesn't rely on information about a running game +- the [`perfetto`](https://ui.perfetto.dev/) performance profiler or a time travel debugger like [rerun-io's Revy](https://github.com/rerun-io/revy/tree/main): these relies on logs created, although in-process equivalents could be built +- a scene editor, pixel art tool or audio workstation: these are used to create assets, and do not need to be accessible at runtime +- gizmos: while these are commonly used for *making* dev tools, they don't inherently provide a way to inspect or manipulate the game world + As you can see, these tools are currently and will continue to be created by the creators of indidvidual games, third-party crates in Bevy's ecosystem, and Bevy itself. +Looking at the usage examples more closely, we can classify them into two patterns: **modal dev tools** and **dev commands**. + +- Modal dev tools (like an FPS meter): can be toggled off and on, and want persistent configuration. +- Dev commands (like adding items): immediately alter the game world + +As a result, we require two distinct (but inter-related) abstractions to handle these patterns. -As a result of this usage pattern, dev tools are: +Regardless of this classification, dev tools are: - not intended to be shipped to end users - as a result, they can be enabled or disabled at compile time, generally behind a single `dev` flag in the application @@ -106,20 +128,13 @@ As a result of this usage pattern, dev tools are: - expansive and ad hoc: it's common to end up with dozens of assorted dev tools in a finished game, added to help solve specific problems as they occur - some structure is generally needed to present these options to the developer, and avoid ovewhelming them with dozens of chorded hotkeys -Some tools, while useful for development, *don't* match this pattern! -For example: - -- a system graph visualizer like [`bevy_mod_debugdump`](https://github.com/jakobhellermann/bevy_mod_debugdump): this doesn't rely on information about a running game -- the [`perfetto`](https://ui.perfetto.dev/) performance profiler or a time travel debugger like [rerun-io's Revy](https://github.com/rerun-io/revy/tree/main): these relies on logs created, although in-process equivalents could be built -- a scene editor, pixel art tool or audio workstation: these are used to create assets, and do not need to be accessible at runtime -- gizmos: while these are commonly used for *making* dev tools, they don't inherently provide a way to inspect or manipulate the game world - In order to manage the complexity of an eclectic collection of one-off debugging tools, dev tools are commonly organized into a **toolbox**: an abstraction used to provide a unified interface over the available dev tools. This might be: - an in-game dev console (like in Quake) - special developer commands entered into the chat box, commonly with a `/` to denote their non-textual effect - cheat codes +- a unified set of hotkeys - game editor widgets Regardless of the exact interface used, toolboxes have a few shared needs: @@ -129,7 +144,127 @@ Regardless of the exact interface used, toolboxes have a few shared needs: - execute one-time commands (like spawning enemies or setting the player's gold) - list the various options for the user, ideally with help text -### A shared abstraction +### Toggling modes: `ModalDevTool` + +In order to facilitate the creation of toolboxes, Bevy provides the `ModalDevTool` trait. + +```rust +/// Modal dev tools are used by developers to inspect their application in a toggleable way, +/// such as an FPS meter or a fly camera. +/// +/// Their configuration is stored as a resource (the type that this trait is implemented for), +/// and they can be enabled, disabled and reconfigured at runtime. +/// +/// The documentation on this struct is reflected, and can be read by toolboxes to provide help text to users. +trait ModalDevTool: Resource + Reflect { + /// Turns this dev tool on (true) or off (false). + fn set_enabled(&mut self, enabled: bool); + + /// Is this dev tool currently enabled? + fn is_enabled(&self) -> bool; + + /// Enables this dev tool. + fn enable(&mut self) { + self.toggle(true); + } + + /// Disables this dev tool. + fn disable(&mut self) { + self.toggle(false); + } + + /// Enables this dev tool if it's disabled, or disables it if it's enabled. + fn toggle(&mut self){ + if self.is_enabled(){ + self.disable(); + } else { + self.enable(); + } + } +} +``` + +Modal dev tools are registered via `app.init_modal_dev_tool::()` (to use default config based on the `FromWorld` implementation) or via `app.insert_modal_dev_tool(config: D)`. +This adds them as a resource (just like ), but also registers their `ComponentId` with the central `DevToolsRegistry`, which can be consumed by toolboxes to get a list of the available dev tools. + +To build your own modal dev tool, simply create a configuration resource, implement the `ModalDevTool` trait, and then register it in the app when the correct feature flag is enabled. + +```rust +/// A flying camera controller that lets you disconnect your camera from the player to freely explore the environment. +/// +/// When this mode is disabled +#[derive(Resource, Reflect)] +struct DevFlyCamera { + enabled: bool, + /// How fast the camera travels forwards, backwards, left, right, up and down, in world units. + movement_speed: f32, + /// How fast the camera turns, in radians per second. + turn_speed: f32, +} + +impl ModalDevTool for DevFlyCamera { + fn set_enabled(&mut self, enabled: bool) { + self.enabled = enabled; + } + + fn is_enabled(&self) -> bool { + self.enabled + } +} +``` + +To configure a dev tool at runtime, simply access the resource, and either mutate or overwrite the `ModalDevTools` struct. + +### Immediate dev tools + +Immediate dev tools modify the world a single time, and can be called with arguments. +To model this, we leverage Bevy's existing `Command` trait, which exists to perform complex one-off modifications to the world. + +```rust +/// Dev commands are used by developers to modify the `World` in order to easily debug and test their application. +/// +/// Dev commands can be called with arguments to specify the exact behavior: if you are creating a toolbox, parse the provided arguments +/// to construct an instance of the type that implements this type, and then send it as a `Command` to execute it. +/// +/// The documentation on this struct is reflected, and can be read by toolboxes to provide help text to users. +trait DevCommand: Command + Reflect; + +/// `DevCommand` uses a blanket implementation over all appropriate commands, as it has no special requirements. +impl DevCommand for C {} +``` + +Creating your own dev command is simple! Create a struct for any config, implement `Command` for it and then register it using `app.register_dev_command::()`, making it available for various toolboxes to inspect and send. + +```rust +/// Sets the player's gold to the provided value. +#[derive(Reflect)] +struct SetGold { + amount: u64, +} + +impl Command for SetGold { + fn apply(&self, world: &mut World){ + let mut current_gold = world.resource_mut::(); + current_gold.0 = self.amount; + } +} +``` + +### Building toolboxes + +TODO + +### Conventions for building dev tools + +Not everything can or should be defined by a trait! To supplement the `DevTool` trait, we recommend that Bevy and its ecosystem follow the following conventions: + +1. Dev tools can be toggled at compile time. +2. Dev tools can be toggled at run time. +3. Dev tools implement the `DevTool` trait, and if the corresponding feature is enabled, are added to the app via `.init_dev_tool` or `.insert_dev_tool`. + 1. This can be done either in the main plugin or a dedicated dev tools plugin. + 1. If configuration is required, splitting this into a dedicated dev tools plugin is preferred. +4. Dev tools are disabled by default, both at compile and run-time. +5. Each dev tools should be configured independently via a single resource. ## Implementation strategy @@ -149,17 +284,21 @@ When writing this section be mindful of the following [repo guidelines](https:// - **RFCs should avoid ambiguity:** Two developers implementing the same RFC should come up with nearly identical implementations. - **RFCs should be "implementable":** Merged RFCs should only depend on features from other merged RFCs and existing Bevy features. It is ok to create multiple dependent RFCs, but they should either be merged at the same time or have a clear merge order that ensures the "implementable" rule is respected. +### What is a `DevToolsRegistry`? + +TODO + ## Drawbacks -Why should we *not* do this? +1. Third-party and end user dev tools will be pushed to conform to this standard. Without the use of a toolbox, this is added work for no benefit. +2. This abstraction may not fit all possible tools and toolboxes. The manual wiring approach is more flexible, and so if our abstraction is overly prescriptive, it may not work correctly. ## Rationale and alternatives -- Why is this design the best in the space of possible designs? -- What other designs have been considered and what is the rationale for not choosing them? -- What objections immediately spring to mind? How have you addressed them? -- What is the impact of not doing this? -- Why is this important to implement as a feature of Bevy itself, rather than an ecosystem crate? +### Why should we use commands rather than one-shot systems for immediate dev tools? + +Immediate dev tools commonly require input from the developer to take effect: what level to warp to, what item to spawn, what to set the player's gold to. +This pattern is very straightforward with commands, and quite complex to do with one-shot systems, requiring an associated type for the input. ## \[Optional\] Prior art @@ -181,15 +320,16 @@ Note that while precedent set by other engines is some motivation, it does not o ## \[Optional\] Future possibilities -Think about what the natural extension and evolution of your proposal would -be and how it would affect Bevy as a whole in a holistic way. -Try to use this section as a tool to more fully consider other possible -interactions with the engine in your proposal. +Related but out-of-scope questions: + +- [where should dev tools live](https://github.com/bevyengine/bevy/pull/12354#issuecomment-1982284605)? + - where the primitives used are defined? e.g. bevy_gizmos + - where they're used for debugging? e.g. bevy_sprite + - in bevy_dev_tools? + - in dedicated crates, saving bevy_dev_tools for higher level abstractions? +- should dev tools be embedded in each application or should testing be done through an editor which controls these dev tools? + - this is one of the [key questions](https://github.com/bevyengine/bevy_editor_prototypes/discussions/1) for the bevy_editor efforts -This is also a good place to "dump ideas", if they are out of scope for the -RFC you are writing but otherwise related. +Future possibilities: -Note that having something written down in the future-possibilities section -is not a reason to accept the current or a future RFC; such notes should be -in the section on motivation or rationale in this or subsequent RFCs. -If a feature or change has no direct value on its own, expand your RFC to include the first valuable feature that would build on it. +1. Over time, we can extend the `ModalDevTool` trait with optional methods like `set_font`, enabling gradual unification of more complex shared configuration. From 30508b71ced6334bfd6149d756f8883c28311479 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 18:10:02 -0400 Subject: [PATCH 06/50] Implementation strategies --- rfcs/77-dev-tools-abstraction.md | 370 +++++++++++++++++++++++++++---- 1 file changed, 330 insertions(+), 40 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 356797dd..d8189032 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -103,7 +103,7 @@ For example: - a system graph visualizer like [`bevy_mod_debugdump`](https://github.com/jakobhellermann/bevy_mod_debugdump): this doesn't rely on information about a running game - the [`perfetto`](https://ui.perfetto.dev/) performance profiler or a time travel debugger like [rerun-io's Revy](https://github.com/rerun-io/revy/tree/main): these relies on logs created, although in-process equivalents could be built -- a scene editor, pixel art tool or audio workstation: these are used to create assets, and do not need to be accessible at runtime +- a scene editor, pixel art tool, asset preprocessing solutions or audio workstation: these are used to create assets, and do not need to be accessible at runtime - gizmos: while these are commonly used for *making* dev tools, they don't inherently provide a way to inspect or manipulate the game world As you can see, these tools are currently and will continue to be created by the creators of indidvidual games, third-party crates in Bevy's ecosystem, and Bevy itself. @@ -156,7 +156,18 @@ In order to facilitate the creation of toolboxes, Bevy provides the `ModalDevToo /// and they can be enabled, disabled and reconfigured at runtime. /// /// The documentation on this struct is reflected, and can be read by toolboxes to provide help text to users. -trait ModalDevTool: Resource + Reflect { +trait ModalDevTool: Resource + Reflect + FromReflect + Debug { + /// The name of this tool, as might be supplied by a command line interface. + fn name() -> &'static str { + Self::type_name().to_snake_case() + } + + /// Attempts to create an object of type `Self` from a provided string, + /// as might be supplied by a command-line style interface. + fn parse_from_str(string: &str) -> Result { + Err(DevToolParseError::NotImplemented) + } + /// Turns this dev tool on (true) or off (false). fn set_enabled(&mut self, enabled: bool); @@ -193,16 +204,53 @@ To build your own modal dev tool, simply create a configuration resource, implem /// A flying camera controller that lets you disconnect your camera from the player to freely explore the environment. /// /// When this mode is disabled -#[derive(Resource, Reflect)] +#[derive(Resource, Reflect, FromReflect, Debug)] struct DevFlyCamera { enabled: bool, /// How fast the camera travels forwards, backwards, left, right, up and down, in world units. - movement_speed: f32, + movement_speed: Option, /// How fast the camera turns, in radians per second. - turn_speed: f32, + turn_speed: Option, +} + +impl Default for DevFlyCamera { + fn default() -> Self { + DevFlyCamera { + enabled: false, + movement_speed: 3., + turn_speed: 10. + } + } } impl ModalDevTool for DevFlyCamera { + /// Expects a call that looks like `dev_fly_camera` (default settings) + /// or `dev_fly_camera 5 10` (to specify settings) + fn parse_from_str(string: &str) -> Result{ + let parts = string.split_whitespace(); + let name = parts.iter.next()?; + if name != self.name() { + return Err(DevToolParseError::InvalidName); + } + + let movement_speed: f32 = match parts.iter.next() { + Some(string) = parse(string)?, + None => Self::default().movement_speed, + }; + + let turn_speed: f32 = match parts.iter.next() { + Some(string) = parse(string)?, + None => Self::default().turn_speed, + }; + + Ok(DevFlyCamera { + enabled: true, + movmeent_speed, + turn_speed, + } + ) + } + fn set_enabled(&mut self, enabled: bool) { self.enabled = enabled; } @@ -215,9 +263,9 @@ impl ModalDevTool for DevFlyCamera { To configure a dev tool at runtime, simply access the resource, and either mutate or overwrite the `ModalDevTools` struct. -### Immediate dev tools +### Dev commands -Immediate dev tools modify the world a single time, and can be called with arguments. +Dev commands modify the world a single time, and can be called with arguments. To model this, we leverage Bevy's existing `Command` trait, which exists to perform complex one-off modifications to the world. ```rust @@ -227,17 +275,36 @@ To model this, we leverage Bevy's existing `Command` trait, which exists to perf /// to construct an instance of the type that implements this type, and then send it as a `Command` to execute it. /// /// The documentation on this struct is reflected, and can be read by toolboxes to provide help text to users. -trait DevCommand: Command + Reflect; +trait DevCommand: Command + Reflect + FromReflect + Debug + 'static { + /// The name of this tool, as might be supplied by a command line interface. + fn name() -> &'static str { + Self::type_name().to_snake_case() + } + + /// The metadata for this dev command. + fn metadata() -> DevToolMetaData { + DevToolMetaData { + name: self.name(), + type_id: Self::type_id(), + type_info: Self::type_info(), + // A function pointer, based on the parse_from_str method + parse_from_str_fn: Self::parse_from_str + } + } -/// `DevCommand` uses a blanket implementation over all appropriate commands, as it has no special requirements. -impl DevCommand for C {} + /// Attempts to create an object of type `Self` from a provided string, + /// as might be supplied by a command-line style interface. + fn parse_from_str(string: &str) -> Result { + Err(DevToolParseError::NotImplemented) + } +} ``` -Creating your own dev command is simple! Create a struct for any config, implement `Command` for it and then register it using `app.register_dev_command::()`, making it available for various toolboxes to inspect and send. +Creating your own dev command is straightforward! Create a struct for any config, implement `Command` for it and then register it using `app.register_dev_command::()`, making it available for various toolboxes to inspect and send. ```rust /// Sets the player's gold to the provided value. -#[derive(Reflect)] +#[derive(Reflect, FromReflect, Debug)] struct SetGold { amount: u64, } @@ -248,11 +315,26 @@ impl Command for SetGold { current_gold.0 = self.amount; } } -``` -### Building toolboxes +impl DevCommand for SetGold { + /// We're fine using Bevy's built-in error type for this. + type StringParseError = DevToolParseError; -TODO + /// Expects a call that looks like `set_gold 500` + fn parse_from_str(string: &str) -> Result{ + let parts = string.split_whitespace(); + let name = parts.iter.next()?; + if name != self.name() { + return Err(DevToolParseError::InvalidName); + } + + let amount_string = parts.iter.next()?; + let amount = amount_string.parse()?; + + Ok(SetGold {amount} ) + } +} +``` ### Conventions for building dev tools @@ -265,33 +347,213 @@ Not everything can or should be defined by a trait! To supplement the `DevTool` 1. If configuration is required, splitting this into a dedicated dev tools plugin is preferred. 4. Dev tools are disabled by default, both at compile and run-time. 5. Each dev tools should be configured independently via a single resource. +6. If a meaningful defaults exist for fields on dev commands, that field should be modelled as an `Option`. + 1. This allows CLI-style toolboxes to more easily populate your struct. + +### Building toolboxes + +Let's take a look at how this all fits together by attempting to build a Quake-style dev console, much like [`bevy-console`](https://github.com/RichoDemus/bevy-console). +Obviously, this is too big to cover properly in an RFC example, so we're going to hand-wave the user input side of things by showing how you might deal with the parsed events, and simplify output by just logging to `println!`. + +Our first task is getting the list of dev tools. + +```rust +#[derive(Event)] +struct ListDevTools; + +fn list_dev_tools(mut event_reader: EventReader, dev_tools_registry: Res){ + // Clear the events, and act if at least one is found + if event_reader.drain().next().is_some() { + println!("Available modal dev tools:"); + for (_component_id, modal_dev_tool) in dev_tools_registry.iter_modal_dev_tools() { + println!("{}", modal_dev_tool.type_name()); + println!("{}", modal_dev_tool.docs()); + + for field in modal_dev_tools.fields(){ + println!("{}", field.type_name()); + println!("{}", field.docs()) + } + } + } +} +``` + +Next, we want to be able to toggle each modal tool by name. + +```rust +#[derive(Event)] +struct ToggleDevTool{ + name: String, +} + +fn toggle_dev_tools(world: &mut World){ + // Move the events out of the world, clearing them and avoiding a persistent borrow + let events = world.resource_mut::>().drain(); + + // Use a resource scope to allow us to access both the dev tools registry and the rest of the world at the same time + world.resource_scope(|(world, dev_tools_registry: Mut)|{ + for event in events { + // This gives us a mutable reference to the underlying resource as a `&mut dyn ModalDevTool` + let Some(dev_command_metadata) = dev_tools_registry.get_command_by_name(world, event.name) else { + warn!("No dev tool was found for {}). Did you forget to register it?", event.name); + continue; + }; + + // Create a concrete instance of our dev command from the supplied string. + let Ok(command) = dev_command_metadata.parse_from_str(&event.0) else { + warn!("Could not parse the command from the supplied string"); + continue; + } + + // Now we can run the command directly on the `World` using `Command::apply`! + command.apply(world); + } + }) +} +``` + +Finally, we want to be able to pass in a user supplied string, parse it into a dev command and then evaluate it on the world. + +```rust +#[derive(Event)] +struct DevCommandInput(String); + +fn parse_and_run_dev_commands(world: &mut World){ + // Move the events out of the world, clearing them and avoiding a persistent borrow + let events = world.resource_mut::>().drain(); + + // Use a resource scope to allow us to access both the dev tools registry and the rest of the world at the same time + world.resource_scope(|(world, dev_tools_registry: Mut)|{ + for event in events { + // This gives us a mutable reference to the underlying resource as a `&mut dyn ModalDevTool` + let Some(dev_tool) = dev_tools_registry.get_command_mut_by_name(world, event.name) else { + warn!("No dev command was found for {}). Did you forget to register it?", event.name); + continue; + }; + + // Since we know that this object always implements `ModalDevTool`, + // we can use any of the methods on it, or the traits that it requires (like `Reflect`) + dev_tool.toggle(); + } + }) + +} +``` + +While a number of other features could sensibly be added to this API (configuring modal dev tools, a `--help` flag, saving and loading config to disk), +this MVP should be sufficient to prove out the viability of the core architecture. ## Implementation strategy -This is the technical portion of the RFC. -Try to capture the broad implementation strategy, -and then focus in on the tricky details so that: +### What metadata do toolboxes need? + +We can start simple: + +- the type name of the dev tool +- any doc strings on the dev tool struct +- the value and types of any configuration that must be supplied -- Its interaction with other features is clear. -- It is reasonably clear how the feature would be implemented. -- Corner cases are dissected by example. +By relying on simple structs to configure both our modal dev tools and commands, we can extract this information automatically, via reflection. +Additional dev tool specific metadata (such as a classification scheme) can be added to the traits on an opt-in basis in the future. -When necessary, this section should return to the examples given in the previous section and explain the implementation details that make them work. +Optional methods on both `ModalDevTool` and `DevCommand` will allow us to override the supplied defaults if needed. -When writing this section be mindful of the following [repo guidelines](https://github.com/bevyengine/rfcs): +### What do the registries for our dev tools look like? -- **RFCs should be scoped:** Try to avoid creating RFCs for huge design spaces that span many features. Try to pick a specific feature slice and describe it in as much detail as possible. Feel free to create multiple RFCs if you need multiple features. -- **RFCs should avoid ambiguity:** Two developers implementing the same RFC should come up with nearly identical implementations. -- **RFCs should be "implementable":** Merged RFCs should only depend on features from other merged RFCs and existing Bevy features. It is ok to create multiple dependent RFCs, but they should either be merged at the same time or have a clear merge order that ensures the "implementable" rule is respected. +Keeping track of the registered dev tools without storing them all in a dedicated collection is quite challenging! +How can we keep track of it under the hood? -### What is a `DevToolsRegistry`? +```rust +#[derive(Resource)] +struct DevToolsRegistry { + /// The stored collection of modal dev tools, tracked in a type-erased way using [`ComponentId`] + /// + /// The key is the `name()` provided by the `ModalDevTool` trait. + modal_dev_tools: HashMap, + /// The metadata for all registered dev commands. + /// + /// The key is the `name()` provided by the `DevCommand` trait. + dev_commands: HashMap +} +``` -TODO +There are a few key operations that we will need to perform: + +```rust +impl DevToolsRegistry { + /// Gets a reference to the specified modal dev tool by `ComponentId`. + fn get_tool_by_id(world: &World, component_id: ComponentId) -> Option<&dyn ModalDevTool> { + let resource = world.get_resource_by_id(component_id)?; + resource.downcast_ref() + } + + /// Gets a mutable reference to the specified modal dev tool by `ComponentId`. + fn get_tool_mut_by_id(mut world: &mut World, component_id: ComponentId) -> Option<&mut dyn ModalDevTool> { + let resource = world.get_resource_mut_by_id(component_id)?; + resource.downcast_mut() + } + + /// Looks up the `ComponentId` associated with the given name, as supplied by the `ModalDevTool` trait. + fn lookup_tool_component_id(&self, name: &str) -> Option { + self.modal_dev_tools.get(name).copied() + } + + /// Gets a reference to the specified modal dev tool by type name, as supplied by the `ModalDevTool` trait. + fn get_tool_by_name(&self, world: &World, name: &str) -> Option<&dyn ModalDevTool> { + let component_id = self.lookup_tool_component_id(name)?; + DevToolsRegistry::get_by_id(world, component_id) + } + + /// Gets a mutable reference to the specified modal dev tool by name. + fn get_tool_mut_by_name(&self, mut world: &mut World, component_id: ComponentId) -> Option<&mut dyn ModalDevTool> { + let component_id = self.lookup_tool_component_id(name)?; + DevToolsRegistry::get_mut_by_id(world, component_id) + } + + /// Iterates over the list of registered modal dev tools. + fn iter_tools(&self, world: &World) -> impl Iterator { + self.modal_dev_tools.iter().map(|&id| (id, self.get(world, id))) + } + + /// Mutably iterates over the list of registered modal dev tools. + fn iter_tools_mut(&self, mut world: &mut World) -> impl Iterator { + self.modal_dev_tools.iter_mut().map(|&id| (id, self.get(world, id))) + } + + /// Gets the `DevCommandMetadata` for a given dev tool by name. + /// + /// The supplied name should match the `DevCommand::name()` method. + fn get_command(name: &str) -> Option<&DevCommandMetadata> { + self.dev_commands.get(name) + } + + /// Iterates over the list registered dev commands, returning their name and `DevCommandMetadata`. + fn iter_commands(&self) -> impl Iterator { + self.dev_commands.iter() + } +} +``` + +Once the user has a `dyn ModalDevTool`, they can perform whatever operations they'd like: enabling and disabling it, reading metadata and so on. +There's no need to add further duplicate API: users building toolboxes are generally sophisticated, and the number of calls should be quite small. + +While we can use the [dynamic resource APIs](https://docs.rs/bevy/latest/bevy/ecs/world/struct.World.html#method.get_resource_by_id) and trait casting to get references to the list of modal dev tools, we cannot do the same for the dev commands! +While this may seem unintuitive, the reason for this is fairly simple: modal configuration is always present, while the configuration for each dev command is generated when it is sent. +As a result, we can only access its metadata: the arguments it takes, its doc strings and so on. + +```rust +#[derive(Resource)] +struct DevCommandsRegistry { + /// The set of modal dev tools, tracked in a type-erased way using [`ComponentId`] + modal_dev_tools: HashMap, +} +``` ## Drawbacks 1. Third-party and end user dev tools will be pushed to conform to this standard. Without the use of a toolbox, this is added work for no benefit. 2. This abstraction may not fit all possible tools and toolboxes. The manual wiring approach is more flexible, and so if our abstraction is overly prescriptive, it may not work correctly. +3. The parse-from-string methods are currently quite heavy on boilerplate. A `clap`-inspired derive macro would be lovely here. ## Rationale and alternatives @@ -300,25 +562,48 @@ TODO Immediate dev tools commonly require input from the developer to take effect: what level to warp to, what item to spawn, what to set the player's gold to. This pattern is very straightforward with commands, and quite complex to do with one-shot systems, requiring an associated type for the input. -## \[Optional\] Prior art +### Why should we store the configuration in resources? + +As [Cart pointed out](https://github.com/bevyengine/bevy/pull/12427#issuecomment-2005328288), using a dedicated collection to store information about the state of various dev tools has several drawbacks: + +- unidiomatic: these are currently configured via stand-alone resources, and changing that would require either synchronization or the imposition of this pattern on every tool that could be used as a dev tool +- breaks the granularity of change detection: we can't determine *which* dev tool's config has changed +- accessing information about the current config of a dev tool requires an added layer of indirection, and will simply fail if the dev tool was not registered, even in cases where no toolbox is being used + +While this makes our internals more complex, that's a trade-off we're willing to accept. + +### Why not use trait queries? + +If we need to access a set of objects, all of which implement the same ttrait, why not simple use [trait queries](https://github.com/bevyengine/rfcs/pull/39). + +There are two arguments against this: + +1. While [`bevy_trait_query`] exists, it is currently third-party, and upstreaming it would be a major initiative. +2. This config is idiomatically stored in resources, not singleton entities. +3. This still doesn't solve the problem of how to track the set of registered dev commands. -Discuss prior art, both the good and the bad, in relation to this proposal. -This can include: +### Why not add a `Display` bound to `ModalDevTool` and `DevCommand`? -- Does this feature exist in other libraries and what experiences have their community had? -- Papers: Are there any published papers or great posts that discuss this? +We want to be able to directly display their value to users in a generic way: why not rely on the `Display` trait? -This section is intended to encourage you as an author to think about the lessons from other tools and provide readers of your RFC with a fuller picture. +There are three arguments against this: -Note that while precedent set by other engines is some motivation, it does not on its own motivate an RFC. +1. Implementing `Display` is relatively tedious due to string formatting. +2. We can get good enough results to print via `Reflect` and `Debug`. +3. Toolkits will typically rely on their own custom UI for formatting this information, and need more structured data than a simple string. ## Unresolved questions -- What parts of the design do you expect to resolve through the RFC process before this gets merged? -- What parts of the design do you expect to resolve through the implementation of this feature before the feature PR is merged? -- What related issues do you consider out of scope for this RFC that could be addressed in the future independently of the solution that comes out of this RFC? +1. Are the provided methods (along with those on `Reflect`) sufficient to allow toolkits to populate and run a wide range of dev tools without requiring an additional trait and manual registration? +2. Is the use of `downcast_ref` / `downcast_mut` correct in `DevToolsRegistry::get`? +3. Are `ModalDevTool` and `DevCommand` the best names? +4. How, precisely, can we achieve the sort of API shown in [Building toolboxes] using reflection? + 1. A prototype would be great here. +5. Should we replace our custom `parse_from_str` methods with a dependency on `clap`? +6. Does the function pointer approach used by `DevCommandMetadata` compile and work successfully? + 1. Again, prototype please! -## \[Optional\] Future possibilities +## Future possibilities Related but out-of-scope questions: @@ -332,4 +617,9 @@ Related but out-of-scope questions: Future possibilities: -1. Over time, we can extend the `ModalDevTool` trait with optional methods like `set_font`, enabling gradual unification of more complex shared configuration. +1. Over time, we can extend the `ModalDevTool` and `DevCommand` traits with optional methods, enabling: + 1. Gradual unification of more complex shared configuration (e.g. a `set_font` method). + 2. Opt-in metadata about the type of tool they are (e.g. an overlay, a camera controller, a 2D vs. 3D tool), allowing toolkits to seamlessly organize the options. + 3. Parsing from a `.bsn` file. +2. Add support for a trait-queries-on-resources operation, and stop storing modal dev tools in the `DevToolsRegistry`. +3. A derive macro for `DevCommand`, that aligns with `clap`'s calling conventions. From abbaaaa1c38bd7c69ae4b96b38cc821e64d94e8f Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 18:10:42 -0400 Subject: [PATCH 07/50] Remove initial notes --- rfcs/77-dev-tools-abstraction.md | 58 -------------------------------- 1 file changed, 58 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index d8189032..251de62d 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -1,63 +1,5 @@ # Feature Name: `dev-tools-abstraction` -Key questions: - -- what are the defining features of a dev tool? - - eases Bevy development experience, typically be presenting information about the app or its function - - not intended for end-user use - - must be able to be disabled at compile time - - contextually useful based on encountered weirdness in the app - - must be able to be toggled on and off at runtime without a reboot - - should not interfere with normal operation of the app - - can be customized to meet the needs of both the app and the developer -- what are clear examples of dev tools that we might want? - - system graph visualizer - - bevy_inspector_egui - - fly camera - - UI node visualizer - - FPS display - - resetting a level - - toggling god mode -- who creates dev tools? - - Bevy itself: FPS display - - third-party crates: bevy_rapier collider overlay - - end users: toggle god mode -- who creates dev tool consumers? - - Bevy itself: scene editor - - third-party crates: dev console - - end users: custom level editors -- how might dev tools be toggled and consumed? - - Quake-style dev console - - unified set of hot keys - - menu bar from scene editor -- how might dev tools be displayed? - - screen overlay - - pop-out window - - embedded panel widget - - cli -- how might dev tools be configured? - - color palettes - - fonts - - font size - - screen location - - dev tool specific features: camera speed, entities to ignore etc -- why do dev tools need to be aware of each other? - - overlays can clash: toggle off current overlay before enabling next -- how can we make it easy for third-party consumers and producers to play nice together? - - Cart's suggestion: let each consumer decide which tools it wants to use - - creates a quadratic problem: must wire up plumbing separately for each tool and consumer - - define a standard API for available dev tools and essential operations - - existing tools will have to conform to this in some way - -Out of scope: - -- [where should dev tools live](https://github.com/bevyengine/bevy/pull/12354)? - - where the primitives used are defined? e.g. bevy_gizmos - - where they're used for debugging? e.g. bevy_sprite - - in bevy_dev_tools? - - in dedicated crates, saving bevy_dev_tools for higher level abstractions? -- should dev tools be embedded in each application or should testing be done through an editor which controls these dev tools? - ## Summary One paragraph explanation of the feature. From 9e4dd40b3888ed5cced4ecafc71dce3d24349e4c Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 18:18:29 -0400 Subject: [PATCH 08/50] Write summary --- rfcs/77-dev-tools-abstraction.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 251de62d..b738986e 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -2,7 +2,12 @@ ## Summary -One paragraph explanation of the feature. +Dev tools are an essential but poorly-defined part of the game development experience. +Designed to allow developers (but generally not end users) to quickly inspect and manipulate the game world, +they accelerate debugging and testing by allowing common operations to be done at runtime, without having to write and then delete code. +To support a robust ecosystem of dev tools, +`bevy_dev_tools` comes with two core traits: `ModalDevTool` (for tools that can be toggled) and `DevCommand` (for operations with an immediate effect on the game world). +These are registered in the `DevToolsRegistry` via methods on `App`, allowing tool boxes (such as a dev console) to easily identify the available options and interact with them without the need for ad-hoc user glue code. ## Motivation From 9faa03f763d965d01793ff0d94c13d89afdb74dd Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 19:57:09 -0400 Subject: [PATCH 09/50] Fix mistakes Co-authored-by: Pablo Reinhardt <126117294+pablo-lua@users.noreply.github.com> --- rfcs/77-dev-tools-abstraction.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index b738986e..d8b2c53c 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -123,12 +123,12 @@ trait ModalDevTool: Resource + Reflect + FromReflect + Debug { /// Enables this dev tool. fn enable(&mut self) { - self.toggle(true); + self.set_enabled(true); } /// Disables this dev tool. fn disable(&mut self) { - self.toggle(false); + self.set_enabled(false); } /// Enables this dev tool if it's disabled, or disables it if it's enabled. @@ -521,7 +521,7 @@ While this makes our internals more complex, that's a trade-off we're willing to ### Why not use trait queries? -If we need to access a set of objects, all of which implement the same ttrait, why not simple use [trait queries](https://github.com/bevyengine/rfcs/pull/39). +If we need to access a set of objects, all of which implement the same trait, why not simple use [trait queries](https://github.com/bevyengine/rfcs/pull/39). There are two arguments against this: From 4c43e222ae77854c5d387e2368ecb3a7658cc7c6 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 20:02:17 -0400 Subject: [PATCH 10/50] Use std::FromStr per Bushrat's suggestion --- rfcs/77-dev-tools-abstraction.md | 52 ++++++++++++-------------------- 1 file changed, 19 insertions(+), 33 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index d8b2c53c..4eeb0781 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -103,18 +103,12 @@ In order to facilitate the creation of toolboxes, Bevy provides the `ModalDevToo /// and they can be enabled, disabled and reconfigured at runtime. /// /// The documentation on this struct is reflected, and can be read by toolboxes to provide help text to users. -trait ModalDevTool: Resource + Reflect + FromReflect + Debug { +trait ModalDevTool: Resource + Reflect + FromReflect + FromStr + Debug { /// The name of this tool, as might be supplied by a command line interface. fn name() -> &'static str { Self::type_name().to_snake_case() } - /// Attempts to create an object of type `Self` from a provided string, - /// as might be supplied by a command-line style interface. - fn parse_from_str(string: &str) -> Result { - Err(DevToolParseError::NotImplemented) - } - /// Turns this dev tool on (true) or off (false). fn set_enabled(&mut self, enabled: bool); @@ -171,10 +165,18 @@ impl Default for DevFlyCamera { } impl ModalDevTool for DevFlyCamera { - /// Expects a call that looks like `dev_fly_camera` (default settings) - /// or `dev_fly_camera 5 10` (to specify settings) - fn parse_from_str(string: &str) -> Result{ - let parts = string.split_whitespace(); + fn set_enabled(&mut self, enabled: bool) { + self.enabled = enabled; + } + + fn is_enabled(&self) -> bool { + self.enabled + } +} + +impl FromStr for DevFlyCamera { + fn from_str(s: &str) -> Result{ + let parts = s.split_whitespace(); let name = parts.iter.next()?; if name != self.name() { return Err(DevToolParseError::InvalidName); @@ -197,14 +199,6 @@ impl ModalDevTool for DevFlyCamera { } ) } - - fn set_enabled(&mut self, enabled: bool) { - self.enabled = enabled; - } - - fn is_enabled(&self) -> bool { - self.enabled - } } ``` @@ -222,7 +216,7 @@ To model this, we leverage Bevy's existing `Command` trait, which exists to perf /// to construct an instance of the type that implements this type, and then send it as a `Command` to execute it. /// /// The documentation on this struct is reflected, and can be read by toolboxes to provide help text to users. -trait DevCommand: Command + Reflect + FromReflect + Debug + 'static { +trait DevCommand: Command + Reflect + FromReflect + FromStr + Debug + 'static { /// The name of this tool, as might be supplied by a command line interface. fn name() -> &'static str { Self::type_name().to_snake_case() @@ -235,15 +229,9 @@ trait DevCommand: Command + Reflect + FromReflect + Debug + 'static { type_id: Self::type_id(), type_info: Self::type_info(), // A function pointer, based on the parse_from_str method - parse_from_str_fn: Self::parse_from_str + parse_from_str_fn: ::from_str } } - - /// Attempts to create an object of type `Self` from a provided string, - /// as might be supplied by a command-line style interface. - fn parse_from_str(string: &str) -> Result { - Err(DevToolParseError::NotImplemented) - } } ``` @@ -263,13 +251,11 @@ impl Command for SetGold { } } -impl DevCommand for SetGold { - /// We're fine using Bevy's built-in error type for this. - type StringParseError = DevToolParseError; +impl DevCommand for SetGold {} - /// Expects a call that looks like `set_gold 500` - fn parse_from_str(string: &str) -> Result{ - let parts = string.split_whitespace(); +impl FromStr for SetGold { + fn from_str(s: &str) -> Result{ + let parts = s.split_whitespace(); let name = parts.iter.next()?; if name != self.name() { return Err(DevToolParseError::InvalidName); From 6e2c7839d6f459b1b8ef051ab5c61fd2de57ac1a Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 20:05:37 -0400 Subject: [PATCH 11/50] Clean up parse_from_str calls --- rfcs/77-dev-tools-abstraction.md | 44 +++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 4eeb0781..2535388b 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -228,8 +228,8 @@ trait DevCommand: Command + Reflect + FromReflect + FromStr::from_str + // A function pointer, based on the std::str::from_str method + from_str_fn: ::from_str } } } @@ -333,7 +333,40 @@ fn toggle_dev_tools(world: &mut World){ }; // Create a concrete instance of our dev command from the supplied string. - let Ok(command) = dev_command_metadata.parse_from_str(&event.0) else { + let Ok(command) = dev_command_metadata.from_str(&event.0) else { + warn!("Could not parse the command from the supplied string"); + continue; + } + + // Now we can run the command directly on the `World` using `Command::apply`! + command.apply(world); + } + }) +} +``` + +Next, we want to be able to configure modal dev tools at run time. + +```rust + +#[derive(Event)] +struct ConfigureDevTool(String); + +fn toggle_dev_tools(world: &mut World){ + // Move the events out of the world, clearing them and avoiding a persistent borrow + let events = world.resource_mut::>().drain(); + + // Use a resource scope to allow us to access both the dev tools registry and the rest of the world at the same time + world.resource_scope(|(world, dev_tools_registry: Mut)|{ + for event in events { + // This gives us a mutable reference to the underlying resource as a `&mut dyn ModalDevTool` + let Some(dev_command_metadata) = dev_tools_registry.get_command_by_name(world, event.name) else { + warn!("No dev tool was found for {}). Did you forget to register it?", event.name); + continue; + }; + + // Create a concrete instance of our dev command from the supplied string. + let Ok(command) = dev_command_metadata.from_str(&event.0) else { warn!("Could not parse the command from the supplied string"); continue; } @@ -373,7 +406,7 @@ fn parse_and_run_dev_commands(world: &mut World){ } ``` -While a number of other features could sensibly be added to this API (configuring modal dev tools, a `--help` flag, saving and loading config to disk), +While a number of other features could sensibly be added to this API (a `--help` flag, saving and loading config to disk, managing compatibility between dev tools), this MVP should be sufficient to prove out the viability of the core architecture. ## Implementation strategy @@ -532,8 +565,7 @@ There are three arguments against this: 3. Are `ModalDevTool` and `DevCommand` the best names? 4. How, precisely, can we achieve the sort of API shown in [Building toolboxes] using reflection? 1. A prototype would be great here. -5. Should we replace our custom `parse_from_str` methods with a dependency on `clap`? -6. Does the function pointer approach used by `DevCommandMetadata` compile and work successfully? +5. Does the function pointer approach used by `DevCommandMetadata` compile and work successfully? 1. Again, prototype please! ## Future possibilities From 95df4f25bddd14f3bc06060717ba750b95f3351a Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 20:23:43 -0400 Subject: [PATCH 12/50] Extend and fix toolkit examples --- rfcs/77-dev-tools-abstraction.md | 77 ++++++++++++++++---------------- 1 file changed, 39 insertions(+), 38 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 2535388b..17e4af37 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -327,55 +327,43 @@ fn toggle_dev_tools(world: &mut World){ world.resource_scope(|(world, dev_tools_registry: Mut)|{ for event in events { // This gives us a mutable reference to the underlying resource as a `&mut dyn ModalDevTool` - let Some(dev_command_metadata) = dev_tools_registry.get_command_by_name(world, event.name) else { + let Some(dev_tool) = dev_tools_registry.get_tool_by_name(world, event.name) else { warn!("No dev tool was found for {}). Did you forget to register it?", event.name); continue; }; - // Create a concrete instance of our dev command from the supplied string. - let Ok(command) = dev_command_metadata.from_str(&event.0) else { - warn!("Could not parse the command from the supplied string"); - continue; - } - - // Now we can run the command directly on the `World` using `Command::apply`! - command.apply(world); + // Since we know that this object always implements `ModalDevTool`, + // we can use any of the methods on it, or the traits that it requires (like `Reflect`) + dev_tool.toggle(); } }) } + + ``` Next, we want to be able to configure modal dev tools at run time. ```rust - #[derive(Event)] -struct ConfigureDevTool(String); +struct ConfigureDevTool{ + tool_string: String, +}; -fn toggle_dev_tools(world: &mut World){ +fn configure_dev_tools(world: &mut World){ // Move the events out of the world, clearing them and avoiding a persistent borrow let events = world.resource_mut::>().drain(); // Use a resource scope to allow us to access both the dev tools registry and the rest of the world at the same time world.resource_scope(|(world, dev_tools_registry: Mut)|{ for event in events { - // This gives us a mutable reference to the underlying resource as a `&mut dyn ModalDevTool` - let Some(dev_command_metadata) = dev_tools_registry.get_command_by_name(world, event.name) else { - warn!("No dev tool was found for {}). Did you forget to register it?", event.name); - continue; - }; - - // Create a concrete instance of our dev command from the supplied string. - let Ok(command) = dev_command_metadata.from_str(&event.0) else { - warn!("Could not parse the command from the supplied string"); - continue; + // Check the implementation details for information about how this works! + let result = dev_tools_registry.parse_and_insert_tool(event.tool_string); + if let Err(error) = result { + warn!(error); } - - // Now we can run the command directly on the `World` using `Command::apply`! - command.apply(world); } - }) -} + })} ``` Finally, we want to be able to pass in a user supplied string, parse it into a dev command and then evaluate it on the world. @@ -391,18 +379,22 @@ fn parse_and_run_dev_commands(world: &mut World){ // Use a resource scope to allow us to access both the dev tools registry and the rest of the world at the same time world.resource_scope(|(world, dev_tools_registry: Mut)|{ for event in events { - // This gives us a mutable reference to the underlying resource as a `&mut dyn ModalDevTool` - let Some(dev_tool) = dev_tools_registry.get_command_mut_by_name(world, event.name) else { - warn!("No dev command was found for {}). Did you forget to register it?", event.name); + // This gives us access to the metadata needed to inspect the dev command and construct a new one. + let Some(dev_command_metadata) = dev_tools_registry.get_command_by_name(world, event.name) else { + warn!("No dev tool was found for {}). Did you forget to register it?", event.name); continue; }; - // Since we know that this object always implements `ModalDevTool`, - // we can use any of the methods on it, or the traits that it requires (like `Reflect`) - dev_tool.toggle(); + // Create a concrete instance of our dev command from the supplied string. + let Ok(command) = dev_command_metadata.from_str(&event.0) else { + warn!("Could not parse the command from the supplied string"); + continue; + } + + // Now we can run the command directly on the `World` using `Command::apply`! + command.apply(world); } }) - } ``` @@ -507,12 +499,21 @@ While we can use the [dynamic resource APIs](https://docs.rs/bevy/latest/bevy/ec While this may seem unintuitive, the reason for this is fairly simple: modal configuration is always present, while the configuration for each dev command is generated when it is sent. As a result, we can only access its metadata: the arguments it takes, its doc strings and so on. +### Parsing and inserting modal dev tools + +One method on `DevToolsRegistry` is worth special attention: `parse_and_insert_tool`. +This tool takes a common pattern, parsing the configuration for a dev tool from a provided string, and provides a user friendly, safe API over it. + ```rust -#[derive(Resource)] -struct DevCommandsRegistry { - /// The set of modal dev tools, tracked in a type-erased way using [`ComponentId`] - modal_dev_tools: HashMap, +impl DevToolsRegistry { + /// Parses the given string into a modal dev tool corresponding to its name if possible. + /// + /// For a typed equivalent, simply use the `FromStr` trait that this method relies on directly. + fn parse_and_insert_tool(s: &str) -> Result<(), DevToolParseError>{ + todo!(); + } } + ``` ## Drawbacks From 5424491f514f6456c77c26610d8519b0924b9956 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 20:40:06 -0400 Subject: [PATCH 13/50] Flesh out parse_and_insert_tool --- rfcs/77-dev-tools-abstraction.md | 53 +++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 17e4af37..29a64ffd 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -380,7 +380,7 @@ fn parse_and_run_dev_commands(world: &mut World){ world.resource_scope(|(world, dev_tools_registry: Mut)|{ for event in events { // This gives us access to the metadata needed to inspect the dev command and construct a new one. - let Some(dev_command_metadata) = dev_tools_registry.get_command_by_name(world, event.name) else { + let Some(dev_command_metadata) = dev_tools_registry.get_command_metadata_by_name(world, event.name) else { warn!("No dev tool was found for {}). Did you forget to register it?", event.name); continue; }; @@ -427,7 +427,7 @@ struct DevToolsRegistry { /// The stored collection of modal dev tools, tracked in a type-erased way using [`ComponentId`] /// /// The key is the `name()` provided by the `ModalDevTool` trait. - modal_dev_tools: HashMap, + modal_dev_tools: HashMap, /// The metadata for all registered dev commands. /// /// The key is the `name()` provided by the `DevCommand` trait. @@ -451,9 +451,23 @@ impl DevToolsRegistry { resource.downcast_mut() } + /// Gets the `DevCommandMetadata` for a given dev tool by name. + /// + /// The supplied name should match the `DevCommand::name()` method. + fn get_command_metadata(name: &str) -> Option<&DevCommandMetadata> { + self.dev_commands.get(name) + } + + /// Gets the `DevToolMetadata` for a given dev tool by name. + /// + /// The supplied name should match the `DevCommand::name()` method. + fn get_tool_metadata(name: &str) -> Option<&DevCommandMetadata> { + self.modal_dev_tools.get(name) + } + /// Looks up the `ComponentId` associated with the given name, as supplied by the `ModalDevTool` trait. fn lookup_tool_component_id(&self, name: &str) -> Option { - self.modal_dev_tools.get(name).copied() + *self.get_tool_metadata(name)?.component_id } /// Gets a reference to the specified modal dev tool by type name, as supplied by the `ModalDevTool` trait. @@ -478,13 +492,6 @@ impl DevToolsRegistry { self.modal_dev_tools.iter_mut().map(|&id| (id, self.get(world, id))) } - /// Gets the `DevCommandMetadata` for a given dev tool by name. - /// - /// The supplied name should match the `DevCommand::name()` method. - fn get_command(name: &str) -> Option<&DevCommandMetadata> { - self.dev_commands.get(name) - } - /// Iterates over the list registered dev commands, returning their name and `DevCommandMetadata`. fn iter_commands(&self) -> impl Iterator { self.dev_commands.iter() @@ -506,14 +513,32 @@ This tool takes a common pattern, parsing the configuration for a dev tool from ```rust impl DevToolsRegistry { - /// Parses the given string into a modal dev tool corresponding to its name if possible. + /// Parses the given str `s` into a modal dev tool corresponding to its name if possible. /// /// For a typed equivalent, simply use the `FromStr` trait that this method relies on directly. - fn parse_and_insert_tool(s: &str) -> Result<(), DevToolParseError>{ - todo!(); + fn parse_and_insert_tool(&self, world: &mut World, s: &str) -> Result<(), DevToolParseError>{ + // Parse out the name + let name = s.clone().split_whitespace.next()?; + + // Get the associated `ComponentId`, so we can use it to insert a resource of a dynamic type + let component_id = self.lookup_tool_component_id(name); + // Look-up the existing resource to get access to get access to the metadata we need + let tool_metadata: &dyn DevTool = self.get_tool_metadata(component_d)?; + // Parse the string into a new copy of the tool using the stored function pointer + let new_tool = tool_metadata.from_str(s); + // Construct an `OwningPointer` so we can dynamically insert the resource we just made + // TODO: what is the second argument of this method supposed to be? + let owning_pointer = OwningPointer::make(new_tool, todo!()); + + // SAFETY: the value referenced by value is valid for the given `ComponentId` of this world, + // as the component id is cached upon initialization of the resource / dev tool. + unsafe { + world.insert_resource_by_id(component_id, owning_pointer); + } + + Ok(()) } } - ``` ## Drawbacks From dabee2c4b62e714a50e159677a7927d097483128 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 20:43:25 -0400 Subject: [PATCH 14/50] Fix usage of OwningPointer thanks to james-o-brien --- rfcs/77-dev-tools-abstraction.md | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 29a64ffd..88e2883a 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -527,14 +527,13 @@ impl DevToolsRegistry { // Parse the string into a new copy of the tool using the stored function pointer let new_tool = tool_metadata.from_str(s); // Construct an `OwningPointer` so we can dynamically insert the resource we just made - // TODO: what is the second argument of this method supposed to be? - let owning_pointer = OwningPointer::make(new_tool, todo!()); - - // SAFETY: the value referenced by value is valid for the given `ComponentId` of this world, - // as the component id is cached upon initialization of the resource / dev tool. - unsafe { - world.insert_resource_by_id(component_id, owning_pointer); - } + OwningPointer::make(new_tool, |owning_pointer|{ + // SAFETY: the value referenced by value is valid for the given `ComponentId` of this world, + // as the component id is cached upon initialization of the resource / dev tool. + unsafe { + world.insert_resource_by_id(component_id, owning_pointer); + } + }); Ok(()) } From 3ac08045c7b1b6a95e12f0947d21dce6fa01e8bf Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 20:54:15 -0400 Subject: [PATCH 15/50] Try to flesh out the metadata fields further --- rfcs/77-dev-tools-abstraction.md | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 88e2883a..81211de9 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -416,6 +416,24 @@ Additional dev tool specific metadata (such as a classification scheme) can be a Optional methods on both `ModalDevTool` and `DevCommand` will allow us to override the supplied defaults if needed. +We also need access to one other critical piece of information: a function pointer that allows us to construct a new value of this type from a string. + +As a result our `DevToolMetadata` looks like: + +```rust +struct DevToolMetadata { + name: String, + type_info: TypeInfo, + from_str: StringConstructorFn, +} +``` + +The tricky bit comes when we get to `StringConstructorFn`: we need to be able to store a function pointer that will take us from a string, and return an object that implements our core traits. + +TODO: how can this be done? + +The definition for `DevCommandMetadata` is effectively identical to start, but over time we should expect these to diverge: splitting them from the beginning will ease migrations going forward. + ### What do the registries for our dev tools look like? Keeping track of the registered dev tools without storing them all in a dedicated collection is quite challenging! @@ -544,7 +562,8 @@ impl DevToolsRegistry { 1. Third-party and end user dev tools will be pushed to conform to this standard. Without the use of a toolbox, this is added work for no benefit. 2. This abstraction may not fit all possible tools and toolboxes. The manual wiring approach is more flexible, and so if our abstraction is overly prescriptive, it may not work correctly. -3. The parse-from-string methods are currently quite heavy on boilerplate. A `clap`-inspired derive macro would be lovely here. +3. The `from_str` methods are currently quite heavy on boilerplate. A `clap`-inspired derive macro would be lovely here. Perhaps a crate already exists? +4. How can we store type-erased `StringConstructorFn`s in our metadata? ## Rationale and alternatives From 472cc76c3e1cb5ef824cba509d228d7fb77b48e9 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 20:54:58 -0400 Subject: [PATCH 16/50] Fix associated type bounds Co-authored-by: Pablo Reinhardt <126117294+pablo-lua@users.noreply.github.com> --- rfcs/77-dev-tools-abstraction.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 81211de9..739dcccc 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -103,7 +103,7 @@ In order to facilitate the creation of toolboxes, Bevy provides the `ModalDevToo /// and they can be enabled, disabled and reconfigured at runtime. /// /// The documentation on this struct is reflected, and can be read by toolboxes to provide help text to users. -trait ModalDevTool: Resource + Reflect + FromReflect + FromStr + Debug { +trait ModalDevTool: Resource + Reflect + FromReflect + FromStr + Debug { /// The name of this tool, as might be supplied by a command line interface. fn name() -> &'static str { Self::type_name().to_snake_case() @@ -216,7 +216,7 @@ To model this, we leverage Bevy's existing `Command` trait, which exists to perf /// to construct an instance of the type that implements this type, and then send it as a `Command` to execute it. /// /// The documentation on this struct is reflected, and can be read by toolboxes to provide help text to users. -trait DevCommand: Command + Reflect + FromReflect + FromStr + Debug + 'static { +trait DevCommand: Command + Reflect + FromReflect + FromStr + Debug + 'static { /// The name of this tool, as might be supplied by a command line interface. fn name() -> &'static str { Self::type_name().to_snake_case() From 45506d2e923e6ce261790e2f1c69476b6c158c87 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 21:54:22 -0400 Subject: [PATCH 17/50] Discuss adding layout information to traits --- rfcs/77-dev-tools-abstraction.md | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 739dcccc..ca59340e 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -602,6 +602,16 @@ There are three arguments against this: 2. We can get good enough results to print via `Reflect` and `Debug`. 3. Toolkits will typically rely on their own custom UI for formatting this information, and need more structured data than a simple string. +### Why not add information about layout to our traits? + +This sort of information is really helpful for toolboxes like editors to ensure that dev tools don't overlap each other: allowing them to be laid out beside each other or enabled one at a time when necessary. + +However: + +1. This RFC is complex enough: keeping its scope small will make it easier to review and implement. +2. Unlike with a CLI-style interface the correct abstractions aren't immediately obvious: we should prototype more toolkits and tools to explore the needs of the space further. +3. Not all dev tools need layout information: we would need to use optional methods and/or subtraits to fill this need, adding further complexity. + ## Unresolved questions 1. Are the provided methods (along with those on `Reflect`) sufficient to allow toolkits to populate and run a wide range of dev tools without requiring an additional trait and manual registration? @@ -626,9 +636,11 @@ Related but out-of-scope questions: Future possibilities: -1. Over time, we can extend the `ModalDevTool` and `DevCommand` traits with optional methods, enabling: +1. Over time, we can extend the `ModalDevTool` and `DevCommand` traits with more methods (mandatory and optional), enabling: 1. Gradual unification of more complex shared configuration (e.g. a `set_font` method). - 2. Opt-in metadata about the type of tool they are (e.g. an overlay, a camera controller, a 2D vs. 3D tool), allowing toolkits to seamlessly organize the options. - 3. Parsing from a `.bsn` file. + 2. Serializing and deserializing structs from a `.bsn` file, rather than simple CLI-style strings, allowing toolboxes to build abstractions for persistent config. 2. Add support for a trait-queries-on-resources operation, and stop storing modal dev tools in the `DevToolsRegistry`. 3. A derive macro for `DevCommand`, that aligns with `clap`'s calling conventions. +4. Add specialized types of dev tools, likely each with their own trait to establish conventions and resolve conflicts between them. + 1. For example, we may want to coordinate between various UI elements or overlays to avoid positioning conflicts, dev tools that change the materials of objects, or camera controllers. + 2. This could be queried via Opt-in metadata about the type of tool they are (e.g. an overlay, a camera controller, a 2D vs. 3D tool), allowing toolkits to seamlessly organize the options. From a7d8059cc9f9b894c487ef089d51a2267271129c Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 22:00:10 -0400 Subject: [PATCH 18/50] Add the command palette idea to the list of possible toolboxes --- rfcs/77-dev-tools-abstraction.md | 1 + 1 file changed, 1 insertion(+) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index ca59340e..caa6f880 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -79,6 +79,7 @@ In order to manage the complexity of an eclectic collection of one-off debugging This might be: - an in-game dev console (like in Quake) +- a command palette in an editor - special developer commands entered into the chat box, commonly with a `/` to denote their non-textual effect - cheat codes - a unified set of hotkeys From 931ea9b97565003a30370c7973920740466725ce Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 22:04:05 -0400 Subject: [PATCH 19/50] Fix typo Co-authored-by: Pablo Reinhardt <126117294+pablo-lua@users.noreply.github.com> --- rfcs/77-dev-tools-abstraction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index caa6f880..3cb0bb7f 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -542,7 +542,7 @@ impl DevToolsRegistry { // Get the associated `ComponentId`, so we can use it to insert a resource of a dynamic type let component_id = self.lookup_tool_component_id(name); // Look-up the existing resource to get access to get access to the metadata we need - let tool_metadata: &dyn DevTool = self.get_tool_metadata(component_d)?; + let tool_metadata: &dyn DevTool = self.get_tool_metadata(component_id)?; // Parse the string into a new copy of the tool using the stored function pointer let new_tool = tool_metadata.from_str(s); // Construct an `OwningPointer` so we can dynamically insert the resource we just made From b58fdffbad4e24d5b087e5a3f3aa2908415329e9 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 22:06:39 -0400 Subject: [PATCH 20/50] Make sure the `ModalDevTool` has access to `DevToolMetadata` --- rfcs/77-dev-tools-abstraction.md | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index caa6f880..0fabac8e 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -110,6 +110,17 @@ trait ModalDevTool: Resource + Reflect + FromReflect + FromStr DevToolMetaData { + DevToolMetaData { + name: self.name(), + type_id: Self::type_id(), + type_info: Self::type_info(), + // A function pointer, based on the std::str::from_str method + from_str_fn: ::from_str + } + } + /// Turns this dev tool on (true) or off (false). fn set_enabled(&mut self, enabled: bool); @@ -224,8 +235,8 @@ trait DevCommand: Command + Reflect + FromReflect + FromStr DevToolMetaData { - DevToolMetaData { + fn metadata() -> DevCommandMetadata { + DevCommandMetadata { name: self.name(), type_id: Self::type_id(), type_info: Self::type_info(), From 5c31490162b25179bf865a15ff1c710846425755 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 19 Mar 2024 22:07:18 -0400 Subject: [PATCH 21/50] Use consistent naming --- rfcs/77-dev-tools-abstraction.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 5f37d7dd..52f63a8f 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -283,11 +283,11 @@ impl FromStr for SetGold { ### Conventions for building dev tools -Not everything can or should be defined by a trait! To supplement the `DevTool` trait, we recommend that Bevy and its ecosystem follow the following conventions: +Not everything can or should be defined by a trait! To supplement the `ModalDevTool` trait, we recommend that Bevy and its ecosystem follow the following conventions: 1. Dev tools can be toggled at compile time. 2. Dev tools can be toggled at run time. -3. Dev tools implement the `DevTool` trait, and if the corresponding feature is enabled, are added to the app via `.init_dev_tool` or `.insert_dev_tool`. +3. Dev tools implement the `ModalDevTool` trait, and if the corresponding feature is enabled, are added to the app via `.init_dev_tool` or `.insert_dev_tool`. 1. This can be done either in the main plugin or a dedicated dev tools plugin. 1. If configuration is required, splitting this into a dedicated dev tools plugin is preferred. 4. Dev tools are disabled by default, both at compile and run-time. From 0b02a75fa26f6d73e9a81fbdc2ccf8097e2e65b1 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 20 Mar 2024 00:09:08 -0400 Subject: [PATCH 22/50] Fix typo Co-authored-by: Thierry Berger --- rfcs/77-dev-tools-abstraction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 52f63a8f..a5a0b7a1 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -53,7 +53,7 @@ For example: - a scene editor, pixel art tool, asset preprocessing solutions or audio workstation: these are used to create assets, and do not need to be accessible at runtime - gizmos: while these are commonly used for *making* dev tools, they don't inherently provide a way to inspect or manipulate the game world -As you can see, these tools are currently and will continue to be created by the creators of indidvidual games, third-party crates in Bevy's ecosystem, and Bevy itself. +As you can see, these tools are currently and will continue to be created by the creators of individual games, third-party crates in Bevy's ecosystem, and Bevy itself. Looking at the usage examples more closely, we can classify them into two patterns: **modal dev tools** and **dev commands**. - Modal dev tools (like an FPS meter): can be toggled off and on, and want persistent configuration. From fd8887977b5d8071f6795fcd169de780c58b133e Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 20 Mar 2024 00:09:19 -0400 Subject: [PATCH 23/50] Fix sentence fragment --- rfcs/77-dev-tools-abstraction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index a5a0b7a1..95f07a60 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -149,7 +149,7 @@ trait ModalDevTool: Resource + Reflect + FromReflect + FromStr()` (to use default config based on the `FromWorld` implementation) or via `app.insert_modal_dev_tool(config: D)`. -This adds them as a resource (just like ), but also registers their `ComponentId` with the central `DevToolsRegistry`, which can be consumed by toolboxes to get a list of the available dev tools. +This adds them as a resource (just like the familiar `.init_resource` or `insert_resource`), but also registers their `ComponentId` with the central `DevToolsRegistry`, which can be consumed by toolboxes to get a list of the available dev tools. To build your own modal dev tool, simply create a configuration resource, implement the `ModalDevTool` trait, and then register it in the app when the correct feature flag is enabled. From ea1bf8a4006879be506646f608479212bc057420 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 20 Mar 2024 00:10:09 -0400 Subject: [PATCH 24/50] Fix reference to end user when Bevy user was meant --- rfcs/77-dev-tools-abstraction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 95f07a60..0cb2abfd 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -15,7 +15,7 @@ These are registered in the `DevToolsRegistry` via methods on `App`, allowing to However, since its inception, it has been plagued by debate over ["what is a dev tool?"](https://github.com/bevyengine/bevy/issues/12358) and every **tool** within both Bevy and its ecosystem follows its own ad hoc conventions about [how it might be enabled and configured](https://github.com/bevyengine/bevy/issues/12368). -This is frustrating to navigate as an end user: every tool has its own way to configure it, with no rhyme, reason or way to set the same property to the same value across tools. +This is frustrating to navigate as a Bevy user: every tool has its own way to configure it, with no rhyme, reason or way to set the same property to the same value across tools. It also makes the creation of **toolboxes**, interfaces designed to collect multiple dev tools in a single place (such as a Quake-style dev console) needlessly painful, requiring manual glue work by either the toolbox creator or application developer for every new tool that's added to it. From 4093bab53db30fa06de219b710d282caa7ee5675 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 20 Mar 2024 00:11:25 -0400 Subject: [PATCH 25/50] Add link to discussion on layout extensions --- rfcs/77-dev-tools-abstraction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 0cb2abfd..fa5348d5 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -654,5 +654,5 @@ Future possibilities: 2. Add support for a trait-queries-on-resources operation, and stop storing modal dev tools in the `DevToolsRegistry`. 3. A derive macro for `DevCommand`, that aligns with `clap`'s calling conventions. 4. Add specialized types of dev tools, likely each with their own trait to establish conventions and resolve conflicts between them. - 1. For example, we may want to coordinate between various UI elements or overlays to avoid positioning conflicts, dev tools that change the materials of objects, or camera controllers. + 1. For example, we may want to coordinate between various UI elements or overlays to avoid positioning conflicts, dev tools that change the materials of objects, or camera controllers. See [Discussion #12589](https://github.com/bevyengine/bevy/discussions/12589) for some initial exploration. 2. This could be queried via Opt-in metadata about the type of tool they are (e.g. an overlay, a camera controller, a 2D vs. 3D tool), allowing toolkits to seamlessly organize the options. From 0ca692c695ca02c2bb91afb55abdb831d3a92401 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 21 Mar 2024 11:17:34 -0400 Subject: [PATCH 26/50] Compiler error Co-authored-by: rewin --- rfcs/77-dev-tools-abstraction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index fa5348d5..b4eda93f 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -113,7 +113,7 @@ trait ModalDevTool: Resource + Reflect + FromReflect + FromStr DevToolMetaData { DevToolMetaData { - name: self.name(), + name: Self::name(), type_id: Self::type_id(), type_info: Self::type_info(), // A function pointer, based on the std::str::from_str method From 5ada1b441c71f0cd15873d58c76b6e2319b052d1 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 21 Mar 2024 11:19:27 -0400 Subject: [PATCH 27/50] Add a `short_description` method Co-authored-by: rewin --- rfcs/77-dev-tools-abstraction.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index b4eda93f..f424d210 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -109,6 +109,8 @@ trait ModalDevTool: Resource + Reflect + FromReflect + FromStr &'static str { Self::type_name().to_snake_case() } + + fn short_description() -> Option<&'static str>; /// The metadata for this modal dev tool. fn metadata() -> DevToolMetaData { @@ -117,7 +119,8 @@ trait ModalDevTool: Resource + Reflect + FromReflect + FromStr::from_str + from_str_fn: ::from_str, + short_description: Self::short_description() } } From e318a65eedc5c7660527b313eaf2c53e99ddc600 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 21 Mar 2024 14:59:19 -0400 Subject: [PATCH 28/50] FromReflect yeet Co-authored-by: rewin --- rfcs/77-dev-tools-abstraction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index f424d210..597e63fb 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -160,7 +160,7 @@ To build your own modal dev tool, simply create a configuration resource, implem /// A flying camera controller that lets you disconnect your camera from the player to freely explore the environment. /// /// When this mode is disabled -#[derive(Resource, Reflect, FromReflect, Debug)] +#[derive(Resource, Reflect, Debug)] struct DevFlyCamera { enabled: bool, /// How fast the camera travels forwards, backwards, left, right, up and down, in world units. From 6f00808aeee5f928c2396cc1eb783f99b272d8de Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 21 Mar 2024 14:59:30 -0400 Subject: [PATCH 29/50] Command trait typo Co-authored-by: rewin --- rfcs/77-dev-tools-abstraction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 597e63fb..26411f62 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -260,7 +260,7 @@ struct SetGold { } impl Command for SetGold { - fn apply(&self, world: &mut World){ + fn apply(self, world: &mut World){ let mut current_gold = world.resource_mut::(); current_gold.0 = self.amount; } From 98588a8d6c9e9f257518b4af30435b6f7b752f1b Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 21 Mar 2024 15:00:19 -0400 Subject: [PATCH 30/50] Use the updated type name for `DevToolMetaData` Co-authored-by: Mateusz Wachowiak --- rfcs/77-dev-tools-abstraction.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 26411f62..48b934ba 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -460,7 +460,7 @@ struct DevToolsRegistry { /// The stored collection of modal dev tools, tracked in a type-erased way using [`ComponentId`] /// /// The key is the `name()` provided by the `ModalDevTool` trait. - modal_dev_tools: HashMap, + modal_dev_tools: HashMap, /// The metadata for all registered dev commands. /// /// The key is the `name()` provided by the `DevCommand` trait. @@ -494,7 +494,7 @@ impl DevToolsRegistry { /// Gets the `DevToolMetadata` for a given dev tool by name. /// /// The supplied name should match the `DevCommand::name()` method. - fn get_tool_metadata(name: &str) -> Option<&DevCommandMetadata> { + fn get_tool_metadata(name: &str) -> Option<&DevToolMetadata> { self.modal_dev_tools.get(name) } From d5617e444682258d19d2017142fe7cb184364990 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 21 Mar 2024 15:20:51 -0400 Subject: [PATCH 31/50] Explain why trait objects won't work :( --- rfcs/77-dev-tools-abstraction.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 48b934ba..1e4f5366 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -627,6 +627,15 @@ However: 2. Unlike with a CLI-style interface the correct abstractions aren't immediately obvious: we should prototype more toolkits and tools to explore the needs of the space further. 3. Not all dev tools need layout information: we would need to use optional methods and/or subtraits to fill this need, adding further complexity. +### Why can't we simply store boxed trait objects in our `DevToolsRegistry`? + +Fundamentally, there are two problems with this design: + +1. Many nice traits (like `Reflect`, `FromWorld` and `FromStr`) aren't object-safe, and we may want non-object safe methods on our traits. Forcing this restriction seriously compromises the functionality of this architecture. +2. While reasonable defaults always exist for modal dev tools, the same is not generally true for dev commands: if we want to have a dev command to despawn an entity, there's no way to store a copy of this at rest. Adding an `Option` field reduces type safety, while storing a placeholder entity is both a footgun and not a good general solution. + +Instead, we're forced to turn to the dark arts of reflection and type registration. + ## Unresolved questions 1. Are the provided methods (along with those on `Reflect`) sufficient to allow toolkits to populate and run a wide range of dev tools without requiring an additional trait and manual registration? From 69c603453ae320282eb2fe20747d2f5a90da1c25 Mon Sep 17 00:00:00 2001 From: rewin Date: Fri, 22 Mar 2024 01:50:06 +0300 Subject: [PATCH 32/50] Fix type info getting and extend DevCommand metadata --- rfcs/77-dev-tools-abstraction.md | 53 ++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 1e4f5366..eabdd232 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -104,22 +104,24 @@ In order to facilitate the creation of toolboxes, Bevy provides the `ModalDevToo /// and they can be enabled, disabled and reconfigured at runtime. /// /// The documentation on this struct is reflected, and can be read by toolboxes to provide help text to users. -trait ModalDevTool: Resource + Reflect + FromReflect + FromStr + Debug { +pub trait ModalDevTool: Resource + Reflect + FromReflect + GetTypeRegistration + FromStr + Debug { /// The name of this tool, as might be supplied by a command line interface. fn name() -> &'static str { - Self::type_name().to_snake_case() + Self::get_type_registration().type_info().type_path_table().short_path() } - fn short_description() -> Option<&'static str>; + fn short_description() -> Option<&'static str> { + None + } /// The metadata for this modal dev tool. fn metadata() -> DevToolMetaData { DevToolMetaData { name: Self::name(), - type_id: Self::type_id(), - type_info: Self::type_info(), + type_id: Self::get_type_registration().type_id(), + type_info: Self::get_type_registration().type_info(), // A function pointer, based on the std::str::from_str method - from_str_fn: ::from_str, + from_str_fn: |s| ::from_str(s).map(|x| Box::new(x) as Box), short_description: Self::short_description() } } @@ -149,6 +151,14 @@ trait ModalDevTool: Resource + Reflect + FromReflect + FromStr Result, DevToolParseError>, + pub short_description: Option<&'static str> +} ``` Modal dev tools are registered via `app.init_modal_dev_tool::()` (to use default config based on the `FromWorld` implementation) or via `app.insert_modal_dev_tool(config: D)`. @@ -231,23 +241,42 @@ To model this, we leverage Bevy's existing `Command` trait, which exists to perf /// to construct an instance of the type that implements this type, and then send it as a `Command` to execute it. /// /// The documentation on this struct is reflected, and can be read by toolboxes to provide help text to users. -trait DevCommand: Command + Reflect + FromReflect + FromStr + Debug + 'static { +pub trait DevCommand: bevy::ecs::world::Command + Reflect + FromReflect + GetTypeRegistration + Default + FromStr + Debug + 'static { /// The name of this tool, as might be supplied by a command line interface. fn name() -> &'static str { - Self::type_name().to_snake_case() + Self::get_type_registration().type_info().type_path_table().short_path() } + fn short_description() -> Option<&'static str>; + /// The metadata for this dev command. fn metadata() -> DevCommandMetadata { DevCommandMetadata { - name: self.name(), - type_id: Self::type_id(), - type_info: Self::type_info(), + name: Self::name(), + type_id: Self::get_type_registration().type_id(), + type_info: Self::get_type_registration().type_info(), // A function pointer, based on the std::str::from_str method - from_str_fn: ::from_str + from_str_fn: |s| ::from_str(s).map(|x| Box::new(x) as Box), + // + create_default_fn: || Box::new(Self::default()), + // A function pointer that adds the DevCommand to the provided Commands + // This is needed because we can'n add Box to Commands withh commmands.add method + // So we need to do it in typed way + add_self_to_commands_fn: |commands, reflected_self| commands.add(::from_reflect(reflected_self).unwrap()), + short_description: Self::short_description() } } } + +pub struct DevCommandMetadata { + pub name: &'static str, + pub type_id: TypeId, + pub type_info: &'static TypeInfo, + pub from_str_fn: fn(&str) -> Result, DevToolParseError>, + pub create_default_fn: fn() -> Box, + pub add_self_to_commands_fn: fn(commands: &mut Commands, reflected_self: &dyn Reflect), + pub short_description: Option<&'static str> +} ``` Creating your own dev command is straightforward! Create a struct for any config, implement `Command` for it and then register it using `app.register_dev_command::()`, making it available for various toolboxes to inspect and send. From dc8057f1091b752341960f2be78bee63e141e9ec Mon Sep 17 00:00:00 2001 From: rewin Date: Fri, 22 Mar 2024 01:52:35 +0300 Subject: [PATCH 33/50] fix typo --- rfcs/77-dev-tools-abstraction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index eabdd232..6ec9e54d 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -260,7 +260,7 @@ pub trait DevCommand: bevy::ecs::world::Command + Reflect + FromReflect + GetTyp // create_default_fn: || Box::new(Self::default()), // A function pointer that adds the DevCommand to the provided Commands - // This is needed because we can'n add Box to Commands withh commmands.add method + // This is needed because we can't add Box to Commands withh commmands.add method // So we need to do it in typed way add_self_to_commands_fn: |commands, reflected_self| commands.add(::from_reflect(reflected_self).unwrap()), short_description: Self::short_description() From fc028b8b9ca63a3bf02c4b35fbce680dd66b5d2b Mon Sep 17 00:00:00 2001 From: "a.yamaev" Date: Fri, 22 Mar 2024 12:43:00 +0300 Subject: [PATCH 34/50] Add CLI example --- rfcs/77-dev-tools-abstraction.md | 150 +++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 6ec9e54d..a4be75c0 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -442,9 +442,159 @@ fn parse_and_run_dev_commands(world: &mut World){ } ``` + While a number of other features could sensibly be added to this API (a `--help` flag, saving and loading config to disk, managing compatibility between dev tools), this MVP should be sufficient to prove out the viability of the core architecture. + + +Another valuable approach we can undertake involves constructing a comprehensive Command Line Interface (CLI) interface utilizing the capabilities of the Reflect trait. A Command Line Interface (CLI) serves as a text-based gateway through which users can interact with computer systems or software by issuing commands via a terminal or console. In a typical CLI command structure, elements are organized as follows: + +```bash +command_name arg0 arg1 arg2 --named-arg4 value --named-arg5 value +| command | positional args | named args | +``` + +* `command_name` represents the name of the command being executed. +* `arg0`, `arg1`, and `arg2` are positional arguments, which are required parameters specified in a particular order. +* `--named-arg4 value` and `--named-arg5 value` are named arguments or options, preceded by `--` and followed by their respective values, separated by a space. + +This structure enables users to provide the necessary information and instructions to the game through typed commands. + +For example, setting 999 gold using the SetGold command in CLI style could look like this: +```bash +SetGold 999 +or +SetGold --amount 999 +``` + +Similarly, changing the turn\_speed in FlyDevCamera can be done with this command: +```bash +FlyDevCamera --turn_speed Some(0.5) +``` + +Thus, to implement the CLI interface, we need to do three things: +1. be able to set the value of a command structure field by its name +2. be able to set the value of a command structure field by its sequence number +3. be able to convert strings into field values + +Reflect trait allows to retrieve by sequence number for all data types in rust (Struct, TupleStruct, List, etc). Example +```rust +let field = match command.reflect_mut() { + bevy::reflect::ReflectMut::Struct(r) => { + let Some(field) = r.field_at_mut(idx) else { + error!("Invalid index: {}", idx); + return Err(DevToolParseError::InvalidToolData); + }; + field + }, + ... +``` +And also Reflect trait allows you to get fields by their name for Strut and Enum. Example +```rust + let field = match command.reflect_mut() { + bevy::reflect::ReflectMut::Struct(r) => { + let Some(field) = r.field_mut(name) else { + error!("Invalid name: {}", name); + return Err(DevToolParseError::InvalidToolData); + }; + field + }, + ... +``` + +With the ability to set separate values for DevCommand and ModalDevTool we can build a simple CLI perserver with minimal code + +```rust +fn parse_reflect_from_cli(&self, words: Vec<&str>, target: &mut Box) -> Result<(), DevToolParseError> { + // The current named parameter being parsed + let mut named_param = None; + // Whether or not we are currently in named style + let mut is_named_style = false; + // Index of the next parameter to expect in positional style + let mut idx = 0; + + // Parse all words following the command name + for word in words.iter().skip(1) { + // Named style parameter + if word.starts_with("--") { + is_named_style = true; + named_param = Some(word.trim_start_matches("--").to_string()); + } else { + // Positional style parameter + + // Get the field to apply the value to + if is_named_style { + // Retrieve the named parameter + let Some(named_param) = &named_param else { + error!("Not found name for value: {}", word); + return Err(DevToolParseError::InvalidToolData); + }; + + // Find the field with the matching name + let Ok(field) = get_field_by_name(target.as_mut(), named_param) else { + error!("Invalid name: {}", named_param); + return Err(DevToolParseError::InvalidToolData); + }; + + // Convert the word into the field's value with refistered applyer (FromStr implementations) + let mut ok = false; + for applyer in self.apply_from_string.iter() { + if applyer(field, &word) { + ok = true; + break; + } + } + if !ok { + error!("Not found applyer for value: {}", word); + return Err(DevToolParseError::InvalidToolData); + } + } else { + // Find the next field in positional style + let Ok(field) = get_field_by_idx(target.as_mut(), idx) else { + error!("Invalid index: {}", idx); + return Err(DevToolParseError::InvalidToolData); + }; + + // Convert the word into the field's value with refistered applyer (FromStr implementations) + let mut ok = false; + for applyer in self.apply_from_string.iter() { + if applyer(field, &word) { + ok = true; + break; + } + } + if !ok { + error!("Not found applyer for value: {}", word); + return Err(DevToolParseError::InvalidToolData); + } + + // Increment the index of the next positional style parameter + idx += 1; + } + } + } + Ok(()) +} + +struct CLIDebom { + /// Functions to convert strings into field values and set field by converted value + /// Return true if successful, false if not + pub apply_from_string: Vec bool>, + ... +} +``` + +And after creating a Box command, we can send it using the function registered in metadata + +```rust + (metadata.add_self_to_commands_fn)(&mut commands, reflected_command.as_ref()); +``` +Thus, with the proposed API, we can construct a CLI interface efficiently. This interface can be employed to create a developer console akin to those found in Half-Life or Quake. Importantly, rapid prototyping of developer commands becomes feasible as there's no need to manually configure the CLI interface for each command. + +MVP implementation of CLI parser can be found at [CLI-Parser](https://github.com/rewin123/bevy_dev_CLI_prototype/tree/main) + + ## Implementation strategy ### What metadata do toolboxes need? From 68293b593fd167f23842d6be468fc2f0ccca4904 Mon Sep 17 00:00:00 2001 From: rewin Date: Fri, 22 Mar 2024 12:45:19 +0300 Subject: [PATCH 35/50] Fix typo --- rfcs/77-dev-tools-abstraction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index a4be75c0..a4e9ac0d 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -577,7 +577,7 @@ fn parse_reflect_from_cli(&self, words: Vec<&str>, target: &mut Box Ok(()) } -struct CLIDebom { +struct CLIDemo { /// Functions to convert strings into field values and set field by converted value /// Return true if successful, false if not pub apply_from_string: Vec bool>, From dfb2b26e00dbe2893ee5d9cc9c6240acd189dabe Mon Sep 17 00:00:00 2001 From: rewin Date: Fri, 22 Mar 2024 12:46:07 +0300 Subject: [PATCH 36/50] Fix formating --- rfcs/77-dev-tools-abstraction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index a4e9ac0d..c4e0a80c 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -452,7 +452,7 @@ Another valuable approach we can undertake involves constructing a comprehensive ```bash command_name arg0 arg1 arg2 --named-arg4 value --named-arg5 value -| command | positional args | named args | +| command | positional args| named args | ``` * `command_name` represents the name of the command being executed. From b28a869c0e41095d35b5877a684d871bcc02049e Mon Sep 17 00:00:00 2001 From: rewin Date: Fri, 22 Mar 2024 12:47:21 +0300 Subject: [PATCH 37/50] Fix typp --- rfcs/77-dev-tools-abstraction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index c4e0a80c..8c74a062 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -503,7 +503,7 @@ And also Reflect trait allows you to get fields by their name for Strut and Enum ... ``` -With the ability to set separate values for DevCommand and ModalDevTool we can build a simple CLI perserver with minimal code +With the ability to set separate values for DevCommand and ModalDevTool we can build a simple CLI parser with minimal code ```rust fn parse_reflect_from_cli(&self, words: Vec<&str>, target: &mut Box) -> Result<(), DevToolParseError> { From 3f175bb46930a71d225eca573d464835e54d8c7d Mon Sep 17 00:00:00 2001 From: rewin Date: Fri, 22 Mar 2024 12:48:41 +0300 Subject: [PATCH 38/50] Fix typo --- rfcs/77-dev-tools-abstraction.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 8c74a062..3cd30a59 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -537,7 +537,7 @@ fn parse_reflect_from_cli(&self, words: Vec<&str>, target: &mut Box return Err(DevToolParseError::InvalidToolData); }; - // Convert the word into the field's value with refistered applyer (FromStr implementations) + // Convert the word into the field's value with registered applyer (FromStr implementations) let mut ok = false; for applyer in self.apply_from_string.iter() { if applyer(field, &word) { @@ -556,7 +556,7 @@ fn parse_reflect_from_cli(&self, words: Vec<&str>, target: &mut Box return Err(DevToolParseError::InvalidToolData); }; - // Convert the word into the field's value with refistered applyer (FromStr implementations) + // Convert the word into the field's value with registered applyer (FromStr implementations) let mut ok = false; for applyer in self.apply_from_string.iter() { if applyer(field, &word) { From 6e64f00c1a7f42e5a11dc4a01ad4195a8f357ec1 Mon Sep 17 00:00:00 2001 From: rewin Date: Fri, 22 Mar 2024 12:50:40 +0300 Subject: [PATCH 39/50] Fix metadata --- rfcs/77-dev-tools-abstraction.md | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 3cd30a59..913bf402 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -151,14 +151,6 @@ pub trait ModalDevTool: Resource + Reflect + FromReflect + GetTypeRegistration + } } } - -pub struct DevToolMetaData { - pub name: &'static str, - pub type_id: TypeId, - pub type_info: &'static TypeInfo, - pub from_str_fn: fn(&str) -> Result, DevToolParseError>, - pub short_description: Option<&'static str> -} ``` Modal dev tools are registered via `app.init_modal_dev_tool::()` (to use default config based on the `FromWorld` implementation) or via `app.insert_modal_dev_tool(config: D)`. @@ -267,16 +259,6 @@ pub trait DevCommand: bevy::ecs::world::Command + Reflect + FromReflect + GetTyp } } } - -pub struct DevCommandMetadata { - pub name: &'static str, - pub type_id: TypeId, - pub type_info: &'static TypeInfo, - pub from_str_fn: fn(&str) -> Result, DevToolParseError>, - pub create_default_fn: fn() -> Box, - pub add_self_to_commands_fn: fn(commands: &mut Commands, reflected_self: &dyn Reflect), - pub short_description: Option<&'static str> -} ``` Creating your own dev command is straightforward! Create a struct for any config, implement `Command` for it and then register it using `app.register_dev_command::()`, making it available for various toolboxes to inspect and send. @@ -615,10 +597,12 @@ We also need access to one other critical piece of information: a function point As a result our `DevToolMetadata` looks like: ```rust -struct DevToolMetadata { - name: String, - type_info: TypeInfo, - from_str: StringConstructorFn, +struct DevToolMetaData { + name: &'static str, + type_id: TypeId, + type_info: &'static TypeInfo, + from_str_fn: fn(&str) -> Result, DevToolParseError>, + short_description: Option<&'static str> } ``` From 8df43799e776af42a64b86e724f972309b489ac9 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 22 Mar 2024 08:39:22 -0400 Subject: [PATCH 40/50] Typo Co-authored-by: Afonso Lage --- rfcs/77-dev-tools-abstraction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 913bf402..62e2df5d 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -211,7 +211,7 @@ impl FromStr for DevFlyCamera { Ok(DevFlyCamera { enabled: true, - movmeent_speed, + movement_speed, turn_speed, } ) From 0a7e3bf114bf33fa429fb93345a4df9a697028df Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 22 Mar 2024 10:14:16 -0400 Subject: [PATCH 41/50] Formatting and typos --- rfcs/77-dev-tools-abstraction.md | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 62e2df5d..b48df03b 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -252,7 +252,7 @@ pub trait DevCommand: bevy::ecs::world::Command + Reflect + FromReflect + GetTyp // create_default_fn: || Box::new(Self::default()), // A function pointer that adds the DevCommand to the provided Commands - // This is needed because we can't add Box to Commands withh commmands.add method + // This is needed because we can't add Box to Commands with the commands.add method // So we need to do it in typed way add_self_to_commands_fn: |commands, reflected_self| commands.add(::from_reflect(reflected_self).unwrap()), short_description: Self::short_description() @@ -424,12 +424,9 @@ fn parse_and_run_dev_commands(world: &mut World){ } ``` - While a number of other features could sensibly be added to this API (a `--help` flag, saving and loading config to disk, managing compatibility between dev tools), this MVP should be sufficient to prove out the viability of the core architecture. - - Another valuable approach we can undertake involves constructing a comprehensive Command Line Interface (CLI) interface utilizing the capabilities of the Reflect trait. A Command Line Interface (CLI) serves as a text-based gateway through which users can interact with computer systems or software by issuing commands via a terminal or console. In a typical CLI command structure, elements are organized as follows: ```bash @@ -437,13 +434,14 @@ command_name arg0 arg1 arg2 --named-arg4 value --named-arg5 value | command | positional args| named args | ``` -* `command_name` represents the name of the command being executed. -* `arg0`, `arg1`, and `arg2` are positional arguments, which are required parameters specified in a particular order. -* `--named-arg4 value` and `--named-arg5 value` are named arguments or options, preceded by `--` and followed by their respective values, separated by a space. +- `command_name` represents the name of the command being executed. +- `arg0`, `arg1`, and `arg2` are positional arguments, which are required parameters specified in a particular order. +- `--named-arg4 value` and `--named-arg5 value` are named arguments or options, preceded by `--` and followed by their respective values, separated by a space. This structure enables users to provide the necessary information and instructions to the game through typed commands. For example, setting 999 gold using the SetGold command in CLI style could look like this: + ```bash SetGold 999 or @@ -451,16 +449,19 @@ SetGold --amount 999 ``` Similarly, changing the turn\_speed in FlyDevCamera can be done with this command: + ```bash FlyDevCamera --turn_speed Some(0.5) ``` Thus, to implement the CLI interface, we need to do three things: + 1. be able to set the value of a command structure field by its name 2. be able to set the value of a command structure field by its sequence number 3. be able to convert strings into field values -Reflect trait allows to retrieve by sequence number for all data types in rust (Struct, TupleStruct, List, etc). Example +Reflect trait allows to retrieve by sequence number for all data types in rust (Struct, TupleStruct, List, etc). For example: + ```rust let field = match command.reflect_mut() { bevy::reflect::ReflectMut::Struct(r) => { @@ -472,7 +473,9 @@ let field = match command.reflect_mut() { }, ... ``` + And also Reflect trait allows you to get fields by their name for Strut and Enum. Example + ```rust let field = match command.reflect_mut() { bevy::reflect::ReflectMut::Struct(r) => { @@ -567,16 +570,16 @@ struct CLIDemo { } ``` -And after creating a Box command, we can send it using the function registered in metadata +And after creating a `Box` command, we can send it using the function registered in metadata: ```rust (metadata.add_self_to_commands_fn)(&mut commands, reflected_command.as_ref()); ``` + Thus, with the proposed API, we can construct a CLI interface efficiently. This interface can be employed to create a developer console akin to those found in Half-Life or Quake. Importantly, rapid prototyping of developer commands becomes feasible as there's no need to manually configure the CLI interface for each command. MVP implementation of CLI parser can be found at [CLI-Parser](https://github.com/rewin123/bevy_dev_CLI_prototype/tree/main) - ## Implementation strategy ### What metadata do toolboxes need? From 9855dd075acae2ea35ef4a0e13ec1830b56cc074 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 22 Mar 2024 10:19:38 -0400 Subject: [PATCH 42/50] Add link to bevy_reflect::Documentation --- rfcs/77-dev-tools-abstraction.md | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index b48df03b..00a3d419 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -587,7 +587,7 @@ MVP implementation of CLI parser can be found at [CLI-Parser](https://github.com We can start simple: - the type name of the dev tool -- any doc strings on the dev tool struct +- any doc strings on the dev tool struct: see the [`Documentation` struct](https://github.com/bevyengine/bevy/blob/e33b93e31230c44d3b269d0c781544872fbd3909/crates/bevy_reflect/bevy_reflect_derive/src/documentation.rs#L43) from `bevy_reflect` - the value and types of any configuration that must be supplied By relying on simple structs to configure both our modal dev tools and commands, we can extract this information automatically, via reflection. @@ -804,13 +804,7 @@ Instead, we're forced to turn to the dark arts of reflection and type registrati ## Unresolved questions -1. Are the provided methods (along with those on `Reflect`) sufficient to allow toolkits to populate and run a wide range of dev tools without requiring an additional trait and manual registration? -2. Is the use of `downcast_ref` / `downcast_mut` correct in `DevToolsRegistry::get`? -3. Are `ModalDevTool` and `DevCommand` the best names? -4. How, precisely, can we achieve the sort of API shown in [Building toolboxes] using reflection? - 1. A prototype would be great here. -5. Does the function pointer approach used by `DevCommandMetadata` compile and work successfully? - 1. Again, prototype please! +1. Can we relax the `FromStr` trait bound and simply construct our structs from CLI-style strings from the reflected type information? ## Future possibilities From e04c8ca7452425b49e0ed87c953786fb67d7fd79 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 22 Mar 2024 10:20:06 -0400 Subject: [PATCH 43/50] Clean up resolved TODO --- rfcs/77-dev-tools-abstraction.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 00a3d419..52ba3459 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -609,12 +609,6 @@ struct DevToolMetaData { } ``` -The tricky bit comes when we get to `StringConstructorFn`: we need to be able to store a function pointer that will take us from a string, and return an object that implements our core traits. - -TODO: how can this be done? - -The definition for `DevCommandMetadata` is effectively identical to start, but over time we should expect these to diverge: splitting them from the beginning will ease migrations going forward. - ### What do the registries for our dev tools look like? Keeping track of the registered dev tools without storing them all in a dedicated collection is quite challenging! From 0d17495b0712178b029228e8a48c78bf7a0890e1 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Fri, 22 Mar 2024 10:22:31 -0400 Subject: [PATCH 44/50] Note focus handling as out-of-scope --- rfcs/77-dev-tools-abstraction.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 52ba3459..1a019303 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -811,6 +811,8 @@ Related but out-of-scope questions: - in dedicated crates, saving bevy_dev_tools for higher level abstractions? - should dev tools be embedded in each application or should testing be done through an editor which controls these dev tools? - this is one of the [key questions](https://github.com/bevyengine/bevy_editor_prototypes/discussions/1) for the bevy_editor efforts +- how should focus and input handling be controlled by the dev tools? For example, a fly camera would disable ordinary game controls. + - this is a [complex open question](https://github.com/bevyengine/bevy/issues/3570), and deserves its own independent design work. Future possibilities: From 30eed15da9011503c6e75e7fe38baac8ab182443 Mon Sep 17 00:00:00 2001 From: Mateusz Wachowiak Date: Thu, 28 Mar 2024 19:13:39 +0100 Subject: [PATCH 45/50] Use TypeRegistry --- rfcs/77-dev-tools-abstraction.md | 229 ++++--------------------------- 1 file changed, 28 insertions(+), 201 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 1a019303..78a0da17 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -104,7 +104,7 @@ In order to facilitate the creation of toolboxes, Bevy provides the `ModalDevToo /// and they can be enabled, disabled and reconfigured at runtime. /// /// The documentation on this struct is reflected, and can be read by toolboxes to provide help text to users. -pub trait ModalDevTool: Resource + Reflect + FromReflect + GetTypeRegistration + FromStr + Debug { +pub trait ModalDevTool: Resource + Reflect + FromReflect + GetTypeRegistration + Debug { /// The name of this tool, as might be supplied by a command line interface. fn name() -> &'static str { Self::get_type_registration().type_info().type_path_table().short_path() @@ -120,8 +120,6 @@ pub trait ModalDevTool: Resource + Reflect + FromReflect + GetTypeRegistration + name: Self::name(), type_id: Self::get_type_registration().type_id(), type_info: Self::get_type_registration().type_info(), - // A function pointer, based on the std::str::from_str method - from_str_fn: |s| ::from_str(s).map(|x| Box::new(x) as Box), short_description: Self::short_description() } } @@ -154,7 +152,7 @@ pub trait ModalDevTool: Resource + Reflect + FromReflect + GetTypeRegistration + ``` Modal dev tools are registered via `app.init_modal_dev_tool::()` (to use default config based on the `FromWorld` implementation) or via `app.insert_modal_dev_tool(config: D)`. -This adds them as a resource (just like the familiar `.init_resource` or `insert_resource`), but also registers their `ComponentId` with the central `DevToolsRegistry`, which can be consumed by toolboxes to get a list of the available dev tools. +This adds them as a resource (just like the familiar `.init_resource` or `insert_resource`), but also registers their type (via `.register_type::()`) and adds a few `DevCommands` for enabling, disabling and toggling. To build your own modal dev tool, simply create a configuration resource, implement the `ModalDevTool` trait, and then register it in the app when the correct feature flag is enabled. @@ -190,36 +188,9 @@ impl ModalDevTool for DevFlyCamera { self.enabled } } - -impl FromStr for DevFlyCamera { - fn from_str(s: &str) -> Result{ - let parts = s.split_whitespace(); - let name = parts.iter.next()?; - if name != self.name() { - return Err(DevToolParseError::InvalidName); - } - - let movement_speed: f32 = match parts.iter.next() { - Some(string) = parse(string)?, - None => Self::default().movement_speed, - }; - - let turn_speed: f32 = match parts.iter.next() { - Some(string) = parse(string)?, - None => Self::default().turn_speed, - }; - - Ok(DevFlyCamera { - enabled: true, - movement_speed, - turn_speed, - } - ) - } -} ``` -To configure a dev tool at runtime, simply access the resource, and either mutate or overwrite the `ModalDevTools` struct. +To configure a dev tool at runtime, simply access the resource, and either mutate or overwrite the `ModalDevTools` struct. For changing state of the `ModalDevTool` you can use `DevCommands`. ### Dev commands @@ -233,7 +204,7 @@ To model this, we leverage Bevy's existing `Command` trait, which exists to perf /// to construct an instance of the type that implements this type, and then send it as a `Command` to execute it. /// /// The documentation on this struct is reflected, and can be read by toolboxes to provide help text to users. -pub trait DevCommand: bevy::ecs::world::Command + Reflect + FromReflect + GetTypeRegistration + Default + FromStr + Debug + 'static { +pub trait DevCommand: bevy::ecs::world::Command + Reflect + FromReflect + GetTypeRegistration + Default + Debug + 'static { /// The name of this tool, as might be supplied by a command line interface. fn name() -> &'static str { Self::get_type_registration().type_info().type_path_table().short_path() @@ -247,8 +218,6 @@ pub trait DevCommand: bevy::ecs::world::Command + Reflect + FromReflect + GetTyp name: Self::name(), type_id: Self::get_type_registration().type_id(), type_info: Self::get_type_registration().type_info(), - // A function pointer, based on the std::str::from_str method - from_str_fn: |s| ::from_str(s).map(|x| Box::new(x) as Box), // create_default_fn: || Box::new(Self::default()), // A function pointer that adds the DevCommand to the provided Commands @@ -278,23 +247,10 @@ impl Command for SetGold { } impl DevCommand for SetGold {} - -impl FromStr for SetGold { - fn from_str(s: &str) -> Result{ - let parts = s.split_whitespace(); - let name = parts.iter.next()?; - if name != self.name() { - return Err(DevToolParseError::InvalidName); - } - - let amount_string = parts.iter.next()?; - let amount = amount_string.parse()?; - - Ok(SetGold {amount} ) - } -} ``` +todo: mention enable, disable, and toggle commands + ### Conventions for building dev tools Not everything can or should be defined by a trait! To supplement the `ModalDevTool` trait, we recommend that Bevy and its ecosystem follow the following conventions: @@ -320,11 +276,11 @@ Our first task is getting the list of dev tools. #[derive(Event)] struct ListDevTools; -fn list_dev_tools(mut event_reader: EventReader, dev_tools_registry: Res){ +fn list_dev_tools(mut event_reader: EventReader, type_registry: Res){ // Clear the events, and act if at least one is found if event_reader.drain().next().is_some() { println!("Available modal dev tools:"); - for (_component_id, modal_dev_tool) in dev_tools_registry.iter_modal_dev_tools() { + for (_component_id, modal_dev_tool) in type_registry.iter_with_data::() { println!("{}", modal_dev_tool.type_name()); println!("{}", modal_dev_tool.docs()); @@ -342,58 +298,29 @@ Next, we want to be able to toggle each modal tool by name. ```rust #[derive(Event)] struct ToggleDevTool{ - name: String, + component_id: ComponentId, } fn toggle_dev_tools(world: &mut World){ // Move the events out of the world, clearing them and avoiding a persistent borrow let events = world.resource_mut::>().drain(); + + for event in events { + // This gives us a mutable reference to the underlying resource as a `&mut dyn ModalDevTool` + let dev_tool = world.get_resource_mut_by_id(event.component_id).unwrap().downcast_mut::().unwrap(); - // Use a resource scope to allow us to access both the dev tools registry and the rest of the world at the same time - world.resource_scope(|(world, dev_tools_registry: Mut)|{ - for event in events { - // This gives us a mutable reference to the underlying resource as a `&mut dyn ModalDevTool` - let Some(dev_tool) = dev_tools_registry.get_tool_by_name(world, event.name) else { - warn!("No dev tool was found for {}). Did you forget to register it?", event.name); - continue; - }; - - // Since we know that this object always implements `ModalDevTool`, - // we can use any of the methods on it, or the traits that it requires (like `Reflect`) - dev_tool.toggle(); - } - }) + // Since we know that this object always implements `ModalDevTool`, + // we can use any of the methods on it, or the traits that it requires (like `Reflect`) + dev_tool.toggle(); + } } ``` -Next, we want to be able to configure modal dev tools at run time. - -```rust -#[derive(Event)] -struct ConfigureDevTool{ - tool_string: String, -}; - -fn configure_dev_tools(world: &mut World){ - // Move the events out of the world, clearing them and avoiding a persistent borrow - let events = world.resource_mut::>().drain(); - - // Use a resource scope to allow us to access both the dev tools registry and the rest of the world at the same time - world.resource_scope(|(world, dev_tools_registry: Mut)|{ - for event in events { - // Check the implementation details for information about how this works! - let result = dev_tools_registry.parse_and_insert_tool(event.tool_string); - if let Err(error) = result { - warn!(error); - } - } - })} -``` - Finally, we want to be able to pass in a user supplied string, parse it into a dev command and then evaluate it on the world. +todo: how can this use type registry? ```rust #[derive(Event)] struct DevCommandInput(String); @@ -595,8 +522,6 @@ Additional dev tool specific metadata (such as a classification scheme) can be a Optional methods on both `ModalDevTool` and `DevCommand` will allow us to override the supplied defaults if needed. -We also need access to one other critical piece of information: a function pointer that allows us to construct a new value of this type from a string. - As a result our `DevToolMetadata` looks like: ```rust @@ -604,7 +529,6 @@ struct DevToolMetaData { name: &'static str, type_id: TypeId, type_info: &'static TypeInfo, - from_str_fn: fn(&str) -> Result, DevToolParseError>, short_description: Option<&'static str> } ``` @@ -612,84 +536,21 @@ struct DevToolMetaData { ### What do the registries for our dev tools look like? Keeping track of the registered dev tools without storing them all in a dedicated collection is quite challenging! -How can we keep track of it under the hood? - -```rust -#[derive(Resource)] -struct DevToolsRegistry { - /// The stored collection of modal dev tools, tracked in a type-erased way using [`ComponentId`] - /// - /// The key is the `name()` provided by the `ModalDevTool` trait. - modal_dev_tools: HashMap, - /// The metadata for all registered dev commands. - /// - /// The key is the `name()` provided by the `DevCommand` trait. - dev_commands: HashMap -} -``` +How can we keep track of it under the hood? We can use already existing `TypeRegistry` for that. There are a few key operations that we will need to perform: ```rust -impl DevToolsRegistry { - /// Gets a reference to the specified modal dev tool by `ComponentId`. - fn get_tool_by_id(world: &World, component_id: ComponentId) -> Option<&dyn ModalDevTool> { - let resource = world.get_resource_by_id(component_id)?; - resource.downcast_ref() - } - - /// Gets a mutable reference to the specified modal dev tool by `ComponentId`. - fn get_tool_mut_by_id(mut world: &mut World, component_id: ComponentId) -> Option<&mut dyn ModalDevTool> { - let resource = world.get_resource_mut_by_id(component_id)?; - resource.downcast_mut() - } - - /// Gets the `DevCommandMetadata` for a given dev tool by name. - /// - /// The supplied name should match the `DevCommand::name()` method. - fn get_command_metadata(name: &str) -> Option<&DevCommandMetadata> { - self.dev_commands.get(name) - } - - /// Gets the `DevToolMetadata` for a given dev tool by name. - /// - /// The supplied name should match the `DevCommand::name()` method. - fn get_tool_metadata(name: &str) -> Option<&DevToolMetadata> { - self.modal_dev_tools.get(name) - } - - /// Looks up the `ComponentId` associated with the given name, as supplied by the `ModalDevTool` trait. - fn lookup_tool_component_id(&self, name: &str) -> Option { - *self.get_tool_metadata(name)?.component_id - } +// You can get [`ModalDevTool`] like any other resource. +world.get_resource::(); - /// Gets a reference to the specified modal dev tool by type name, as supplied by the `ModalDevTool` trait. - fn get_tool_by_name(&self, world: &World, name: &str) -> Option<&dyn ModalDevTool> { - let component_id = self.lookup_tool_component_id(name)?; - DevToolsRegistry::get_by_id(world, component_id) - } +// todo: show how to get `Resource` from `TypeId`. - /// Gets a mutable reference to the specified modal dev tool by name. - fn get_tool_mut_by_name(&self, mut world: &mut World, component_id: ComponentId) -> Option<&mut dyn ModalDevTool> { - let component_id = self.lookup_tool_component_id(name)?; - DevToolsRegistry::get_mut_by_id(world, component_id) - } - - /// Iterates over the list of registered modal dev tools. - fn iter_tools(&self, world: &World) -> impl Iterator { - self.modal_dev_tools.iter().map(|&id| (id, self.get(world, id))) - } +// Iterates over tools from `TypeRegistry`. Note: This is not yet implemented. +type_registry.iter_with_data::(); - /// Mutably iterates over the list of registered modal dev tools. - fn iter_tools_mut(&self, mut world: &mut World) -> impl Iterator { - self.modal_dev_tools.iter_mut().map(|&id| (id, self.get(world, id))) - } - - /// Iterates over the list registered dev commands, returning their name and `DevCommandMetadata`. - fn iter_commands(&self) -> impl Iterator { - self.dev_commands.iter() - } -} +// Iterates over dev commands from `TypeRegistry`. Note: This is not yet implemented. +type_registry.iter_with_data::(); ``` Once the user has a `dyn ModalDevTool`, they can perform whatever operations they'd like: enabling and disabling it, reading metadata and so on. @@ -699,46 +560,10 @@ While we can use the [dynamic resource APIs](https://docs.rs/bevy/latest/bevy/ec While this may seem unintuitive, the reason for this is fairly simple: modal configuration is always present, while the configuration for each dev command is generated when it is sent. As a result, we can only access its metadata: the arguments it takes, its doc strings and so on. -### Parsing and inserting modal dev tools - -One method on `DevToolsRegistry` is worth special attention: `parse_and_insert_tool`. -This tool takes a common pattern, parsing the configuration for a dev tool from a provided string, and provides a user friendly, safe API over it. - -```rust -impl DevToolsRegistry { - /// Parses the given str `s` into a modal dev tool corresponding to its name if possible. - /// - /// For a typed equivalent, simply use the `FromStr` trait that this method relies on directly. - fn parse_and_insert_tool(&self, world: &mut World, s: &str) -> Result<(), DevToolParseError>{ - // Parse out the name - let name = s.clone().split_whitespace.next()?; - - // Get the associated `ComponentId`, so we can use it to insert a resource of a dynamic type - let component_id = self.lookup_tool_component_id(name); - // Look-up the existing resource to get access to get access to the metadata we need - let tool_metadata: &dyn DevTool = self.get_tool_metadata(component_id)?; - // Parse the string into a new copy of the tool using the stored function pointer - let new_tool = tool_metadata.from_str(s); - // Construct an `OwningPointer` so we can dynamically insert the resource we just made - OwningPointer::make(new_tool, |owning_pointer|{ - // SAFETY: the value referenced by value is valid for the given `ComponentId` of this world, - // as the component id is cached upon initialization of the resource / dev tool. - unsafe { - world.insert_resource_by_id(component_id, owning_pointer); - } - }); - - Ok(()) - } -} -``` - ## Drawbacks 1. Third-party and end user dev tools will be pushed to conform to this standard. Without the use of a toolbox, this is added work for no benefit. 2. This abstraction may not fit all possible tools and toolboxes. The manual wiring approach is more flexible, and so if our abstraction is overly prescriptive, it may not work correctly. -3. The `from_str` methods are currently quite heavy on boilerplate. A `clap`-inspired derive macro would be lovely here. Perhaps a crate already exists? -4. How can we store type-erased `StringConstructorFn`s in our metadata? ## Rationale and alternatives @@ -799,6 +624,8 @@ Instead, we're forced to turn to the dark arts of reflection and type registrati ## Unresolved questions 1. Can we relax the `FromStr` trait bound and simply construct our structs from CLI-style strings from the reflected type information? +2. How can we access `DevCommand` from type_registry by name? +3. How can we get `ModalDevTool` resource from `TypeId`? ## Future possibilities From ca4a5708dc5fa45b87b4c985093fa60a903b5b51 Mon Sep 17 00:00:00 2001 From: Mateusz Wachowiak Date: Thu, 28 Mar 2024 19:21:48 +0100 Subject: [PATCH 46/50] Add common DevCommands for enabling, disabling and toggling ModalDevTools --- rfcs/77-dev-tools-abstraction.md | 37 +++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 78a0da17..76db926f 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -249,7 +249,42 @@ impl Command for SetGold { impl DevCommand for SetGold {} ``` -todo: mention enable, disable, and toggle commands +We also have a few common `DevCommands` for `ModalDevTools`, which are automatically registered with `.register_modal_dev_tool::()`: + +```rust +/// Reusable dev command to enable [`ModalDevTool`]. +struct Enable { + modal_dev_tool: D +} + +impl() Command for Enable { + fn apply(mut self, world: &mut World) { + self.modal_dev_tool.set_enabled(true); + } +} + +/// Reusable dev command to disable [`ModalDevTool`]. +struct Disable { + modal_dev_tool: D +} + +impl() Command for Disable { + fn apply(mut self, world: &mut World) { + self.modal_dev_tool.set_enabled(false); + } +} + +/// Reusable dev command to toggle [`ModalDevTool`]. +struct Toggle { + modal_dev_tool: D +} + +impl() Command for Toggle { + fn apply(mut self, world: &mut World) { + self.modal_dev_tool.set_enabled(!self.modal_dev_tool.is_enabled()); + } +} +``` ### Conventions for building dev tools From 8b425182781e7b2f9b6def0790aa0221692acc54 Mon Sep 17 00:00:00 2001 From: Mateusz Wachowiak Date: Thu, 28 Mar 2024 19:26:25 +0100 Subject: [PATCH 47/50] why not configure ModalDevTools with DevCommands --- rfcs/77-dev-tools-abstraction.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 76db926f..eb379634 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -656,6 +656,11 @@ Fundamentally, there are two problems with this design: Instead, we're forced to turn to the dark arts of reflection and type registration. +### Why not configure `ModalDevTools` with `DevCommands`? + +While configuring `ModalDevTools` with `DevCommands` makes sense for common setting, such as enabling, disabling, and toggling dev tools, it is not ideal for specific properties. +Those commands are meant to provide a single interface for toolboxes to work with, and should exist on every `ModalDevTool`. + ## Unresolved questions 1. Can we relax the `FromStr` trait bound and simply construct our structs from CLI-style strings from the reflected type information? From 0aca22a09d526795eae4f56fcf01834d7f3d3eb4 Mon Sep 17 00:00:00 2001 From: Mateusz Wachowiak Date: Thu, 28 Mar 2024 19:38:02 +0100 Subject: [PATCH 48/50] Apply suggestions from code review Co-authored-by: Alice Cecile --- rfcs/77-dev-tools-abstraction.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index eb379634..533d5a3a 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -571,7 +571,7 @@ struct DevToolMetaData { ### What do the registries for our dev tools look like? Keeping track of the registered dev tools without storing them all in a dedicated collection is quite challenging! -How can we keep track of it under the hood? We can use already existing `TypeRegistry` for that. +How can we keep track of it under the hood? We can use the pre-existing `TypeRegistry` for that. There are a few key operations that we will need to perform: @@ -658,7 +658,8 @@ Instead, we're forced to turn to the dark arts of reflection and type registrati ### Why not configure `ModalDevTools` with `DevCommands`? -While configuring `ModalDevTools` with `DevCommands` makes sense for common setting, such as enabling, disabling, and toggling dev tools, it is not ideal for specific properties. +While configuring `ModalDevTools` with `DevCommands` makes sense for universal configuration options such as enabling, disabling, and toggling dev tools, it is not ideal for configuring specific properties due to the amount of boilerplate required. +Instead, toolboxes should use a reflection-based workflow to set fields on the various `ModalDevTool` resources directly. Those commands are meant to provide a single interface for toolboxes to work with, and should exist on every `ModalDevTool`. ## Unresolved questions From 5b64aa3214f5894bc8dfeefa4194808081cc11a0 Mon Sep 17 00:00:00 2001 From: Mateusz Wachowiak Date: Thu, 28 Mar 2024 19:42:23 +0100 Subject: [PATCH 49/50] remove question about FromStr trait --- rfcs/77-dev-tools-abstraction.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index 533d5a3a..f2b9b0ed 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -664,9 +664,8 @@ Those commands are meant to provide a single interface for toolboxes to work wit ## Unresolved questions -1. Can we relax the `FromStr` trait bound and simply construct our structs from CLI-style strings from the reflected type information? -2. How can we access `DevCommand` from type_registry by name? -3. How can we get `ModalDevTool` resource from `TypeId`? +1. How can we access `DevCommand` from type_registry by name? +2. How can we get `ModalDevTool` resource from `TypeId`? ## Future possibilities From 8dc97dc23381dde8173ee7c380024328f9f524b5 Mon Sep 17 00:00:00 2001 From: Mateusz Wachowiak Date: Thu, 28 Mar 2024 19:50:32 +0100 Subject: [PATCH 50/50] Add implementation strategy about iter_with_data method --- rfcs/77-dev-tools-abstraction.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rfcs/77-dev-tools-abstraction.md b/rfcs/77-dev-tools-abstraction.md index f2b9b0ed..3968068c 100644 --- a/rfcs/77-dev-tools-abstraction.md +++ b/rfcs/77-dev-tools-abstraction.md @@ -595,6 +595,12 @@ While we can use the [dynamic resource APIs](https://docs.rs/bevy/latest/bevy/ec While this may seem unintuitive, the reason for this is fairly simple: modal configuration is always present, while the configuration for each dev command is generated when it is sent. As a result, we can only access its metadata: the arguments it takes, its doc strings and so on. +### Iterating through `TypeRegistry` with data + +We are using *currently* non-existing method `.iter_with_data::()` on `TypeRegistry`. +This method will iterate through all types that implement a specific trait. If this method will not be implemented, we can achieve something similar with filtering `.iter()` method. +See: [Cart's comment](https://github.com/bevyengine/rfcs/pull/77#discussion_r1536286977) + ## Drawbacks 1. Third-party and end user dev tools will be pushed to conform to this standard. Without the use of a toolbox, this is added work for no benefit.