From ba4acba638b3ef44c44de2d745ff2fc2fcba44a6 Mon Sep 17 00:00:00 2001
From: SeanOMik <seanomik@gmail.com>
Date: Tue, 7 Jan 2025 19:18:00 -0500
Subject: [PATCH] lua: remove ReflectBranch::as_component_unchecked and improve
error handling
---
Cargo.lock | 1 +
crates/lyra-scripting/src/lib.rs | 12 ----
crates/lyra-scripting/src/lua/ecs/view.rs | 41 ++++++++----
crates/lyra-scripting/src/lua/ecs/view_one.rs | 63 ++++++++++++-----
crates/lyra-scripting/src/lua/entity_ref.rs | 39 ++++++-----
crates/lyra-scripting/src/lua/proxy.rs | 67 ++++++++++---------
crates/lyra-scripting/src/lua/world.rs | 11 ++-
7 files changed, 139 insertions(+), 95 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
index 56ee634..df77544 100644
--- a/Cargo.lock
+++ b/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",
diff --git a/crates/lyra-scripting/src/lib.rs b/crates/lyra-scripting/src/lib.rs
index f6d0a75..442e0f5 100644
--- a/crates/lyra-scripting/src/lib.rs
+++ b/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.
diff --git a/crates/lyra-scripting/src/lua/ecs/view.rs b/crates/lyra-scripting/src/lua/ecs/view.rs
index bb051ed..242a7ce 100644
--- a/crates/lyra-scripting/src/lua/ecs/view.rs
+++ b/crates/lyra-scripting/src/lua/ecs/view.rs
@@ -53,7 +53,7 @@ impl ViewQueryItem {
}
/// Returns `true` if self is a Query.
- ///
+ ///
/// If self is a function, it will return true. Else, it checks for a function with the
/// name of [`FN_NAME_INTERNAL_ECS_QUERY_RESULT`] on the table or userdata. If the function
/// is found, it returns true.
@@ -105,7 +105,7 @@ impl mlua::UserData for View {
}
/// Results of queries in a View.
-///
+///
/// Represents the results of multiple queries.
#[derive(Debug, Clone)]
pub(crate) enum ViewQueryResult {
@@ -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");
- let refl_comp = reflect.reflect_branch.as_component()
- .expect("`self` is not an instance of `ReflectBranch::Component`");
+ .map_err(|_| 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 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!(),
}
}
@@ -223,19 +238,19 @@ impl ViewResult {
let mut index_mod = 0;
for (query, i) in &self.queries {
let qres = query.get_query_result(self.world.clone(), entity)?;
-
+
match qres {
LuaQueryResult::None => return Ok(ViewQueryResult::None),
LuaQueryResult::AlwaysNone => return Ok(ViewQueryResult::AlwaysNone),
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
diff --git a/crates/lyra-scripting/src/lua/ecs/view_one.rs b/crates/lyra-scripting/src/lua/ecs/view_one.rs
index b22b259..de6acd6 100644
--- a/crates/lyra-scripting/src/lua/ecs/view_one.rs
+++ b/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");
- let refl_comp = reflect.reflect_branch.as_component()
- .expect("`self` is not an instance of `ReflectBranch::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 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!(),
}
}
@@ -103,19 +127,19 @@ impl ViewOneResult {
let mut index_mod = 0;
for (query, i) in &self.queries {
let qres = query.get_query_result(self.world.clone(), entity)?;
-
+
match qres {
LuaQueryResult::None => return Ok(ViewQueryResult::None),
LuaQueryResult::AlwaysNone => return Ok(ViewQueryResult::AlwaysNone),
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));
- },
+ }
}
}
@@ -139,14 +163,19 @@ impl ViewOneResult {
let mut vals = vec![];
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()
- .into_lua(lua).unwrap();
+ let value = proxy
+ .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, ()| {
- this.get_res_impl(lua)
- });
+ methods.add_method_mut("get", |lua, this, ()| this.get_res_impl(lua));
methods.add_meta_method(mlua::MetaMethod::Call, |lua, this, ()| {
this.get_res_impl(lua)
});
diff --git a/crates/lyra-scripting/src/lua/entity_ref.rs b/crates/lyra-scripting/src/lua/entity_ref.rs
index 2444413..bdc7ed1 100644
--- a/crates/lyra-scripting/src/lua/entity_ref.rs
+++ b/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,14 +52,14 @@ 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())
}
}
}
/// Call the internal reflect type function and return the result.
- ///
+ ///
/// This calls the [`FN_NAME_INTERNAL_REFLECT_TYPE`] function on the Component.
pub fn reflect_type(&self) -> Result<ScriptBorrow, Error> {
self.call_function(FN_NAME_INTERNAL_REFLECT_TYPE, ())
@@ -65,9 +67,13 @@ 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),
@@ -75,9 +81,13 @@ 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) => {
- ud.get::<mlua::Value>(name).map(|v| !v.is_nil())
- },
- LuaComponent::Table(t) => {
- t.contains_key(name)
- },
+ LuaComponent::UserData(ud) => ud.get::<mlua::Value>(name).map(|v| !v.is_nil()),
+ 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 {
- en,
- world,
- }
+ Self { en, world }
}
}
@@ -139,7 +142,7 @@ impl mlua::UserData for LuaEntityRef {
let arch_idx = *arch.entity_indexes().get(&this.en).unwrap();
let col = arch.get_column_mut(tid).unwrap();
let col_ptr = col.component_ptr(*arch_idx as usize, &world_tick).cast();
-
+
// get the type registry to apply the new value
let reg = world.get_resource::<TypeRegistry>().unwrap();
let reg_type = reg.get_type(tid).unwrap();
diff --git a/crates/lyra-scripting/src/lua/proxy.rs b/crates/lyra-scripting/src/lua/proxy.rs
index 758a080..67eda65 100644
--- a/crates/lyra-scripting/src/lua/proxy.rs
+++ b/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;
@@ -18,7 +18,7 @@ pub trait LuaWrapper: Sized {
}
fn into_wrapped(self) -> Self::Wrap;
-
+
#[inline(always)]
fn from_wrapped(wrap: Self::Wrap) -> Option<Self> {
let _ = wrap;
@@ -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(
- lua: &mlua::Lua,
- this: &dyn Reflect,
- ) -> mlua::Result<mlua::Value>;
+ fn as_lua_value(lua: &mlua::Lua, this: &dyn Reflect) -> mlua::Result<mlua::Value>;
- fn apply(
- lua: &mlua::Lua,
- this: &mut dyn Reflect,
- value: &mlua::Value,
- ) -> mlua::Result<()>;
+ fn apply(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(
- lua: &mlua::Lua,
- this: &dyn Reflect,
- ) -> mlua::Result<mlua::Value> {
+ fn as_lua_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(
- lua: &mlua::Lua,
- this: &mut dyn Reflect,
- apply: &mlua::Value,
- ) -> mlua::Result<()> {
+ fn apply(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)?;
@@ -67,7 +53,7 @@ where
}
/// ECS resource that can be used to lookup types via name.
-///
+///
/// You can get the [`TypeId`] of the type via name, or the [`ComponentInfo`].
#[derive(Default)]
pub struct TypeLookup {
@@ -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:
- for<'a> fn(lua: &'a mlua::Lua, this_ptr: NonNull<()>) -> mlua::Result<mlua::Value>,
+ fn_as_lua: 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<()>,
@@ -90,8 +75,8 @@ pub struct ReflectLuaProxy {
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
+ where
+ 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> {
@@ -120,7 +105,7 @@ impl ReflectLuaProxy {
let new_val = T::from_lua(value.clone(), lua)?;
*this = new_val;
-
+
Ok(())
},
}
@@ -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);
@@ -160,4 +161,4 @@ impl mlua::UserData for ScriptDynamicBundle {
Ok(())
});
}
-}
\ No newline at end of file
+}
diff --git a/crates/lyra-scripting/src/lua/world.rs b/crates/lyra-scripting/src/lua/world.rs
index f926d17..2a2a1da 100644
--- a/crates/lyra-scripting/src/lua/world.rs
+++ b/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);
}