Make get_short_name return Cow instead of String

crates/bevy_ecs/src/schedule/schedule.rs

@@ -1,4 +1,5 @@
 use std::{
+    borrow::Cow,
     fmt::{Debug, Write},
     result::Result,
 };
@@ -23,6 +24,8 @@ use crate::{
     world::World,
 };
 
+type CowStr = Cow<'static, str>;
+
 /// Resource that stores [`Schedule`]s mapped to [`ScheduleLabel`]s.
 #[derive(Default, Resource)]
 pub struct Schedules {
@@ -1241,18 +1244,18 @@ enum ReportCycles {
 
 // methods for reporting errors
 impl ScheduleGraph {
-    fn get_node_name(&self, id: &NodeId) -> String {
-        let mut name = match id {
+    fn get_node_name(&self, id: &NodeId) -> CowStr {
+        let mut name: Cow<_> = match id {
             NodeId::System(_) => {
-                let name = self.systems[id.index()].get().unwrap().name().to_string();
+                let name = self.systems[id.index()].get().unwrap().name();
                 if self.settings.report_sets {
                     let sets = self.names_of_sets_containing_node(id);
                     if sets.is_empty() {
                         name
                     } else if sets.len() == 1 {
-                        format!("{name} (in set {})", sets[0])
+                        format!("{name} (in set {})", sets[0]).into()
                     } else {
-                        format!("{name} (in sets {})", sets.join(", "))
+                        format!("{name} (in sets {})", sets.join(", ")).into()
                     }
                 } else {
                     name
@@ -1261,14 +1264,17 @@ impl ScheduleGraph {
             NodeId::Set(_) => {
                 let set = &self.system_sets[id.index()];
                 if set.is_anonymous() {
-                    self.anonymous_set_name(id)
+                    self.anonymous_set_name(id).into()
                 } else {
-                    set.name()
+                    set.name().into()
                 }
             }
         };
         if self.settings.use_shortnames {
-            name = bevy_utils::get_short_name(&name);
+            name = match name {
+                Cow::Borrowed(borrowed) => bevy_utils::get_short_name(borrowed),
+                Cow::Owned(string) => Cow::Owned(bevy_utils::get_short_name(&string).to_string()),
+            };
         }
         name
     }
@@ -1280,7 +1286,12 @@ impl ScheduleGraph {
                 .graph
                 .edges_directed(*id, Direction::Outgoing)
                 .map(|(_, member_id, _)| self.get_node_name(&member_id))
-                .reduce(|a, b| format!("{a}, {b}"))
+                .reduce(|mut a, b| {
+                    let a_mut = a.to_mut();
+                    *a_mut += ", ";
+                    *a_mut += &b;
+                    a
+                })
                 .unwrap_or_default()
         )
     }
@@ -1463,7 +1474,7 @@ impl ScheduleGraph {
         }
     }
 
-    fn names_of_sets_containing_node(&self, id: &NodeId) -> Vec<String> {
+    fn names_of_sets_containing_node(&self, id: &NodeId) -> Vec<CowStr> {
         let mut sets = HashSet::new();
         self.traverse_sets_containing_node(*id, &mut |set_id| {
             !self.system_sets[set_id.index()].is_system_type() && sets.insert(set_id)
@@ -1482,8 +1493,8 @@ impl ScheduleGraph {
 #[non_exhaustive]
 pub enum ScheduleBuildError {
     /// A system set contains itself.
-    #[error("`{0:?}` contains itself.")]
-    HierarchyLoop(String),
+    #[error("`{0}` contains itself.")]
+    HierarchyLoop(CowStr),
     /// The hierarchy of system sets contains a cycle.
     #[error("System set hierarchy contains cycle(s).")]
     HierarchyCycle,
@@ -1493,20 +1504,20 @@ pub enum ScheduleBuildError {
     #[error("System set hierarchy contains redundant edges.")]
     HierarchyRedundancy,
     /// A system (set) has been told to run before itself.
-    #[error("`{0:?}` depends on itself.")]
-    DependencyLoop(String),
+    #[error("`{0}` depends on itself.")]
+    DependencyLoop(CowStr),
     /// The dependency graph contains a cycle.
     #[error("System dependencies contain cycle(s).")]
     DependencyCycle,
     /// Tried to order a system (set) relative to a system set it belongs to.
-    #[error("`{0:?}` and `{1:?}` have both `in_set` and `before`-`after` relationships (these might be transitive). This combination is unsolvable as a system cannot run before or after a set it belongs to.")]
-    CrossDependency(String, String),
+    #[error("`{0}` and `{1}` have both `in_set` and `before`-`after` relationships (these might be transitive). This combination is unsolvable as a system cannot run before or after a set it belongs to.")]
+    CrossDependency(CowStr, CowStr),
     /// Tried to order system sets that share systems.
-    #[error("`{0:?}` and `{1:?}` have a `before`-`after` relationship (which may be transitive) but share systems.")]
-    SetsHaveOrderButIntersect(String, String),
+    #[error("`{0}` and `{1}` have a `before`-`after` relationship (which may be transitive) but share systems.")]
+    SetsHaveOrderButIntersect(CowStr, CowStr),
     /// Tried to order a system (set) relative to all instances of some system function.
-    #[error("Tried to order against `fn {0:?}` in a schedule that has more than one `{0:?}` instance. `fn {0:?}` is a `SystemTypeSet` and cannot be used for ordering if ambiguous. Use a different set without this restriction.")]
-    SystemTypeSetAmbiguity(String),
+    #[error("Tried to order against `fn {0}` in a schedule that has more than one `{0}` instance. `fn {0}` is a `SystemTypeSet` and cannot be used for ordering if ambiguous. Use a different set without this restriction.")]
+    SystemTypeSetAmbiguity(CowStr),
     /// Systems with conflicting access have indeterminate run order.
     ///
     /// This error is disabled by default, but can be opted-in using [`ScheduleBuildSettings`].

crates/bevy_reflect/src/lib.rs

@@ -1914,7 +1914,7 @@ bevy_reflect::tests::should_reflect_debug::Test {
             fn short_type_path() -> &'static str {
                 static CELL: GenericTypePathCell = GenericTypePathCell::new();
                 CELL.get_or_insert::<Self, _>(|| {
-                    bevy_utils::get_short_name(std::any::type_name::<Self>())
+                    bevy_utils::get_short_name(std::any::type_name::<Self>()).to_string()
                 })
             }
 

crates/bevy_reflect/src/type_registry.rs

@@ -1,10 +1,12 @@
-use crate::{serde::Serializable, Reflect, TypeInfo, Typed};
+use std::{any::TypeId, borrow::Cow, fmt::Debug, sync::Arc};
+
 use bevy_ptr::{Ptr, PtrMut};
 use bevy_utils::{HashMap, HashSet};
 use downcast_rs::{impl_downcast, Downcast};
 use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard};
 use serde::Deserialize;
-use std::{any::TypeId, fmt::Debug, sync::Arc};
+
+use crate::{serde::Serializable, Reflect, TypeInfo, Typed};
 
 /// A registry of [reflected] types.
 ///
@@ -303,7 +305,7 @@ impl TypeRegistryArc {
 /// [short name]: bevy_utils::get_short_name
 /// [crate-level documentation]: crate
 pub struct TypeRegistration {
-    short_name: String,
+    short_name: Cow<'static, str>,
     data: HashMap<TypeId, Box<dyn TypeData>>,
     type_info: &'static TypeInfo,
 }

crates/bevy_utils/src/lib.rs

@@ -14,7 +14,7 @@ pub mod prelude {
 pub mod futures;
 pub mod label;
 mod short_names;
-pub use short_names::get_short_name;
+pub use short_names::{get_short_name, DisplayShortName};
 pub mod synccell;
 pub mod syncunsafecell;
 

crates/bevy_utils/src/short_names.rs

@@ -1,65 +1,72 @@
+use std::{borrow::Cow, fmt, str};
+
+const SPECIAL_TYPE_CHARS: [u8; 9] = *b" <>()[],;";
 /// Shortens a type name to remove all module paths.
 ///
 /// The short name of a type is its full name as returned by
 /// [`std::any::type_name`], but with the prefix of all paths removed. For
 /// example, the short name of `alloc::vec::Vec<core::option::Option<u32>>`
 /// would be `Vec<Option<u32>>`.
-pub fn get_short_name(full_name: &str) -> String {
+pub fn get_short_name(full_name: &str) -> Cow<str> {
     // Generics result in nested paths within <..> blocks.
     // Consider "bevy_render::camera::camera::extract_cameras<bevy_render::camera::bundle::Camera3d>".
     // To tackle this, we parse the string from left to right, collapsing as we go.
-    let mut index: usize = 0;
-    let end_of_string = full_name.len();
-    let mut parsed_name = String::new();
-
-    while index < end_of_string {
-        let rest_of_string = full_name.get(index..end_of_string).unwrap_or_default();
+    let mut remaining = full_name.as_bytes();
+    let mut parsed_name = Vec::new();
+    let mut complex_type = false;
 
+    loop {
         // Collapse everything up to the next special character,
         // then skip over it
-        if let Some(special_character_index) = rest_of_string.find(|c: char| {
-            (c == ' ')
-                || (c == '<')
-                || (c == '>')
-                || (c == '(')
-                || (c == ')')
-                || (c == '[')
-                || (c == ']')
-                || (c == ',')
-                || (c == ';')
-        }) {
-            let segment_to_collapse = rest_of_string
-                .get(0..special_character_index)
-                .unwrap_or_default();
-            parsed_name += collapse_type_name(segment_to_collapse);
-            // Insert the special character
-            let special_character =
-                &rest_of_string[special_character_index..=special_character_index];
-            parsed_name.push_str(special_character);
-
-            match special_character {
-                ">" | ")" | "]"
-                    if rest_of_string[special_character_index + 1..].starts_with("::") =>
-                {
-                    parsed_name.push_str("::");
+        let is_special = |c| SPECIAL_TYPE_CHARS.contains(c);
+        if let Some(next_special_index) = remaining.iter().position(is_special) {
+            complex_type = true;
+            if parsed_name.is_empty() {
+                parsed_name.reserve(remaining.len());
+            }
+            let (pre_special, post_special) = remaining.split_at(next_special_index + 1);
+            parsed_name.extend_from_slice(collapse_type_name(pre_special));
+            match pre_special.last().unwrap() {
+                b'>' | b')' | b']' if post_special.get(..2) == Some(b"::") => {
+                    parsed_name.extend_from_slice(b"::");
                     // Move the index past the "::"
-                    index += special_character_index + 3;
+                    remaining = &post_special[2..];
                 }
                 // Move the index just past the special character
-                _ => index += special_character_index + 1,
+                _ => remaining = post_special,
             }
+        } else if !complex_type {
+            let collapsed = collapse_type_name(remaining);
+            // SAFETY: We only split on ASCII characters, and the input is valid UTF8, since
+            // it was a &str
+            let str = unsafe { str::from_utf8_unchecked(collapsed) };
+            return Cow::Borrowed(str);
         } else {
             // If there are no special characters left, we're done!
-            parsed_name += collapse_type_name(rest_of_string);
-            index = end_of_string;
+            parsed_name.extend_from_slice(collapse_type_name(remaining));
+            // SAFETY: see above
+            let utf8_name = unsafe { String::from_utf8_unchecked(parsed_name) };
+            return Cow::Owned(utf8_name);
         }
     }
-    parsed_name
+}
+
+/// Wrapper around `AsRef<str>` that uses the [`get_short_name`] format when
+/// displayed.
+pub struct DisplayShortName<T: AsRef<str>>(pub T);
+
+impl<T: AsRef<str>> fmt::Display for DisplayShortName<T> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let as_short_name = get_short_name(self.0.as_ref());
+        write!(f, "{as_short_name}")
+    }
 }
 
 #[inline(always)]
-fn collapse_type_name(string: &str) -> &str {
-    string.split("::").last().unwrap()
+fn collapse_type_name(string: &[u8]) -> &[u8] {
+    let find = |(index, window)| (window == b"::").then_some(index + 2);
+    let split_index = string.windows(2).enumerate().rev().find_map(find);
+    &string[split_index.unwrap_or(0)..]
 }
 
 #[cfg(test)]
@@ -110,6 +117,14 @@ mod name_formatting_tests {
         );
     }
 
+    #[test]
+    fn utf8_generics() {
+        assert_eq!(
+            get_short_name("bévï::camérą::łørđ::_öñîòñ<ķràźÿ::Москва::東京>"),
+            "_öñîòñ<東京>".to_string()
+        );
+    }
+
     #[test]
     fn nested_generics() {
         assert_eq!(