Skip to content

feat: Don't panic! #216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Jan 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,9 @@ debug = true
name = "game_of_life"
path = "examples/game_of_life.rs"
required-features = ["lua54", "bevy/file_watcher", "bevy/multi_threaded"]

[workspace.lints.clippy]
panic = "deny"
unwrap_used = "deny"
expect_used = "deny"
todo = "deny"
1 change: 0 additions & 1 deletion assets/scripts/game_of_life.lua
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ end

function on_update()
local cells = fetch_life_state().cells
world.log_all_allocations()
local settings = world.get_resource(Settings)
local dimensions = settings.physical_grid_dimensions
local dimension_x = dimensions._1
Expand Down
13 changes: 13 additions & 0 deletions badges/no-panics.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
type-complexity-threshold = 1000
too-many-arguments-threshold = 999
allow-panic-in-tests = true
allow-unwrap-in-tests = true
allow-expect-in-tests = true
2 changes: 0 additions & 2 deletions crates/bevy_api_gen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,3 @@ chrono = "0.4"

[build-dependencies]
toml = "0.8"

[workspace]
3 changes: 3 additions & 0 deletions crates/bevy_mod_scripting_core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,6 @@ itertools = "0.13"

[dev-dependencies]
test_utils = { workspace = true }

[lints]
workspace = true
4 changes: 1 addition & 3 deletions crates/bevy_mod_scripting_core/src/asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -544,9 +544,7 @@ mod tests {
type C = ();
const LANGUAGE: Language = Language::Lua;

fn build_runtime() -> Self::R {
todo!()
}
fn build_runtime() -> Self::R {}
}

#[test]
Expand Down
62 changes: 36 additions & 26 deletions crates/bevy_mod_scripting_core/src/bindings/access_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use bevy::{
use dashmap::{DashMap, Entry};
use smallvec::SmallVec;

use crate::error::InteropError;

use super::{ReflectAllocationId, ReflectBase};

#[derive(Debug, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -80,8 +82,8 @@ pub enum ReflectAccessKind {
/// for script owned values this is an allocationId, this is used to ensure we have permission to access the value.
#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)]
pub struct ReflectAccessId {
kind: ReflectAccessKind,
id: u64,
pub(crate) kind: ReflectAccessKind,
pub(crate) id: u64,
}

impl AccessMapKey for ReflectAccessId {
Expand Down Expand Up @@ -111,19 +113,25 @@ impl AccessMapKey for ReflectAccessId {
}

impl ReflectAccessId {
pub fn for_resource<R: Resource>(cell: &UnsafeWorldCell) -> Option<Self> {
Some(Self {
pub fn for_resource<R: Resource>(cell: &UnsafeWorldCell) -> Result<Self, InteropError> {
let resource_id = cell.components().resource_id::<R>().ok_or_else(|| {
InteropError::unregistered_component_or_resource_type(std::any::type_name::<R>())
})?;

Ok(Self {
kind: ReflectAccessKind::ComponentOrResource,
id: cell.components().resource_id::<R>()?.index() as u64,
id: resource_id.index() as u64,
})
}

pub fn for_component<C: bevy::ecs::component::Component>(
cell: &UnsafeWorldCell,
) -> Option<Self> {
let component_id = cell.components().component_id::<C>()?;
) -> Result<Self, InteropError> {
let component_id = cell.components().component_id::<C>().ok_or_else(|| {
InteropError::unregistered_component_or_resource_type(std::any::type_name::<C>())
})?;

Some(Self::for_component_id(component_id))
Ok(Self::for_component_id(component_id))
}

pub fn for_allocation(id: ReflectAllocationId) -> Self {
Expand All @@ -140,11 +148,11 @@ impl ReflectAccessId {
}
}

pub fn for_reference(base: ReflectBase) -> Option<Self> {
pub fn for_reference(base: ReflectBase) -> Self {
match base {
ReflectBase::Resource(id) => Some(Self::for_component_id(id)),
ReflectBase::Component(_, id) => Some(Self::for_component_id(id)),
ReflectBase::Owned(id) => Some(Self::for_allocation(id)),
ReflectBase::Resource(id) => Self::for_component_id(id),
ReflectBase::Component(_, id) => Self::for_component_id(id),
ReflectBase::Owned(id) => Self::for_allocation(id),
}
}
}
Expand Down Expand Up @@ -173,6 +181,12 @@ impl From<ReflectAccessId> for ComponentId {
}
}

impl From<ReflectAccessId> for ReflectAllocationId {
fn from(val: ReflectAccessId) -> Self {
ReflectAllocationId::new(val.id)
}
}

#[derive(Debug, Default)]
pub struct AccessMap {
individual_accesses: DashMap<u64, AccessCount>,
Expand Down Expand Up @@ -323,17 +337,15 @@ impl DisplayCodeLocation for Option<std::panic::Location<'_>> {
macro_rules! with_access_read {
($access_map:expr, $id:expr, $msg:expr, $body:block) => {{
if !$access_map.claim_read_access($id) {
panic!(
"{}. Aliasing access is held somewhere else: {}",
Err($crate::error::InteropError::cannot_claim_access(
$id,
$access_map.access_location($id),
$msg,
$crate::bindings::access_map::DisplayCodeLocation::display_location(
$access_map.access_location($id)
)
);
))
} else {
let result = $body;
$access_map.release_access($id);
result
Ok(result)
}
}};
}
Expand All @@ -342,17 +354,15 @@ macro_rules! with_access_read {
macro_rules! with_access_write {
($access_map:expr, $id:expr, $msg:expr, $body:block) => {
if !$access_map.claim_write_access($id) {
panic!(
"{}. Aliasing access is held somewhere else: {}",
Err($crate::error::InteropError::cannot_claim_access(
$id,
$access_map.access_location($id),
$msg,
$crate::bindings::access_map::DisplayCodeLocation::display_location(
$access_map.access_location($id)
)
);
))
} else {
let result = $body;
$access_map.release_access($id);
result
Ok(result)
}
};
}
Expand Down
20 changes: 9 additions & 11 deletions crates/bevy_mod_scripting_core/src/bindings/function/from.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ impl FromScript for char {
{
match value {
ScriptValue::Integer(i) => Ok(i as u8 as char),
ScriptValue::String(c) if c.len() == 1 => Ok(c.chars().next().expect("invariant")),
ScriptValue::String(c) => c.chars().next().ok_or_else(|| {
InteropError::value_mismatch(TypeId::of::<char>(), ScriptValue::String(c))
}),
ScriptValue::Reference(r) => r.downcast::<Self>(world),
_ => Err(InteropError::value_mismatch(
std::any::TypeId::of::<Self>(),
Expand Down Expand Up @@ -226,10 +228,7 @@ impl<T: FromReflect> FromScript for Ref<'_, T> {
) -> Result<Self::This<'_>, InteropError> {
match value {
ScriptValue::Reference(reflect_reference) => {
let raid = ReflectAccessId::for_reference(reflect_reference.base.base_id.clone())
.ok_or_else(|| {
InteropError::unregistered_base(reflect_reference.base.clone())
})?;
let raid = ReflectAccessId::for_reference(reflect_reference.base.base_id.clone());

if world.claim_read_access(raid) {
// Safety: we just claimed access
Expand All @@ -243,8 +242,9 @@ impl<T: FromReflect> FromScript for Ref<'_, T> {
Ok(Ref(cast))
} else {
Err(InteropError::cannot_claim_access(
reflect_reference.base,
raid,
world.get_access_location(raid),
format!("In conversion to type: Ref<{}>", std::any::type_name::<T>()),
))
}
}
Expand Down Expand Up @@ -300,10 +300,7 @@ impl<T: FromReflect> FromScript for Mut<'_, T> {
) -> Result<Self::This<'_>, InteropError> {
match value {
ScriptValue::Reference(reflect_reference) => {
let raid = ReflectAccessId::for_reference(reflect_reference.base.base_id.clone())
.ok_or_else(|| {
InteropError::unregistered_base(reflect_reference.base.clone())
})?;
let raid = ReflectAccessId::for_reference(reflect_reference.base.base_id.clone());

if world.claim_write_access(raid) {
// Safety: we just claimed write access
Expand All @@ -315,8 +312,9 @@ impl<T: FromReflect> FromScript for Mut<'_, T> {
Ok(Mut(cast))
} else {
Err(InteropError::cannot_claim_access(
reflect_reference.base,
raid,
world.get_access_location(raid),
format!("In conversion to type: Mut<{}>", std::any::type_name::<T>()),
))
}
}
Expand Down
11 changes: 4 additions & 7 deletions crates/bevy_mod_scripting_core/src/bindings/function/from_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,11 @@ impl FromScriptRef for Box<dyn PartialReflect> {
)
})?;

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

if type_info.is_list() {
let inner_type = type_info.list_inner_type().expect("invariant");

if let Some(inner_list_type) = type_info.list_inner_type() {
if let ScriptValue::List(vec) = value {
let mut dynamic_list = DynamicList::default();
for item in vec {
let inner = Self::from_script_ref(inner_type, item, world.clone())?;
let inner = Self::from_script_ref(inner_list_type, item, world.clone())?;
dynamic_list.push_box(inner);
}

Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_mod_scripting_core/src/bindings/function/into.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{borrow::Cow, ffi::OsString, path::PathBuf};

use bevy::reflect::PartialReflect;
use bevy::reflect::Reflect;

use crate::{
bindings::{script_value::ScriptValue, ReflectReference, WorldGuard},
Expand Down Expand Up @@ -112,9 +112,9 @@ impl IntoScript for ReflectReference {
}
}

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

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{ffi::OsString, path::PathBuf};

use bevy::reflect::{ParsedPath, PartialReflect};
use bevy::reflect::{Access, PartialReflect};

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

Ok(ScriptValue::Reference(self_))
}

const FIRST_TUPLE_FIELD_ACCESS: Access = Access::TupleIndex(0);
Loading
Loading