Skip to content

Commit de854d5

Browse files
makspllmakspll
committed
DONT PANIC
1 parent fea0442 commit de854d5

File tree

18 files changed

+638
-388
lines changed

18 files changed

+638
-388
lines changed

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

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ use bevy::{
77
use dashmap::{DashMap, Entry};
88
use smallvec::SmallVec;
99

10+
use crate::error::InteropError;
11+
1012
use super::{ReflectAllocationId, ReflectBase};
1113

1214
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -80,8 +82,8 @@ pub enum ReflectAccessKind {
8082
/// for script owned values this is an allocationId, this is used to ensure we have permission to access the value.
8183
#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)]
8284
pub struct ReflectAccessId {
83-
kind: ReflectAccessKind,
84-
id: u64,
85+
pub(crate) kind: ReflectAccessKind,
86+
pub(crate) id: u64,
8587
}
8688

8789
impl AccessMapKey for ReflectAccessId {
@@ -111,19 +113,25 @@ impl AccessMapKey for ReflectAccessId {
111113
}
112114

113115
impl ReflectAccessId {
114-
pub fn for_resource<R: Resource>(cell: &UnsafeWorldCell) -> Option<Self> {
115-
Some(Self {
116+
pub fn for_resource<R: Resource>(cell: &UnsafeWorldCell) -> Result<Self, InteropError> {
117+
let resource_id = cell.components().resource_id::<R>().ok_or_else(|| {
118+
InteropError::unregistered_component_or_resource_type(std::any::type_name::<R>())
119+
})?;
120+
121+
Ok(Self {
116122
kind: ReflectAccessKind::ComponentOrResource,
117-
id: cell.components().resource_id::<R>()?.index() as u64,
123+
id: resource_id.index() as u64,
118124
})
119125
}
120126

121127
pub fn for_component<C: bevy::ecs::component::Component>(
122128
cell: &UnsafeWorldCell,
123-
) -> Option<Self> {
124-
let component_id = cell.components().component_id::<C>()?;
129+
) -> Result<Self, InteropError> {
130+
let component_id = cell.components().component_id::<C>().ok_or_else(|| {
131+
InteropError::unregistered_component_or_resource_type(std::any::type_name::<C>())
132+
})?;
125133

126-
Some(Self::for_component_id(component_id))
134+
Ok(Self::for_component_id(component_id))
127135
}
128136

129137
pub fn for_allocation(id: ReflectAllocationId) -> Self {
@@ -140,11 +148,11 @@ impl ReflectAccessId {
140148
}
141149
}
142150

143-
pub fn for_reference(base: ReflectBase) -> Option<Self> {
151+
pub fn for_reference(base: ReflectBase) -> Self {
144152
match base {
145-
ReflectBase::Resource(id) => Some(Self::for_component_id(id)),
146-
ReflectBase::Component(_, id) => Some(Self::for_component_id(id)),
147-
ReflectBase::Owned(id) => Some(Self::for_allocation(id)),
153+
ReflectBase::Resource(id) => Self::for_component_id(id),
154+
ReflectBase::Component(_, id) => Self::for_component_id(id),
155+
ReflectBase::Owned(id) => Self::for_allocation(id),
148156
}
149157
}
150158
}
@@ -173,6 +181,12 @@ impl From<ReflectAccessId> for ComponentId {
173181
}
174182
}
175183

184+
impl From<ReflectAccessId> for ReflectAllocationId {
185+
fn from(val: ReflectAccessId) -> Self {
186+
ReflectAllocationId::new(val.id)
187+
}
188+
}
189+
176190
#[derive(Debug, Default)]
177191
pub struct AccessMap {
178192
individual_accesses: DashMap<u64, AccessCount>,
@@ -323,17 +337,15 @@ impl DisplayCodeLocation for Option<std::panic::Location<'_>> {
323337
macro_rules! with_access_read {
324338
($access_map:expr, $id:expr, $msg:expr, $body:block) => {{
325339
if !$access_map.claim_read_access($id) {
326-
panic!(
327-
"{}. Aliasing access is held somewhere else: {}",
340+
Err($crate::error::InteropError::cannot_claim_access(
341+
$id,
342+
$access_map.access_location($id),
328343
$msg,
329-
$crate::bindings::access_map::DisplayCodeLocation::display_location(
330-
$access_map.access_location($id)
331-
)
332-
);
344+
))
333345
} else {
334346
let result = $body;
335347
$access_map.release_access($id);
336-
result
348+
Ok(result)
337349
}
338350
}};
339351
}
@@ -342,17 +354,15 @@ macro_rules! with_access_read {
342354
macro_rules! with_access_write {
343355
($access_map:expr, $id:expr, $msg:expr, $body:block) => {
344356
if !$access_map.claim_write_access($id) {
345-
panic!(
346-
"{}. Aliasing access is held somewhere else: {}",
357+
Err($crate::error::InteropError::cannot_claim_access(
358+
$id,
359+
$access_map.access_location($id),
347360
$msg,
348-
$crate::bindings::access_map::DisplayCodeLocation::display_location(
349-
$access_map.access_location($id)
350-
)
351-
);
361+
))
352362
} else {
353363
let result = $body;
354364
$access_map.release_access($id);
355-
result
365+
Ok(result)
356366
}
357367
};
358368
}

‎crates/bevy_mod_scripting_core/src/bindings/function/from.rs

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,9 @@ impl FromScript for char {
114114
{
115115
match value {
116116
ScriptValue::Integer(i) => Ok(i as u8 as char),
117-
ScriptValue::String(c) if c.len() == 1 => Ok(c.chars().next().expect("invariant")),
117+
ScriptValue::String(c) => c.chars().next().ok_or_else(|| {
118+
InteropError::value_mismatch(TypeId::of::<char>(), ScriptValue::String(c))
119+
}),
118120
ScriptValue::Reference(r) => r.downcast::<Self>(world),
119121
_ => Err(InteropError::value_mismatch(
120122
std::any::TypeId::of::<Self>(),
@@ -226,10 +228,7 @@ impl<T: FromReflect> FromScript for Ref<'_, T> {
226228
) -> Result<Self::This<'_>, InteropError> {
227229
match value {
228230
ScriptValue::Reference(reflect_reference) => {
229-
let raid = ReflectAccessId::for_reference(reflect_reference.base.base_id.clone())
230-
.ok_or_else(|| {
231-
InteropError::unregistered_base(reflect_reference.base.clone())
232-
})?;
231+
let raid = ReflectAccessId::for_reference(reflect_reference.base.base_id.clone());
233232

234233
if world.claim_read_access(raid) {
235234
// Safety: we just claimed access
@@ -243,8 +242,9 @@ impl<T: FromReflect> FromScript for Ref<'_, T> {
243242
Ok(Ref(cast))
244243
} else {
245244
Err(InteropError::cannot_claim_access(
246-
reflect_reference.base,
245+
raid,
247246
world.get_access_location(raid),
247+
format!("In conversion to type: Ref<{}>", std::any::type_name::<T>()),
248248
))
249249
}
250250
}
@@ -300,10 +300,7 @@ impl<T: FromReflect> FromScript for Mut<'_, T> {
300300
) -> Result<Self::This<'_>, InteropError> {
301301
match value {
302302
ScriptValue::Reference(reflect_reference) => {
303-
let raid = ReflectAccessId::for_reference(reflect_reference.base.base_id.clone())
304-
.ok_or_else(|| {
305-
InteropError::unregistered_base(reflect_reference.base.clone())
306-
})?;
303+
let raid = ReflectAccessId::for_reference(reflect_reference.base.base_id.clone());
307304

308305
if world.claim_write_access(raid) {
309306
// Safety: we just claimed write access
@@ -315,8 +312,9 @@ impl<T: FromReflect> FromScript for Mut<'_, T> {
315312
Ok(Mut(cast))
316313
} else {
317314
Err(InteropError::cannot_claim_access(
318-
reflect_reference.base,
315+
raid,
319316
world.get_access_location(raid),
317+
format!("In conversion to type: Mut<{}>", std::any::type_name::<T>()),
320318
))
321319
}
322320
}

‎crates/bevy_mod_scripting_core/src/bindings/function/from_ref.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,11 @@ impl FromScriptRef for Box<dyn PartialReflect> {
6767
)
6868
})?;
6969

70-
if type_info.is_option() {
71-
let inner_type = type_info.option_inner_type().expect("invariant");
70+
if let Some(inner_option_type) = type_info.option_inner_type() {
7271
let mut dynamic_enum = match value {
7372
ScriptValue::Unit => DynamicEnum::new("None", DynamicVariant::Unit),
7473
_ => {
75-
let inner = Self::from_script_ref(inner_type, value, world)?;
74+
let inner = Self::from_script_ref(inner_option_type, value, world)?;
7675
DynamicEnum::new(
7776
"Some",
7877
DynamicVariant::Tuple(DynamicTuple::from_iter(vec![inner])),
@@ -84,13 +83,11 @@ impl FromScriptRef for Box<dyn PartialReflect> {
8483
return Ok(Box::new(dynamic_enum));
8584
}
8685

87-
if type_info.is_list() {
88-
let inner_type = type_info.list_inner_type().expect("invariant");
89-
86+
if let Some(inner_list_type) = type_info.list_inner_type() {
9087
if let ScriptValue::List(vec) = value {
9188
let mut dynamic_list = DynamicList::default();
9289
for item in vec {
93-
let inner = Self::from_script_ref(inner_type, item, world.clone())?;
90+
let inner = Self::from_script_ref(inner_list_type, item, world.clone())?;
9491
dynamic_list.push_box(inner);
9592
}
9693

‎crates/bevy_mod_scripting_core/src/bindings/function/into.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::{borrow::Cow, ffi::OsString, path::PathBuf};
22

3-
use bevy::reflect::PartialReflect;
3+
use bevy::reflect::Reflect;
44

55
use crate::{
66
bindings::{script_value::ScriptValue, ReflectReference, WorldGuard},
@@ -112,9 +112,9 @@ impl IntoScript for ReflectReference {
112112
}
113113
}
114114

115-
impl<T: PartialReflect> IntoScript for Val<T> {
115+
impl<T: Reflect> IntoScript for Val<T> {
116116
fn into_script(self, world: WorldGuard) -> Result<ScriptValue, InteropError> {
117-
let boxed: Box<dyn PartialReflect> = Box::new(self.0);
117+
let boxed = Box::new(self.0);
118118
let allocator = world.allocator();
119119
let mut allocator = allocator.write();
120120

‎crates/bevy_mod_scripting_core/src/bindings/function/into_ref.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::{ffi::OsString, path::PathBuf};
22

3-
use bevy::reflect::{ParsedPath, PartialReflect};
3+
use bevy::reflect::{Access, PartialReflect};
44

55
use crate::{
66
bindings::{function::into::IntoScript, ReflectReference, WorldGuard},
@@ -99,7 +99,7 @@ fn into_script_ref(
9999
// either return nil or ref into
100100
if let Ok(as_option) = r.as_option() {
101101
return if let Some(s) = as_option {
102-
self_.index_path(ParsedPath::parse_static(".0").expect("invariant"));
102+
self_.index_path(vec![FIRST_TUPLE_FIELD_ACCESS]);
103103
into_script_ref(self_, s, world)
104104
} else {
105105
Ok(ScriptValue::Unit)
@@ -108,3 +108,5 @@ fn into_script_ref(
108108

109109
Ok(ScriptValue::Reference(self_))
110110
}
111+
112+
const FIRST_TUPLE_FIELD_ACCESS: Access = Access::TupleIndex(0);

‎crates/bevy_mod_scripting_core/src/bindings/function/script_function.rs

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,9 @@ impl ScriptFunctionRegistry {
398398
}
399399

400400
/// Remove a function from the registry if it exists. Returns the removed function if it was found.
401+
///
402+
/// Note if the function is overloaded, you will need to remove each overload individually.
403+
/// Use [`ScriptFunctionRegistry::remove_all_overloads`] to remove all overloads at once.
401404
pub fn remove(
402405
&mut self,
403406
namespace: Namespace,
@@ -407,6 +410,21 @@ impl ScriptFunctionRegistry {
407410
self.functions.remove(&FunctionKey { name, namespace })
408411
}
409412

413+
pub fn remove_all_overloads(
414+
&mut self,
415+
namespace: Namespace,
416+
name: impl Into<Cow<'static, str>>,
417+
) -> Result<Vec<DynamicScriptFunction>, Cow<'static, str>> {
418+
let overloads: Vec<_> = self.iter_overloads(namespace, name)?.cloned().collect();
419+
for overload in overloads.iter() {
420+
self.functions.remove(&FunctionKey {
421+
name: overload.info.name.clone(),
422+
namespace,
423+
});
424+
}
425+
Ok(overloads)
426+
}
427+
410428
fn register_overload<F, M>(
411429
&mut self,
412430
namespace: Namespace,
@@ -428,17 +446,13 @@ impl ScriptFunctionRegistry {
428446
return;
429447
}
430448

431-
for i in 1..16 {
449+
for i in 1.. {
432450
let overload = format!("{name}-{i}");
433451
if !self.contains(namespace, overload.clone()) {
434452
self.register(namespace, overload, func);
435453
return;
436454
}
437455
}
438-
439-
panic!(
440-
"Could not register overload for function {name}. Maximum number of overloads reached"
441-
);
442456
}
443457

444458
pub fn contains(&self, namespace: Namespace, name: impl Into<Cow<'static, str>>) -> bool {
@@ -476,7 +490,7 @@ impl ScriptFunctionRegistry {
476490
Err(name) => return Err(name),
477491
};
478492

479-
let overloads = (1..16)
493+
let overloads = (1..)
480494
.map(move |i| {
481495
if i == 0 {
482496
self.get_function(namespace, name.clone())
@@ -486,7 +500,7 @@ impl ScriptFunctionRegistry {
486500
}
487501
})
488502
.take_while(|o| o.is_ok())
489-
.map(|o| o.unwrap());
503+
.filter_map(|o| o.ok());
490504

491505
Ok(seed.chain(overloads))
492506
}
@@ -691,4 +705,20 @@ mod test {
691705
let removed = registry.remove(namespace, "test");
692706
assert!(removed.is_none());
693707
}
708+
709+
#[test]
710+
fn test_remove_all_overloads() {
711+
let mut registry = ScriptFunctionRegistry::default();
712+
let fn_ = |a: usize, b: usize| a + b;
713+
let namespace = Namespace::Global;
714+
registry.register(namespace, "test", fn_);
715+
let fn_2 = |a: usize, b: i32| a + (b as usize);
716+
registry.register(namespace, "test", fn_2);
717+
718+
let removed = registry
719+
.remove_all_overloads(namespace, "test")
720+
.expect("Failed to remove overloads");
721+
assert_eq!(removed.len(), 2);
722+
assert!(registry.get_function(namespace, "test").is_err());
723+
}
694724
}

0 commit comments

Comments
 (0)