Skip to content

Commit 9b32e09

Browse files
authored
bevy_reflect: Add clone registrations project-wide (#18307)
# Objective Now that #13432 has been merged, it's important we update our reflected types to properly opt into this feature. If we do not, then this could cause issues for users downstream who want to make use of reflection-based cloning. ## Solution This PR is broken into 4 commits: 1. Add `#[reflect(Clone)]` on all types marked `#[reflect(opaque)]` that are also `Clone`. This is mandatory as these types would otherwise cause the cloning operation to fail for any type that contains it at any depth. 2. Update the reflection example to suggest adding `#[reflect(Clone)]` on opaque types. 3. Add `#[reflect(clone)]` attributes on all fields marked `#[reflect(ignore)]` that are also `Clone`. This prevents the ignored field from causing the cloning operation to fail. Note that some of the types that contain these fields are also `Clone`, and thus can be marked `#[reflect(Clone)]`. This makes the `#[reflect(clone)]` attribute redundant. However, I think it's safer to keep it marked in the case that the `Clone` impl/derive is ever removed. I'm open to removing them, though, if people disagree. 4. Finally, I added `#[reflect(Clone)]` on all types that are also `Clone`. While not strictly necessary, it enables us to reduce the generated output since we can just call `Clone::clone` directly instead of calling `PartialReflect::reflect_clone` on each variant/field. It also means we benefit from any optimizations or customizations made in the `Clone` impl, including directly dereferencing `Copy` values and increasing reference counters. Along with that change I also took the liberty of adding any missing registrations that I saw could be applied to the type as well, such as `Default`, `PartialEq`, and `Hash`. There were hundreds of these to edit, though, so it's possible I missed quite a few. That last commit is **_massive_**. There were nearly 700 types to update. So it's recommended to review the first three before moving onto that last one. Additionally, I can break the last commit off into its own PR or into smaller PRs, but I figured this would be the easiest way of doing it (and in a timely manner since I unfortunately don't have as much time as I used to for code contributions). ## Testing You can test locally with a `cargo check`: ``` cargo check --workspace --all-features ```
1 parent e61b5a1 commit 9b32e09

File tree

189 files changed

+999
-506
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

189 files changed

+999
-506
lines changed

crates/bevy_a11y/src/lib.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,11 @@ pub struct ActionRequest(pub accesskit::ActionRequest);
5454
/// Useful if a third-party plugin needs to conditionally integrate with
5555
/// `AccessKit`
5656
#[derive(Resource, Default, Clone, Debug, Deref, DerefMut)]
57-
#[cfg_attr(feature = "bevy_reflect", derive(Reflect), reflect(Default, Resource))]
57+
#[cfg_attr(
58+
feature = "bevy_reflect",
59+
derive(Reflect),
60+
reflect(Default, Clone, Resource)
61+
)]
5862
pub struct AccessibilityRequested(Arc<AtomicBool>);
5963

6064
impl AccessibilityRequested {
@@ -78,7 +82,11 @@ impl AccessibilityRequested {
7882
/// will generate conflicting updates.
7983
#[derive(Resource, Clone, Debug, Deref, DerefMut)]
8084
#[cfg_attr(feature = "serialize", derive(Serialize, Deserialize))]
81-
#[cfg_attr(feature = "bevy_reflect", derive(Reflect), reflect(Resource))]
85+
#[cfg_attr(
86+
feature = "bevy_reflect",
87+
derive(Reflect),
88+
reflect(Resource, Clone, Default)
89+
)]
8290
#[cfg_attr(
8391
all(feature = "bevy_reflect", feature = "serialize"),
8492
reflect(Serialize, Deserialize)
@@ -127,7 +135,7 @@ impl From<Node> for AccessibilityNode {
127135
#[cfg_attr(feature = "bevy_reflect", derive(Reflect))]
128136
#[cfg_attr(
129137
all(feature = "bevy_reflect", feature = "serialize"),
130-
reflect(Serialize, Deserialize)
138+
reflect(Serialize, Deserialize, Clone)
131139
)]
132140
pub enum AccessibilitySystem {
133141
/// Update the accessibility tree

crates/bevy_animation/src/gltf_curves.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ impl<T> CubicKeyframeCurve<T> {
111111
/// A keyframe-defined curve that uses cubic spline interpolation, special-cased for quaternions
112112
/// since it uses `Vec4` internally.
113113
#[derive(Debug, Clone, Reflect)]
114+
#[reflect(Clone)]
114115
pub struct CubicRotationCurve {
115116
// Note: The sample width here should be 3.
116117
core: ChunkedUnevenCore<Vec4>,
@@ -374,6 +375,7 @@ impl<T> WideCubicKeyframeCurve<T> {
374375
///
375376
/// [`MorphWeights`]: bevy_render::prelude::MorphWeights
376377
#[derive(Debug, Clone, Reflect)]
378+
#[reflect(Clone)]
377379
pub enum WeightsCurve {
378380
/// A curve which takes a constant value over its domain. Notably, this is how animations with
379381
/// only a single keyframe are interpreted.

crates/bevy_animation/src/graph.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ use crate::{AnimationClip, AnimationTargetId};
108108
///
109109
/// [RFC 51]: https://github.com/bevyengine/rfcs/blob/main/rfcs/51-animation-composition.md
110110
#[derive(Asset, Reflect, Clone, Debug, Serialize)]
111-
#[reflect(Serialize, Debug)]
111+
#[reflect(Serialize, Debug, Clone)]
112112
#[serde(into = "SerializedAnimationGraph")]
113113
pub struct AnimationGraph {
114114
/// The `petgraph` data structure that defines the animation graph.
@@ -131,7 +131,7 @@ pub struct AnimationGraph {
131131

132132
/// A [`Handle`] to the [`AnimationGraph`] to be used by the [`AnimationPlayer`](crate::AnimationPlayer) on the same entity.
133133
#[derive(Component, Clone, Debug, Default, Deref, DerefMut, Reflect, PartialEq, Eq, From)]
134-
#[reflect(Component, Default)]
134+
#[reflect(Component, Default, Clone)]
135135
pub struct AnimationGraphHandle(pub Handle<AnimationGraph>);
136136

137137
impl From<AnimationGraphHandle> for AssetId<AnimationGraph> {
@@ -164,6 +164,7 @@ pub type AnimationNodeIndex = NodeIndex<u32>;
164164
/// of the graph, contain animation clips to play. Blend and add nodes describe
165165
/// how to combine their children to produce a final animation.
166166
#[derive(Clone, Reflect, Debug)]
167+
#[reflect(Clone)]
167168
pub struct AnimationGraphNode {
168169
/// Animation node data specific to the type of node (clip, blend, or add).
169170
///
@@ -205,6 +206,7 @@ pub struct AnimationGraphNode {
205206
/// In the case of clip nodes, this contains the actual animation clip
206207
/// associated with the node.
207208
#[derive(Clone, Default, Reflect, Debug)]
209+
#[reflect(Clone)]
208210
pub enum AnimationNodeType {
209211
/// A *clip node*, which plays an animation clip.
210212
///

crates/bevy_animation/src/lib.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,23 +96,26 @@ impl VariableCurve {
9696
/// Because animation clips refer to targets by UUID, they can target any
9797
/// [`AnimationTarget`] with that ID.
9898
#[derive(Asset, Reflect, Clone, Debug, Default)]
99+
#[reflect(Clone, Default)]
99100
pub struct AnimationClip {
100101
// This field is ignored by reflection because AnimationCurves can contain things that are not reflect-able
101-
#[reflect(ignore)]
102+
#[reflect(ignore, clone)]
102103
curves: AnimationCurves,
103104
events: AnimationEvents,
104105
duration: f32,
105106
}
106107

107108
#[derive(Reflect, Debug, Clone)]
109+
#[reflect(Clone)]
108110
struct TimedAnimationEvent {
109111
time: f32,
110112
event: AnimationEvent,
111113
}
112114

113115
#[derive(Reflect, Debug, Clone)]
116+
#[reflect(Clone)]
114117
struct AnimationEvent {
115-
#[reflect(ignore)]
118+
#[reflect(ignore, clone)]
116119
trigger: AnimationEventFn,
117120
}
118121

@@ -124,6 +127,7 @@ impl AnimationEvent {
124127

125128
#[derive(Reflect, Clone)]
126129
#[reflect(opaque)]
130+
#[reflect(Clone, Default, Debug)]
127131
struct AnimationEventFn(Arc<dyn Fn(&mut Commands, Entity, f32, f32) + Send + Sync>);
128132

129133
impl Default for AnimationEventFn {
@@ -139,6 +143,7 @@ impl Debug for AnimationEventFn {
139143
}
140144

141145
#[derive(Reflect, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone)]
146+
#[reflect(Clone)]
142147
enum AnimationEventTarget {
143148
Root,
144149
Node(AnimationTargetId),
@@ -172,6 +177,7 @@ pub type AnimationCurves = HashMap<AnimationTargetId, Vec<VariableCurve>, NoOpHa
172177
///
173178
/// [UUID]: https://en.wikipedia.org/wiki/Universally_unique_identifier
174179
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Reflect, Debug, Serialize, Deserialize)]
180+
#[reflect(Clone)]
175181
pub struct AnimationTargetId(pub Uuid);
176182

177183
impl Hash for AnimationTargetId {
@@ -203,7 +209,7 @@ impl Hash for AnimationTargetId {
203209
/// time. However, you can change [`AnimationTarget`]'s `player` property at
204210
/// runtime to change which player is responsible for animating the entity.
205211
#[derive(Clone, Copy, Component, Reflect)]
206-
#[reflect(Component)]
212+
#[reflect(Component, Clone)]
207213
pub struct AnimationTarget {
208214
/// The ID of this animation target.
209215
///
@@ -425,6 +431,7 @@ impl AnimationClip {
425431

426432
/// Repetition behavior of an animation.
427433
#[derive(Reflect, Debug, PartialEq, Eq, Copy, Clone, Default)]
434+
#[reflect(Clone, Default)]
428435
pub enum RepeatAnimation {
429436
/// The animation will finish after running once.
430437
#[default]
@@ -462,6 +469,7 @@ pub enum AnimationEvaluationError {
462469
///
463470
/// A stopped animation is considered no longer active.
464471
#[derive(Debug, Clone, Copy, Reflect)]
472+
#[reflect(Clone, Default)]
465473
pub struct ActiveAnimation {
466474
/// The factor by which the weight from the [`AnimationGraph`] is multiplied.
467475
weight: f32,
@@ -674,7 +682,7 @@ impl ActiveAnimation {
674682
/// Automatically added to any root animations of a scene when it is
675683
/// spawned.
676684
#[derive(Component, Default, Reflect)]
677-
#[reflect(Component, Default)]
685+
#[reflect(Component, Default, Clone)]
678686
pub struct AnimationPlayer {
679687
active_animations: HashMap<AnimationNodeIndex, ActiveAnimation>,
680688
blend_weights: HashMap<AnimationNodeIndex, f32>,

crates/bevy_animation/src/transition.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use crate::{graph::AnimationNodeIndex, ActiveAnimation, AnimationPlayer};
2929
/// component to get confused about which animation is the "main" animation, and
3030
/// transitions will usually be incorrect as a result.
3131
#[derive(Component, Default, Reflect)]
32-
#[reflect(Component, Default)]
32+
#[reflect(Component, Default, Clone)]
3333
pub struct AnimationTransitions {
3434
main_animation: Option<AnimationNodeIndex>,
3535
transitions: Vec<AnimationTransition>,
@@ -52,6 +52,7 @@ impl Clone for AnimationTransitions {
5252

5353
/// An animation that is being faded out as part of a transition
5454
#[derive(Debug, Clone, Copy, Reflect)]
55+
#[reflect(Clone)]
5556
pub struct AnimationTransition {
5657
/// The current weight. Starts at 1.0 and goes to 0.0 during the fade-out.
5758
current_weight: f32,

crates/bevy_asset/src/handle.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ impl core::fmt::Debug for StrongHandle {
129129
///
130130
/// [`Handle::Strong`], via [`StrongHandle`] also provides access to useful [`Asset`] metadata, such as the [`AssetPath`] (if it exists).
131131
#[derive(Reflect)]
132-
#[reflect(Default, Debug, Hash, PartialEq)]
132+
#[reflect(Default, Debug, Hash, PartialEq, Clone)]
133133
pub enum Handle<A: Asset> {
134134
/// A "strong" reference to a live (or loading) [`Asset`]. If a [`Handle`] is [`Handle::Strong`], the [`Asset`] will be kept
135135
/// alive until the [`Handle`] is dropped. Strong handles also provide access to additional asset metadata.

crates/bevy_asset/src/id.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::{Asset, AssetIndex};
2-
use bevy_reflect::Reflect;
2+
use bevy_reflect::{std_traits::ReflectDefault, Reflect};
33
use serde::{Deserialize, Serialize};
44
use uuid::Uuid;
55

@@ -19,6 +19,7 @@ use thiserror::Error;
1919
///
2020
/// For an "untyped" / "generic-less" id, see [`UntypedAssetId`].
2121
#[derive(Reflect, Serialize, Deserialize, From)]
22+
#[reflect(Clone, Default, Debug, PartialEq, Hash)]
2223
pub enum AssetId<A: Asset> {
2324
/// A small / efficient runtime identifier that can be used to efficiently look up an asset stored in [`Assets`]. This is
2425
/// the "default" identifier used for assets. The alternative(s) (ex: [`AssetId::Uuid`]) will only be used if assets are
@@ -29,7 +30,7 @@ pub enum AssetId<A: Asset> {
2930
/// The unstable, opaque index of the asset.
3031
index: AssetIndex,
3132
/// A marker to store the type information of the asset.
32-
#[reflect(ignore)]
33+
#[reflect(ignore, clone)]
3334
marker: PhantomData<fn() -> A>,
3435
},
3536
/// A stable-across-runs / const asset identifier. This will only be used if an asset is explicitly registered in [`Assets`]

crates/bevy_asset/src/path.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ use thiserror::Error;
5353
/// This also means that you should use [`AssetPath::parse`] in cases where `&str` is the explicit type.
5454
#[derive(Eq, PartialEq, Hash, Clone, Default, Reflect)]
5555
#[reflect(opaque)]
56-
#[reflect(Debug, PartialEq, Hash, Serialize, Deserialize)]
56+
#[reflect(Debug, PartialEq, Hash, Clone, Serialize, Deserialize)]
5757
pub struct AssetPath<'a> {
5858
source: AssetSourceId<'a>,
5959
path: CowArc<'a, Path>,

crates/bevy_asset/src/render_asset.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ bitflags::bitflags! {
2727
#[repr(transparent)]
2828
#[derive(Serialize, Deserialize, Hash, Clone, Copy, PartialEq, Eq, Debug, Reflect)]
2929
#[reflect(opaque)]
30-
#[reflect(Serialize, Deserialize, Hash, PartialEq, Debug)]
30+
#[reflect(Serialize, Deserialize, Hash, Clone, PartialEq, Debug)]
3131
pub struct RenderAssetUsages: u8 {
3232
/// The bit flag for the main world.
3333
const MAIN_WORLD = 1 << 0;

crates/bevy_audio/src/audio.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use bevy_reflect::prelude::*;
66

77
/// The way Bevy manages the sound playback.
88
#[derive(Debug, Clone, Copy, Reflect)]
9+
#[reflect(Clone)]
910
pub enum PlaybackMode {
1011
/// Play the sound once. Do nothing when it ends.
1112
///
@@ -29,7 +30,7 @@ pub enum PlaybackMode {
2930
/// [`AudioSink`][crate::AudioSink] or [`SpatialAudioSink`][crate::SpatialAudioSink]
3031
/// components. Changes to this component will *not* be applied to already-playing audio.
3132
#[derive(Component, Clone, Copy, Debug, Reflect)]
32-
#[reflect(Default, Component, Debug)]
33+
#[reflect(Clone, Default, Component, Debug)]
3334
pub struct PlaybackSettings {
3435
/// The desired playback behavior.
3536
pub mode: PlaybackMode,
@@ -142,7 +143,7 @@ impl PlaybackSettings {
142143
/// This must be accompanied by `Transform` and `GlobalTransform`.
143144
/// Only one entity with a `SpatialListener` should be present at any given time.
144145
#[derive(Component, Clone, Debug, Reflect)]
145-
#[reflect(Default, Component, Debug)]
146+
#[reflect(Clone, Default, Component, Debug)]
146147
pub struct SpatialListener {
147148
/// Left ear position relative to the `GlobalTransform`.
148149
pub left_ear_offset: Vec3,
@@ -174,6 +175,7 @@ impl SpatialListener {
174175
///
175176
/// Default is `Vec3::ONE`.
176177
#[derive(Clone, Copy, Debug, Reflect)]
178+
#[reflect(Clone, Default)]
177179
pub struct SpatialScale(pub Vec3);
178180

179181
impl SpatialScale {
@@ -202,7 +204,7 @@ impl Default for SpatialScale {
202204
///
203205
/// Default is `Vec3::ONE`.
204206
#[derive(Resource, Default, Clone, Copy, Reflect)]
205-
#[reflect(Resource, Default)]
207+
#[reflect(Resource, Default, Clone)]
206208
pub struct DefaultSpatialScale(pub SpatialScale);
207209

208210
/// A component for playing a sound.
@@ -218,7 +220,7 @@ pub struct DefaultSpatialScale(pub SpatialScale);
218220
/// Playback can be configured using the [`PlaybackSettings`] component. Note that changes to the
219221
/// `PlaybackSettings` component will *not* affect already-playing audio.
220222
#[derive(Component, Reflect)]
221-
#[reflect(Component)]
223+
#[reflect(Component, Clone)]
222224
#[require(PlaybackSettings)]
223225
pub struct AudioPlayer<Source = AudioSource>(pub Handle<Source>)
224226
where

0 commit comments

Comments
 (0)