feat: warn on duplicate globals

Author: makspll

crates/bevy_mod_scripting_bindings/src/globals/core.rs

@@ -7,6 +7,7 @@ use ::{
     bevy_reflect::TypeRegistration,
 };
 use bevy_app::App;
+use bevy_log::warn;
 use bevy_mod_scripting_asset::ScriptAsset;
 use bevy_mod_scripting_derive::script_globals;
 use bevy_platform::collections::HashMap;
@@ -63,6 +64,7 @@ impl Plugin for CoreScriptGlobalsPlugin {
         register_core_globals(app.world_mut());
     }
 }
+const MSG_DUPLICATE_GLOBAL: &str = "This can cause confusing issues for script users, use `CoreScriptGlobalsPlugin.filter` to filter out uneeded duplicate types.";
 
 #[profiling::function]
 fn register_static_core_globals(world: &mut World, filter: fn(&TypeRegistration) -> bool) {
@@ -82,20 +84,41 @@ fn register_static_core_globals(world: &mut World, filter: fn(&TypeRegistration)
         if let Some(global_name) = registration.type_info().type_path_table().ident() {
             let documentation = "A reference to the type, allowing you to call static methods.";
             let type_info = registration.type_info();
-            global_registry.register_static_documented_dynamic(
-                registration.type_id(),
-                into_through_type_info(type_info),
-                global_name.into(),
-                documentation.into(),
-            );
+            if global_registry
+                .register_static_documented_dynamic(
+                    registration.type_id(),
+                    into_through_type_info(type_info),
+                    global_name.into(),
+                    documentation.into(),
+                )
+                .is_some()
+            {
+                warn!(
+                    "Duplicate global registration for type: '{}'. {MSG_DUPLICATE_GLOBAL}",
+                    type_info.type_path_table().short_path()
+                )
+            };
         }
     }
 
     // register basic globals
-    global_registry.register_dummy::<World>("world", "The current ECS world.");
-    global_registry
-        .register_dummy::<Entity>("entity", "The entity this script is attached to if any.");
-    global_registry.register_dummy_typed::<Val<Handle<ScriptAsset>>>("script_asset", "the asset handle for this script. If the asset is ever unloaded, the handle will be less useful.");
+    if global_registry
+        .register_dummy::<World>("world", "The current ECS world.")
+        .is_some()
+    {
+        warn!("existing `world` global was replaced by the core `world` dummy type.")
+    };
+
+    if global_registry
+        .register_dummy::<Entity>("entity", "The entity this script is attached to if any.")
+        .is_some()
+    {
+        warn!("existing `entity` global was replaced by the core `entity` dummy type.")
+    }
+
+    if global_registry.register_dummy_typed::<Val<Handle<ScriptAsset>>>("script_asset", "the asset handle for this script. If the asset is ever unloaded, the handle will be less useful.").is_some() {
+        warn!("existing `script_asset` global was replaced by the core `script_asset` dummy type.")
+    };
 }
 
 #[script_globals(bms_bindings_path = "crate", name = "core_globals")]
@@ -128,7 +151,15 @@ impl CoreGlobals {
             let registration = guard.clone().get_type_registration(registration)?;
             let registration =
                 registration.map_both(Val::from, |u| u.map_both(Val::from, Val::from));
-            type_cache.insert(type_path.to_owned(), registration);
+            if type_cache
+                .insert(type_path.to_owned(), registration)
+                .is_some()
+            {
+                warn!(
+                    "duplicate entry inside `types` global for type: {}. {MSG_DUPLICATE_GLOBAL}",
+                    type_path
+                )
+            };
         }
 
         Ok(type_cache)

crates/bevy_mod_scripting_bindings/src/globals/mod.rs

@@ -116,6 +116,7 @@ impl ScriptGlobalsRegistry {
     }
 
     /// Inserts a global into the registry, returns the previous value if it existed
+    #[must_use]
     pub fn register<
         T: ScriptReturn + 'static + Typed,
         F: Fn(WorldGuard) -> Result<T, InteropError> + 'static + Send + Sync,
@@ -140,40 +141,43 @@ impl ScriptGlobalsRegistry {
     /// This can be useful for globals which you cannot expose normally.
     ///
     /// Dummy globals are stored as non-static instances, i.e. they're expected to be values not type references.
+    #[must_use]
     pub fn register_dummy<T: 'static>(
         &mut self,
         name: impl Into<Cow<'static, str>>,
         documentation: impl Into<Cow<'static, str>>,
-    ) {
+    ) -> Option<ScriptGlobalDummy> {
         self.dummies.insert(
             name.into(),
             ScriptGlobalDummy {
                 documentation: Some(documentation.into()),
                 type_id: TypeId::of::<T>(),
                 type_information: None,
             },
-        );
+        )
     }
 
     /// Typed equivalent to [`Self::register_dummy`].
+    #[must_use]
     pub fn register_dummy_typed<T: 'static + TypedThrough>(
         &mut self,
         name: impl Into<Cow<'static, str>>,
         documentation: impl Into<Cow<'static, str>>,
-    ) {
+    ) -> Option<ScriptGlobalDummy> {
         self.dummies.insert(
             name.into(),
             ScriptGlobalDummy {
                 documentation: Some(documentation.into()),
                 type_id: TypeId::of::<T>(),
                 type_information: Some(T::through_type_info()),
             },
-        );
+        )
     }
 
     /// Inserts a global into the registry, returns the previous value if it existed.
     ///
     /// This is a version of [`Self::register`] which stores type information regarding the global.
+    #[must_use]
     pub fn register_documented<
         T: TypedScriptReturn + 'static,
         F: Fn(WorldGuard) -> Result<T, InteropError> + 'static + Send + Sync,
@@ -195,7 +199,11 @@ impl ScriptGlobalsRegistry {
     }
 
     /// Registers a static global into the registry.
-    pub fn register_static<T: 'static + Typed>(&mut self, name: Cow<'static, str>) {
+    #[must_use]
+    pub fn register_static<T: 'static + Typed>(
+        &mut self,
+        name: Cow<'static, str>,
+    ) -> Option<ScriptGlobal> {
         self.globals.insert(
             name,
             ScriptGlobal {
@@ -204,17 +212,18 @@ impl ScriptGlobalsRegistry {
                 type_id: TypeId::of::<T>(),
                 type_information: into_through_type_info(T::type_info()),
             },
-        );
+        )
     }
 
     /// Registers a static global into the registry.
     ///
     /// This is a version of [`Self::register_static`] which stores rich type information regarding the global.
+    #[must_use]
     pub fn register_static_documented<T: TypedScriptReturn + 'static>(
         &mut self,
         name: Cow<'static, str>,
         documentation: Cow<'static, str>,
-    ) {
+    ) -> Option<ScriptGlobal> {
         self.globals.insert(
             name,
             ScriptGlobal {
@@ -223,19 +232,20 @@ impl ScriptGlobalsRegistry {
                 type_id: TypeId::of::<T>(),
                 type_information: T::through_type_info(),
             },
-        );
+        )
     }
 
     /// Registers a static global into the registry.
     ///
     /// This is a version of [`Self::register_static_documented`] which does not require compile time type knowledge.
+    #[must_use]
     pub fn register_static_documented_dynamic(
         &mut self,
         type_id: TypeId,
         type_information: ThroughTypeInfo,
         name: Cow<'static, str>,
         documentation: Cow<'static, str>,
-    ) {
+    ) -> Option<ScriptGlobal> {
         self.globals.insert(
             name,
             ScriptGlobal {
@@ -244,7 +254,7 @@ impl ScriptGlobalsRegistry {
                 type_id,
                 type_information,
             },
-        );
+        )
     }
 }
 
@@ -307,14 +317,14 @@ mod test {
     fn test_static_globals() {
         let mut registry = ScriptGlobalsRegistry::default();
 
-        registry.register_static::<i32>(Cow::Borrowed("foo"));
+        _ = registry.register_static::<i32>(Cow::Borrowed("foo"));
 
         let global = registry.get("foo").unwrap();
         assert!(global.maker.is_none());
         assert_eq!(global.type_id, TypeId::of::<i32>());
 
         // the same but documented
-        registry.register_static_documented::<i32>(
+        _ = registry.register_static_documented::<i32>(
             Cow::Borrowed("bar"),
             Cow::Borrowed("This is a test"),
         );

crates/bevy_mod_scripting_derive/src/derive/script_globals.rs

@@ -42,8 +42,11 @@ pub fn script_globals(
             let registry = world.get_resource_or_init::<#bms_bindings_path::globals::AppScriptGlobalsRegistry>();
             let mut registry = registry.write();
 
-            registry
-                #(#function_registrations)*;
+            #(
+                if (registry #function_registrations).is_some() {
+                    warn!("conflicting global registration under name: {}. This might cause confusing problems, use `CoreScriptGlobalsPlugin.filter` to filter out uneeded duplicate types.", stringify!(#function_name))
+                }
+            )*;
         }
     };