lua: remove ReflectBranch::as_component_unchecked and improve error handling

Cargo.lock

@@ -1949,6 +1949,7 @@ name = "lyra-scripting-derive"
 version = "0.1.0"
 dependencies = [
  "paste",
+ "proc-macro-crate",
  "proc-macro2",
  "quote",
  "syn 2.0.77",

crates/lyra-scripting/src/lib.rs

@@ -35,18 +35,6 @@ pub enum ReflectBranch {
 }
 
 impl ReflectBranch {
-    /// Gets self as a [`ReflectedComponent`].
-    ///
-    /// # Panics
-    /// If `self` is not a variant of [`ReflectBranch::Component`].
-    #[deprecated(note = "use ReflectBranch::as_component instead")]
-    pub fn as_component_unchecked(&self) -> &ReflectedComponent {
-        match self {
-            ReflectBranch::Component(c) => c,
-            _ => panic!("`self` is not an instance of `ReflectBranch::Component`"),
-        }
-    }
-
     /// Gets self as a [`ReflectedComponent`].
     ///
     /// Returns `None` if `self` is not a component.

crates/lyra-scripting/src/lua/ecs/view.rs

@@ -163,16 +163,31 @@ impl ViewResult {
                 ViewQueryItem::UserData(ud) => {
                     let reflect = ud
                         .call_function::<ScriptBorrow>(FN_NAME_INTERNAL_REFLECT_TYPE, ())
-                        .expect("Type does not implement 'reflect_type' properly");
+                        .map_err(|_| mlua::Error::BadArgument {
-                    let refl_comp = reflect.reflect_branch.as_component()
+                            to: Some("ViewResult.new".into()),
-                        .expect("`self` is not an instance of `ReflectBranch::Component`");
+                            pos: 2 + idx,
+                            name: Some("query...".into()),
+                            cause: Arc::new(mlua::Error::external(WorldError::LuaInvalidUsage(
+                                format!("userdata does not implement type reflection"),
+                            ))),
+                        })?;
+                    let refl_comp = reflect.reflect_branch.as_component().ok_or_else(|| {
+                        mlua::Error::BadArgument {
+                            to: Some("ViewResult.new".into()),
+                            pos: 2 + idx,
+                            name: Some("query...".into()),
+                            cause: Arc::new(mlua::Error::external(WorldError::LuaInvalidUsage(
+                                format!("userdata is not a Component"),
+                            ))),
+                        }
+                    })?;
 
                     let dyn_type = QueryDynamicType::from_info(refl_comp.info);
                     view.push(dyn_type);
                 }
                 // functions are queries, the if statement at the start would cause this to
                 // be unreachable.
-                ViewQueryItem::Function(_) => unreachable!()
+                ViewQueryItem::Function(_) => unreachable!(),
             }
         }
 
@@ -230,12 +245,12 @@ impl ViewResult {
                 LuaQueryResult::FilterPass => {
                     // do not push a boolean to values, its considered a filter
                     index_mod += 1;
-                },
+                }
                 LuaQueryResult::FilterDeny => return Ok(ViewQueryResult::FilterDeny),
                 LuaQueryResult::Some(value) => {
                     let idx = (*i - index_mod).max(0);
                     query_vals.push((value, idx));
-                },
+                }
             }
         }
 
@@ -268,11 +283,11 @@ impl mlua::UserData for ViewResult {
                             ViewQueryResult::Some(v) => v,
                             ViewQueryResult::AlwaysNone => {
                                 return mlua::Value::Nil.into_lua_multi(lua);
-                            },
+                            }
                             ViewQueryResult::None | ViewQueryResult::FilterDeny => {
                                 // try to get it next loop
                                 continue;
-                            },
+                            }
                         };
 
                         // insert query values to the result row
@@ -309,11 +324,11 @@ impl mlua::UserData for ViewResult {
                                     ViewQueryResult::Some(v) => v,
                                     ViewQueryResult::AlwaysNone => {
                                         return mlua::Value::Nil.into_lua_multi(lua);
-                                    },
+                                    }
                                     ViewQueryResult::None | ViewQueryResult::FilterDeny => {
                                         // try to get it next loop
                                         continue;
-                                    },
+                                    }
                                 };
 
                                 // insert query values to the result row

crates/lyra-scripting/src/lua/ecs/view_one.rs

@@ -1,12 +1,21 @@
 use std::sync::Arc;
 
-use lyra_ecs::{query::dynamic::{DynamicViewOneOwned, QueryDynamicType}, Entity};
+use lyra_ecs::{
+    query::dynamic::{DynamicViewOneOwned, QueryDynamicType},
+    Entity,
+};
 use lyra_reflect::TypeRegistry;
 use mlua::{IntoLua, IntoLuaMulti, ObjectLike};
 
-use crate::{lua::{ReflectLuaProxy, TypeLookup, WorldError, FN_NAME_INTERNAL_REFLECT_TYPE}, ScriptBorrow, ScriptWorldPtr};
+use crate::{
+    lua::{ReflectLuaProxy, TypeLookup, WorldError, FN_NAME_INTERNAL_REFLECT_TYPE},
+    ScriptBorrow, ScriptWorldPtr,
+};
 
-use super::{query::{LuaQuery, LuaQueryResult}, View, ViewQueryItem, ViewQueryResult};
+use super::{
+    query::{LuaQuery, LuaQueryResult},
+    View, ViewQueryItem, ViewQueryResult,
+};
 
 /// The result of an ecs world View of a single entity.
 #[derive(Clone)]
@@ -69,16 +78,31 @@ impl ViewOneResult {
                 ViewQueryItem::UserData(ud) => {
                     let reflect = ud
                         .call_function::<ScriptBorrow>(FN_NAME_INTERNAL_REFLECT_TYPE, ())
-                        .expect("Type does not implement 'reflect_type' properly");
+                        .ok_or_else(|| mlua::Error::BadArgument {
-                    let refl_comp = reflect.reflect_branch.as_component()
+                            to: Some("ViewOneResult.new".into()),
-                        .expect("`self` is not an instance of `ReflectBranch::Component`");
+                            pos: 2 + idx,
+                            name: Some("query...".into()),
+                            cause: Arc::new(mlua::Error::external(WorldError::LuaInvalidUsage(
+                                format!("userdata does not implement type reflection"),
+                            ))),
+                        })?;
+                    let refl_comp = reflect.reflect_branch.as_component().ok_or_else(|| {
+                        mlua::Error::BadArgument {
+                            to: Some("ViewOneResult.new".into()),
+                            pos: 2 + idx,
+                            name: Some("query...".into()),
+                            cause: Arc::new(mlua::Error::external(WorldError::LuaInvalidUsage(
+                                format!("userdata is not a Component"),
+                            ))),
+                        }
+                    })?;
 
                     let dyn_type = QueryDynamicType::from_info(refl_comp.info);
                     view.queries.push(dyn_type);
                 }
                 // functions are queries, the if statement at the start would cause this to
                 // be unreachable.
-                ViewQueryItem::Function(_) => unreachable!()
+                ViewQueryItem::Function(_) => unreachable!(),
             }
         }
 
@@ -110,12 +134,12 @@ impl ViewOneResult {
                 LuaQueryResult::FilterPass => {
                     // do not push a boolean to values, its considered a filter
                     index_mod += 1;
-                },
+                }
                 LuaQueryResult::FilterDeny => return Ok(ViewQueryResult::FilterDeny),
                 LuaQueryResult::Some(value) => {
                     let idx = (*i - index_mod).max(0);
                     query_vals.push((value, idx));
-                },
+                }
             }
         }
 
@@ -140,13 +164,18 @@ impl ViewOneResult {
             for d in row.iter() {
                 let id = d.info.type_id().as_rust();
 
-                let reg_type = reg.get_type(id)
+                let reg_type = reg
+                    .get_type(id)
                     .expect("Requested type was not found in TypeRegistry");
-                let proxy = reg_type.get_data::<ReflectLuaProxy>()
+                let proxy = reg_type
+                    .get_data::<ReflectLuaProxy>()
                     // TODO: properly handle this error
                     .expect("Type does not have ReflectLuaProxy as a TypeData");
-                let value = proxy.as_lua(lua, d.ptr.cast()).unwrap()
+                let value = proxy
-                    .into_lua(lua).unwrap();
+                    .as_lua(lua, d.ptr.cast())
+                    .unwrap()
+                    .into_lua(lua)
+                    .unwrap();
 
                 vals.push(value);
             }
@@ -165,9 +194,7 @@ impl ViewOneResult {
 
 impl mlua::UserData for ViewOneResult {
     fn add_methods<M: mlua::UserDataMethods<Self>>(methods: &mut M) {
-        methods.add_method_mut("get", |lua, this, ()| {
+        methods.add_method_mut("get", |lua, this, ()| this.get_res_impl(lua));
-            this.get_res_impl(lua)
-        });
         methods.add_meta_method(mlua::MetaMethod::Call, |lua, this, ()| {
             this.get_res_impl(lua)
         });

crates/lyra-scripting/src/lua/entity_ref.rs

@@ -6,7 +6,9 @@ use mlua::{IntoLua, ObjectLike};
 
 use crate::{ScriptBorrow, ScriptWorldPtr};
 
-use super::{reflect_type_user_data, Error, ReflectLuaProxy, TypeLookup, FN_NAME_INTERNAL_REFLECT_TYPE};
+use super::{
+    reflect_type_user_data, Error, ReflectLuaProxy, TypeLookup, FN_NAME_INTERNAL_REFLECT_TYPE,
+};
 
 #[derive(Clone)]
 pub enum LuaComponent {
@@ -50,7 +52,7 @@ impl LuaComponent {
             }
             Self::UserData(ud) => {
                 let lua_comp = reflect_type_user_data(ud);
-                let refl_comp = lua_comp.reflect_branch.as_component_unchecked();
+                let refl_comp = lua_comp.reflect_branch.as_component()?;
                 Some(refl_comp.info.type_id().as_rust())
             }
         }
@@ -67,7 +69,11 @@ impl LuaComponent {
     /// Call a Lua function on the Component.
     ///
     /// This is a helper function so you don't have to match on the component.
-    pub fn call_function<R: mlua::FromLuaMulti>(&self, name: &str, args: impl mlua::IntoLuaMulti) -> mlua::Result<R> {
+    pub fn call_function<R: mlua::FromLuaMulti>(
+        &self,
+        name: &str,
+        args: impl mlua::IntoLuaMulti,
+    ) -> mlua::Result<R> {
         match self {
             LuaComponent::UserData(ud) => ud.call_function(name, args),
             LuaComponent::Table(t) => t.call_function(name, args),
@@ -77,7 +83,11 @@ impl LuaComponent {
     /// Call a Lua method on the Component.
     ///
     /// This is a helper function so you don't have to match on the component.
-    pub fn call_method<R: mlua::FromLuaMulti>(&self, name: &str, args: impl mlua::IntoLuaMulti) -> mlua::Result<R> {
+    pub fn call_method<R: mlua::FromLuaMulti>(
+        &self,
+        name: &str,
+        args: impl mlua::IntoLuaMulti,
+    ) -> mlua::Result<R> {
         match self {
             LuaComponent::UserData(ud) => ud.call_method(name, args),
             LuaComponent::Table(t) => t.call_method(name, args),
@@ -87,12 +97,8 @@ impl LuaComponent {
     /// Returns `true` if the Component has a function of `name`.
     pub fn has_function(&self, name: &str) -> mlua::Result<bool> {
         match self {
-            LuaComponent::UserData(ud) => {
+            LuaComponent::UserData(ud) => ud.get::<mlua::Value>(name).map(|v| !v.is_nil()),
-                ud.get::<mlua::Value>(name).map(|v| !v.is_nil())
+            LuaComponent::Table(t) => t.contains_key(name),
-            },
-            LuaComponent::Table(t) => {
-                t.contains_key(name)
-            },
         }
     }
 }
@@ -107,10 +113,7 @@ pub struct LuaEntityRef {
 
 impl LuaEntityRef {
     pub fn new(world: ScriptWorldPtr, en: Entity) -> Self {
-        Self {
+        Self { en, world }
-            en,
-            world,
-        }
     }
 }
 

crates/lyra-scripting/src/lua/proxy.rs

@@ -1,12 +1,12 @@
-use std::{any::TypeId, collections::HashMap, ptr::NonNull};
+use std::{any::TypeId, collections::HashMap, ptr::NonNull, sync::Arc};
 
-use mlua::ObjectLike;
 use lyra_ecs::{ComponentInfo, DynamicBundle};
 use lyra_reflect::Reflect;
+use mlua::ObjectLike;
 
 use crate::{ScriptBorrow, ScriptDynamicBundle};
 
-use super::{Error, FN_NAME_INTERNAL_REFLECT};
+use super::{Error, WorldError, FN_NAME_INTERNAL_REFLECT};
 
 pub trait LuaWrapper: Sized {
     type Wrap: 'static;
@@ -28,35 +28,21 @@ pub trait LuaWrapper: Sized {
 
 /// A trait that used to convert something into lua, or to set something to a value from lua.
 pub trait LuaProxy {
-    fn as_lua_value(
+    fn as_lua_value(lua: &mlua::Lua, this: &dyn Reflect) -> mlua::Result<mlua::Value>;
-        lua: &mlua::Lua,
-        this: &dyn Reflect,
-    ) -> mlua::Result<mlua::Value>;
 
-    fn apply(
+    fn apply(lua: &mlua::Lua, this: &mut dyn Reflect, value: &mlua::Value) -> mlua::Result<()>;
-        lua: &mlua::Lua,
-        this: &mut dyn Reflect,
-        value: &mlua::Value,
-    ) -> mlua::Result<()>;
 }
 
 impl<'a, T> LuaProxy for T
 where
-    T: Reflect + Clone + mlua::FromLua + mlua::IntoLua
+    T: Reflect + Clone + mlua::FromLua + mlua::IntoLua,
 {
-    fn as_lua_value(
+    fn as_lua_value(lua: &mlua::Lua, this: &dyn Reflect) -> mlua::Result<mlua::Value> {
-        lua: &mlua::Lua,
-        this: &dyn Reflect,
-    ) -> mlua::Result<mlua::Value> {
         let this = this.as_any().downcast_ref::<T>().unwrap();
         this.clone().into_lua(lua)
     }
 
-    fn apply(
+    fn apply(lua: &mlua::Lua, this: &mut dyn Reflect, apply: &mlua::Value) -> mlua::Result<()> {
-        lua: &mlua::Lua,
-        this: &mut dyn Reflect,
-        apply: &mlua::Value,
-    ) -> mlua::Result<()> {
         let this = this.as_any_mut().downcast_mut::<T>().unwrap();
         let apply = T::from_lua(apply.clone(), lua)?;
 
@@ -78,8 +64,7 @@ pub struct TypeLookup {
 /// A struct used for Proxying types to and from Lua.
 #[derive(Clone)]
 pub struct ReflectLuaProxy {
-    fn_as_lua:
+    fn_as_lua: for<'a> fn(lua: &'a mlua::Lua, this_ptr: NonNull<()>) -> mlua::Result<mlua::Value>,
-        for<'a> fn(lua: &'a mlua::Lua, this_ptr: NonNull<()>) -> mlua::Result<mlua::Value>,
     fn_apply: for<'a> fn(
         lua: &'a mlua::Lua,
         this_ptr: NonNull<()>,
@@ -91,7 +76,7 @@ impl ReflectLuaProxy {
     /// Create from a type that implements LuaProxy (among some other required traits)
     pub fn from_lua_proxy<'a, T>() -> Self
     where
-        T: Reflect + LuaProxy
+        T: Reflect + LuaProxy,
     {
         Self {
             fn_as_lua: |lua, this| -> mlua::Result<mlua::Value> {
@@ -108,7 +93,7 @@ impl ReflectLuaProxy {
     /// Create from a type that implements FromLua and AsLua
     pub fn from_as_and_from_lua<T>() -> Self
     where
-        T: mlua::FromLua + mlua::IntoLua + Clone
+        T: mlua::FromLua + mlua::IntoLua + Clone,
     {
         Self {
             fn_as_lua: |lua, this| -> mlua::Result<mlua::Value> {
@@ -132,7 +117,12 @@ impl ReflectLuaProxy {
     }
 
     /// Set the contents in the pointer to a Lua value.
-    pub fn apply(&self, lua: &mlua::Lua, this_ptr: NonNull<()>, value: &mlua::Value) -> mlua::Result<()> {
+    pub fn apply(
+        &self,
+        lua: &mlua::Lua,
+        this_ptr: NonNull<()>,
+        value: &mlua::Value,
+    ) -> mlua::Result<()> {
         (self.fn_apply)(lua, this_ptr, value)
     }
 }
@@ -152,7 +142,18 @@ impl mlua::UserData for ScriptDynamicBundle {
         methods.add_function("new", |_, ()| Ok(ScriptDynamicBundle(DynamicBundle::new())));
         methods.add_method_mut("push", |_, this, comp: mlua::AnyUserData| {
             let script_brw = comp.call_method::<ScriptBorrow>(FN_NAME_INTERNAL_REFLECT, ())?;
-            let reflect = script_brw.reflect_branch.as_component_unchecked();
+            let reflect =
+                script_brw
+                    .reflect_branch
+                    .as_component()
+                    .ok_or(mlua::Error::BadArgument {
+                        to: Some("DynamicBundle:push".into()),
+                        pos: 2,
+                        name: Some("component...".into()),
+                        cause: Arc::new(mlua::Error::external(WorldError::LuaInvalidUsage(
+                            "userdata is not a Component".into(),
+                        ))),
+                    })?;
 
             let refl_data = script_brw.data.unwrap();
             reflect.bundle_insert(&mut this.0, refl_data);

crates/lyra-scripting/src/lua/world.rs

@@ -83,7 +83,16 @@ impl mlua::UserData for ScriptWorldPtr {
                     }
                 };
 
-                let reflect = comp_borrow.reflect_branch.as_component_unchecked();
+                let reflect =
+                    comp_borrow
+                        .reflect_branch
+                        .as_component()
+                        .ok_or(mlua::Error::BadArgument {
+                            to: Some("World:spawn".into()),
+                            pos: 2 + i, // i starts at 0
+                            name: Some("components...".into()),
+                            cause: Arc::new(mlua::Error::runtime("userdata is not a Component")),
+                        })?;
                 let refl_data = comp_borrow.data.unwrap();
                 reflect.bundle_insert(&mut bundle, refl_data);
             }