From fc0c76fd59e91fcbc7510817555160c612234eea Mon Sep 17 00:00:00 2001 From: Mark Suckerberg Date: Fri, 15 Nov 2024 05:17:40 -0600 Subject: [PATCH 1/5] Adds sanity check on visuals update callback for turfs with null air mixes --- src/turfs.rs | 12 +++++++----- src/turfs/processing.rs | 9 +++++++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/turfs.rs b/src/turfs.rs index 5ba7a0bc..8779fedb 100644 --- a/src/turfs.rs +++ b/src/turfs.rs @@ -533,14 +533,16 @@ fn update_visuals(src: ByondValue) -> Result { // gas_overlays: list( GAS_ID = list( VIS_FACTORS = OVERLAYS )) got it? I don't let gas_overlays = ByondValue::new_global_ref() .read_var_id(byond_string!("GLOB")) - .wrap_err("GLOB is null")? + .wrap_err("Unable to get GLOB from BYOND globals")? .read_var_id(byond_string!("gas_data")) - .wrap_err("gas_data is null")? + .wrap_err("gas_data is undefined on GLOB")? .read_var_id(byond_string!("overlays")) - .wrap_err("overlays is null")?; + .wrap_err("overlays is undefined in GLOB.gas_data")?; let ptr = air - .read_number_id(byond_string!("_extools_pointer_gasmixture")) - .wrap_err("Gas mixture doesn't have a valid pointer")? as usize; + .read_var_id(byond_string!("_extools_pointer_gasmixture")) + .wrap_err("air is undefined on turf")? + .get_number() + .wrap_err("Gas mixture has invalid pointer")? as usize; let overlay_types = GasArena::with_gas_mixture(ptr, |mix| { Ok(mix .enumerate() diff --git a/src/turfs/processing.rs b/src/turfs/processing.rs index 42d2a145..8da1313c 100644 --- a/src/turfs/processing.rs +++ b/src/turfs/processing.rs @@ -420,6 +420,15 @@ fn post_process() { if should_update_vis { drop(sender.try_send(Box::new(move || { let turf = ByondValue::new_ref(ValueType::Turf, id); + + // Not valid for visuals updating if it doesn't have air defined now + if !turf + .read_var_id(byond_string!("air")) + .is_ok_and(|air| !air.is_null()) + { + return Ok(()); + } + update_visuals(turf).wrap_err("Updating Visuals")?; Ok(()) }))); From b96d72c48e128f5489491b0d5acc2b175b0a9678 Mon Sep 17 00:00:00 2001 From: Mark Suckerberg Date: Fri, 15 Nov 2024 19:13:42 -0600 Subject: [PATCH 2/5] Does the same for reaction pushing, and saves a lookup call on the turf's air --- src/turfs.rs | 87 +++++++++++++++++++---------------------- src/turfs/processing.rs | 14 ++++--- 2 files changed, 49 insertions(+), 52 deletions(-) diff --git a/src/turfs.rs b/src/turfs.rs index 8779fedb..60223926 100644 --- a/src/turfs.rs +++ b/src/turfs.rs @@ -525,53 +525,48 @@ fn hook_infos(src: ByondValue) -> Result { /// Will use a cached overlay list if one exists. /// # Errors /// If auxgm wasn't implemented properly or there's an invalid gas mixture. -fn update_visuals(src: ByondValue) -> Result { +fn update_visuals(src: ByondValue, air: ByondValue) -> Result { use super::gas; - match src.read_var_id(byond_string!("air")) { - Err(_) => Ok(ByondValue::null()), - Ok(air) => { - // gas_overlays: list( GAS_ID = list( VIS_FACTORS = OVERLAYS )) got it? I don't - let gas_overlays = ByondValue::new_global_ref() - .read_var_id(byond_string!("GLOB")) - .wrap_err("Unable to get GLOB from BYOND globals")? - .read_var_id(byond_string!("gas_data")) - .wrap_err("gas_data is undefined on GLOB")? - .read_var_id(byond_string!("overlays")) - .wrap_err("overlays is undefined in GLOB.gas_data")?; - let ptr = air - .read_var_id(byond_string!("_extools_pointer_gasmixture")) - .wrap_err("air is undefined on turf")? - .get_number() - .wrap_err("Gas mixture has invalid pointer")? as usize; - let overlay_types = GasArena::with_gas_mixture(ptr, |mix| { - Ok(mix - .enumerate() - .filter_map(|(idx, moles)| Some((idx, moles, gas::types::gas_visibility(idx)?))) - .filter(|(_, moles, amt)| moles > amt) - // getting the list(VIS_FACTORS = OVERLAYS) with GAS_ID - .filter_map(|(idx, moles, _)| { - Some(( - gas_overlays.read_list_index(gas::gas_idx_to_id(idx)).ok()?, - moles, - )) - }) - // getting the OVERLAYS with VIS_FACTOR - .filter_map(|(this_overlay_list, moles)| { - this_overlay_list - .read_list_index(gas::mixture::visibility_step(moles) as f32) - .ok() - }) - .collect::>()) - })?; - - Ok(src - .call_id( - byond_string!("set_visuals"), - &[overlay_types.as_slice().try_into()?], - ) - .wrap_err("Calling set_visuals")?) - } - } + // gas_overlays: list( GAS_ID = list( VIS_FACTORS = OVERLAYS )) got it? I don't + let gas_overlays = ByondValue::new_global_ref() + .read_var_id(byond_string!("GLOB")) + .wrap_err("Unable to get GLOB from BYOND globals")? + .read_var_id(byond_string!("gas_data")) + .wrap_err("gas_data is undefined on GLOB")? + .read_var_id(byond_string!("overlays")) + .wrap_err("overlays is undefined in GLOB.gas_data")?; + let ptr = air + .read_var_id(byond_string!("_extools_pointer_gasmixture")) + .wrap_err("air is undefined on turf")? + .get_number() + .wrap_err("Gas mixture has invalid pointer")? as usize; + let overlay_types = GasArena::with_gas_mixture(ptr, |mix| { + Ok(mix + .enumerate() + .filter_map(|(idx, moles)| Some((idx, moles, gas::types::gas_visibility(idx)?))) + .filter(|(_, moles, amt)| moles > amt) + // getting the list(VIS_FACTORS = OVERLAYS) with GAS_ID + .filter_map(|(idx, moles, _)| { + Some(( + gas_overlays.read_list_index(gas::gas_idx_to_id(idx)).ok()?, + moles, + )) + }) + // getting the OVERLAYS with VIS_FACTOR + .filter_map(|(this_overlay_list, moles)| { + this_overlay_list + .read_list_index(gas::mixture::visibility_step(moles) as f32) + .ok() + }) + .collect::>()) + })?; + + Ok(src + .call_id( + byond_string!("set_visuals"), + &[overlay_types.as_slice().try_into()?], + ) + .wrap_err("Calling set_visuals")?) } const fn adjacent_tile_id(id: u8, i: TurfID, max_x: i32, max_y: i32) -> TurfID { diff --git a/src/turfs/processing.rs b/src/turfs/processing.rs index 8da1313c..e4829f00 100644 --- a/src/turfs/processing.rs +++ b/src/turfs/processing.rs @@ -412,6 +412,9 @@ fn post_process() { let Ok(air) = turf.read_var_id(byond_string!("air")) else { return Ok(()); }; + if air.is_null() { + return Ok(()); + } react_hook(air, turf).wrap_err("Reacting")?; Ok(()) }))); @@ -422,14 +425,13 @@ fn post_process() { let turf = ByondValue::new_ref(ValueType::Turf, id); // Not valid for visuals updating if it doesn't have air defined now - if !turf - .read_var_id(byond_string!("air")) - .is_ok_and(|air| !air.is_null()) - { + let Ok(air) = turf.read_var_id(byond_string!("air")) else { + return Ok(()); + }; + if air.is_null() { return Ok(()); } - - update_visuals(turf).wrap_err("Updating Visuals")?; + update_visuals(turf, air).wrap_err("Updating Visuals")?; Ok(()) }))); } From 795eca891599cef61a704c93e4b147142bc495b0 Mon Sep 17 00:00:00 2001 From: Mark Suckerberg Date: Sat, 16 Nov 2024 12:37:19 -0600 Subject: [PATCH 3/5] Tidies it up --- src/turfs.rs | 92 +++++++++++++++++++++++------------------ src/turfs/processing.rs | 26 +++++------- 2 files changed, 61 insertions(+), 57 deletions(-) diff --git a/src/turfs.rs b/src/turfs.rs index 60223926..fc1f2a62 100644 --- a/src/turfs.rs +++ b/src/turfs.rs @@ -525,48 +525,58 @@ fn hook_infos(src: ByondValue) -> Result { /// Will use a cached overlay list if one exists. /// # Errors /// If auxgm wasn't implemented properly or there's an invalid gas mixture. -fn update_visuals(src: ByondValue, air: ByondValue) -> Result { +fn update_visuals(src: ByondValue) -> Result { use super::gas; - // gas_overlays: list( GAS_ID = list( VIS_FACTORS = OVERLAYS )) got it? I don't - let gas_overlays = ByondValue::new_global_ref() - .read_var_id(byond_string!("GLOB")) - .wrap_err("Unable to get GLOB from BYOND globals")? - .read_var_id(byond_string!("gas_data")) - .wrap_err("gas_data is undefined on GLOB")? - .read_var_id(byond_string!("overlays")) - .wrap_err("overlays is undefined in GLOB.gas_data")?; - let ptr = air - .read_var_id(byond_string!("_extools_pointer_gasmixture")) - .wrap_err("air is undefined on turf")? - .get_number() - .wrap_err("Gas mixture has invalid pointer")? as usize; - let overlay_types = GasArena::with_gas_mixture(ptr, |mix| { - Ok(mix - .enumerate() - .filter_map(|(idx, moles)| Some((idx, moles, gas::types::gas_visibility(idx)?))) - .filter(|(_, moles, amt)| moles > amt) - // getting the list(VIS_FACTORS = OVERLAYS) with GAS_ID - .filter_map(|(idx, moles, _)| { - Some(( - gas_overlays.read_list_index(gas::gas_idx_to_id(idx)).ok()?, - moles, - )) - }) - // getting the OVERLAYS with VIS_FACTOR - .filter_map(|(this_overlay_list, moles)| { - this_overlay_list - .read_list_index(gas::mixture::visibility_step(moles) as f32) - .ok() - }) - .collect::>()) - })?; - - Ok(src - .call_id( - byond_string!("set_visuals"), - &[overlay_types.as_slice().try_into()?], - ) - .wrap_err("Calling set_visuals")?) + match src.read_var_id(byond_string!("air")) { + Ok(air) if !air.is_null() => { + // gas_overlays: list( GAS_ID = list( VIS_FACTORS = OVERLAYS )) got it? I don't + let gas_overlays = ByondValue::new_global_ref() + .read_var_id(byond_string!("GLOB")) + .wrap_err("Unable to get GLOB from BYOND globals")? + .read_var_id(byond_string!("gas_data")) + .wrap_err("gas_data is undefined on GLOB")? + .read_var_id(byond_string!("overlays")) + .wrap_err("overlays is undefined in GLOB.gas_data")?; + let ptr = air + .read_var_id(byond_string!("_extools_pointer_gasmixture")) + .wrap_err("air is undefined on turf")? + .get_number() + .wrap_err("Gas mixture has invalid pointer")? as usize; + let overlay_types = GasArena::with_gas_mixture(ptr, |mix| { + Ok(mix + .enumerate() + .filter_map(|(idx, moles)| Some((idx, moles, gas::types::gas_visibility(idx)?))) + .filter(|(_, moles, amt)| moles > amt) + // getting the list(VIS_FACTORS = OVERLAYS) with GAS_ID + .filter_map(|(idx, moles, _)| { + Some(( + gas_overlays.read_list_index(gas::gas_idx_to_id(idx)).ok()?, + moles, + )) + }) + // getting the OVERLAYS with VIS_FACTOR + .filter_map(|(this_overlay_list, moles)| { + this_overlay_list + .read_list_index(gas::mixture::visibility_step(moles) as f32) + .ok() + }) + .collect::>()) + })?; + + Ok(src + .call_id( + byond_string!("set_visuals"), + &[overlay_types.as_slice().try_into()?], + ) + .wrap_err("Calling set_visuals")?) + } + // If air is not defined or is null, just call set_visuals with no args + _ => { + return Ok(src + .call_id(byond_string!("set_visuals"), &[]) + .wrap_err("Calling set_visuals with no args")?); + } + } } const fn adjacent_tile_id(id: u8, i: TurfID, max_x: i32, max_y: i32) -> TurfID { diff --git a/src/turfs/processing.rs b/src/turfs/processing.rs index e4829f00..083ab21b 100644 --- a/src/turfs/processing.rs +++ b/src/turfs/processing.rs @@ -408,15 +408,14 @@ fn post_process() { if should_react { drop(sender.try_send(Box::new(move || { let turf = ByondValue::new_ref(ValueType::Turf, id); - //turf is no longer valid for reactions - let Ok(air) = turf.read_var_id(byond_string!("air")) else { - return Ok(()); - }; - if air.is_null() { - return Ok(()); + match turf.read_var_id(byond_string!("air")) { + Ok(air) if !air.is_null() => { + react_hook(air, turf).wrap_err("Reacting")?; + Ok(()) + } + //turf is no longer valid for reactions + _ => Ok(()), } - react_hook(air, turf).wrap_err("Reacting")?; - Ok(()) }))); } @@ -424,14 +423,9 @@ fn post_process() { drop(sender.try_send(Box::new(move || { let turf = ByondValue::new_ref(ValueType::Turf, id); - // Not valid for visuals updating if it doesn't have air defined now - let Ok(air) = turf.read_var_id(byond_string!("air")) else { - return Ok(()); - }; - if air.is_null() { - return Ok(()); - } - update_visuals(turf, air).wrap_err("Updating Visuals")?; + //Turf is checked for validity in update_visuals + + update_visuals(turf).wrap_err("Updating Visuals")?; Ok(()) }))); } From 55a534aeff1c2e05a56b3aabe7cdbffceeee8c3f Mon Sep 17 00:00:00 2001 From: Mark Suckerberg Date: Sat, 16 Nov 2024 12:41:17 -0600 Subject: [PATCH 4/5] Clippy changes and consistency --- src/turfs.rs | 4 ++-- src/turfs/processing.rs | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/turfs.rs b/src/turfs.rs index fc1f2a62..440ddd72 100644 --- a/src/turfs.rs +++ b/src/turfs.rs @@ -568,13 +568,13 @@ fn update_visuals(src: ByondValue) -> Result { byond_string!("set_visuals"), &[overlay_types.as_slice().try_into()?], ) - .wrap_err("Calling set_visuals")?) + .wrap_err("Calling set_visuals")) } // If air is not defined or is null, just call set_visuals with no args _ => { return Ok(src .call_id(byond_string!("set_visuals"), &[]) - .wrap_err("Calling set_visuals with no args")?); + .wrap_err("Calling set_visuals with no args")); } } } diff --git a/src/turfs/processing.rs b/src/turfs/processing.rs index 083ab21b..57299314 100644 --- a/src/turfs/processing.rs +++ b/src/turfs/processing.rs @@ -423,8 +423,7 @@ fn post_process() { drop(sender.try_send(Box::new(move || { let turf = ByondValue::new_ref(ValueType::Turf, id); - //Turf is checked for validity in update_visuals - + //turf is checked for validity in update_visuals update_visuals(turf).wrap_err("Updating Visuals")?; Ok(()) }))); From 5953f1594005ebebfa0237e2d91cd65d5a374e08 Mon Sep 17 00:00:00 2001 From: Mark Suckerberg Date: Sat, 16 Nov 2024 12:49:58 -0600 Subject: [PATCH 5/5] This is actually the proper solution --- src/turfs.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/turfs.rs b/src/turfs.rs index 440ddd72..e7b1a8f0 100644 --- a/src/turfs.rs +++ b/src/turfs.rs @@ -568,14 +568,14 @@ fn update_visuals(src: ByondValue) -> Result { byond_string!("set_visuals"), &[overlay_types.as_slice().try_into()?], ) - .wrap_err("Calling set_visuals")) - } - // If air is not defined or is null, just call set_visuals with no args - _ => { - return Ok(src - .call_id(byond_string!("set_visuals"), &[]) - .wrap_err("Calling set_visuals with no args")); + .wrap_err("Calling set_visuals")?) } + // If air is null, clear the visuals + Ok(_) => Ok(src + .call_id(byond_string!("set_visuals"), &[]) + .wrap_err("Calling set_visuals with no args")?), + // If air is not defined, it must be a closed turf. Do .othing + Err(_) => Ok(ByondValue::null()), } }