Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Abstraction of bevy_dev_tools #12368

Open
matiqo15 opened this issue Mar 7, 2024 · 7 comments · May be fixed by #12427
Open

Abstraction of bevy_dev_tools #12368

matiqo15 opened this issue Mar 7, 2024 · 7 comments · May be fixed by #12427
Labels
A-Dev-Tools Tools used to debug Bevy applications. C-Feature A new feature, making something new possible S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged X-Controversial There is active debate or serious implications around merging this PR

Comments

@matiqo15
Copy link
Member

matiqo15 commented Mar 7, 2024

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

Allow for configuration of dev tools

What solution would you like?

Ideally, we're looking for something that:

  • Provides an easy way for user to enable/disable tool
  • Is configurable during runtime

Additional context

see #12354 (comment)

@matiqo15 matiqo15 added C-Feature A new feature, making something new possible A-Dev-Tools Tools used to debug Bevy applications. labels Mar 7, 2024
@pablo-lua
Copy link
Contributor

Ideally, we would Fix #11083 and follow with some type of API from that.

But while we don't have it, one challenge of doing this now is the fact that we need to use something other than a plugin for it or similar. A problem that I can see is the fact that the major part of the tools need to add systems and so in the App building for working, something that I can't imagine doing without plugins, the only API that I can imagine working somehow is One Shot systems.

@matiqo15
Copy link
Member Author

matiqo15 commented Mar 7, 2024

I don't think #11083 will be fixed anytime soon though, we could just drop configuration during runtime for now but keep it in mind for the future. We need some abstraction before merging more than few tools.

@alice-i-cecile
Copy link
Member

Yeah, I would like to avoid coupling this too tightly to the current Plugin trait.

The other key requirements I have are:

  1. It's easy to toggle one on and off independently.
  2. It's easy to toggle all of them on and off independently.
  3. There's a simple standard way to add third-party or user-created dev tools to this group, which can then be toggled on and off.

@alice-i-cecile alice-i-cecile added this to the 0.14 milestone Mar 8, 2024
@BD103
Copy link
Member

BD103 commented Mar 8, 2024

3. There's a simple standard way to add third-party or user-created dev tools to this group, which can then be toggled on and off.

What if dev tools were stored in a Resource, which could be registered with a trait on App?

fn main() {
    App::new()
        .register_dev_tool(MyEpicEditorDoodad)
        .run();
}

#[derive(Resource)]
struct DevTools(Vec<Box<dyn DevTool>>);

trait DevTool {
    // TODO: What methods are needed here?
}

trait DevToolsExt: Sealed {
    fn register_dev_tool(&mut self, tool: impl DevTool) -> &mut Self;
}

impl DevToolsExt for App {
    fn register_dev_tool(&mut self, tool: impl DevTool) -> &mut Self {
        self.world.get_resource_or_something::<DevTools>().0.push(Box::new(tool));
        self
    }
}

@pablo-lua
Copy link
Contributor

pablo-lua commented Mar 8, 2024

What if dev tools were stored in a Resource

That was what I was thinking. But first, we would probably need a TypeIdMap. I'm thinking in something like the Multiple gizmos config (see #10342).

@matiqo15
Copy link
Member Author

matiqo15 commented Mar 8, 2024

I was thinking of something similar to diagnostics:

#[derive(Hash, Eq, PartialEq)
pub struct DevToolId(pub Uuid);

pub struct DevTool {
    pub id: DevToolId,
    pub is_enabled: bool,
    // ...
}

#[derive(Resource, Default)]
pub struct DevToolsStore {
    pub dev_tools: HashMap<DevtoolId, DevTool>,
}

This should meet all of our requirements. Though with the configuration during runtime it would require something like:

fn tool_system(dev_tools_store: Res<DevToolsStore>) {
    let Some(tool) = dev_tools_store.dev_tools.get(&uuid);
    if !tool.is_enabled {
        return;
    }
    // rest of the system
}

@alice-i-cecile
Copy link
Member

I wrote an RFC to better explore this space: bevyengine/rfcs#77

@alice-i-cecile alice-i-cecile added S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged X-Controversial There is active debate or serious implications around merging this PR labels Mar 19, 2024
@alice-i-cecile alice-i-cecile removed this from the 0.14 milestone May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Dev-Tools Tools used to debug Bevy applications. C-Feature A new feature, making something new possible S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants