-
-
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
Dev tools abstraction #13582
base: main
Are you sure you want to change the base?
Dev tools abstraction #13582
Conversation
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
The generated |
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.
Very interesting topic, took a very quick look at the PR!
I'm not super sold on the reflection-driver approach, but this seems to be the decision from RFC discussion. If you were to sum it up, why is Reflection preferred here?
use crate::dev_command::*; | ||
use bevy_ecs::world::{Command, World}; | ||
use bevy_reflect::std_traits::ReflectDefault; | ||
use bevy_reflect::FromReflect; | ||
use bevy_reflect::Reflect; | ||
use bevy_reflect::TypePath; | ||
|
||
/// Trait that represents a toggable dev tool | ||
pub trait Toggable { | ||
/// Enables the dev tool in the given `world`. | ||
fn enable(world: &mut World); | ||
/// Disables the dev tool in the given `world`. | ||
fn disable(world: &mut World); | ||
/// Checks if the dev tool is enabled in the given `world`. | ||
fn is_enabled(world: &World) -> bool; | ||
} | ||
|
||
/// Command to enable a `Toggable` component or system. | ||
#[derive(Reflect, Default)] | ||
#[reflect(DevCommand, Default)] | ||
pub struct Enable<T: Toggable + FromReflect + Send + Sync + 'static + Default> { | ||
/// PhantomData to hold the type `T`. | ||
#[reflect(ignore)] | ||
_phantom: std::marker::PhantomData<T>, | ||
} | ||
|
||
impl<T: Toggable + Send + Sync + 'static + TypePath + FromReflect + Default> DevCommand for Enable<T> {} | ||
|
||
impl<T: Toggable + FromReflect + Send + Sync + 'static + Default> Command for Enable<T> { | ||
/// Applies the enable command, enabling the `Toggable` dev tool in the `world`. | ||
fn apply(self, world: &mut World) { | ||
T::enable(world); | ||
} | ||
} | ||
|
||
/// Command to disable a `Toggable` dev tool. | ||
#[derive(Reflect, Default)] | ||
#[reflect(DevCommand, Default)] | ||
pub struct Disable<T: Toggable + FromReflect + Send + Sync + 'static + Default> { | ||
/// PhantomData to hold the type `T`. | ||
#[reflect(ignore)] | ||
_phantom: std::marker::PhantomData<T>, | ||
} | ||
|
||
impl<T: Toggable + Send + Sync + 'static + TypePath + FromReflect + Default> DevCommand for Disable<T> {} | ||
|
||
impl<T: Toggable + FromReflect + Send + Sync + 'static + Default> Command for Disable<T> { | ||
/// Applies the disable command, disabling the `Toggable` dev tool in the `world`. | ||
fn apply(self, world: &mut World) { | ||
T::disable(world); | ||
} | ||
} |
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.
This is my favorite part, intermediate traits can be used to define commands 🎉
Thank you for reviewing the PR! The PR and RFC aim to achieve three primary objectives and one additional goal:
Additional Goal:
Reflection was preferred for several reasons:
For me these benefits make Reflection an best choice for achieving the goals. And in this PR I implemented dev console mode for Bevy to test that selected goals are achived. |
@rewin123 What's the status? Is this still meant to be a draft? |
I apologize for the long delay in replying, been on vacation. |
Objective
Implementation of bevyengine/rfcs#77 rfc
This rfc introduces the traits and concepts of DevTool and DevCommand as tools and commands that speed up the development process and are not normally built into the final build of a game. RFC target is create unified input for DevCommand creation and unified controls to enable/disable/configure DevTools. I have tried to implement this RFC in order to improve it. The implementation does not match the RFC text completely, as my solution takes into account both the RFC itself and comments to it that have not yet been corrected in the text.
PR and RFC aim to achieve three primary objectives and one additional goal:
Additional Goal:
Solution
app.register_toggable_dev_tool<T: DevTool>()
which will register in type registry DevTool and default commands for itFor starting this example use
cargo run --example dev_cli --features="bevy_dev_tools"
To try this demo you must print into your console while app is running
Try this:
disable fpsoverlay
-- will hide fps overlayenable fpsoverlay
-- will show fps overlaysetfield fpsoverlay text_config.font_size 16
-- will change font size in fps overlayprintcommands
-- will list all dev commandssetgold 100
-- will set gold amountprintgold
-- will print gold amountdisable showgold
-- will hide gold overlay (right top corner on screen)Testing
I created 21 tests for CLIDeserialization. Since this is a draft designed to improve RFC and not merge, that's enough for now. If this PR reaches a finished state, everything possible will be covered in tests.
Migration Guide
No migration needed