Skip to content

Commit d11ff2a

Browse files
makspllmakspll
committed
WIP, refactor things
1 parent e01c516 commit d11ff2a

File tree

4 files changed

+101
-252
lines changed

4 files changed

+101
-252
lines changed

‎crates/bevy_mod_scripting_core/src/bindings/reference.rs

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
//! we need wrapper types which have owned and ref variants.
77
use lockable::LockableHashMap;
88

9+
pub use itertools::Either;
10+
911
use std::{
1012
any::TypeId,
1113
cell::UnsafeCell,
@@ -28,7 +30,7 @@ use bevy::{
2830
},
2931
ptr::Ptr,
3032
reflect::{
31-
Access, DynamicEnum, DynamicTuple, ParsedPath, PartialReflect, Reflect, ReflectFromPtr, ReflectPath, ReflectPathError, TypeInfo, TypeRegistry
33+
Access, DynamicEnum, DynamicTuple, ParsedPath, PartialReflect, Reflect, ReflectFromPtr, ReflectPath, ReflectPathError, TypeData, TypeInfo, TypeRegistry
3234
},
3335
};
3436
use smallvec::SmallVec;
@@ -203,6 +205,38 @@ impl ReflectReference {
203205
reflect
204206
}
205207

208+
/// convenience for:
209+
/// - retrieving the type id at the tip of the reference
210+
/// - retrieving the type data for that type id if it exists
211+
/// - calling the function with the type data if it exists
212+
/// - or calling the function with [`self`] only if the type data does not exist
213+
pub fn map_type_data<D,D2,O,F>(self, world: &WorldAccessGuard, map: F) -> ScriptResult<O>
214+
where
215+
F: FnOnce(Self, Option<Either<D,D2>>) -> O,
216+
D: TypeData + Clone,
217+
D2: TypeData + Clone,
218+
{
219+
if let Some(type_id) = self.with_reflect(&world, |r, _, _| {
220+
r.get_represented_type_info().map(|t| t.type_id())
221+
})? {
222+
223+
if let Some(type_data) = world.with_resource(|_, type_registry: Mut<AppTypeRegistry>| {
224+
let type_registry = type_registry.read();
225+
type_registry.get_type_data::<D>(type_id).cloned()
226+
}) {
227+
return Ok(map(self, Some(Either::Left(type_data))));
228+
} else if let Some(type_data) = world.with_resource(|_, type_registry: Mut<AppTypeRegistry>| {
229+
let type_registry = type_registry.read();
230+
type_registry.get_type_data::<D2>(type_id).cloned()
231+
}) {
232+
return Ok(map(self, Some(Either::Right(type_data))));
233+
}
234+
}
235+
236+
Ok(map(self, None))
237+
}
238+
239+
206240
/// Returns `Ok(())` if the given access is sufficient to read the value or an appropriate error otherwise
207241
pub fn expect_read_access<'w>(
208242
&self,
@@ -255,7 +289,7 @@ impl ReflectReference {
255289
self.expect_read_access(access, type_registry, allocator, world)?;
256290
// Safety: since we have read access to the underlying componentId we can safely access the component
257291
// and we can return a reference tied to its lifetime, which will prevent invalid aliasing
258-
return unsafe { self.reflect_unsafe(world, type_registry, allocator) };
292+
unsafe { self.reflect_unsafe(world, type_registry, allocator) }
259293
}
260294

261295
/// Retrieves a reference to the underlying `dyn PartialReflect` type valid for the 'w lifetime of the world cell.
@@ -272,7 +306,7 @@ impl ReflectReference {
272306
self.expect_write_access(access, type_registry, allocator, world)?;
273307
// Safety: since we have write access to the underlying reflect access id we can safely access the component
274308
// and we can return a reference tied to its lifetime, which will prevent invalid aliasing
275-
return unsafe { self.reflect_mut_unsafe(world, type_registry, allocator) };
309+
unsafe { self.reflect_mut_unsafe(world, type_registry, allocator) }
276310
}
277311

278312
/// Retrieves a reference to the underlying `dyn PartialReflect` type valid for the 'w lifetime of the world cell
@@ -416,9 +450,9 @@ impl ReflectBaseType {
416450
let type_name = Self::type_name(self.type_id, type_registry);
417451

418452
let kind = match self.base_id {
419-
ReflectBase::Component(entity, _) => "Component",
453+
ReflectBase::Component(_, _) => "Component",
420454
ReflectBase::Resource(_) => "Resource",
421-
ReflectBase::Owned(id) => "Allocation",
455+
ReflectBase::Owned(_) => "Allocation",
422456
};
423457

424458
format!("{}({})", kind, type_name)

‎crates/bevy_mod_scripting_core/src/reflection_extensions.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::{any::TypeId, cmp::max};
22

3-
use bevy::reflect::{List, PartialReflect};
3+
use bevy::reflect::{FromType, List, PartialReflect, TypeData};
44
use itertools::Itertools;
55

66
use crate::error::ScriptError;
@@ -39,11 +39,11 @@ pub trait PartialReflectExt {
3939
fn insert_at(&mut self, index: usize, value: Box<dyn PartialReflect>) -> Result<(), ScriptError>;
4040
}
4141
pub trait TypeIdExtensions {
42-
fn type_id_or_dummy(&self) -> TypeId;
42+
fn type_id_or_fake_id(&self) -> TypeId;
4343
}
4444

4545
impl TypeIdExtensions for Option<TypeId> {
46-
fn type_id_or_dummy(&self) -> TypeId {
46+
fn type_id_or_fake_id(&self) -> TypeId {
4747
struct UknownType;
4848
match self {
4949
Some(t) => *t,
@@ -189,13 +189,11 @@ impl<T: PartialReflect + ?Sized> PartialReflectExt for T {
189189

190190
}
191191

192-
193-
194-
195192
#[cfg(test)]
196193
mod test {
197194
use super::*;
198195

196+
199197
#[test]
200198
fn test_type_no_crate() {
201199
assert!(42.is_type(None, "i32"));

‎crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs

Lines changed: 57 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use bevy::{
99
};
1010
use bevy_mod_scripting_core::{
1111
bindings::{
12-
ReflectAllocator, ReflectRefIter, ReflectReference, ReflectionPathElem, Unproxy,
12+
Either, ReflectAllocator, ReflectRefIter, ReflectReference, ReflectionPathElem, Unproxy,
1313
WorldCallbackAccess,
1414
},
1515
error::ScriptError,
@@ -54,99 +54,64 @@ impl LuaReflectReference {
5454
pub fn to_lua_proxy(self, lua: &Lua) -> Result<Value<'_>, mlua::Error> {
5555
// note we do not need to refer to LuaWorld here, it does not matter what the proxy is, that's pretty neat,
5656
let world = lua.get_world();
57-
// TODO: i don't like the pingponging between errors here, need something more ergonomic
58-
// first we need the type id of the pointed to object to figure out how to work with it
59-
let type_id = self.0.with_reflect(&world, |r, _, _| {
60-
r.get_represented_type_info().map(|t| t.type_id())
61-
})?;
62-
63-
// convenience, ideally we probably should just avoid lookups when no type id is here, but for now we just use a dummy type nothing will ever
64-
// be registered for. If the type we're reflecting doesn't represent anything or a registered type, we use a generic reflect reference.
65-
struct Dummy;
66-
let type_id_or_dummy = type_id.unwrap_or(TypeId::of::<Dummy>());
67-
68-
if let Some(type_data) = world.with_resource::<AppTypeRegistry, _, _>(|_, type_registry| {
69-
type_registry
70-
.read()
71-
.get_type_data::<ReflectLuaValue>(type_id_or_dummy)
72-
.cloned()
73-
}) {
74-
self.0
75-
.with_reflect(&world, |r, _, _| (type_data.into_value)(r, lua))?
76-
} else if let Some(type_data) =
77-
world.with_resource::<AppTypeRegistry, _, _>(|_, type_registry| {
78-
type_registry
79-
.read()
80-
.get_type_data::<ReflectLuaProxied>(type_id_or_dummy)
81-
.cloned()
82-
})
83-
{
84-
Ok((type_data.into_proxy)(self.0.clone(), lua)?)
85-
} else {
86-
Ok(self.clone().into_lua(lua)?)
87-
}
57+
58+
self.0.map_type_data(
59+
&world,
60+
|self_, type_data: Option<Either<ReflectLuaValue, ReflectLuaProxied>>| match type_data {
61+
Some(Either::Left(value_data)) => {
62+
self_.with_reflect(&world, |r, _, _| (value_data.into_value)(r, lua))?
63+
}
64+
Some(Either::Right(proxy_data)) => Ok((proxy_data.into_proxy)(self_, lua)?),
65+
None => Ok(LuaReflectReference(self_).into_lua(lua)?),
66+
},
67+
)?
8868
}
8969

90-
pub fn set_with_lua_proxy(&self, lua: &Lua, value: Value) -> Result<(), mlua::Error> {
70+
pub fn set_with_lua_proxy(self, lua: &Lua, value: Value) -> Result<(), mlua::Error> {
9171
bevy::log::debug!("Setting lua reflect reference with value: {:?}", value);
9272

9373
let world = lua.get_world();
94-
let type_id = self.0.with_reflect(&world, |r, _, _| {
95-
r.get_represented_type_info().map(|t| t.type_id())
96-
})?;
97-
98-
// convenience, ideally we probably should just avoid lookups when no type id is here, but for now we just use a dummy type nothing will ever
99-
// be registered for. If the type we're reflecting doesn't represent anything or a registered type, we use a generic reflect reference.
100-
struct Unknown;
101-
let type_id_or_dummy = type_id.unwrap_or(TypeId::of::<Unknown>());
102-
103-
if let Some(type_data) = world.with_resource::<AppTypeRegistry, _, _>(|_, type_registry| {
104-
type_registry
105-
.read()
106-
.get_type_data::<ReflectLuaValue>(type_id_or_dummy)
107-
.cloned()
108-
}) {
109-
bevy::log::debug!("Setting value with ReflectLuaValue registration");
110-
let other = (type_data.from_value)(value, lua)?;
111-
self.0.with_reflect_mut(&world, |r, _, _| {
112-
r.try_apply(other.as_partial_reflect())
113-
.map_err(ScriptError::new_reflection_error)
114-
})??;
115-
} else if let Some(type_data) =
116-
world.with_resource::<AppTypeRegistry, _, _>(|_, type_registry| {
117-
type_registry
118-
.read()
119-
.get_type_data::<ReflectLuaProxied>(type_id_or_dummy)
120-
.cloned()
121-
})
122-
{
123-
bevy::log::debug!("Setting value with ReflectLuaProxied registration");
124-
let other = (type_data.from_proxy)(value, lua)?;
125-
let other = other.with_reflect(&world, |r, _, _| r.clone_value())?;
126-
// now we can set it
127-
self.0.with_reflect_mut(&world, |r, _, _| {
128-
if let Some(set) = type_data.opt_set {
129-
set(r, other)
130-
} else {
131-
r.try_apply(other.as_partial_reflect())
132-
.map_err(ScriptError::new_reflection_error)?;
133-
Ok(())
134-
}
135-
})??;
136-
} else {
137-
bevy::log::debug!("No registration found, throwing error");
138-
// we don't know how to assign the value
139-
// prod to see if it's a common container (i.e. Option or Vec)
140-
world.with_resource::<AppTypeRegistry, _, _>(|_,type_registry| {
141-
let type_registry = type_registry.read();
142-
Err(ScriptError::new_runtime_error(format!(
143-
"Invalid assignment `{:?}` = `{:?}`. The underlying type does: `{}` not support assignment.",
144-
self.0.print_with_type_registry(&type_registry),
145-
value,
146-
type_registry.get_type_info(type_id_or_dummy).map(|t| t.type_path()).unwrap_or_else(|| "Unknown")
74+
75+
self.0.map_type_data(
76+
&world.clone(),
77+
move |self_, type_data: Option<Either<ReflectLuaValue, ReflectLuaProxied>>| {
78+
// let world = world.clone(); // move copy into closure
79+
match type_data {
80+
Some(Either::Left(value_data)) => {
81+
let other = (value_data.from_value)(value, lua)?;
82+
self_.with_reflect_mut(&world, |r, _, _| {
83+
r.try_apply(other.as_partial_reflect())
84+
.map_err(ScriptError::new_reflection_error)
85+
})?
86+
}
87+
Some(Either::Right(proxy_data)) => {
88+
let other = (proxy_data.from_proxy)(value, lua)?;
89+
let other = other.with_reflect(&world, |r, _, _| r.clone_value())?;
90+
// now we can set it
91+
self_.with_reflect_mut(&world, |r, _, _| {
92+
if let Some(set) = proxy_data.opt_set {
93+
set(r, other)
94+
} else {
95+
r.try_apply(other.as_partial_reflect())
96+
.map_err(ScriptError::new_reflection_error)?;
97+
Ok(())
98+
}
99+
})?
100+
}
101+
None => {
102+
world.with_resource(|_, type_registry: Mut<AppTypeRegistry>| {
103+
let type_registry = type_registry.read();
104+
Err(ScriptError::new_runtime_error(format!(
105+
"Invalid assignment `{:?}` = `{:?}`. The underlying type does not support assignment.",
106+
self_.print_with_type_registry(&type_registry),
107+
value
147108
)))
148-
})?;
149-
};
109+
})?;
110+
Ok(())
111+
},
112+
}
113+
},
114+
)??;
150115
Ok(())
151116
}
152117

@@ -250,8 +215,10 @@ impl TealData for LuaReflectReference {
250215

251216
m.add_function_mut(
252217
"insert",
253-
|l, (mut self_, key): (LuaReflectReference, usize, LuaReflect)| {
254-
self_.0.insert_at(key, l.get_world())
218+
|l, (mut self_, key): (LuaReflectReference, usize, Value)| {
219+
// check target type for a from_lua function
220+
// let world = l.get_world();
221+
// self.0.
255222
},
256223
);
257224

0 commit comments

Comments
 (0)