make TypeIdMap iteration order respect its comment (#23864)

# Objective

- Fixes #23840
- Make `TypeIdMap` iteration order depends only on insertion/removals

## Solution

- Replace the backing `HashMap` by an `IndexMap`

## Testing

- CI

Author: mockersf

crates/bevy_animation/src/lib.rs

@@ -748,10 +748,10 @@ impl AnimationCurveEvaluators {
                 .component_property_curve_evaluators
                 .get_or_insert_with(component_property, func),
             EvaluatorId::Type(type_id) => match self.type_id_curve_evaluators.entry(type_id) {
-                bevy_platform::collections::hash_map::Entry::Occupied(occupied_entry) => {
+                bevy_utils::TypeIdMapEntry::Occupied(occupied_entry) => {
                     &mut **occupied_entry.into_mut()
                 }
-                bevy_platform::collections::hash_map::Entry::Vacant(vacant_entry) => {
+                bevy_utils::TypeIdMapEntry::Vacant(vacant_entry) => {
                     &mut **vacant_entry.insert(func())
                 }
             },
@@ -781,7 +781,7 @@ impl CurrentEvaluators {
             (visit)(EvaluatorId::ComponentField(&key))?;
         }
 
-        for (key, _) in self.type_ids.drain() {
+        for (key, _) in self.type_ids.drain(..) {
             (visit)(EvaluatorId::Type(key))?;
         }
 

crates/bevy_app/src/plugin_group.rs

@@ -4,8 +4,7 @@ use alloc::{
     string::{String, ToString},
     vec::Vec,
 };
-use bevy_platform::collections::hash_map::Entry;
-use bevy_utils::TypeIdMap;
+use bevy_utils::{TypeIdMap, TypeIdMapEntry as Entry};
 use core::any::TypeId;
 use log::{debug, warn};
 
@@ -368,7 +367,7 @@ impl PluginGroupBuilder {
         for plugin_id in order {
             self.upsert_plugin_entry_state(
                 plugin_id,
-                plugins.remove(&plugin_id).unwrap(),
+                plugins.shift_remove(&plugin_id).unwrap(),
                 self.order.len(),
             );
 
@@ -517,7 +516,7 @@ impl PluginGroupBuilder {
     #[track_caller]
     pub fn finish(mut self, app: &mut App) {
         for ty in &self.order {
-            if let Some(entry) = self.plugins.remove(ty)
+            if let Some(entry) = self.plugins.shift_remove(ty)
                 && entry.enabled
             {
                 debug!("added plugin: {}", entry.plugin.name());

crates/bevy_asset/src/server/info.rs

@@ -13,7 +13,7 @@ use alloc::{
 use bevy_ecs::world::World;
 use bevy_platform::collections::{hash_map::Entry, HashMap, HashSet};
 use bevy_tasks::Task;
-use bevy_utils::TypeIdMap;
+use bevy_utils::{TypeIdMap, TypeIdMapEntry};
 use core::{
     any::{type_name, TypeId},
     task::Waker,
@@ -222,7 +222,7 @@ impl AssetInfos {
             .ok_or(GetOrCreateHandleInternalError::HandleMissingButTypeIdNotSpecified)?;
 
         match handles.entry(type_id) {
-            Entry::Occupied(entry) => {
+            TypeIdMapEntry::Occupied(entry) => {
                 let index = *entry.get();
                 // if there is a path_to_id entry, info always exists
                 let info = self
@@ -264,7 +264,7 @@ impl AssetInfos {
                 }
             }
             // The entry does not exist, so this is a "fresh" asset load. We must create a new handle
-            Entry::Vacant(entry) => {
+            TypeIdMapEntry::Vacant(entry) => {
                 let should_load = match loading_mode {
                     HandleLoadingMode::NotLoading => false,
                     HandleLoadingMode::Request | HandleLoadingMode::Force => true,
@@ -746,7 +746,7 @@ impl AssetInfos {
         }
 
         if let Some(map) = path_to_id.get_mut(path) {
-            map.remove(&type_id);
+            map.shift_remove(&type_id);
 
             if map.is_empty() {
                 path_to_id.remove(path);

crates/bevy_ecs/src/component/register.rs

@@ -133,7 +133,12 @@ impl<'w> ComponentsRegistrator<'w> {
                 .unwrap_or_else(PoisonError::into_inner);
             queued.components.keys().next().copied().map(|type_id| {
                 // SAFETY: the id just came from a valid iterator.
-                unsafe { queued.components.remove(&type_id).debug_checked_unwrap() }
+                unsafe {
+                    queued
+                        .components
+                        .shift_remove(&type_id)
+                        .debug_checked_unwrap()
+                }
             })
         } {
             registrator.register(self);
@@ -189,7 +194,7 @@ impl<'w> ComponentsRegistrator<'w> {
             .get_mut()
             .unwrap_or_else(PoisonError::into_inner)
             .components
-            .remove(&type_id)
+            .shift_remove(&type_id)
         {
             // If we are trying to register something that has already been queued, we respect the queue.
             // Just like if we are trying to register something that already is, we respect the first registration.
@@ -338,7 +343,7 @@ impl<'w> ComponentsRegistrator<'w> {
             .get_mut()
             .unwrap_or_else(PoisonError::into_inner)
             .components
-            .remove(&type_id)
+            .shift_remove(&type_id)
         {
             // If we are trying to register something that has already been queued, we respect the queue.
             // Just like if we are trying to register something that already is, we respect the first registration.

crates/bevy_reflect/src/type_registry.rs

@@ -292,22 +292,18 @@ impl TypeRegistry {
         type_id: TypeId,
         get_registration: impl FnOnce() -> TypeRegistration,
     ) -> bool {
-        use bevy_platform::collections::hash_map::Entry;
-
-        match self.registrations.entry(type_id) {
-            Entry::Occupied(_) => false,
-            Entry::Vacant(entry) => {
-                let registration = get_registration();
-                Self::update_registration_indices(
-                    &registration,
-                    &mut self.short_path_to_id,
-                    &mut self.type_path_to_id,
-                    &mut self.ambiguous_names,
-                );
-                entry.insert(registration);
-                true
-            }
+        if self.registrations.contains_key(&type_id) {
+            return false;
         }
+        let registration = get_registration();
+        Self::update_registration_indices(
+            &registration,
+            &mut self.short_path_to_id,
+            &mut self.type_path_to_id,
+            &mut self.ambiguous_names,
+        );
+        self.registrations.insert(type_id, registration);
+        true
     }
 
     /// Internal method to register additional lookups for a given [`TypeRegistration`].

crates/bevy_reflect/src/utility.rs

@@ -278,7 +278,7 @@ impl<T: TypedProperty> GenericTypeCell<T> {
 
         write_lock
             .entry(type_id)
-            .insert({
+            .insert_entry({
                 // We leak here in order to obtain a `&'static` reference.
                 // Otherwise, we won't be able to return a reference due to the `RwLock`.
                 // This should be okay, though, since we expect it to remain statically

crates/bevy_render/src/batching/gpu_preprocessing.rs

@@ -199,7 +199,7 @@ where
         BatchedInstanceBuffers {
             current_input_buffer: InstanceInputUniformBuffer::new(),
             previous_input_buffer: PreviousInstanceInputUniformBuffer::new(),
-            phase_instance_buffers: HashMap::default(),
+            phase_instance_buffers: TypeIdMap::default(),
         }
     }
 }

crates/bevy_utils/Cargo.toml

@@ -26,6 +26,7 @@ bevy_platform = { path = "../bevy_platform", version = "0.19.0-dev", default-fea
 disqualified = { version = "1.0", default-features = false }
 thread_local = { version = "1.0", optional = true }
 async-channel = { version = "2.3.0", optional = true }
+indexmap = { version = "2", default-features = false }
 
 [dev-dependencies]
 static_assertions = "1.1.0"

crates/bevy_utils/src/map.rs

@@ -1,9 +1,13 @@
 use core::{any::TypeId, hash::Hash};
 
 use bevy_platform::{
-    collections::{hash_map::Entry, HashMap},
+    collections::HashMap,
     hash::{Hashed, NoOpHash, PassHash},
 };
+use indexmap::map::IndexMap;
+
+/// The [`Entry`][indexmap::map::Entry] type for [`TypeIdMap`].
+pub use indexmap::map::Entry as TypeIdMapEntry;
 
 /// A [`HashMap`] pre-configured to use [`Hashed`] keys and [`PassHash`] passthrough hashing.
 /// Iteration order only depends on the order of insertions and deletions.
@@ -34,14 +38,14 @@ impl<K: Hash + Eq + PartialEq + Clone, V> PreHashMapExt<K, V> for PreHashMap<K,
     }
 }
 
-/// A specialized hashmap type with Key of [`TypeId`]
+/// A specialized map type with Key of [`TypeId`]
 /// Iteration order only depends on the order of insertions and deletions.
-pub type TypeIdMap<V> = HashMap<TypeId, V, NoOpHash>;
+pub type TypeIdMap<V> = IndexMap<TypeId, V, NoOpHash>;
 
 /// Extension trait to make use of [`TypeIdMap`] more ergonomic.
 ///
 /// Each function on this trait is a trivial wrapper for a function
-/// on [`HashMap`], replacing a `TypeId` key with a
+/// on [`IndexMap`], replacing a `TypeId` key with a
 /// generic parameter `T`.
 ///
 /// # Examples
@@ -80,7 +84,7 @@ pub trait TypeIdMapExt<V> {
     fn remove_type<T: ?Sized + 'static>(&mut self) -> Option<V>;
 
     /// Gets the type `T`'s entry in the map for in-place manipulation.
-    fn entry_type<T: ?Sized + 'static>(&mut self) -> Entry<'_, TypeId, V, NoOpHash>;
+    fn entry_type<T: ?Sized + 'static>(&mut self) -> TypeIdMapEntry<'_, TypeId, V>;
 }
 
 impl<V> TypeIdMapExt<V> for TypeIdMap<V> {
@@ -101,11 +105,11 @@ impl<V> TypeIdMapExt<V> for TypeIdMap<V> {
 
     #[inline]
     fn remove_type<T: ?Sized + 'static>(&mut self) -> Option<V> {
-        self.remove(&TypeId::of::<T>())
+        self.shift_remove(&TypeId::of::<T>())
     }
 
     #[inline]
-    fn entry_type<T: ?Sized + 'static>(&mut self) -> Entry<'_, TypeId, V, NoOpHash> {
+    fn entry_type<T: ?Sized + 'static>(&mut self) -> TypeIdMapEntry<'_, TypeId, V> {
         self.entry(TypeId::of::<T>())
     }
 }
@@ -152,4 +156,4 @@ mod tests {
             );
         }
     }
-}
+}