Skip to content

Move aabb gizmos to bevy_dev_tools #12354

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Move aabb gizmos to bevy_dev_tools #12354

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9de5b0b
Move Aabb gizmos to `bevy_dev_tools`
chompaa Mar 6, 2024
fa72006
Fix wrong variable name
chompaa Mar 6, 2024
4716a5d
Rename aabb -> aabb_gizmo
chompaa Mar 7, 2024
0c2fa0e
Fix module name
chompaa Mar 7, 2024
38df5e1
Update dependencies
chompaa Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions crates/bevy_gizmos/src/aabb.rs → crates/bevy_dev_tools/src/aabb.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
//! A module adding debug visualization of [`Aabb`]s.

use crate as bevy_gizmos;
//! Debug visualization tools of [`Aabb`].

use bevy_app::{Plugin, PostUpdate};
use bevy_color::{Color, Oklcha};
Expand All @@ -12,19 +10,18 @@ use bevy_ecs::{
schedule::IntoSystemConfigs,
system::{Query, Res},
};
use bevy_gizmos::{
config::{GizmoConfigGroup, GizmoConfigStore},
gizmos::Gizmos,
AppGizmoBuilder,
};
use bevy_reflect::{std_traits::ReflectDefault, Reflect};
use bevy_render::primitives::Aabb;
use bevy_transform::{
components::{GlobalTransform, Transform},
TransformSystem,
};

use crate::{
config::{GizmoConfigGroup, GizmoConfigStore},
gizmos::Gizmos,
AppGizmoBuilder,
};

/// A [`Plugin`] that provides visualization of [`Aabb`]s for debugging.
pub struct AabbGizmoPlugin;

Expand Down
9 changes: 7 additions & 2 deletions crates/bevy_dev_tools/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
//! This crate provides additional utilities for the [Bevy game engine](https://bevyengine.org),
//! focused on improving developer experience.

use bevy_app::prelude::*;
pub mod aabb;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this probably makes mores sense as aabb_gizmo or something here :)

#[cfg(feature = "bevy_ci_testing")]
pub mod ci_testing;

use aabb::AabbGizmoPlugin;
use bevy_app::prelude::*;

/// Enables developer tools in an [`App`]. This plugin is added automatically with `bevy_dev_tools`
/// feature.
///
Expand Down Expand Up @@ -35,7 +38,9 @@ pub mod ci_testing;
pub struct DevToolsPlugin;

impl Plugin for DevToolsPlugin {
fn build(&self, _app: &mut App) {
fn build(&self, app: &mut App) {
app.add_plugins(AabbGizmoPlugin);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, for now, we should hide it behind a feature and change it later when we create some other abstraction for toggling specific tools

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. We have to create this abstraction soon, though, the amount of created features can be very high if we don't handle with care.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can do DevToolsPlugin into a PluginGroup, to make it easy disable and enable things, but I'm not sure how this will play with DefaultPlugins though

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've thought about it too, but wouldn't it then be impossible to change dev tools during runtime? I also thought about making a config as a resource but there might be a better way

Copy link
Contributor

@pablo-lua pablo-lua Mar 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is a way at disabling this at runtime right now, at least not in the way we are currently doing
we add plugins to the app after all, is there a way to currently disabling a plugin, adding a plugin, or even adding systems at runtime right now? If we want, we can at least make it into a group now and mitigate the effects I guess, at least while we don't have the runtime tool manager

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we can't add/remove plugins at runtime (see #11083), but maybe we could achieve something without plugins? I think we should create an issue regarding this to decide which direction we want to go for now


#[cfg(feature = "bevy_ci_testing")]
{
ci_testing::setup_app(_app);
Expand Down
4 changes: 0 additions & 4 deletions crates/bevy_gizmos/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ pub enum GizmoRenderSystem {
QueueLineGizmos3d,
}

pub mod aabb;
pub mod arcs;
pub mod arrows;
pub mod circles;
Expand All @@ -44,7 +43,6 @@ mod pipeline_3d;
pub mod prelude {
#[doc(hidden)]
pub use crate::{
aabb::{AabbGizmoConfigGroup, ShowAabbGizmo},
config::{DefaultGizmoConfigGroup, GizmoConfig, GizmoConfigGroup, GizmoConfigStore},
gizmos::Gizmos,
light::{LightGizmoColor, LightGizmoConfigGroup, ShowLightGizmo},
Expand All @@ -53,7 +51,6 @@ pub mod prelude {
};
}

use aabb::AabbGizmoPlugin;
use bevy_app::{App, Last, Plugin};
use bevy_asset::{load_internal_asset, Asset, AssetApp, Assets, Handle};
use bevy_color::LinearRgba;
Expand Down Expand Up @@ -113,7 +110,6 @@ impl Plugin for GizmoPlugin {
.init_resource::<LineGizmoHandles>()
// We insert the Resource GizmoConfigStore into the world implicitly here if it does not exist.
.init_gizmo_group::<DefaultGizmoConfigGroup>()
.add_plugins(AabbGizmoPlugin)
.add_plugins(LightGizmoPlugin);

let Ok(render_app) = app.get_sub_app_mut(RenderApp) else {
Expand Down