Skip to content
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

Jump to bottom

Make bevy_time optionally depend on bevy_reflect #13263

Merged
merged 1 commit into from
May 12, 2024
Merged

Make bevy_time optionally depend on bevy_reflect #13263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/bevy_time/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ license = "MIT OR Apache-2.0"
keywords = ["bevy"]

[features]
default = []
default = ["bevy_reflect"]
serialize = ["serde"]

[dependencies]
Expand All @@ -20,7 +20,7 @@ bevy_ecs = { path = "../bevy_ecs", version = "0.14.0-dev", features = [
] }
bevy_reflect = { path = "../bevy_reflect", version = "0.14.0-dev", features = [
"bevy",
] }
], optional = true }
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a feature for enabling this dependency? And to control the feature flag for bevy_ecs? Right now it looks like bevy_ecs will always enable bevy_reflect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean something like adding a bevy_reflect feature to the root workspace that will enable another bevy_reflect feature in bevy_internal?

I was wondering why some crates have an optional dependency on bevy_reflect crate while enabling it in their defaults.

I'm new to contributing to bevy.

Copy link
Contributor

@notmd notmd May 7, 2024 •
edited

Choose a reason for hiding this comment

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

You need to add this to the cargo file. Assume the feature is called bevy_reflect. However, I think it should be renamed to reflect for short. Nvm, I just check the bevy_ecs, it also called bevy_reflect

[features]
default = ["dep:bevy_reflect"]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, I've just added it! Though, I noticed bevy_app also uses bevy_reflect by default.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah It need to be change to default = ["bevy_reflect"] instead. Sorry for the bad suggestion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, no problem! I'm changing the feature name right now.

bevy_utils = { path = "../bevy_utils", version = "0.14.0-dev" }

# other
Expand Down
4 changes: 3 additions & 1 deletion crates/bevy_time/src/fixed.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use bevy_app::FixedMain;
use bevy_ecs::world::World;
#[cfg(feature = "bevy_reflect")]
use bevy_reflect::Reflect;
use bevy_utils::Duration;

Expand Down Expand Up @@ -63,7 +64,8 @@ use crate::{time::Time, virt::Virtual};
/// [`FixedUpdate`](bevy_app::FixedUpdate), even if it is still during the same
/// frame. Any [`overstep()`](Time::overstep) present in the accumulator will be
/// processed according to the new [`timestep()`](Time::timestep) value.
#[derive(Debug, Copy, Clone, Reflect)]
#[derive(Debug, Copy, Clone)]
#[cfg_attr(feature = "bevy_reflect", derive(Reflect))]
pub struct Fixed {
timestep: Duration,
overstep: Duration,
Expand Down
19 changes: 12 additions & 7 deletions crates/bevy_time/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,18 @@ impl Plugin for TimePlugin {
.init_resource::<Time<Real>>()
.init_resource::<Time<Virtual>>()
.init_resource::<Time<Fixed>>()
.init_resource::<TimeUpdateStrategy>()
.register_type::<Time>()
.register_type::<Time<Real>>()
.register_type::<Time<Virtual>>()
.register_type::<Time<Fixed>>()
.register_type::<Timer>()
.add_systems(First, time_system.in_set(TimeSystem))
.init_resource::<TimeUpdateStrategy>();

#[cfg(feature = "bevy_reflect")]
{
app.register_type::<Time>()
.register_type::<Time<Real>>()
.register_type::<Time<Virtual>>()
.register_type::<Time<Fixed>>()
.register_type::<Timer>();
}

app.add_systems(First, time_system.in_set(TimeSystem))
.add_systems(RunFixedMainLoop, run_fixed_main_schedule);

// ensure the events are not dropped until `FixedMain` systems can observe them
Expand Down
4 changes: 3 additions & 1 deletion crates/bevy_time/src/real.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#[cfg(feature = "bevy_reflect")]
use bevy_reflect::Reflect;
use bevy_utils::{Duration, Instant};

Expand Down Expand Up @@ -28,7 +29,8 @@ use crate::time::Time;
/// [`Instant`]s for [`startup()`](Time::startup),
/// [`first_update()`](Time::first_update) and
/// [`last_update()`](Time::last_update) are recorded and accessible.
#[derive(Debug, Copy, Clone, Reflect)]
#[derive(Debug, Copy, Clone)]
#[cfg_attr(feature = "bevy_reflect", derive(Reflect))]
pub struct Real {
startup: Instant,
first_update: Option<Instant>,
Expand Down
5 changes: 3 additions & 2 deletions crates/bevy_time/src/stopwatch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#[cfg(feature = "bevy_reflect")]
use bevy_reflect::{prelude::*, Reflect};
use bevy_utils::Duration;

Expand All @@ -22,9 +23,9 @@ use bevy_utils::Duration;
/// assert!(stopwatch.paused());
/// assert_eq!(stopwatch.elapsed_secs(), 0.0);
/// ```
#[derive(Clone, Debug, Default, PartialEq, Eq, Reflect)]
#[derive(Clone, Debug, Default, PartialEq, Eq)]
#[cfg_attr(feature = "serialize", derive(serde::Deserialize, serde::Serialize))]
#[reflect(Default)]
#[cfg_attr(feature = "bevy_reflect", derive(Reflect), reflect(Default))]
pub struct Stopwatch {
elapsed: Duration,
paused: bool,
Expand Down
9 changes: 6 additions & 3 deletions crates/bevy_time/src/time.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use bevy_ecs::{reflect::ReflectResource, system::Resource};
#[cfg(feature = "bevy_reflect")]
use bevy_ecs::reflect::ReflectResource;
use bevy_ecs::system::Resource;
#[cfg(feature = "bevy_reflect")]
use bevy_reflect::{std_traits::ReflectDefault, Reflect};
use bevy_utils::Duration;

Expand Down Expand Up @@ -183,8 +186,8 @@ use bevy_utils::Duration;
/// }
/// }
/// ```
#[derive(Resource, Debug, Copy, Clone, Reflect)]
#[reflect(Resource, Default)]
#[derive(Resource, Debug, Copy, Clone)]
#[cfg_attr(feature = "bevy_reflect", derive(Reflect), reflect(Resource, Default))]
pub struct Time<T: Default = ()> {
context: T,
wrap_period: Duration,
Expand Down
9 changes: 5 additions & 4 deletions crates/bevy_time/src/timer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::Stopwatch;
#[cfg(feature = "bevy_reflect")]
use bevy_reflect::prelude::*;
use bevy_utils::Duration;

Expand All @@ -9,9 +10,9 @@ use bevy_utils::Duration;
/// exceeded, and can still be reset at any given point.
///
/// Paused timers will not have elapsed time increased.
#[derive(Clone, Debug, Default, PartialEq, Eq, Reflect)]
#[derive(Clone, Debug, Default, PartialEq, Eq)]
#[cfg_attr(feature = "serialize", derive(serde::Deserialize, serde::Serialize))]
#[reflect(Default)]
#[cfg_attr(feature = "bevy_reflect", derive(Reflect), reflect(Default))]
pub struct Timer {
stopwatch: Stopwatch,
duration: Duration,
Expand Down Expand Up @@ -425,9 +426,9 @@ impl Timer {
}

/// Specifies [`Timer`] behavior.
#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, Default, Reflect)]
#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, Default)]
#[cfg_attr(feature = "serialize", derive(serde::Deserialize, serde::Serialize))]
#[reflect(Default)]
#[cfg_attr(feature = "bevy_reflect", derive(Reflect), reflect(Default))]
pub enum TimerMode {
/// Run once and stop.
#[default]
Expand Down
4 changes: 3 additions & 1 deletion crates/bevy_time/src/virt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#[cfg(feature = "bevy_reflect")]
use bevy_reflect::Reflect;
use bevy_utils::{tracing::debug, Duration};

Expand Down Expand Up @@ -67,7 +68,8 @@ use crate::{real::Real, time::Time};
/// time. You should also consider how stable your FPS is, as the limit will
/// also dictate how big of an FPS drop you can accept without losing time and
/// falling behind real time.
#[derive(Debug, Copy, Clone, Reflect)]
#[derive(Debug, Copy, Clone)]
#[cfg_attr(feature = "bevy_reflect", derive(Reflect))]
pub struct Virtual {
max_delta: Duration,
paused: bool,
Expand Down