-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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 dev tools #12427
base: main
Are you sure you want to change the base?
Abstraction dev tools #12427
Changes from all commits
9e18452
2b5cef5
dd62c29
77f7bfb
ef68643
8355a39
8a81cd1
e2f8f89
93e697f
1ad3469
d999850
8e6eab9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -3,6 +3,10 @@ | |||||||||
#![cfg_attr(docsrs, feature(doc_auto_cfg))] | ||||||||||
|
||||||||||
use bevy_app::prelude::*; | ||||||||||
use bevy_ecs::system::Resource; | ||||||||||
use bevy_reflect::Reflect; | ||||||||||
use bevy_utils::HashMap; | ||||||||||
use std::{any::TypeId, fmt::Debug}; | ||||||||||
|
||||||||||
#[cfg(feature = "bevy_ci_testing")] | ||||||||||
pub mod ci_testing; | ||||||||||
|
@@ -45,3 +49,128 @@ impl Plugin for DevToolsPlugin { | |||||||||
} | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
/// Trait implemented for every dev tool. | ||||||||||
pub trait DevTool: Sync + Send + Debug + Reflect + 'static {} | ||||||||||
|
||||||||||
/// Information about dev tool. | ||||||||||
#[derive(Debug)] | ||||||||||
pub struct DevToolConfig { | ||||||||||
/// Identifier of a dev tool. | ||||||||||
pub id: TypeId, | ||||||||||
/// Tool specific configuration. | ||||||||||
tool_config: Box<dyn DevTool>, | ||||||||||
is_enabled: bool, | ||||||||||
} | ||||||||||
|
||||||||||
impl DevToolConfig { | ||||||||||
/// Returns true if [`DevTool`] is enabled. | ||||||||||
pub fn is_enabled(&self) -> bool { | ||||||||||
self.is_enabled | ||||||||||
} | ||||||||||
|
||||||||||
/// Enables [`DevTool`]. | ||||||||||
pub fn enable(&mut self) { | ||||||||||
self.is_enabled = true; | ||||||||||
} | ||||||||||
|
||||||||||
/// Disables | ||||||||||
pub fn disable(&mut self) { | ||||||||||
self.is_enabled = false; | ||||||||||
} | ||||||||||
|
||||||||||
/// Toggles [`DevTool`]. | ||||||||||
pub fn toggle(&mut self) { | ||||||||||
self.is_enabled = !self.is_enabled; | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
impl DevToolConfig { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason why those two impl Blocks aren't together? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, I think we might want helper methods to return the inner config as &dyn Reflect and &mut dyn Reflect. |
||||||||||
/// Creates a new [`DevTool`] from a specified [`TypeId`]. | ||||||||||
/// New tool is enabled by default. | ||||||||||
pub fn new(id: TypeId, tool_config: impl DevTool) -> DevToolConfig { | ||||||||||
DevToolConfig { | ||||||||||
id, | ||||||||||
tool_config: Box::new(tool_config), | ||||||||||
is_enabled: true, | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
/// Returns a tool specific configuration. | ||||||||||
pub fn get_tool_config<D: DevTool + 'static>(&self) -> Option<&D> { | ||||||||||
self.tool_config.as_any().downcast_ref::<D>() | ||||||||||
} | ||||||||||
|
||||||||||
/// Returns a mutable tool specific configuration. | ||||||||||
pub fn get_tool_config_mut<D: DevTool + 'static>(&mut self) -> Option<&mut D> { | ||||||||||
self.tool_config.as_any_mut().downcast_mut::<D>() | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
/// A collection of [`DevTool`]s. | ||||||||||
#[derive(Resource, Default, Debug)] | ||||||||||
pub struct DevToolsStore { | ||||||||||
dev_tools: HashMap<TypeId, DevToolConfig>, | ||||||||||
} | ||||||||||
|
||||||||||
impl DevToolsStore { | ||||||||||
/// Adds a new [`DevTool`]. | ||||||||||
/// | ||||||||||
/// If possible, prefer calling [`App::init_dev_tool`] or [`App::insert_dev_tool`]. | ||||||||||
pub fn add(&mut self, dev_tool: DevToolConfig) { | ||||||||||
self.dev_tools.insert(dev_tool.id, dev_tool); | ||||||||||
} | ||||||||||
|
||||||||||
/// Removes a [`DevTool`]. | ||||||||||
pub fn remove(&mut self, id: &TypeId) { | ||||||||||
self.dev_tools.remove(id); | ||||||||||
} | ||||||||||
|
||||||||||
/// Returns a reference to the given [`DevTool`] if present. | ||||||||||
pub fn get(&self, id: &TypeId) -> Option<&DevToolConfig> { | ||||||||||
self.dev_tools.get(id) | ||||||||||
} | ||||||||||
|
||||||||||
/// Returns a mutable reference to the given [`DevTool`] if present. | ||||||||||
pub fn get_mut(&mut self, id: &TypeId) -> Option<&mut DevToolConfig> { | ||||||||||
self.dev_tools.get_mut(id) | ||||||||||
} | ||||||||||
|
||||||||||
/// Returns an iterator over all [`DevTool`]s, by reference. | ||||||||||
pub fn iter(&self) -> impl Iterator<Item = &DevToolConfig> { | ||||||||||
self.dev_tools.values() | ||||||||||
} | ||||||||||
|
||||||||||
/// Returns an iterator over all [`DevTool`]s, by mutable reference. | ||||||||||
pub fn iter_mut(&mut self) -> impl Iterator<Item = &mut DevToolConfig> { | ||||||||||
self.dev_tools.values_mut() | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
/// Extends [`App`] with new `init_dev_tool` and `insert_dev_tool` functions. | ||||||||||
pub trait DevToolApp { | ||||||||||
/// Initialize a new [`DevTool`]. | ||||||||||
fn init_dev_tool<D: DevTool + Default>(&mut self) -> &mut Self; | ||||||||||
/// Insert a new [`DevTool`] with configuration. | ||||||||||
fn insert_dev_tool<D: DevTool>(&mut self, value: D) -> &mut Self; | ||||||||||
} | ||||||||||
|
||||||||||
impl DevToolApp for App { | ||||||||||
fn init_dev_tool<D: DevTool + Default>(&mut self) -> &mut Self { | ||||||||||
let dev_tool = DevToolConfig::new(TypeId::of::<D>(), D::default()); | ||||||||||
let mut dev_tools = self | ||||||||||
.world | ||||||||||
.get_resource_or_insert_with::<DevToolsStore>(Default::default); | ||||||||||
dev_tools.add(dev_tool); | ||||||||||
self | ||||||||||
} | ||||||||||
|
||||||||||
fn insert_dev_tool<D: DevTool>(&mut self, value: D) -> &mut Self { | ||||||||||
let dev_tool = DevToolConfig::new(TypeId::of::<D>(), value); | ||||||||||
Comment on lines
+168
to
+169
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a better naming here, but can be a opinion question. |
||||||||||
let mut dev_tools = self | ||||||||||
.world | ||||||||||
.get_resource_or_insert_with::<DevToolsStore>(Default::default); | ||||||||||
dev_tools.add(dev_tool); | ||||||||||
self | ||||||||||
} | ||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like this. This will run every time any tool has been changed, but I can't think of a solution to only run if a specific tool has been changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can relax on that somehow by just making a global system that manages this, based on one-shot systems, I'm not sure how effective this can be do though. At the same time, that would be useless without some kind o cache to store what was changed and what was not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we had some kind of cache storing what was changed, I don't think there would be any reason for complicating it with the global system. I'd just let the user interact with the cache and use it in a
run_if
condition.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a fine level of granularity for now: I wouldn't expect
DevToolsStore
to change regularly at all.If it becomes a problem we can also cache a
Local
within the system to short-circuit.