[Merged by Bors] - Unique WorldId

crates/bevy_core/src/time/fixed_timestep.rs

@@ -4,7 +4,7 @@ use bevy_ecs::{
  component::ComponentId,
  query::Access,
  schedule::ShouldRun,
- system::{ConfigurableSystem, IntoSystem, Local, Res, ResMut, System, SystemId},
+ system::{ConfigurableSystem, IntoSystem, Local, Res, ResMut, System},
  world::World,
 };
 use bevy_utils::HashMap;
@@ -148,10 +148,6 @@ impl System for FixedTimestep {
  Cow::Borrowed(std::any::type_name::<FixedTimestep>())
  }
 
- fn id(&self) -> SystemId {
- self.internal_system.id()
- }
-
  fn new_archetype(&mut self, archetype: &Archetype) {
  self.internal_system.new_archetype(archetype);
  }

crates/bevy_ecs/Cargo.toml

@@ -24,11 +24,11 @@ fixedbitset = "0.4"
 fxhash = "0.2"
 thiserror = "1.0"
 downcast-rs = "1.2"
-rand = "0.8"
 serde = "1"
 
 [dev-dependencies]
 parking_lot = "0.11"
+rand = "0.8"
 
 [[example]]
 name = "events"

crates/bevy_ecs/src/schedule/run_criteria.rs

@@ -3,7 +3,7 @@ use crate::{
  component::ComponentId,
  query::Access,
  schedule::{BoxedRunCriteriaLabel, GraphNode, RunCriteriaLabel},
- system::{BoxedSystem, IntoSystem, System, SystemId},
+ system::{BoxedSystem, IntoSystem, System},
  world::World,
 };
 use std::borrow::Cow;
@@ -398,7 +398,6 @@ where
 
 pub struct RunOnce {
  ran: bool,
- system_id: SystemId,
  archetype_component_access: Access<ArchetypeComponentId>,
  component_access: Access<ComponentId>,
 }
@@ -407,7 +406,6 @@ impl Default for RunOnce {
  fn default() -> Self {
  Self {
  ran: false,
- system_id: SystemId::new(),
  archetype_component_access: Default::default(),
  component_access: Default::default(),
  }
@@ -422,10 +420,6 @@ impl System for RunOnce {
  Cow::Borrowed(std::any::type_name::<RunOnce>())
  }
 
- fn id(&self) -> SystemId {
- self.system_id
- }
-
  fn new_archetype(&mut self, _archetype: &Archetype) {}
 
  fn component_access(&self) -> &Access<ComponentId> {

crates/bevy_ecs/src/system/exclusive_system.rs

@@ -1,15 +1,13 @@
 use crate::{
  archetype::ArchetypeGeneration,
- system::{check_system_change_tick, BoxedSystem, IntoSystem, SystemId},
+ system::{check_system_change_tick, BoxedSystem, IntoSystem},
  world::World,
 };
 use std::borrow::Cow;
 
 pub trait ExclusiveSystem: Send + Sync + 'static {
  fn name(&self) -> Cow<'static, str>;
 
- fn id(&self) -> SystemId;
-
  fn run(&mut self, world: &mut World);
 
  fn initialize(&mut self, world: &mut World);
@@ -20,7 +18,6 @@ pub trait ExclusiveSystem: Send + Sync + 'static {
 pub struct ExclusiveSystemFn {
  func: Box<dyn FnMut(&mut World) + Send + Sync + 'static>,
  name: Cow<'static, str>,
- id: SystemId,
  last_change_tick: u32,
 }
 
@@ -29,10 +26,6 @@ impl ExclusiveSystem for ExclusiveSystemFn {
  self.name.clone()
  }
 
- fn id(&self) -> SystemId {
- self.id
- }
-
  fn run(&mut self, world: &mut World) {
  // The previous value is saved in case this exclusive system is run by another exclusive
  // system
@@ -67,7 +60,6 @@ where
  ExclusiveSystemFn {
  func: Box::new(self),
  name: core::any::type_name::<F>().into(),
- id: SystemId::new(),
  last_change_tick: 0,
  }
  }
@@ -83,10 +75,6 @@ impl ExclusiveSystem for ExclusiveSystemCoerced {
  self.system.name()
  }
 
- fn id(&self) -> SystemId {
- self.system.id()
- }
-
  fn run(&mut self, world: &mut World) {
  let archetypes = world.archetypes();
  let new_generation = archetypes.generation();

crates/bevy_ecs/src/system/function_system.rs

@@ -3,8 +3,8 @@ use crate::{
  component::ComponentId,
  query::{Access, FilteredAccessSet},
  system::{
- check_system_change_tick, ReadOnlySystemParamFetch, System, SystemId, SystemParam,
- SystemParamFetch, SystemParamState,
+ check_system_change_tick, ReadOnlySystemParamFetch, System, SystemParam, SystemParamFetch,
+ SystemParamState,
  },
  world::{World, WorldId},
 };
@@ -13,7 +13,6 @@ use std::{borrow::Cow, marker::PhantomData};
 
 /// The metadata of a [`System`].
 pub struct SystemMeta {
- pub(crate) id: SystemId,
  pub(crate) name: Cow<'static, str>,
  pub(crate) component_access_set: FilteredAccessSet<ComponentId>,
  pub(crate) archetype_component_access: Access<ArchetypeComponentId>,
@@ -30,7 +29,6 @@ impl SystemMeta {
  archetype_component_access: Access::default(),
  component_access_set: FilteredAccessSet::default(),
  is_send: true,
- id: SystemId::new(),
  last_change_tick: 0,
  }
  }
@@ -340,11 +338,6 @@ where
  self.system_meta.name.clone()
  }
 
- #[inline]
- fn id(&self) -> SystemId {
- self.system_meta.id
- }
-
  #[inline]
  fn new_archetype(&mut self, archetype: &Archetype) {
  let param_state = self.param_state.as_mut().unwrap();

crates/bevy_ecs/src/system/system.rs

@@ -8,18 +8,6 @@ use crate::{
 };
 use std::borrow::Cow;
 
-/// A [`System`] identifier.
-#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
-pub struct SystemId(pub usize);
-
-impl SystemId {
- /// Creates a new random `SystemId`.
- #[allow(clippy::new_without_default)]
- pub fn new() -> Self {
- SystemId(rand::random::<usize>())
- }
-}
-
 /// An ECS system that can be added to a [Schedule](crate::schedule::Schedule)
 ///
 /// Systems are functions with all arguments implementing [SystemParam](crate::system::SystemParam).
@@ -38,8 +26,6 @@ pub trait System: Send + Sync + 'static {
  type Out;
  /// Returns the system's name.
  fn name(&self) -> Cow<'static, str>;
- /// Returns the system's [`SystemId`].
- fn id(&self) -> SystemId;
  /// Register a new archetype for this system.
  fn new_archetype(&mut self, archetype: &Archetype);
  /// Returns the system's component [`Access`].

crates/bevy_ecs/src/system/system_chaining.rs

@@ -2,7 +2,7 @@ use crate::{
  archetype::{Archetype, ArchetypeComponentId},
  component::ComponentId,
  query::Access,
- system::{IntoSystem, System, SystemId},
+ system::{IntoSystem, System},
  world::World,
 };
 use std::borrow::Cow;
@@ -48,7 +48,6 @@ pub struct ChainSystem<SystemA, SystemB> {
  system_a: SystemA,
  system_b: SystemB,
  name: Cow<'static, str>,
- id: SystemId,
  component_access: Access<ComponentId>,
  archetype_component_access: Access<ArchetypeComponentId>,
 }
@@ -61,10 +60,6 @@ impl<SystemA: System, SystemB: System<In = SystemA::Out>> System for ChainSystem
  self.name.clone()
  }
 
- fn id(&self) -> SystemId {
- self.id
- }
-
  fn new_archetype(&mut self, archetype: &Archetype) {
  self.system_a.new_archetype(archetype);
  self.system_b.new_archetype(archetype);
@@ -143,7 +138,6 @@ where
  system_b,
  archetype_component_access: Default::default(),
  component_access: Default::default(),
- id: SystemId::new(),
  }
  }
 }

crates/bevy_ecs/src/world/identifier.rs

@@ -0,0 +1,56 @@
+use std::sync::atomic::{AtomicUsize, Ordering};
+
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+// We use usize here because that is the largest `Atomic` we want to require
+/// A unique identifier for a [`super::World`].
+// Note that this *is* used by external crates as well as for internal safety checks
+pub struct WorldId(usize);
+
+/// The next [`WorldId`].
+static MAX_WORLD_ID: AtomicUsize = AtomicUsize::new(0);
+
+impl WorldId {
+ /// Create a new, unique [`WorldId`]. Returns [`None`] if the supply of unique
+ /// [`WorldId`]s has been exhausted
+ ///
+ /// Please note that the [`WorldId`]s created from this method are unique across
+ /// time - if a given [`WorldId`] is [`Drop`]ped its value still cannot be reused
+ pub fn new() -> Option<Self> {
+ MAX_WORLD_ID
+ // We use `Relaxed` here since this atomic only needs to be consistent with itself
+ .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |val| {
+ val.checked_add(1)
+ })
+ .map(WorldId)
+ .ok()
+ }
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ #[test]
+ fn world_ids_unique() {
+ let ids = std::iter::repeat_with(WorldId::new)
+ .take(50)
+ .map(Option::unwrap)
+ .collect::<Vec<_>>();
+ for (i, &id1) in ids.iter().enumerate() {
+ // For the first element, i is 0 - so skip 1
+ for &id2 in ids.iter().skip(i + 1) {
+ assert_ne!(id1, id2, "WorldIds should not repeat");
+ }
+ }
+ }
+
+ // We cannot use this test as-is, as it causes other tests to panic due to using the same atomic variable.
+ // #[test]
+ // #[should_panic]
+ // fn panic_on_overflow() {
+ // MAX_WORLD_ID.store(usize::MAX - 50, Ordering::Relaxed);
+ // std::iter::repeat_with(WorldId::new)
+ // .take(500)
+ // .for_each(|_| ());
+ // }
+}

crates/bevy_ecs/src/world/mod.rs

@@ -25,15 +25,9 @@ use std::{
  sync::atomic::{AtomicU32, Ordering},
 };
 
-#[derive(Copy, Clone, PartialEq, Eq, Debug)]
-pub struct WorldId(u64);
-
-impl Default for WorldId {
- fn default() -> Self {
- WorldId(rand::random())
- }
-}
+mod identifier;
 
+pub use identifier::WorldId;
 /// Stores and exposes operations on [entities](Entity), [components](Component), resources,
 /// and their associated metadata.
 ///
@@ -94,7 +88,7 @@ pub struct World {
 impl Default for World {
  fn default() -> Self {
  Self {
- id: Default::default(),
+ id: WorldId::new().expect("More `bevy` `World`s have been created than is supported"),
  entities: Default::default(),
  components: Default::default(),
  archetypes: Default::default(),
@@ -113,12 +107,17 @@ impl Default for World {
 
 impl World {
  /// Creates a new empty [World]
+ /// # Panics
+ ///
+ /// If [`usize::MAX`] [`World`]s have been created.
+ /// This guarantee allows System Parameters to safely uniquely identify a [`World`],
+ /// since its [`WorldId`] is unique
  #[inline]
  pub fn new() -> World {
  World::default()
  }
 
- /// Retrieves this world's unique ID
+ /// Retrieves this [`World`]'s unique ID
  #[inline]
  pub fn id(&self) -> WorldId {
  self.id