Dev tools abstraction

[alice-i-cecile]

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.

Co-authored with @matiqo15 and @rewin123.

Read it RENDERED here.

[pablo-lua]

Looks good! Really liked this RFC, just a couple comments with things that I noticed here and there.

[pablo-lua]

Noticed a error in my previous comment, sending again

[bushrat011899]

This is an interesting problem to try and solve. I'm not fully sold on the current distinction between commands and modals, and feel like it needs a bit more refinement. To me, the biggest difference between the two is one has state, while the other is stateless. Taking the no-clip camera as an example, that is definitely stateful, but you still want commands to interact with it (e.g., /noclip speed 10, /noclip on, etc.).

I think it would be useful to think of the ModalDevTool trait as describing a tool state which can be toggled (omitting the parse_from_str entirely). This lends itself well to adding extra traits for visualising these tool states (e.g., in a log, in a toolbox, etc.). Rich interactions with the dev tool states would then be the job of dev commands.

So for a no-clip camera dev tool, you'd define the ModalDevTool as the state of the tool, and a DevCommand for adjusting parameters. It would make sense for a default DevCommand for toggling an arbitrary ModalDevTool to be provided (e.g. /toggle noclip true ).

[makspll]

This is really cool! And since I am maintaining bevy-console I am very excited about the possibilities here! A few things though:

First of all why have parse_from_str on modals at all? For commands this is clear, a command has a one-to-one relationship with some string representation, for a modal this is less clear cut, and a parse_from_str feels more like deserialisation. If anything, instead of returning a Self instance, modals could have a react_to_str method but this would step on DevCommand territory.

Also parse_from_str seems to be very un-opinionated, which may be on purpose, but I can see different standards arising, i.e. do we include the dev tool "executable" name as first argument, string separators, how do we interpret quoted strings (i.e. does "arg" == arg).

I don't think we necessarily should depend on clap directly, but we could have some more abstractions here, I am thinking something along the lines of a smaller https://docs.rs/clap/latest/clap/trait.FromArgMatches.html, so a slight abstraction from text, which allows room for different standards. For example as an extreme example: both "{ my_arg: 123 }" and "--my-arg 123" would come down to the same thing via parsers, and we could even provide a default one (clap?) and hell could later store that with the dev tools.

let matches = parse(input);
let val: usize = matches.get<usize>("my_arg"); // 123
// or
let val: String = matches.get("my_arg"); //123

EDIT:
I suppose not forcing some sort of "arg" based construction method for modals would then rely on authors supplying commands to modify all config properties of the modal. In which case I can see why having such a method would be beneficial, however if the modal has 20 properties, your console might as well support multi line json haha :P

[alice-i-cecile]

First of all why have parse_from_str on modals at all

I've expanded the toolkit example to try and clarify this: you need it to be able to allow for free-form runtime config of these resources. Obviously you could make dev commands for each way you might want to configure the resource, but I was thinking it would be more user-friendly to allow them to just set the struct's values directly.

Also parse_from_str seems to be very un-opinionted, which may be on purpose, but I can see different standards arising

Yeah, I'm not sure of the best way to handle this. I'd like to interoperate with std's FromStr trait, but it really doesn't impose any structure at all.

however if the modal has 20 properties, your console might as well support multi line json haha :P

Yeah, I actually really want to support parsing these objects from the new bsn format at some point!

[rewin123]

The RFC looks really good! I really like the good analysis done on usage and what should be classified as dev tools and will be used and what will not. From this analysis comes for me a reasonable and good trait structure.

However, I think the RFC did not address one important issue in my opinion.

Part of the dev tools may want to display debugging information on the screen or get some input from the developer. For example, the fps overlay displays the number of fps in the bottom right corner, and the entity inspector may want to show some editable fields and buttons near the right edge of the screen. Or the realtime statistics render graph will want to display rendering times information somewhere in the static area. And if multiple dev_tools claim the same area of the screen to display, it will be a mess. And obviously dev_tools can and will conflict over displaying information. Accordingly, the question not solved and not asked in my opinion in RFC is "how is it planned to organise the location of the displayed information of dev modules?". (Or how do we start building editor UI from dev tools xDDDD, sorry).

[alice-i-cecile]

@rewin123 great question! I've added more thoughts on this to both "Rationale" and "Future work" justifying why it's not included currently: 45506d2

[pablo-lua]

The DevToolsRegistry was a bit confusing, but I tried my best to organize my thoughts here

[pablo-lua]

First: What are we parsing here? I was having a look and ModalDevTool trait doesn't have a metadata at all. If we expect to not only parse commands here, but also tools themselves, we can store in the Metadata some kind of identifier to it: Maybe a enum of some kind?

I Think we need to clarify how are we going to register commands so this can be an easy way forward.
For example, lets say we store in the metadata an enum

enum DevToolType {
   ModalDevTool,
   DevCommand
}

Now, something that maybe can be strange: To do everything the way we currently do with strings, we must ensure that all the tools have different names.
And so, we get the tool metadata, with the enum stored, and we can make a command or insert the tool, as simple as possible.

[alice-i-cecile]

First: What are we parsing here? I was having a look and ModalDevTool trait doesn't have a metadata at all

This is an omission! Lemme fix that...

And yeah, I've been relying on unique names for each dev tool.

[pablo-lua]

For starters, can we unify this two metadatas into a single metadata?

[alice-i-cecile]

For now, maybe. Down the line, I think their implementations will diverge, so I'd like to keep them separate and eat the code duplication.

[naomijub]

From my perspective, I like them separated. In my mind this goes well with the comment I made about they being +- equivalent to Resources and Events, so the separation makes them pretty clear

[pablo-lua]

The approach will change depending on what we do, if we have a function parsing both the commands and modals at the same block, I think we might need to unify the metadatas. At the same time, if we choose to have two functions: A command parsing method and a tool parsing method, this can be simplified and we can have two distinct metadatas.
But that will have significant differences in parsing, as we have to know beforehand if we are parsing a tool or a command.

[codercommand]

On the topic of commands, personally I've come to really like menus like F1 on VScode and ctrl+k on Github.

Personally I like the idea of commands being a feature that has ui inspired from Githubs ctrl+k menu

Then as for features it works like traditional console and have take commands, but it can also provide context. This allows it to provide context relevant info like the file explorer right click context menu or you can search through sub commands/directories.

I'm not sure on the best way it should work. Since it's a hybrid between a console and a context menu, maybe have the user right click and if they have something selected in the scene it can be used by the command.

Imagine your workflow looking something like click on thing, right click to get menu, type command or optionally click on a proposed option. And if you click on an option, it could open more suboptions, etc.

[afonsolage]

I like the idea of having a common pattern for development tools. I'll read more on weekend but for now my first point is this RFC should decouple the parsing and keep Reflect as a generic requirement, since requiring FromStr will only be useful for "toolboxes" like bevy_console, for all others like having UI, remote debugging or even WASI integration, you won't use &str at all, you just want to serialize the command and process it or use Reflect meta info.

If ModalDevTools: Reflect this means any "toolbox" can parse a String without forcing the dev tools devs to do that.

my_awesome_command 10 15.5 enabled=true

Args without name would be used as positional (Reflect::field_at(index)) and args with name would be set with (Reflect::field(name)).

This way we shift the command parsing from MoldaDevTools itself and move to "toolbox" impls, making this even more flexible.

[ricky26]

@alice-i-cecile / @rewin123 future considerations RE: layout is input redirection / modality. Might be kinda bike-sheddy.

The free-cam example seems obviously modal, and I would expect it to capture input (to avoid binding collisions with gameplay systems). (I do wonder whether we want to let the developers determine at which point input is captured, but I think this is a wider issue in bevy as it stands.)

For things like overlays (which is more where layout comes into play), I wouldn't expect them to be 'modal', since that usually refers to whole-window + input capture. We could just have an orthogonal system for allocating dev overlay space, and have these kind of things be DevCommands?

If you toggle multiple dev tools, which should get the input? (I'm expecting a lot of these tools will have overlapping bindings with gameplay, and each other.)

[alice-i-cecile]

Yeah, the focus management thing is generally a big open question in Bevy as a whole: see bevyengine/bevy#3570 for more context there. I think it can and should be tackled independently though, and done ad-hoc to start.

[pablo-lua]

Some comments about object safety.

[pablo-lua]

    pub fn get_tool_by_component_id<'w>(&self, cmp: ComponentId, world: &'w World) -> Option<&'w dyn Reflect> {
        let type_id = world.components().get_info(cmp)?.type_id()?;
        let type_registry = world.get_resource::<AppTypeRegistry>()?.read();
        type_registry.get(type_id)?.data::<ReflectResource>()?.reflect(world)
    }

    pub fn get_tool_by_component_id_mut<'w>(&self, cmp: ComponentId, world: &'w mut World) -> Option<Mut<'w, dyn Reflect>> {
        let type_id = world.components().get_info(cmp)?.type_id()?;
        let reflect_resource = {
            let type_registry = world.get_resource::<AppTypeRegistry>()?.read();
            type_registry.get(type_id)?.data::<ReflectResource>()?.clone()
        };
        reflect_resource.reflect_mut(world)
    }

With this approach, we can return a dyn Reflect instead of dyn ModalDevTool while keeping component_id

[pablo-lua]

I don't know for sure the implications of using ComponentId instead of TypeId, this can simplified if we don't need to get the type_id from components first.

[pablo-lua]

A cleanup is probably needed here.

We should probably have the methods match the impl above (see my comment above), only the name and return will change.

[pablo-lua]

Here we're talking about dyn ModalDevTool, but that's not possible, so this section should probably be changed.

[cart]

Thank you for putting this together. I now understand where you all are coming from (for context: my original comment pushing back against the "dev tool" concept).

I see the appeal of a common way to represent commands:

• Single searchable database of commands
• Common data model for command input

That being said, I think this is trying to be too opinionated, and focusing on "dev tool" as a specific thing might be getting in our way:

1. Every "toolbox" will have a different way of interacting with these commands. Command serialization/deserialization is something the "toolbox" should be responsible for. We cannot just throw FromStr on the impl and let each dev tool trait impl choose what format to expect. We should rely on Reflect for this (with optional reflect-driven Serde support), similar to how we handle scene serialization. Leave things like the "command line format" to the "command line toolbox".
2. There is no reasonable way to solve the "Dev tool UI overlap" problem in a generic system. Each "toolbox" will define a different context for displaying and rendering "dev tools". The rendering/UX of a "dev tool" is fully the responsibility of the toolbox, and most "interactive" functionality will be toolbox specific. A "dev-tool" that renders content inside of the Bevy Editor should define itself relative to the Bevy Editor's constraints (which are notably ... currently undefined). I do not think we should make solving this problem generically across toolboxes "future work". Instead it should be a non-goal.

I believe we should be fully focused on (1) a command-style abstraction and (2) queryable/categorized settings configured by commands. We should try very hard to build on what already exists when possible, and build reusable pieces when it isn't.

1. Reflect (with optional reflect-driven Serde) for all serialization
2. Use Command for "applying a command to a World" (I acknowledge that this is already part of the proposal)
3. Rely on existing registry tooling instead of building a new one. Some options:
   a. TypeRegistry enumeration of types registered with a specific reflected trait (ex: DevCommand)
   b. Make this Component / Query driven instead of Resource driven (ex: use a common DevCommand component)
4. Use Commands for configuring settings (including enabling / disabling features)

I think by shifting focus to these things, we sidestep the "dev tool is a nebulous concept" problem and focus on building the specific functionality we need. This also leaves the door open to other categories of commands / configuration.

For example (not a final design, just illustrating where my head is at):

// Reflect registry-driven commands

#[derive(Reflect, DevCommand, Debug)]
#[reflect(DevCommand)]
struct SetGold {
    amount: u64,
}

impl Command for SetGold {
    fn apply(self, world: &mut World){
        let mut current_gold = world.resource_mut::<Gold>();
        current_gold.0 = self.amount;
    }
}

app.register_type::<SetGold>()

// later, use the type registry to invoke / enumerate / serialize this command
type_registry.iter_with_data::<ReflectDevCommand>()

// Commands for enabling / disabling the settings of a dev tool

#[derive(Resource, Reflect, DevToolSettings, Debug)]
#[reflect(DevToolSettings)]
struct DevFlyCamera {
    enabled: bool,
    /// How fast the camera travels forwards, backwards, left, right, up and down, in world units.
    movement_speed: Option<f32>,
    /// How fast the camera turns, in radians per second.
    turn_speed: Option<f32>,
}

impl Toggleable for DevFlyCamera {
    fn is_enabled(&self) -> bool {
        self.enabled
    }
    
    fn set_enabled(&mut self, enabled: bool) {
        self.enabled = enabled;
    }
}

app.register_type::<DevFlyCamera>()
// Makes these Dev commands available (these could be put behind a helper function):
app.register_type::<Enable<DevFlyCamera>>()
app.register_type::<Disable<DevFlyCamera>>()
app.register_type::<Toggle<DevFlyCamera>>()
// this DevCommand writes a new value of DevFlyCamera on top of the existing value
app.register_type::<Configure<DevFlyCamera>>()

// later (ex: if you want to enumerate DevToolSettings types in a tool)
type_registry.iter_with_data::<DevToolSettings>()

/// Reusable Enable dev command for toggleable things
#[derive(DevCommand)]
pub struct Enable<T: Toggleable>();

impl<T: Toggleable> Command for Enable<T> {
    // TODO
}

[alice-i-cecile]

I can absolutely work with that! I'll do a clean up pass next week, reframing and simplifying to capture these ideas.

[cart]

Awesome!

Also note that type_registry.iter_with_data does not currently exist. You can do something similar with .iter() and filtering the results, but we could consider providing that ourselves (and maybe building an index to accelerate, although building that index might be too detrimental to startup times and I'm not sure we need it).

[matiqo15]

Me and @alice-i-cecile noticed during the re-designing of the abstraction to use TypeRegistry a few issues:

1. TypeRegistry was not built for this. The TypeRegistry was designed to store the registration of types, not tools or metadata of them. If we would use this, we are potentially allowing usage of TypeRegistry for storing anything.
2. Using a dedicated resource (e.g. DevToolsRegistry) is a standard design pattern in the engine. For example, let's take a look at DiagnosticStore resource that stores diagnostic information. Similarly, a DevToolsRegistry could be used to store and manage tools and commands, while hiding a lot of magic that could be intimidating to users.
3. In the DevToolsRegistry we would store metadata about a tool or a command, which is accessible by providing its name. While this still could be possible with TypeRegistry it would be a misuse of the registry, and would create additional overhead.
4. TypeRegistry limits our flexibility and extensibility of the abstraction in the future.

The use of Reflect instead of FromStr is great though!

[cart]

TypeRegistry was not built for this. The TypeRegistry was designed to store the registration of types, not tools or metadata of them. If we would use this, we are potentially allowing usage of TypeRegistry for storing anything.

Tools and metadata are represented as types, and whether a type is a tool (or command) should be encoded as a Rust Trait. From my perspective these line up perfectly with the TypeRegistry's functionality and purpose.

In the DevToolsRegistry we would store metadata about a tool or a command, which is accessible by providing its name. While this still could be possible with TypeRegistry it would be a misuse of the registry, and would create additional overhead.

From my perspective the "dev tool name" should be the type name (short name by default with disambiguation possible via the full type path). Again this feels like a great fit (and the right one). I consider the overhead of querying the reflected trait registration for a type to be acceptable for this use case (and just barely more expensive than the HashMap<String, Data> that a custom registry would use).

I think the biggest missing piece is how DevCommands correlate to dev tools / DevToolSettings (which we could just shorten to DevTool if we want). For example: the DevTool trait could have a function that provides this correlation.

TypeRegistry limits our flexibility and extensibility of the abstraction in the future.

This was intentional. My goal here was to put bounds on this abstraction in a way that prevents it from becoming a new unbounded "framework". I'd like to reconcile the requirements of the "dev tool" space with the rest of Bevy.

[pablo-lua]

I agree here: I don't really see a use for Metadata anymore if we can use the type registry for many things
For example, let's say we have the Toggleable trait, and the ReflectToggleable type and the 3 new commands: Toggle<T>, Enable<T> and Disable<T>, and the CLI tool as an example of functionality here.
We can have the CLI tool be just a HashMap<ToolName, TypeId> (tool name would probably be of type String here). We can just have the CLI tool use the name here and lookup in type registry for the ReflectToggleable data. For inserting new tools with CLI, we would use the same approach, but with ReflectDeserialize instead.
And so, this would lessen the uses for metadata. Note that with this approach, the CLI tool for example would have to use serde with deserialize, as we don't really have a way to store the given function to deserialize the tool without some metadata. But with the main commands this RFC proposes: Enable, disable and toggle, this will go alright without metadata.
At the same time, I don't see a way to go without a ToolBox resource that will store at least the TypeIdand the Tool / command name.

[matiqo15]

At the same time, I don't see a way to go without a ToolBox resource that will store at least the TypeIdand the Tool / command name.

That's why I believe we should have a registry. If we would ask every toolbox creator to "make your own storage for tools", we would end up with multiple resources that have the same purpose.

[pablo-lua]

That's why I believe we should have a registry

Yeah, I believe we at can have a global resource to store what tools the app currently have, and rely more on the app registry for parse and creation of commands

[matiqo15]

I've been experimenting with abstraction for the dev tools for some time now and:

1. I still don't think type registry is a good idea:

From my understanding iter_with_data would require DevTool to be object-safe otherwise, we would never be able to get it, even as a &dyn Reflect. This is because we would need to use a ReflectDevTool to check whether it is a dev tool.

Even if we made DevTool object-safe, we would still run into problems getting &dyn DevTool from ReflectDevTool without type annotation.

    // New method on `TypeRegistry`
    pub fn iter_with_data<T: TypeData>(&self) -> impl Iterator<Item = (&TypeRegistration, &T)> {
        self.registrations.values().filter_map(|item| {
            let type_data = self.get_type_data::<T>(item.type_id());
            type_data.map(|data| (item, data))
        })
    }
    
    // Possible implementation of printing name of every tool
    for dev_tool in type_registry.iter_with_data::<ReflectDevTool>() {
        // This is the problem - we need to specify a tool here if we want to get a `&dyn DevTool`
        let reflected = Box::new(DevFlyCamera::default());
        if let Some(tool) = dev_tool.1.get(&*reflected) {
            println!("{:?}", tool.name());
        }
    }

2. Why shouldn't it be object-safe

We would need to drop FromReflect, and as a result of this, we would run into problems with executing dev commands.

3. DevToolsRegistry

You don't want dev tools to become a new unbounded framework - but actually, our abstraction can restrict what fits in it. With a type registry, you can always register a type, but whether you would be able to register a tool in our registry is going to be decided by the API of the registry. Would it be opinionated? - yes, but I think it should.

[cart]

I'm not seeing any problems here. Your first two hangups have solutions:

trait DevTool: Resource {
    fn name(&self) -> &'static str;
}

#[derive(Clone)]
struct ReflectDevTool {
    get_reflect: fn(&World) -> Option<&dyn Reflect>,
    get: fn(&World) -> Option<&dyn DevTool>,
    from_reflect: fn(&dyn Reflect) -> Option<Box<dyn DevTool>>,
}

impl ReflectDevTool {
    fn get_reflect<'a>(&self, world: &'a World) -> Option<&'a dyn Reflect> {
        (self.get_reflect)(world)
    }

    fn get<'a>(&self, world: &'a World) -> Option<&'a dyn DevTool> {
        (self.get)(world)
    }

    fn from_reflect(&self, reflect: &dyn Reflect) -> Option<Box<dyn DevTool>> {
        (self.from_reflect)(reflect)
    }
}

impl<D: DevTool + Reflect + FromReflect> FromType<D> for ReflectDevTool {
    fn from_type() -> Self {
        Self {
            get_reflect: |world| world.get_resource::<D>().map(|d| d as &dyn Reflect),
            get: |world| world.get_resource::<D>().map(|d| d as &dyn DevTool),
            from_reflect: |reflect| {
                D::from_reflect(reflect).map(|d| {
                    let d: Box<dyn DevTool> = Box::new(d);
                    d
                })
            },
        }
    }
}

#[derive(Resource)]
pub struct DevFlyCamera {}

fn iter_dev_tools(world: &mut World, type_registry: &TypeRegistry) {
    // Possible implementation of printing name of every tool
    for (registration, dev_tool) in type_registry.iter_with_data::<ReflectDevTool>() {
        if let Some(tool) = dev_tool.get(world) {
            println!("{:?}", tool.name());
        }
        let reflect = DynamicStruct::default();
        let d = dev_tool.from_reflect(&reflect);
    }
}

[vladinator1000]

As I'm reading this spec, it sounds exactly what an editor would be used for? Like a temporary solution that alleviates the lack of editor conventions, e.g. command panel.

Regardless of this classification, dev tools are:
not intended to be shipped to end users
highly application specific
enabled or operated one at a time, on an as-needed basis, when problems arise
intended to avoid interfering with the games ordinary operation, especially when disabled
expansive and ad hoc

Also hi everyone, I normally just lurk around while gathering courage to contribute 😅