Skip to content

Commit eb1f46f

Browse files
makspllmakspll
authored
feat: Don't panic! (#216)
* add information on context sharing * expand docs slightly * apply heavy clippy lints to remove all panics * remove event handler panics * remove panics from commands * DONT PANIC * fix fmt * fix failing tests and update docs * remove unnecessary thing * add badges * update badge * Update readme.md * change badge * change badge again
1 parent ecf613d commit eb1f46f

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+1156
-622
lines changed

‎Cargo.toml

+6
Original file line numberDiff line numberDiff line change
@@ -117,3 +117,9 @@ debug = true
117117
name = "game_of_life"
118118
path = "examples/game_of_life.rs"
119119
required-features = ["lua54", "bevy/file_watcher", "bevy/multi_threaded"]
120+
121+
[workspace.lints.clippy]
122+
panic = "deny"
123+
unwrap_used = "deny"
124+
expect_used = "deny"
125+
todo = "deny"

‎assets/scripts/game_of_life.lua

-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ end
7575

7676
function on_update()
7777
local cells = fetch_life_state().cells
78-
world.log_all_allocations()
7978
local settings = world.get_resource(Settings)
8079
local dimensions = settings.physical_grid_dimensions
8180
local dimension_x = dimensions._1

‎badges/no-panics.svg

+13
Loading

‎clippy.toml

+3
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,5 @@
11
type-complexity-threshold = 1000
22
too-many-arguments-threshold = 999
3+
allow-panic-in-tests = true
4+
allow-unwrap-in-tests = true
5+
allow-expect-in-tests = true

‎crates/bevy_api_gen/Cargo.toml

-2
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,3 @@ chrono = "0.4"
5858

5959
[build-dependencies]
6060
toml = "0.8"
61-
62-
[workspace]

‎crates/bevy_mod_scripting_core/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,6 @@ itertools = "0.13"
3939

4040
[dev-dependencies]
4141
test_utils = { workspace = true }
42+
43+
[lints]
44+
workspace = true

‎crates/bevy_mod_scripting_core/src/asset.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -544,9 +544,7 @@ mod tests {
544544
type C = ();
545545
const LANGUAGE: Language = Language::Lua;
546546

547-
fn build_runtime() -> Self::R {
548-
todo!()
549-
}
547+
fn build_runtime() -> Self::R {}
550548
}
551549

552550
#[test]

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

+36-26
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

+9-11
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

+4-7
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

+3-3
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

+4-2
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);

0 commit comments

Comments
 (0)