Skip to content

Commit

Permalink
Split bevy_hierarchy out from bevy_transform (#4168)
Browse files Browse the repository at this point in the history
# Objective

- Hierarchy tools are not just used for `Transform`: they are also used for scenes.
- In the future there's interest in using them for other features, such as visiibility inheritance.
- The fact that these tools are found in `bevy_transform` causes a great deal of user and developer confusion
- Fixes #2758.

## Solution

- Split `bevy_transform` into two!
- Make everything work again.

Note that this is a very tightly scoped PR: I *know* there are code quality and docs issues that existed in bevy_transform that I've just moved around. We should fix those in a seperate PR and try to merge this ASAP to reduce the bitrot involved in splitting an entire crate.

## Frustrations

The API around `GlobalTransform` is a mess: we have massive code and docs duplication, no link between the two types and no clear way to extend this to other forms of inheritance.

In the medium-term, I feel pretty strongly that `GlobalTransform` should be replaced by something like `Inherited<Transform>`, which lives in `bevy_hierarchy`:

- avoids code duplication
- makes the inheritance pattern extensible
- links the types at the type-level
- allows us to remove all references to inheritance from `bevy_transform`, making it more useful as a standalone crate and cleaning up its docs

## Additional context

- double-blessed by @cart in #4141 (comment) and #2758 (comment)
- preparation for more advanced / cleaner hierarchy tools: go read bevyengine/rfcs#53 !
- originally attempted by @finegeometer in #2789. It was a great idea, just needed more discussion!

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
  • Loading branch information
alice-i-cecile and cart committed Mar 15, 2022
1 parent a291b5a commit a304fd9
Show file tree
Hide file tree
Showing 29 changed files with 225 additions and 205 deletions.
1 change: 1 addition & 0 deletions crates/bevy_gltf/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ bevy_app = { path = "../bevy_app", version = "0.6.0" }
bevy_asset = { path = "../bevy_asset", version = "0.6.0" }
bevy_core = { path = "../bevy_core", version = "0.6.0" }
bevy_ecs = { path = "../bevy_ecs", version = "0.6.0" }
bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.6.0" }
bevy_pbr = { path = "../bevy_pbr", version = "0.6.0" }
bevy_reflect = { path = "../bevy_reflect", version = "0.6.0", features = ["bevy"] }
bevy_render = { path = "../bevy_render", version = "0.6.0" }
Expand Down
8 changes: 3 additions & 5 deletions crates/bevy_gltf/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use bevy_asset::{
};
use bevy_core::Name;
use bevy_ecs::world::World;
use bevy_hierarchy::{BuildWorldChildren, WorldChildBuilder};
use bevy_log::warn;
use bevy_math::{Mat4, Vec3};
use bevy_pbr::{
Expand All @@ -24,11 +25,8 @@ use bevy_render::{
view::VisibleEntities,
};
use bevy_scene::Scene;
use bevy_transform::{
hierarchy::{BuildWorldChildren, WorldChildBuilder},
prelude::Transform,
TransformBundle,
};
use bevy_transform::{components::Transform, TransformBundle};

use bevy_utils::{HashMap, HashSet};
use gltf::{
mesh::Mode,
Expand Down
19 changes: 19 additions & 0 deletions crates/bevy_hierarchy/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
[package]
name = "bevy_hierarchy"
version = "0.6.0"
edition = "2021"
description = "Provides hierarchy functionality for Bevy Engine"
homepage = "https://bevyengine.org"
repository = "https://github.com/bevyengine/bevy"
license = "MIT OR Apache-2.0"
keywords = ["bevy"]

[dependencies]
# bevy
bevy_app = { path = "../bevy_app", version = "0.6.0" }
bevy_ecs = { path = "../bevy_ecs", version = "0.6.0", features = ["bevy_reflect"] }
bevy_reflect = { path = "../bevy_reflect", version = "0.6.0", features = ["bevy"] }
bevy_utils = { path = "../bevy_utils", version = "0.6.0" }

# other
smallvec = { version = "1.6", features = ["serde", "union", "const_generics"] }
File renamed without changes.
5 changes: 5 additions & 0 deletions crates/bevy_hierarchy/src/components/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
mod children;
mod parent;

pub use children::Children;
pub use parent::{Parent, PreviousParent};
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ mod tests {
};

use super::DespawnRecursiveExt;
use crate::{components::Children, hierarchy::BuildChildren};
use crate::{child_builder::BuildChildren, components::Children};

#[derive(Component, Clone, Copy, PartialEq, Eq, Ord, PartialOrd, Debug)]
struct Idx(u32);
Expand Down
53 changes: 53 additions & 0 deletions crates/bevy_hierarchy/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
#![warn(missing_docs)]
//! `bevy_hierarchy` can be used to define hierarchies of entities.
//!
//! Most commonly, these hierarchies are used for inheriting `Transform` values
//! from the [`Parent`] to its [`Children`].

mod components;
pub use components::*;

mod hierarchy;
pub use hierarchy::*;

mod child_builder;
pub use child_builder::*;

mod systems;
pub use systems::*;

#[doc(hidden)]
pub mod prelude {
#[doc(hidden)]
pub use crate::{child_builder::*, components::*, hierarchy::*, HierarchyPlugin};
}

use bevy_app::prelude::*;
use bevy_ecs::prelude::*;

/// The base plugin for handling [`Parent`] and [`Children`] components
#[derive(Default)]
pub struct HierarchyPlugin;

/// Label enum for the systems relating to hierarchy upkeep
#[derive(Debug, Hash, PartialEq, Eq, Clone, SystemLabel)]
pub enum HierarchySystem {
/// Updates [`Parent`] when changes in the hierarchy occur
ParentUpdate,
}

impl Plugin for HierarchyPlugin {
fn build(&self, app: &mut App) {
app.register_type::<Children>()
.register_type::<Parent>()
.register_type::<PreviousParent>()
.add_startup_system_to_stage(
StartupStage::PostStartup,
parent_update_system.label(HierarchySystem::ParentUpdate),
)
.add_system_to_stage(
CoreStage::PostUpdate,
parent_update_system.label(HierarchySystem::ParentUpdate),
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,101 +70,3 @@ pub fn parent_update_system(
commands.entity(*e).insert(Children::with(v));
});
}
#[cfg(test)]
mod test {
use bevy_ecs::{
schedule::{Schedule, Stage, SystemStage},
system::CommandQueue,
world::World,
};

use super::*;
use crate::{hierarchy::BuildChildren, transform_propagate_system::transform_propagate_system};

#[test]
fn correct_children() {
let mut world = World::default();

let mut update_stage = SystemStage::parallel();
update_stage.add_system(parent_update_system);
update_stage.add_system(transform_propagate_system);

let mut schedule = Schedule::default();
schedule.add_stage("update", update_stage);

// Add parent entities
let mut command_queue = CommandQueue::default();
let mut commands = Commands::new(&mut command_queue, &world);
let mut children = Vec::new();
let parent = commands
.spawn()
.insert(Transform::from_xyz(1.0, 0.0, 0.0))
.id();
commands.entity(parent).with_children(|parent| {
children.push(
parent
.spawn()
.insert(Transform::from_xyz(0.0, 2.0, 0.0))
.id(),
);
children.push(
parent
.spawn()
.insert(Transform::from_xyz(0.0, 3.0, 0.0))
.id(),
);
});
command_queue.apply(&mut world);
schedule.run(&mut world);

assert_eq!(
world
.get::<Children>(parent)
.unwrap()
.0
.iter()
.cloned()
.collect::<Vec<_>>(),
children,
);

// Parent `e1` to `e2`.
(*world.get_mut::<Parent>(children[0]).unwrap()).0 = children[1];

schedule.run(&mut world);

assert_eq!(
world
.get::<Children>(parent)
.unwrap()
.iter()
.cloned()
.collect::<Vec<_>>(),
vec![children[1]]
);

assert_eq!(
world
.get::<Children>(children[1])
.unwrap()
.iter()
.cloned()
.collect::<Vec<_>>(),
vec![children[0]]
);

assert!(world.despawn(children[0]));

schedule.run(&mut world);

assert_eq!(
world
.get::<Children>(parent)
.unwrap()
.iter()
.cloned()
.collect::<Vec<_>>(),
vec![children[1]]
);
}
}
1 change: 1 addition & 0 deletions crates/bevy_internal/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ bevy_core = { path = "../bevy_core", version = "0.6.0" }
bevy_derive = { path = "../bevy_derive", version = "0.6.0" }
bevy_diagnostic = { path = "../bevy_diagnostic", version = "0.6.0" }
bevy_ecs = { path = "../bevy_ecs", version = "0.6.0" }
bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.6.0" }
bevy_input = { path = "../bevy_input", version = "0.6.0" }
bevy_log = { path = "../bevy_log", version = "0.6.0" }
bevy_math = { path = "../bevy_math", version = "0.6.0" }
Expand Down
2 changes: 2 additions & 0 deletions crates/bevy_internal/src/default_plugins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use bevy_app::{PluginGroup, PluginGroupBuilder};
/// * [`LogPlugin`](bevy_log::LogPlugin)
/// * [`CorePlugin`](bevy_core::CorePlugin)
/// * [`TransformPlugin`](bevy_transform::TransformPlugin)
/// * [`HierarchyPlugin`](bevy_hierarchy::HierarchyPlugin)
/// * [`DiagnosticsPlugin`](bevy_diagnostic::DiagnosticsPlugin)
/// * [`InputPlugin`](bevy_input::InputPlugin)
/// * [`WindowPlugin`](bevy_window::WindowPlugin)
Expand All @@ -27,6 +28,7 @@ impl PluginGroup for DefaultPlugins {
group.add(bevy_log::LogPlugin::default());
group.add(bevy_core::CorePlugin::default());
group.add(bevy_transform::TransformPlugin::default());
group.add(bevy_hierarchy::HierarchyPlugin::default());
group.add(bevy_diagnostic::DiagnosticsPlugin::default());
group.add(bevy_input::InputPlugin::default());
group.add(bevy_window::WindowPlugin::default());
Expand Down
5 changes: 5 additions & 0 deletions crates/bevy_internal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ pub mod tasks {
pub use bevy_tasks::*;
}

pub mod hierarchy {
//! Entity hierarchies and property inheritance
pub use bevy_hierarchy::*;
}

pub mod transform {
//! Local and global transforms (e.g. translation, scale, rotation).
pub use bevy_transform::*;
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_internal/src/prelude.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#[doc(hidden)]
pub use crate::{
app::prelude::*, asset::prelude::*, core::prelude::*, ecs::prelude::*, input::prelude::*,
log::prelude::*, math::prelude::*, reflect::prelude::*, scene::prelude::*,
app::prelude::*, asset::prelude::*, core::prelude::*, ecs::prelude::*, hierarchy::prelude::*,
input::prelude::*, log::prelude::*, math::prelude::*, reflect::prelude::*, scene::prelude::*,
transform::prelude::*, utils::prelude::*, window::prelude::*, DefaultPlugins, MinimalPlugins,
};

Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_render/src/view/visibility/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use bevy_app::{CoreStage, Plugin};
use bevy_asset::{Assets, Handle};
use bevy_ecs::prelude::*;
use bevy_reflect::Reflect;
use bevy_transform::{components::GlobalTransform, TransformSystem};
use bevy_transform::components::GlobalTransform;
use bevy_transform::TransformSystem;

use crate::{
camera::{Camera, CameraProjection, OrthographicProjection, PerspectiveProjection},
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_scene/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ bevy_app = { path = "../bevy_app", version = "0.6.0" }
bevy_asset = { path = "../bevy_asset", version = "0.6.0" }
bevy_ecs = { path = "../bevy_ecs", version = "0.6.0" }
bevy_reflect = { path = "../bevy_reflect", version = "0.6.0", features = ["bevy"] }
bevy_transform = { path = "../bevy_transform", version = "0.6.0" }
bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.6.0" }
bevy_utils = { path = "../bevy_utils", version = "0.6.0" }

# other
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_scene/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use bevy_ecs::{
system::{Command, Commands},
world::World,
};
use bevy_transform::hierarchy::ChildBuilder;
use bevy_hierarchy::ChildBuilder;

use crate::{Scene, SceneSpawner};

Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_scene/src/scene_spawner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use bevy_ecs::{
system::Command,
world::{Mut, World},
};
use bevy_hierarchy::{AddChild, Parent};
use bevy_reflect::TypeRegistryArc;
use bevy_transform::{hierarchy::AddChild, prelude::Parent};
use bevy_utils::{tracing::error, HashMap};
use thiserror::Error;
use uuid::Uuid;
Expand Down
24 changes: 0 additions & 24 deletions crates/bevy_transform/.gitignore

This file was deleted.

7 changes: 2 additions & 5 deletions crates/bevy_transform/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
name = "bevy_transform"
version = "0.6.0"
edition = "2021"
description = "Provides hierarchy and transform functionality for Bevy Engine"
description = "Provides transform functionality for Bevy Engine"
homepage = "https://bevyengine.org"
repository = "https://github.com/bevyengine/bevy"
license = "MIT OR Apache-2.0"
Expand All @@ -12,9 +12,6 @@ keywords = ["bevy"]
# bevy
bevy_app = { path = "../bevy_app", version = "0.6.0" }
bevy_ecs = { path = "../bevy_ecs", version = "0.6.0", features = ["bevy_reflect"] }
bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.6.0"}
bevy_math = { path = "../bevy_math", version = "0.6.0" }
bevy_reflect = { path = "../bevy_reflect", version = "0.6.0", features = ["bevy"] }
bevy_utils = { path = "../bevy_utils", version = "0.6.0" }

# other
smallvec = { version = "1.6", features = ["serde", "union", "const_generics"] }
4 changes: 2 additions & 2 deletions crates/bevy_transform/src/components/global_transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ use std::ops::Mul;
/// ## [`Transform`] and [`GlobalTransform`]
///
/// [`Transform`] is the position of an entity relative to its parent position, or the reference
/// frame if it doesn't have a [`Parent`](super::Parent).
/// frame if it doesn't have a [`Parent`](bevy_hierarchy::Parent).
///
/// [`GlobalTransform`] is the position of an entity relative to the reference frame.
///
/// [`GlobalTransform`] is updated from [`Transform`] in the system
/// [`transform_propagate_system`](crate::transform_propagate_system::transform_propagate_system).
/// [`transform_propagate_system`](crate::transform_propagate_system).
///
/// This system runs in stage [`CoreStage::PostUpdate`](crate::CoreStage::PostUpdate). If you
/// update the[`Transform`] of an entity in this stage or after, you will notice a 1 frame lag
Expand Down
4 changes: 0 additions & 4 deletions crates/bevy_transform/src/components/mod.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
mod children;
mod global_transform;
mod parent;
mod transform;

pub use children::Children;
pub use global_transform::*;
pub use parent::{Parent, PreviousParent};
pub use transform::*;
4 changes: 2 additions & 2 deletions crates/bevy_transform/src/components/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ use std::ops::Mul;
/// ## [`Transform`] and [`GlobalTransform`]
///
/// [`Transform`] is the position of an entity relative to its parent position, or the reference
/// frame if it doesn't have a [`Parent`](super::Parent).
/// frame if it doesn't have a [`Parent`](bevy_hierarchy::Parent).
///
/// [`GlobalTransform`] is the position of an entity relative to the reference frame.
///
/// [`GlobalTransform`] is updated from [`Transform`] in the system
/// [`transform_propagate_system`](crate::transform_propagate_system::transform_propagate_system).
/// [`transform_propagate_system`](crate::transform_propagate_system).
///
/// This system runs in stage [`CoreStage::PostUpdate`](crate::CoreStage::PostUpdate). If you
/// update the[`Transform`] of an entity in this stage or after, you will notice a 1 frame lag
Expand Down
8 changes: 0 additions & 8 deletions crates/bevy_transform/src/hierarchy/mod.rs

This file was deleted.

Loading

0 comments on commit a304fd9

Please sign in to comment.