From 00d596d65683af757bf9f4c6164cf8cc1995603b Mon Sep 17 00:00:00 2001 From: Deukhoofd Date: Sun, 16 Apr 2023 19:57:21 +0200 Subject: [PATCH] A bunch more work on replacing every potential panic with results --- src/dynamic_data/flow/choice_queue.rs | 24 +--- src/dynamic_data/flow/target_resolver.rs | 15 +-- src/dynamic_data/models/battle.rs | 10 +- src/dynamic_data/models/battle_party.rs | 7 +- src/dynamic_data/models/battle_side.rs | 41 ++---- src/dynamic_data/models/pokemon.rs | 37 ++++-- src/dynamic_data/models/pokemon_builder.rs | 9 +- src/dynamic_data/models/pokemon_party.rs | 27 ++-- src/dynamic_data/script_handling/mod.rs | 122 +++++++++++------- src/dynamic_data/script_handling/script.rs | 17 ++- .../script_handling/script_set.rs | 48 ++++--- .../script_handling/volatile_scripts_owner.rs | 2 +- src/ffi/dynamic_data/choices.rs | 27 ++-- src/ffi/dynamic_data/models/battle.rs | 14 +- src/ffi/dynamic_data/models/battle_party.rs | 30 ++++- src/ffi/dynamic_data/models/pokemon.rs | 10 +- src/ffi/dynamic_data/models/pokemon_party.rs | 16 +-- src/ffi/mod.rs | 17 ++- src/ffi/static_data/ability.rs | 31 +++-- src/ffi/static_data/form.rs | 25 +++- src/ffi/static_data/item.rs | 25 +++- .../static_data/libraries/nature_library.rs | 32 +++-- src/ffi/static_data/libraries/type_library.rs | 12 +- src/ffi/static_data/mod.rs | 47 ++++--- src/ffi/static_data/move_data.rs | 37 ++++-- src/ffi/static_data/species.rs | 42 ++++-- src/lib.rs | 12 ++ src/script_implementations/mod.rs | 7 + .../dynamic_data/battle_side.rs | 8 +- .../export_registry/dynamic_data/pokemon.rs | 8 +- src/script_implementations/wasm/mod.rs | 3 - src/static_data/growth_rates.rs | 8 +- src/static_data/libraries/data_library.rs | 8 +- src/utils/mod.rs | 4 + src/utils/vec_ext.rs | 36 ++++++ tests/common/test_case.rs | 2 +- tests/integration.rs | 6 +- 37 files changed, 526 insertions(+), 300 deletions(-) create mode 100644 src/utils/vec_ext.rs diff --git a/src/dynamic_data/flow/choice_queue.rs b/src/dynamic_data/flow/choice_queue.rs index 890f431..16d3724 100755 --- a/src/dynamic_data/flow/choice_queue.rs +++ b/src/dynamic_data/flow/choice_queue.rs @@ -1,7 +1,7 @@ use crate::dynamic_data::choices::TurnChoice; use crate::dynamic_data::script_handling::ScriptSource; use crate::dynamic_data::Pokemon; -use crate::{script_hook, PkmnError, ValueIdentifiable, ValueIdentifier}; +use crate::{script_hook, PkmnError, ValueIdentifiable, ValueIdentifier, VecExt}; use anyhow::Result; use anyhow_ext::anyhow; use parking_lot::lock_api::MappedRwLockReadGuard; @@ -81,9 +81,7 @@ impl ChoiceQueue { let len = self.queue.read().len(); let mut write_lock = self.queue.write(); for index in self.current..len { - let choice = &mut write_lock - .get_mut(index) - .ok_or(PkmnError::IndexOutOfBounds { index, len })?; + let choice = &mut write_lock.get_mut_res(index)?; if let Some(choice) = choice { let mut speed = choice.user().boosted_stats().speed(); script_hook!(change_speed, (*choice), choice, &mut speed); @@ -121,14 +119,9 @@ impl ChoiceQueue { return Ok(true); } - let len = queue_lock.len(); // Take the choice we want to move forward out of it's place. let choice = queue_lock - .get_mut(desired_index) - .ok_or(PkmnError::IndexOutOfBounds { - index: self.current, - len, - })? + .get_mut_res(desired_index)? .take() .ok_or(anyhow!("Choice was already taken"))?; // Iterate backwards from the spot before the choice we want to move up, push them all back @@ -136,15 +129,8 @@ impl ChoiceQueue { for index in (self.current..desired_index).rev() { queue_lock.swap(index, index + 1); } - let len = queue_lock.len(); // Place the choice that needs to be next in the next to be executed position. - let _ = queue_lock - .get_mut(self.current) - .ok_or(PkmnError::IndexOutOfBounds { - index: self.current, - len, - })? - .insert(choice); + let _ = queue_lock.get_mut_res(self.current)?.insert(choice); true } None => false, @@ -173,13 +159,13 @@ impl ValueIdentifiable for ChoiceQueue { } #[cfg(test)] +#[allow(clippy::unwrap_used)] mod tests { use super::*; use crate::defines::LevelInt; use crate::dynamic_data::{DynamicLibrary, PassChoice}; use crate::static_data::{AbilityIndex, Gender}; use std::sync::Arc; - #[test] fn create_empty_queue() { let queue = ChoiceQueue::new(Vec::new()); diff --git a/src/dynamic_data/flow/target_resolver.rs b/src/dynamic_data/flow/target_resolver.rs index cd3b2e5..99b1dfb 100755 --- a/src/dynamic_data/flow/target_resolver.rs +++ b/src/dynamic_data/flow/target_resolver.rs @@ -7,9 +7,9 @@ use num_traits::abs; use crate::dynamic_data::Battle; use crate::dynamic_data::Pokemon; use crate::static_data::MoveTarget; -use crate::PkmnError; +use crate::VecExt; -/// Helper type for the vector of targets we will return. +/// Helper type for the vector of targ ets we will return. pub type TargetList = Vec>>; /// This returns all Pokemon in the battle. @@ -127,16 +127,7 @@ pub fn resolve_targets(side: u8, index: u8, target: MoveTarget, battle: &Battle) // the client deal with what side is passed. MoveTarget::AllAlly | MoveTarget::AllOpponent => { let mut v = Vec::new(); - for pokemon in battle - .sides() - .get(side as usize) - .ok_or(PkmnError::IndexOutOfBounds { - index: side as usize, - len: battle.sides().len(), - })? - .pokemon() - .deref() - { + for pokemon in battle.sides().get_res(side as usize)?.pokemon().deref() { v.push(pokemon.as_ref().cloned()); } v diff --git a/src/dynamic_data/models/battle.rs b/src/dynamic_data/models/battle.rs index 4b72c1c..5cfbab0 100755 --- a/src/dynamic_data/models/battle.rs +++ b/src/dynamic_data/models/battle.rs @@ -20,7 +20,7 @@ use crate::dynamic_data::VolatileScriptsOwner; use crate::dynamic_data::{is_valid_target, ScriptWrapper}; use crate::dynamic_data::{ChoiceQueue, ScriptContainer}; use crate::dynamic_data::{ScriptCategory, ScriptSource, ScriptSourceData}; -use crate::{script_hook, PkmnError, StringKey, ValueIdentifiable, ValueIdentifier}; +use crate::{script_hook, PkmnError, StringKey, ValueIdentifiable, ValueIdentifier, VecExt}; /// A pokemon battle, with any amount of sides and pokemon per side. #[derive(Debug)] @@ -271,13 +271,7 @@ impl Battle { let side = choice.user().get_battle_side_index(); match side { Some(side) => { - self.sides - .get(side as usize) - .ok_or(PkmnError::IndexOutOfBounds { - index: side as usize, - len: self.sides.len(), - })? - .set_choice(choice)?; + self.sides.get_res(side as usize)?.set_choice(choice)?; self.check_choices_set_and_run()?; Ok(true) } diff --git a/src/dynamic_data/models/battle_party.rs b/src/dynamic_data/models/battle_party.rs index 3fb612f..7cb6514 100755 --- a/src/dynamic_data/models/battle_party.rs +++ b/src/dynamic_data/models/battle_party.rs @@ -1,3 +1,4 @@ +use anyhow::Result; use std::sync::Arc; use crate::dynamic_data::models::pokemon::Pokemon; @@ -19,12 +20,12 @@ pub struct BattleParty { impl BattleParty { /// Initializes a battle party with the underlying party, and the indices the party is responsible /// for. - pub fn new(party: Arc, responsible_indices: Vec<(u8, u8)>) -> Self { - Self { + pub fn new(party: Arc, responsible_indices: Vec<(u8, u8)>) -> Result { + Ok(Self { identifier: Default::default(), party, responsible_indices, - } + }) } /// Checks whether the party is responsible for the given index. diff --git a/src/dynamic_data/models/battle_side.rs b/src/dynamic_data/models/battle_side.rs index 2c23f29..8b9b8f2 100755 --- a/src/dynamic_data/models/battle_side.rs +++ b/src/dynamic_data/models/battle_side.rs @@ -14,7 +14,7 @@ use crate::dynamic_data::script_handling::{ScriptSource, ScriptSourceData, Scrip use crate::dynamic_data::Script; use crate::dynamic_data::ScriptSet; use crate::dynamic_data::VolatileScriptsOwner; -use crate::{script_hook, PkmnError, StringKey, ValueIdentifiable, ValueIdentifier}; +use crate::{script_hook, StringKey, ValueIdentifiable, ValueIdentifier, VecExt}; /// A side on a battle. #[derive(Debug)] @@ -149,12 +149,7 @@ impl BattleSide { for (index, pokemon_slot) in self.pokemon.read().iter().enumerate() { if let Some(pokemon) = pokemon_slot { if std::ptr::eq(pokemon.deref(), choice.user().deref()) { - let len = self.choices.read().len(); - self.choices - .write() - .get_mut(index) - .ok_or(PkmnError::IndexOutOfBounds { index, len })? - .replace(choice); + self.choices.write().get_mut_res(index)?.replace(choice); self.choices_set.fetch_add(1, Ordering::SeqCst); return Ok(()); } @@ -180,10 +175,7 @@ impl BattleSide { pub fn set_pokemon(&self, index: u8, pokemon: Option>) -> Result<()> { { let mut write_lock = self.pokemon.write(); - let old = write_lock.get_mut(index as usize).ok_or(PkmnError::IndexOutOfBounds { - index: index as usize, - len: self.pokemon_per_side as usize, - })?; + let old = write_lock.get_mut_res(index as usize)?; let old = match pokemon { Some(pokemon) => old.replace(pokemon), None => old.take(), @@ -193,23 +185,17 @@ impl BattleSide { // side again. drop(write_lock); script_hook!(on_remove, old_pokemon,); - old_pokemon.set_on_battlefield(false); + old_pokemon.set_on_battlefield(false)?; } } let pokemon = { let read_lock = self.pokemon.read(); - &read_lock - .get(index as usize) - .ok_or(PkmnError::IndexOutOfBounds { - index: index as usize, - len: read_lock.len(), - })? - .clone() + &read_lock.get_res(index as usize)?.clone() }; if let Some(pokemon) = pokemon { pokemon.set_battle_data(self.battle, self.index); - pokemon.set_on_battlefield(true); + pokemon.set_on_battlefield(true)?; pokemon.set_battle_index(index); let battle = self.battle()?; @@ -252,11 +238,7 @@ impl BattleSide { /// If this happens, the slot can not be used again. pub(crate) fn mark_slot_as_unfillable(&self, index: u8) -> Result<()> { self.fillable_slots - .get(index as usize) - .ok_or(PkmnError::IndexOutOfBounds { - index: index as usize, - len: self.fillable_slots.len(), - })? + .get_res(index as usize)? .store(false, Ordering::SeqCst); Ok(()) } @@ -266,14 +248,7 @@ impl BattleSide { for (i, slot) in self.pokemon.read().iter().enumerate() { if let Some(p) = slot { if std::ptr::eq(p.deref().deref(), pokemon.deref()) { - return Ok(self - .fillable_slots - .get(i) - .ok_or(PkmnError::IndexOutOfBounds { - index: i, - len: self.fillable_slots.len(), - })? - .load(Ordering::Relaxed)); + return Ok(self.fillable_slots.get_res(i)?.load(Ordering::Relaxed)); } } } diff --git a/src/dynamic_data/models/pokemon.rs b/src/dynamic_data/models/pokemon.rs index 397d1e2..3133259 100755 --- a/src/dynamic_data/models/pokemon.rs +++ b/src/dynamic_data/models/pokemon.rs @@ -22,7 +22,7 @@ use crate::static_data::TypeIdentifier; use crate::static_data::{Ability, Statistic}; use crate::static_data::{ClampedStatisticSet, StatisticSet}; use crate::utils::Random; -use crate::{script_hook, PkmnError, StringKey, ValueIdentifiable, ValueIdentifier}; +use crate::{script_hook, PkmnError, StringKey, ValueIdentifiable, ValueIdentifier, VecExt}; use anyhow::{anyhow, bail, Result}; /// An individual Pokemon as we know and love them. @@ -595,16 +595,17 @@ impl Pokemon { } /// Sets whether or not the Pokemon is on the battlefield. - pub fn set_on_battlefield(&self, value: bool) { + pub fn set_on_battlefield(&self, value: bool) -> Result<()> { let r = self.battle_data.read(); if let Some(data) = &mut r.deref() { data.on_battle_field.store(value, Ordering::SeqCst); if !value { - self.volatile.clear(); + self.volatile.clear()?; self.weight.store(self.form().weight(), Ordering::SeqCst); self.height.store(self.form().height(), Ordering::SeqCst); } } + Ok(()) } /// Sets the index of the slot of the side the Pokemon is on. @@ -676,11 +677,7 @@ impl Pokemon { if !battle.can_slot_be_filled(battle_data.battle_side_index(), battle_data.index()) { battle .sides() - .get(battle_data.battle_side_index() as usize) - .ok_or(PkmnError::IndexOutOfBounds { - index: battle_data.battle_side_index() as usize, - len: battle.sides().len(), - })? + .get_res(battle_data.battle_side_index() as usize)? .mark_slot_as_unfillable(battle_data.index())?; } @@ -721,9 +718,12 @@ impl Pokemon { pub fn learn_move(&self, move_name: &StringKey, learn_method: MoveLearnMethod) -> Result<()> { let mut learned_moves = self.learned_moves().write(); let move_pos = learned_moves.iter().position(|a| a.is_none()); - if move_pos.is_none() { - bail!("No more moves with an empty space found."); - } + let move_pos = match move_pos { + Some(a) => a, + None => { + bail!("No more moves with an empty space found."); + } + }; let move_data = self .library .static_data() @@ -732,7 +732,9 @@ impl Pokemon { .ok_or(PkmnError::InvalidMoveName { move_name: move_name.clone(), })?; - learned_moves[move_pos.unwrap()] = Some(Arc::new(LearnedMove::new(move_data, learn_method))); + learned_moves + .get_mut_res(move_pos)? + .replace(Arc::new(LearnedMove::new(move_data, learn_method))); Ok(()) } @@ -805,7 +807,10 @@ impl ScriptSource for Pokemon { let mut c = 3; if let Some(battle_data) = &self.battle_data.read().deref() { if let Some(battle) = battle_data.battle() { - c += battle.sides()[battle_data.battle_side_index() as usize].get_script_count()?; + c += battle + .sides() + .get_res(battle_data.battle_side_index() as usize)? + .get_script_count()?; } } Ok(c) @@ -826,7 +831,10 @@ impl ScriptSource for Pokemon { self.get_own_scripts(scripts); if let Some(battle_data) = &self.battle_data.read().deref() { if let Some(battle) = battle_data.battle() { - battle.sides()[battle_data.battle_side_index() as usize].collect_scripts(scripts)?; + battle + .sides() + .get_res(battle_data.battle_side_index() as usize)? + .collect_scripts(scripts)?; } } Ok(()) @@ -862,6 +870,7 @@ pub enum DamageSource { } #[cfg(test)] +#[allow(clippy::unwrap_used)] pub mod test { use crate::dynamic_data::libraries::test::MockDynamicLibrary; use crate::dynamic_data::models::pokemon::Pokemon; diff --git a/src/dynamic_data/models/pokemon_builder.rs b/src/dynamic_data/models/pokemon_builder.rs index 762855b..e4e09ea 100755 --- a/src/dynamic_data/models/pokemon_builder.rs +++ b/src/dynamic_data/models/pokemon_builder.rs @@ -5,7 +5,7 @@ use crate::dynamic_data::models::learned_move::MoveLearnMethod; use crate::dynamic_data::models::pokemon::Pokemon; use crate::dynamic_data::DynamicLibrary; use crate::static_data::{AbilityIndex, Gender}; -use crate::{Random, StringKey}; +use crate::{PkmnError, Random, StringKey}; use anyhow::Result; /// This allows for the easy chain building of a Pokemon. @@ -47,7 +47,12 @@ impl PokemonBuilder { Random::default() }; - let species = self.library.static_data().species().get(&self.species).unwrap(); + let species = self + .library + .static_data() + .species() + .get(&self.species) + .ok_or(PkmnError::InvalidSpeciesName { species: self.species })?; let form = species.get_default_form(); let p = Pokemon::new( self.library, diff --git a/src/dynamic_data/models/pokemon_party.rs b/src/dynamic_data/models/pokemon_party.rs index 30accf4..bbe1307 100755 --- a/src/dynamic_data/models/pokemon_party.rs +++ b/src/dynamic_data/models/pokemon_party.rs @@ -1,9 +1,10 @@ +use anyhow::Result; use parking_lot::lock_api::RwLockReadGuard; use parking_lot::{RawRwLock, RwLock}; use std::sync::Arc; use crate::dynamic_data::models::pokemon::Pokemon; -use crate::{ValueIdentifiable, ValueIdentifier}; +use crate::{ValueIdentifiable, ValueIdentifier, VecExt}; /// A list of Pokemon belonging to a trainer. #[derive(Debug)] @@ -52,14 +53,17 @@ impl PokemonParty { } /// Sets the Pokemon at an index to a Pokemon, returning the old Pokemon. - pub fn swap_into(&self, index: usize, pokemon: Option>) -> Option> { + pub fn swap_into(&self, index: usize, pokemon: Option>) -> Result>> { let mut party = self.pokemon.write(); if index >= party.len() { - return pokemon; + return Ok(pokemon); } - let old = party[index].as_ref().cloned(); - party[index] = pokemon; - old + let value = party.get_mut_res(index)?; + let old = match pokemon { + Some(p) => value.replace(p), + None => value.take(), + }; + Ok(old) } /// Whether or not the party still has Pokemon that can be used in battle. @@ -83,18 +87,18 @@ impl PokemonParty { } /// Makes sure there are no empty spots in the party anymore, leaving the length the same. - pub fn pack_party(&self) { + pub fn pack_party(&self) -> Result<()> { let mut first_empty = None; let mut i = 0; let mut party = self.pokemon.write(); loop { - if party[i].is_none() { + if party.get_res(i)?.is_none() { if first_empty.is_none() { first_empty = Some(i) } - } else if first_empty.is_some() { - party.swap(first_empty.unwrap(), i); - i = first_empty.unwrap(); + } else if let Some(f) = first_empty { + party.swap(f, i); + i = f; first_empty = None; } i += 1; @@ -102,6 +106,7 @@ impl PokemonParty { break; } } + Ok(()) } /// Checks if the party contains a given pokemon. diff --git a/src/dynamic_data/script_handling/mod.rs b/src/dynamic_data/script_handling/mod.rs index b5fd4b1..20d75db 100755 --- a/src/dynamic_data/script_handling/mod.rs +++ b/src/dynamic_data/script_handling/mod.rs @@ -1,8 +1,9 @@ -use anyhow::Result; +use anyhow::{anyhow, Result}; use std::sync::{Arc, Weak}; use parking_lot::RwLock; +use crate::VecExt; #[doc(inline)] pub use item_script::*; #[doc(inline)] @@ -27,7 +28,7 @@ mod volatile_scripts_owner; macro_rules! script_hook { ($hook_name: ident, $source: expr, $($parameters: expr),*) => { let mut aggregator = $source.get_script_iterator()?; - while let Some(script_container) = aggregator.get_next() { + while let Some(script_container) = aggregator.get_next()? { let script = script_container.get(); if let Some(script) = script { if let Some(script) = script.read().as_deref() { @@ -157,16 +158,16 @@ impl ScriptIterator { } /// Move to the next valid value in the scripts. - fn increment_to_next_value(&mut self) -> bool { + fn increment_to_next_value(&mut self) -> Result { if self.index != -1 { - let wrapper = unsafe { &(*self.scripts)[self.index as usize] }; + let wrapper = unsafe { &(*self.scripts).get_res(self.index as usize)? }; if let ScriptWrapper::Set(set) = wrapper { if let Some(set) = set.upgrade() { self.set_index += 1; if self.set_index as usize >= set.count() { self.set_index = -1; } else { - return true; + return Ok(true); } } } @@ -175,41 +176,61 @@ impl ScriptIterator { let len = (unsafe { &*self.scripts }).len() as i32; for index in self.index..len { self.index = index; - let wrapper = unsafe { &self.scripts.as_ref().unwrap()[self.index as usize] }; + let wrapper = unsafe { + &self + .scripts + .as_ref() + .ok_or(anyhow!("ScriptIterator scripts pointer is null"))? + .get_res(self.index as usize)? + }; if let ScriptWrapper::Set(s) = wrapper { if let Some(set) = s.upgrade() { if set.count() > 0 { self.set_index = 0; - return true; + return Ok(true); } } } else if let ScriptWrapper::Script(script) = wrapper { if let Some(v) = script.upgrade() { if let Some(..) = v.read().as_ref() { - return true; + return Ok(true); } } } } - false + Ok(false) } /// Gets the next valid script. If none is found, returns None. - pub fn get_next(&mut self) -> Option { - if !self.increment_to_next_value() { - return None; + pub fn get_next(&mut self) -> Result> { + if !self.increment_to_next_value()? { + return Ok(None); } unsafe { - return match &self.scripts.as_ref().unwrap()[self.index as usize] { - // increment_to_next_value - ScriptWrapper::Script(script) => Some(script.upgrade().unwrap().into()), - ScriptWrapper::Set(set) => { - let set = set.upgrade().unwrap(); - let sc = set.at(self.set_index as usize); - return Some(sc); - } - }; + Ok( + match &self + .scripts + .as_ref() + .ok_or(anyhow!("ScriptIterator scripts pointer is null"))? + .get_res(self.index as usize)? + { + // increment_to_next_value + ScriptWrapper::Script(script) => Some( + script + .upgrade() + .ok_or(anyhow!("Couldn't get a strong reference to a script"))? + .into(), + ), + ScriptWrapper::Set(set) => { + let set = set + .upgrade() + .ok_or(anyhow!("Couldn't get a strong reference to a set"))?; + let sc = set.at(self.set_index as usize)?; + return Ok(Some(sc)); + } + }, + ) } } @@ -221,6 +242,7 @@ impl ScriptIterator { } #[cfg(test)] +#[allow(clippy::unwrap_used)] mod tests { use std::any::Any; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; @@ -291,10 +313,10 @@ mod tests { let script = ScriptContainer::new(Arc::new(TestScript::new())); let scripts = vec![ScriptWrapper::from(&script)]; let mut aggregator = ScriptIterator::new(&scripts as *const Vec); - while let Some(v) = aggregator.get_next() { + while let Some(v) = aggregator.get_next().unwrap() { v.get().unwrap().read().as_ref().unwrap().stack(); } - let a = script.get_as::(); + let a = script.get_as::().unwrap(); assert_eq!(a.test_count.load(Ordering::Relaxed), 1); } @@ -305,10 +327,10 @@ mod tests { let mut aggregator = ScriptIterator::new(&scripts as *const Vec); for i in 1..11 { aggregator.reset(); - while let Some(v) = aggregator.get_next() { + while let Some(v) = aggregator.get_next().unwrap() { v.get().unwrap().read().as_ref().unwrap().stack(); } - let a = script.get_as::(); + let a = script.get_as::().unwrap(); assert_eq!(a.test_count.load(Ordering::Relaxed), i); } } @@ -324,14 +346,14 @@ mod tests { ScriptWrapper::from(&script3), ]; let mut aggregator = ScriptIterator::new(&scripts as *const Vec); - while let Some(v) = aggregator.get_next() { + while let Some(v) = aggregator.get_next().unwrap() { v.get().unwrap().read().as_ref().unwrap().stack(); } - let a = script1.get_as::(); + let a = script1.get_as::().unwrap(); assert_eq!(a.test_count.load(Ordering::Relaxed), 1); - let a = script2.get_as::(); + let a = script2.get_as::().unwrap(); assert_eq!(a.test_count.load(Ordering::Relaxed), 1); - let a = script3.get_as::(); + let a = script3.get_as::().unwrap(); assert_eq!(a.test_count.load(Ordering::Relaxed), 1); } @@ -348,14 +370,14 @@ mod tests { let mut aggregator = ScriptIterator::new(&scripts as *const Vec); for i in 1..11 { aggregator.reset(); - while let Some(v) = aggregator.get_next() { + while let Some(v) = aggregator.get_next().unwrap() { v.get().unwrap().read().as_ref().unwrap().stack(); } - let a = script1.get_as::(); + let a = script1.get_as::().unwrap(); assert_eq!(a.test_count.load(Ordering::Relaxed), i); - let a = script2.get_as::(); + let a = script2.get_as::().unwrap(); assert_eq!(a.test_count.load(Ordering::Relaxed), i); - let a = script3.get_as::(); + let a = script3.get_as::().unwrap(); assert_eq!(a.test_count.load(Ordering::Relaxed), i); } } @@ -371,19 +393,19 @@ mod tests { let mut aggregator = ScriptIterator::new(&scripts as *const Vec); for i in 1..11 { aggregator.reset(); - while let Some(v) = aggregator.get_next() { + while let Some(v) = aggregator.get_next().unwrap() { v.get().unwrap().read().as_ref().unwrap().stack(); } - let s = set.at(0); - let s = s.get_as::(); + let s = set.at(0).unwrap(); + let s = s.get_as::().unwrap(); assert_eq!(s.test_count.load(Ordering::Relaxed), i); assert_eq!(s.name().str(), "test_a"); - let s = set.at(1); - let s = s.get_as::(); + let s = set.at(1).unwrap(); + let s = s.get_as::().unwrap(); assert_eq!(s.test_count.load(Ordering::Relaxed), i); assert_eq!(s.name().str(), "test_b"); - let s = set.at(2); - let s = s.get_as::(); + let s = set.at(2).unwrap(); + let s = s.get_as::().unwrap(); assert_eq!(s.test_count.load(Ordering::Relaxed), i); assert_eq!(s.name().str(), "test_c"); } @@ -402,6 +424,7 @@ mod tests { aggregator .get_next() .unwrap() + .unwrap() .get() .unwrap() .read() @@ -415,6 +438,7 @@ mod tests { aggregator .get_next() .unwrap() + .unwrap() .get() .unwrap() .read() @@ -425,8 +449,8 @@ mod tests { "test_b" ); - set.remove(&"test_c".into()); - assert!(aggregator.get_next().is_none()); + set.remove(&"test_c".into()).unwrap(); + assert!(aggregator.get_next().unwrap().is_none()); } #[test] @@ -442,6 +466,7 @@ mod tests { aggregator .get_next() .unwrap() + .unwrap() .get() .unwrap() .read() @@ -452,12 +477,13 @@ mod tests { "test_a" ); - set.remove(&"test_b".into()); + set.remove(&"test_b".into()).unwrap(); assert_eq!( aggregator .get_next() .unwrap() + .unwrap() .get() .unwrap() .read() @@ -467,7 +493,7 @@ mod tests { .str(), "test_c" ); - assert!(aggregator.get_next().is_none()); + assert!(aggregator.get_next().unwrap().is_none()); } pub struct TestScriptSource { @@ -503,10 +529,10 @@ mod tests { let mut aggregator = source.get_script_iterator().unwrap(); aggregator.reset(); - assert!(aggregator.get_next().is_none()); + assert!(aggregator.get_next().unwrap().is_none()); aggregator.reset(); source.script.set(Arc::new(TestScript::new())); - assert!(aggregator.get_next().is_some()); + assert!(aggregator.get_next().unwrap().is_some()); } #[test] @@ -518,9 +544,9 @@ mod tests { let mut aggregator = source.get_script_iterator().unwrap(); source.script.set(Arc::new(TestScript::new())); - assert!(aggregator.get_next().is_some()); + assert!(aggregator.get_next().unwrap().is_some()); aggregator.reset(); source.script.clear(); - assert!(aggregator.get_next().is_none()); + assert!(aggregator.get_next().unwrap().is_none()); } } diff --git a/src/dynamic_data/script_handling/script.rs b/src/dynamic_data/script_handling/script.rs index c169f3a..33f6ab9 100755 --- a/src/dynamic_data/script_handling/script.rs +++ b/src/dynamic_data/script_handling/script.rs @@ -1,3 +1,4 @@ +use anyhow::{anyhow, Result}; use std::any::Any; use std::fmt::{Debug, Formatter}; use std::ops::Deref; @@ -355,12 +356,16 @@ impl ScriptContainer { } /// Get the underlying script as the downcasted value. - pub fn get_as(&self) -> MappedRwLockReadGuard { - RwLockReadGuard::map(self.script.read(), |a| unsafe { - let ptr = a.as_ref().as_ref().unwrap().as_ref() as *const dyn Script; - let any = ptr.as_ref().unwrap().as_any(); - any.downcast_ref::().unwrap() - }) + pub fn get_as(&self) -> Result> { + let r = RwLockReadGuard::try_map(self.script.read(), |a| unsafe { + let ptr = a.as_ref().as_ref()?.as_ref() as *const dyn Script; + let any = ptr.as_ref()?.as_any(); + any.downcast_ref::() + }); + match r { + Ok(a) => Ok(a), + Err(_) => Err(anyhow!("Could not downcast script to requested type")), + } } } diff --git a/src/dynamic_data/script_handling/script_set.rs b/src/dynamic_data/script_handling/script_set.rs index 6d3dbdb..9ab9014 100755 --- a/src/dynamic_data/script_handling/script_set.rs +++ b/src/dynamic_data/script_handling/script_set.rs @@ -6,7 +6,7 @@ use indexmap::IndexMap; use parking_lot::RwLock; use crate::dynamic_data::script_handling::script::{Script, ScriptContainer}; -use crate::StringKey; +use crate::{PkmnError, StringKey}; /// A collection of unique scripts. #[derive(Debug, Default)] @@ -29,10 +29,12 @@ impl ScriptSet { } } } - self.scripts - .write() - .insert(script.name().clone(), ScriptContainer::new(script)); - self.scripts.read().last().unwrap().1.clone() + + // If the existing script does not exist, or is not a script, we can just add the new one. + let name = script.name().clone(); + let container = ScriptContainer::new(script); + self.scripts.write().insert(name, container.clone()); + container } /// Adds a script with a name to the set. If the script with that name already exists in this @@ -54,8 +56,8 @@ impl ScriptSet { if let Some(script) = script { let name = script.name().clone(); let arc = ScriptContainer::new(script); - self.scripts.write().insert(name, arc); - Ok(Some(self.scripts.read().last().unwrap().1.clone())) + self.scripts.write().insert(name, arc.clone()); + Ok(Some(arc)) } else { Ok(None) } @@ -67,27 +69,34 @@ impl ScriptSet { } /// Removes a script from the set using its unique name. - pub fn remove(&self, key: &StringKey) { + pub fn remove(&self, key: &StringKey) -> Result<()> { let value = self.scripts.write().shift_remove(key); if let Some(script) = value { if let Some(script) = script.get() { let script = script.read(); - script.as_ref().unwrap().on_remove(); - script.as_ref().unwrap().mark_for_deletion(); + script.as_ref().ok_or(PkmnError::UnableToAcquireLock)?.on_remove(); + script + .as_ref() + .ok_or(PkmnError::UnableToAcquireLock)? + .mark_for_deletion(); } } + Ok(()) } /// Clears all scripts from the set. - pub fn clear(&self) { + pub fn clear(&self) -> Result<()> { for script in self.scripts.read().deref() { if let Some(script) = script.1.get() { let script = script.read(); - script.as_ref().unwrap().on_remove(); - script.as_ref().unwrap().mark_for_deletion(); + if let Some(script) = &*script { + script.on_remove(); + script.mark_for_deletion(); + } } } self.scripts.write().clear(); + Ok(()) } /// Checks if the set has a script with the given name. @@ -96,8 +105,17 @@ impl ScriptSet { } /// Gets a script from the set at a specific index. - pub fn at(&self, index: usize) -> ScriptContainer { - self.scripts.read()[index].clone() + pub fn at(&self, index: usize) -> Result { + Ok(self + .scripts + .read() + .get_index(index) + .ok_or(PkmnError::IndexOutOfBounds { + index, + len: self.scripts.read().len(), + })? + .1 + .clone()) } /// Gets the number of scripts in the set. diff --git a/src/dynamic_data/script_handling/volatile_scripts_owner.rs b/src/dynamic_data/script_handling/volatile_scripts_owner.rs index ae71fe0..367b642 100755 --- a/src/dynamic_data/script_handling/volatile_scripts_owner.rs +++ b/src/dynamic_data/script_handling/volatile_scripts_owner.rs @@ -35,7 +35,7 @@ pub trait VolatileScriptsOwner { } /// Removes a volatile script by name. - fn remove_volatile_script(&self, key: &StringKey) { + fn remove_volatile_script(&self, key: &StringKey) -> Result<()> { self.volatile_scripts().remove(key) } } diff --git a/src/ffi/dynamic_data/choices.rs b/src/ffi/dynamic_data/choices.rs index c772319..b497362 100644 --- a/src/ffi/dynamic_data/choices.rs +++ b/src/ffi/dynamic_data/choices.rs @@ -1,5 +1,6 @@ use crate::dynamic_data::{FleeChoice, LearnedMove, MoveChoice, PassChoice, Pokemon, TurnChoice}; -use crate::ffi::{ExternPointer, IdentifiablePointer, OwnedPtr}; +use crate::ffi::{ExternPointer, IdentifiablePointer, NativeResult, OwnedPtr}; +use anyhow::anyhow; use std::ptr::drop_in_place; use std::sync::Arc; @@ -57,38 +58,38 @@ extern "C" fn turn_choice_move_new( #[no_mangle] extern "C" fn turn_choice_move_learned_move( choice: ExternPointer, -) -> IdentifiablePointer> { +) -> NativeResult>> { if let TurnChoice::Move(c) = choice.as_ref() { - return c.used_move().clone().into(); + return NativeResult::ok(c.used_move().clone().into()); } - panic!("Turn choice was not a learned move"); + NativeResult::err(anyhow!("Turn choice was not a learned move")) } /// The target side the move is aimed at. #[no_mangle] -extern "C" fn turn_choice_move_target_side(choice: ExternPointer) -> u8 { +extern "C" fn turn_choice_move_target_side(choice: ExternPointer) -> NativeResult { if let TurnChoice::Move(c) = choice.as_ref() { - return c.target_side(); + return NativeResult::ok(c.target_side()); } - panic!("Turn choice was not a learned move"); + NativeResult::err(anyhow!("Turn choice was not a learned move")) } /// The Pokemon index on the side we're aiming at. #[no_mangle] -extern "C" fn turn_choice_move_target_index(choice: ExternPointer) -> u8 { +extern "C" fn turn_choice_move_target_index(choice: ExternPointer) -> NativeResult { if let TurnChoice::Move(c) = choice.as_ref() { - return c.target_index(); + return NativeResult::ok(c.target_index()); } - panic!("Turn choice was not a learned move"); + NativeResult::err(anyhow!("Turn choice was not a learned move")) } /// The priority of the move choice at the beginning of the turn. #[no_mangle] -extern "C" fn turn_choice_move_priority(choice: ExternPointer) -> i8 { +extern "C" fn turn_choice_move_priority(choice: ExternPointer) -> NativeResult { if let TurnChoice::Move(c) = choice.as_ref() { - return c.priority(); + return NativeResult::ok(c.priority()); } - panic!("Turn choice was not a learned move"); + NativeResult::err(anyhow!("Turn choice was not a learned move")) } /// Creates a new Turn Choice for the user to flee. diff --git a/src/ffi/dynamic_data/models/battle.rs b/src/ffi/dynamic_data/models/battle.rs index f027bee..df44137 100644 --- a/src/ffi/dynamic_data/models/battle.rs +++ b/src/ffi/dynamic_data/models/battle.rs @@ -3,6 +3,7 @@ use crate::dynamic_data::{ }; use crate::ffi::dynamic_data::models::native_event_hook::NativeEventHook; use crate::ffi::{ExternPointer, IdentifiablePointer, NativeResult, OwnedPtr}; +use anyhow::anyhow; use std::ffi::{c_char, CStr, CString}; use std::sync::Arc; @@ -178,14 +179,12 @@ extern "C" fn battle_can_use(ptr: ExternPointer>, choice: ExternPoin /// Checks to see whether all Pokemon on the field have set their choices. If so, we then run /// the turn. #[no_mangle] -extern "C" fn battle_try_set_choice(ptr: ExternPointer>, choice: OwnedPtr) -> u8 { +extern "C" fn battle_try_set_choice(ptr: ExternPointer>, choice: OwnedPtr) -> NativeResult { let choice = unsafe { choice.read() }; let result = ptr.as_ref().try_set_choice(choice); match result { - Ok(b) => u8::from(b), - Err(e) => { - panic!("Encountered error: {}", e) - } + Ok(b) => NativeResult::ok(u8::from(b)), + Err(e) => NativeResult::err(e), } } @@ -203,7 +202,10 @@ extern "C" fn battle_set_weather(ptr: ExternPointer>, weather: *cons #[no_mangle] extern "C" fn battle_weather_name(ptr: ExternPointer>) -> NativeResult<*mut c_char> { match ptr.as_ref().weather_name() { - Ok(Some(w)) => CString::new(w.str()).unwrap().into_raw().into(), + Ok(Some(w)) => match CString::new(w.str()) { + Ok(s) => s.into_raw().into(), + Err(e) => NativeResult::err(anyhow!("Failed to convert weather name to CString: {}", e)), + }, Ok(None) => std::ptr::null_mut::().into(), Err(e) => NativeResult::err(e), } diff --git a/src/ffi/dynamic_data/models/battle_party.rs b/src/ffi/dynamic_data/models/battle_party.rs index 2e761fc..98bda5f 100644 --- a/src/ffi/dynamic_data/models/battle_party.rs +++ b/src/ffi/dynamic_data/models/battle_party.rs @@ -1,5 +1,7 @@ use crate::dynamic_data::{BattleParty, Pokemon, PokemonParty}; -use crate::ffi::{ExternPointer, IdentifiablePointer}; +use crate::ffi::{ExternPointer, IdentifiablePointer, NativeResult}; +use crate::VecExt; +use anyhow::{anyhow, Result}; use std::sync::Arc; /// A battle party is a wrapper around a party, with the indices for which the party is responsible @@ -9,18 +11,34 @@ extern "C" fn battle_party_new( party: ExternPointer>, responsible_indices_ptr: *const u8, responsible_indices_length: usize, -) -> IdentifiablePointer> { - if responsible_indices_length % 2 == 1 { - panic!("The length of responsible indices should be dividable by two"); +) -> NativeResult>> { + if responsible_indices_length % 2 != 0 { + return NativeResult::err(anyhow!("The length of responsible indices should be dividable by two")); } + let responsible_indices_slice = unsafe { std::slice::from_raw_parts(responsible_indices_ptr, responsible_indices_length) }; let mut responsible_indices: Vec<(u8, u8)> = Vec::with_capacity(responsible_indices_length / 2); + + let mut split = |i| -> Result<()> { + *responsible_indices.get_mut_res(i)? = ( + *responsible_indices_slice.get_res(i * 2)?, + *responsible_indices_slice.get_res(i * 2 + 1)?, + ); + Ok(()) + }; + for i in 0..responsible_indices_length / 2 { - responsible_indices[i] = (responsible_indices_slice[i * 2], responsible_indices_slice[i * 2 + 1]) + match split(i) { + Ok(_) => (), + Err(e) => return NativeResult::err(e), + } } - Arc::new(BattleParty::new(party.as_ref().clone(), responsible_indices)).into() + match BattleParty::new(party.as_ref().clone(), responsible_indices) { + Ok(v) => NativeResult::ok(Arc::new(v).into()), + Err(e) => NativeResult::err(e), + } } /// Checks whether the party is responsible for the given index. diff --git a/src/ffi/dynamic_data/models/pokemon.rs b/src/ffi/dynamic_data/models/pokemon.rs index 2253fc4..055c407 100644 --- a/src/ffi/dynamic_data/models/pokemon.rs +++ b/src/ffi/dynamic_data/models/pokemon.rs @@ -4,6 +4,7 @@ use crate::ffi::{ffi_arc_getter, ffi_vec_value_getters, ExternPointer, Identifia use crate::static_data::{ Ability, AbilityIndex, Form, Gender, Item, Nature, Species, Statistic, StatisticSet, TypeIdentifier, }; +use anyhow::anyhow; use std::ffi::{c_char, CStr, CString}; use std::ptr::drop_in_place; use std::sync::Arc; @@ -141,12 +142,15 @@ ffi_arc_getter!(Pokemon, height, f32); /// An optional nickname of the Pokemon. #[no_mangle] -extern "C" fn pokemon_nickname(ptr: ExternPointer>) -> *mut c_char { +extern "C" fn pokemon_nickname(ptr: ExternPointer>) -> NativeResult<*mut c_char> { let name = ptr.as_ref().nickname(); if let Some(v) = name { - CString::new(v.as_str()).unwrap().into_raw() + match CString::new(v.as_str()) { + Ok(v) => NativeResult::ok(v.into_raw()), + Err(err) => NativeResult::err(anyhow!("Could not convert nickname to CString: {}", err)), + } } else { - std::ptr::null_mut() + NativeResult::ok(std::ptr::null_mut()) } } diff --git a/src/ffi/dynamic_data/models/pokemon_party.rs b/src/ffi/dynamic_data/models/pokemon_party.rs index ea2472a..958c400 100644 --- a/src/ffi/dynamic_data/models/pokemon_party.rs +++ b/src/ffi/dynamic_data/models/pokemon_party.rs @@ -1,5 +1,5 @@ use crate::dynamic_data::{Pokemon, PokemonParty}; -use crate::ffi::{ExternPointer, IdentifiablePointer}; +use crate::ffi::{ExternPointer, IdentifiablePointer, NativeResult}; use std::sync::Arc; /// Instantiates a party with a set size. @@ -33,16 +33,16 @@ extern "C" fn pokemon_party_swap_into( ptr: ExternPointer>, index: usize, pokemon: ExternPointer>, -) -> IdentifiablePointer> { +) -> NativeResult>> { let pokemon = if pokemon.ptr.is_null() { None } else { Some(pokemon.as_ref().clone()) }; - if let Some(v) = ptr.as_ref().swap_into(index, pokemon) { - v.into() - } else { - IdentifiablePointer::none() + match ptr.as_ref().swap_into(index, pokemon) { + Ok(Some(v)) => NativeResult::ok(v.into()), + Ok(None) => NativeResult::ok(IdentifiablePointer::none()), + Err(e) => NativeResult::err(e), } } @@ -60,8 +60,8 @@ extern "C" fn pokemon_party_length(ptr: ExternPointer>) -> usi /// Makes sure there are no empty spots in the party anymore, leaving the length the same. #[no_mangle] -extern "C" fn pokemon_party_pack_party(ptr: ExternPointer>) { - ptr.as_ref().pack_party() +extern "C" fn pokemon_party_pack_party(ptr: ExternPointer>) -> NativeResult<()> { + ptr.as_ref().pack_party().into() } /// Checks if the party contains a given pokemon. diff --git a/src/ffi/mod.rs b/src/ffi/mod.rs index 9b11953..5dc53db 100644 --- a/src/ffi/mod.rs +++ b/src/ffi/mod.rs @@ -106,6 +106,7 @@ pub(self) struct ExternPointer { impl ExternPointer { /// Get the internal pointer as reference. + #[allow(clippy::panic)] // We currently allow this as these should never be null, but we might want to change this in the future. pub(self) fn as_ref(&self) -> &T { unsafe { self.ptr @@ -114,6 +115,7 @@ impl ExternPointer { } } /// Get the internal pointer as mutable reference. + #[allow(clippy::panic)] // We currently allow this as these should never be null, but we might want to change this in the future. pub(self) fn as_mut(&mut self) -> &mut T { unsafe { self.ptr @@ -241,6 +243,7 @@ impl From> for IdentifiablePointer { } impl From<*const T> for IdentifiablePointer { + #[allow(clippy::unwrap_used)] // We currently allow this as these should never be null, but we might want to change this in the future. fn from(v: *const T) -> Self { let id = unsafe { transmute(v.as_ref().unwrap().value_identifier()) }; Self { ptr: v, id } @@ -284,7 +287,8 @@ impl NativeResult { } } /// Creates a new NativeResult with the given error. - pub fn err(err: anyhow::Error) -> Self { + #[allow(clippy::unwrap_used)] // We know for certain this is not empty. + pub fn err(err: anyhow_ext::Error) -> Self { Self { ok: 0, value: ResultUnion { @@ -292,6 +296,17 @@ impl NativeResult { }, } } + + /// Creates a new NativeResult with the given error string. + #[allow(clippy::unwrap_used)] // We know for certain this is not empty. + pub fn err_from_str(err: &str) -> Self { + Self { + ok: 0, + value: ResultUnion { + err: CString::new(err).unwrap().into_raw(), + }, + } + } } impl From> for NativeResult { diff --git a/src/ffi/static_data/ability.rs b/src/ffi/static_data/ability.rs index 4b69618..7a44063 100644 --- a/src/ffi/static_data/ability.rs +++ b/src/ffi/static_data/ability.rs @@ -1,6 +1,7 @@ -use crate::ffi::{ExternPointer, IdentifiablePointer, OwnedPtr}; +use crate::ffi::{ExternPointer, IdentifiablePointer, NativeResult, OwnedPtr}; use crate::static_data::{Ability, AbilityImpl, EffectParameter}; use crate::StringKey; +use anyhow::anyhow; use std::ffi::{c_char, CStr, CString}; use std::ptr::drop_in_place; use std::sync::Arc; @@ -12,18 +13,24 @@ unsafe extern "C" fn ability_new( effect: *const c_char, parameters: *const OwnedPtr, parameters_length: usize, -) -> IdentifiablePointer> { +) -> NativeResult>> { let parameters = std::slice::from_raw_parts(parameters, parameters_length); let mut parameters_vec: Vec = Vec::with_capacity(parameters_length); for parameter in parameters { parameters_vec.push(*Box::from_raw(*parameter)); } - let name: StringKey = CStr::from_ptr(name).to_str().unwrap().into(); - let effect: StringKey = CStr::from_ptr(effect).to_str().unwrap().into(); + let name: StringKey = match CStr::from_ptr(name).to_str() { + Ok(s) => s.into(), + Err(_) => return NativeResult::err(anyhow!("Failed to convert name to CStr")), + }; + let effect: StringKey = match CStr::from_ptr(effect).to_str() { + Ok(s) => s.into(), + Err(_) => return NativeResult::err(anyhow!("Failed to convert effect to CStr")), + }; let arc: Arc = Arc::new(AbilityImpl::new(&name, &effect, parameters_vec)); - arc.into() + NativeResult::ok(arc.into()) } /// Drops a reference counted ability. @@ -34,14 +41,20 @@ unsafe extern "C" fn ability_drop(ptr: OwnedPtr>) { /// The name of the ability. #[no_mangle] -unsafe extern "C" fn ability_name(ptr: ExternPointer>) -> OwnedPtr { - CString::new(ptr.as_ref().name().str()).unwrap().into_raw() +unsafe extern "C" fn ability_name(ptr: ExternPointer>) -> NativeResult> { + match CString::new(ptr.as_ref().name().str()) { + Ok(s) => NativeResult::ok(s.into_raw()), + Err(_) => NativeResult::err(anyhow!("Failed to convert name to CString")), + } } /// The name of the script effect of the ability. #[no_mangle] -unsafe extern "C" fn ability_effect(ptr: ExternPointer>) -> OwnedPtr { - CString::new(ptr.as_ref().effect().str()).unwrap().into_raw() +unsafe extern "C" fn ability_effect(ptr: ExternPointer>) -> NativeResult> { + match CString::new(ptr.as_ref().effect().str()) { + Ok(s) => NativeResult::ok(s.into_raw()), + Err(_) => NativeResult::err(anyhow!("Failed to convert effect to CString")), + } } /// The length of the parameters for the script effect of the ability. diff --git a/src/ffi/static_data/form.rs b/src/ffi/static_data/form.rs index 9f380b5..e3c8044 100644 --- a/src/ffi/static_data/form.rs +++ b/src/ffi/static_data/form.rs @@ -1,9 +1,10 @@ use crate::ffi::{ ffi_arc_dyn_getter, ffi_vec_stringkey_getters, ffi_vec_value_getters, BorrowedPtr, ExternPointer, - IdentifiablePointer, OwnedPtr, + IdentifiablePointer, NativeResult, OwnedPtr, }; use crate::static_data::{Form, FormImpl, LearnableMoves, StaticStatisticSet, TypeIdentifier}; use crate::StringKey; +use anyhow::anyhow; use hashbrown::HashSet; use std::ffi::{c_char, CStr, CString}; use std::ptr::drop_in_place; @@ -26,8 +27,11 @@ unsafe extern "C" fn form_new( moves: OwnedPtr>, flags: *const *const c_char, flags_length: usize, -) -> IdentifiablePointer> { - let name: StringKey = CStr::from_ptr(name).to_str().unwrap().into(); +) -> NativeResult>> { + let name: StringKey = match CStr::from_ptr(name).to_str() { + Ok(name) => name.into(), + Err(_) => return NativeResult::err(anyhow!("Unable to convert name to string")), + }; let abilities = std::slice::from_raw_parts(abilities, abilities_length); let mut abilities_vec = Vec::with_capacity(abilities_length); @@ -43,7 +47,11 @@ unsafe extern "C" fn form_new( let flags = std::slice::from_raw_parts(flags, flags_length); let mut flags_set: HashSet = HashSet::with_capacity(flags_length); for flag in flags { - flags_set.insert(CStr::from_ptr(*flag).to_str().unwrap().into()); + let flag = match CStr::from_ptr(*flag).to_str() { + Ok(flag) => flag, + Err(_) => return NativeResult::err(anyhow!("Unable to convert flag to string")), + }; + flags_set.insert(flag.into()); } let a: Arc = Arc::new(FormImpl::new( @@ -58,7 +66,7 @@ unsafe extern "C" fn form_new( *Box::from_raw(moves), flags_set, )); - a.into() + NativeResult::ok(a.into()) } /// Drops a reference count for a form. @@ -69,9 +77,12 @@ unsafe extern "C" fn form_drop(ptr: OwnedPtr>) { /// The name of the form. #[no_mangle] -unsafe extern "C" fn form_name(ptr: ExternPointer>) -> OwnedPtr { +unsafe extern "C" fn form_name(ptr: ExternPointer>) -> NativeResult> { let name = ptr.as_ref().name(); - CString::new(name.str()).unwrap().into_raw() + match CString::new(name.str()) { + Ok(name) => NativeResult::ok(name.into_raw()), + Err(_) => NativeResult::err(anyhow!("Unable to convert name `{}` to CString", name.str())), + } } ffi_arc_dyn_getter!(Form, height, f32); diff --git a/src/ffi/static_data/item.rs b/src/ffi/static_data/item.rs index cfc6292..0a62db1 100644 --- a/src/ffi/static_data/item.rs +++ b/src/ffi/static_data/item.rs @@ -1,6 +1,7 @@ -use crate::ffi::{ffi_arc_dyn_getter, ExternPointer, IdentifiablePointer, OwnedPtr}; +use crate::ffi::{ffi_arc_dyn_getter, ExternPointer, IdentifiablePointer, NativeResult, OwnedPtr}; use crate::static_data::{BattleItemCategory, Item, ItemCategory, ItemImpl}; use crate::StringKey; +use anyhow::anyhow; use hashbrown::HashSet; use std::ffi::{c_char, CStr, CString}; use std::ptr::drop_in_place; @@ -15,16 +16,23 @@ unsafe extern "C" fn item_new( price: i32, flags: *const *const c_char, flags_length: usize, -) -> IdentifiablePointer> { +) -> NativeResult>> { let flags = std::slice::from_raw_parts(flags, flags_length); - let name: StringKey = CStr::from_ptr(name).to_str().unwrap().into(); + let name: StringKey = match CStr::from_ptr(name).to_str() { + Ok(name) => name.into(), + Err(_) => return NativeResult::err(anyhow!("Unable to convert name to string")), + }; let mut flags_set: HashSet = HashSet::with_capacity(flags_length); for flag in flags { - flags_set.insert(CStr::from_ptr(*flag).to_str().unwrap().into()); + let flag = match CStr::from_ptr(*flag).to_str() { + Ok(flag) => flag, + Err(_) => return NativeResult::err(anyhow!("Unable to convert flag to string")), + }; + flags_set.insert(flag.into()); } let item: Arc = Arc::new(ItemImpl::new(&name, category, battle_category, price, flags_set)); - item.into() + NativeResult::ok(item.into()) } /// Drops a reference counted item. @@ -35,9 +43,12 @@ unsafe extern "C" fn item_drop(ptr: OwnedPtr>) { /// The name of the item. #[no_mangle] -unsafe extern "C" fn item_name(ptr: ExternPointer>) -> OwnedPtr { +unsafe extern "C" fn item_name(ptr: ExternPointer>) -> NativeResult> { let name = ptr.as_ref().name(); - CString::new(name.str()).unwrap().into_raw() + match CString::new(name.str()) { + Ok(name) => NativeResult::ok(name.into_raw()), + Err(_) => NativeResult::err(anyhow!("Unable to convert name `{}` to CString", name.str())), + } } ffi_arc_dyn_getter!(Item, category, ItemCategory); diff --git a/src/ffi/static_data/libraries/nature_library.rs b/src/ffi/static_data/libraries/nature_library.rs index 982f949..1c25a99 100644 --- a/src/ffi/static_data/libraries/nature_library.rs +++ b/src/ffi/static_data/libraries/nature_library.rs @@ -1,6 +1,7 @@ -use crate::ffi::{BorrowedPtr, ExternPointer, IdentifiablePointer, OwnedPtr}; +use crate::ffi::{BorrowedPtr, ExternPointer, IdentifiablePointer, NativeResult, OwnedPtr}; use crate::static_data::{Nature, NatureLibrary, NatureLibraryImpl}; -use crate::Random; +use crate::{PkmnError, Random}; +use anyhow::anyhow; use std::ffi::{c_char, CStr, CString}; use std::ptr::drop_in_place; use std::sync::Arc; @@ -24,9 +25,15 @@ unsafe extern "C" fn nature_library_load_nature( mut ptr: ExternPointer>, name: BorrowedPtr, nature: OwnedPtr>, -) { - ptr.as_mut() - .load_nature(CStr::from_ptr(name).into(), nature.as_ref().unwrap().clone()) +) -> NativeResult<()> { + let nature = nature.as_ref().ok_or(PkmnError::NullReference); + match nature { + Ok(nature) => { + ptr.as_mut().load_nature(CStr::from_ptr(name).into(), nature.clone()); + NativeResult::ok(()) + } + Err(e) => NativeResult::err(e.into()), + } } /// Gets a nature by name. @@ -57,8 +64,15 @@ unsafe extern "C" fn nature_library_get_random_nature( unsafe extern "C" fn nature_library_get_nature_name( ptr: ExternPointer>, nature: BorrowedPtr>, -) -> OwnedPtr { - CString::new(ptr.as_ref().get_nature_name(nature.as_ref().unwrap()).str()) - .unwrap() - .into_raw() +) -> NativeResult> { + match nature.as_ref() { + Some(nature) => { + let name = ptr.as_ref().get_nature_name(nature); + match CString::new(name.str()) { + Ok(cstr) => NativeResult::ok(cstr.into_raw()), + Err(_) => NativeResult::err(anyhow!("Failed to convert nature name to C string")), + } + } + None => NativeResult::err(PkmnError::NullReference.into()), + } } diff --git a/src/ffi/static_data/libraries/type_library.rs b/src/ffi/static_data/libraries/type_library.rs index 5a9c5ae..bf83a7e 100644 --- a/src/ffi/static_data/libraries/type_library.rs +++ b/src/ffi/static_data/libraries/type_library.rs @@ -1,4 +1,4 @@ -use crate::ffi::{BorrowedPtr, ExternPointer, IdentifiablePointer, OwnedPtr}; +use crate::ffi::{BorrowedPtr, ExternPointer, IdentifiablePointer, NativeResult, OwnedPtr}; use crate::static_data::{TypeIdentifier, TypeLibrary, TypeLibraryImpl}; use std::ffi::{c_char, CStr, CString}; use std::ptr::drop_in_place; @@ -38,13 +38,17 @@ unsafe extern "C" fn type_library_get_type_name( ptr: ExternPointer>, type_id: TypeIdentifier, found: *mut bool, -) -> *mut c_char { +) -> NativeResult<*mut c_char> { if let Some(v) = ptr.as_ref().get_type_name(type_id) { *found = true; - CString::new(v.str()).unwrap().into_raw() + + match CString::new(v.str()) { + Ok(v) => NativeResult::ok(v.into_raw()), + Err(e) => NativeResult::err(e.into()), + } } else { *found = false; - std::ptr::null_mut() + NativeResult::ok(std::ptr::null_mut()) } } diff --git a/src/ffi/static_data/mod.rs b/src/ffi/static_data/mod.rs index 676d945..0ef2c47 100644 --- a/src/ffi/static_data/mod.rs +++ b/src/ffi/static_data/mod.rs @@ -1,6 +1,7 @@ -use crate::ffi::{ExternPointer, IdentifiablePointer, OwnedPtr}; +use crate::ffi::{ExternPointer, IdentifiablePointer, NativeResult, OwnedPtr}; use crate::static_data::EffectParameter; -use crate::StringKey; +use crate::{PkmnError, StringKey}; +use anyhow::anyhow; use std::ffi::{c_char, CStr, CString}; use std::ptr::drop_in_place; @@ -45,9 +46,14 @@ extern "C" fn effect_parameter_new_float(value: f32) -> IdentifiablePointer IdentifiablePointer { - let sk: StringKey = CStr::from_ptr(value).to_str().unwrap().into(); - Box::::new(sk.into()).into() +unsafe extern "C" fn effect_parameter_new_string( + value: *const c_char, +) -> NativeResult> { + let sk: StringKey = match CStr::from_ptr(value).to_str() { + Ok(sk) => sk.into(), + Err(_) => return NativeResult::err(PkmnError::InvalidCString.into()), + }; + NativeResult::ok(Box::::new(sk.into()).into()) } /// Drop an effect parameter. @@ -69,40 +75,47 @@ extern "C" fn effect_parameter_get_type(ptr: ExternPointer) -> /// Get the boolean contained in the effect parameter, panics if the effect parameter is not a bool. #[no_mangle] -extern "C" fn effect_parameter_get_as_bool(ptr: ExternPointer) -> u8 { +extern "C" fn effect_parameter_get_as_bool(ptr: ExternPointer) -> NativeResult { let p = ptr.as_ref(); if let EffectParameter::Bool(_, b) = p { - return u8::from(*b); + NativeResult::ok(u8::from(*b)) + } else { + NativeResult::err(anyhow!("Unexpected effect parameter. Expected bool, was: {}", p)) } - panic!("Unexpected effect parameter. Expected bool, was: {}", p); } /// Get the int contained in the effect parameter, panics if the effect parameter is not a int. #[no_mangle] -extern "C" fn effect_parameter_get_as_int(ptr: ExternPointer) -> i64 { +extern "C" fn effect_parameter_get_as_int(ptr: ExternPointer) -> NativeResult { let p = ptr.as_ref(); if let EffectParameter::Int(_, b) = p { - return *b; + NativeResult::ok(*b) + } else { + NativeResult::err(anyhow!("Unexpected effect parameter. Expected int, was: {}", p)) } - panic!("Unexpected effect parameter. Expected int, was: {}", p); } /// Get the float contained in the effect parameter, panics if the effect parameter is not a float. #[no_mangle] -extern "C" fn effect_parameter_get_as_float(ptr: ExternPointer) -> f32 { +extern "C" fn effect_parameter_get_as_float(ptr: ExternPointer) -> NativeResult { let p = ptr.as_ref(); if let EffectParameter::Float(_, b) = p { - return *b; + NativeResult::ok(*b) + } else { + NativeResult::err(anyhow!("Unexpected effect parameter. Expected float, was: {}", p)) } - panic!("Unexpected effect parameter. Expected float, was: {}", p); } /// Get the string contained in the effect parameter, panics if the effect parameter is not a string. #[no_mangle] -extern "C" fn effect_parameter_get_as_string(ptr: ExternPointer) -> OwnedPtr { +extern "C" fn effect_parameter_get_as_string(ptr: ExternPointer) -> NativeResult> { let p = ptr.as_ref(); if let EffectParameter::String(_, b) = p { - return CString::new(b.str().to_string()).unwrap().into_raw(); + match CString::new(b.str().to_string()) { + Ok(cstr) => NativeResult::ok(cstr.into_raw()), + Err(_) => NativeResult::err(PkmnError::InvalidCString.into()), + } + } else { + NativeResult::err(anyhow!("Unexpected effect parameter. Expected string, was: {}", p)) } - panic!("Unexpected effect parameter. Expected string, was: {}", p); } diff --git a/src/ffi/static_data/move_data.rs b/src/ffi/static_data/move_data.rs index e8113d9..d0be0ab 100644 --- a/src/ffi/static_data/move_data.rs +++ b/src/ffi/static_data/move_data.rs @@ -1,9 +1,10 @@ -use crate::ffi::{ffi_arc_dyn_getter, BorrowedPtr, ExternPointer, IdentifiablePointer, OwnedPtr}; +use crate::ffi::{ffi_arc_dyn_getter, BorrowedPtr, ExternPointer, IdentifiablePointer, NativeResult, OwnedPtr}; use crate::static_data::{ EffectParameter, MoveCategory, MoveData, MoveDataImpl, MoveTarget, SecondaryEffect, SecondaryEffectImpl, TypeIdentifier, }; use crate::StringKey; +use anyhow::anyhow; use hashbrown::HashSet; use std::ffi::{c_char, CStr, CString}; use std::ptr::drop_in_place; @@ -23,12 +24,19 @@ unsafe extern "C" fn move_data_new( secondary_effect: *mut Box, flags: *const *const c_char, flags_length: usize, -) -> IdentifiablePointer> { +) -> NativeResult>> { let flags = std::slice::from_raw_parts(flags, flags_length); - let name: StringKey = CStr::from_ptr(name).to_str().unwrap().into(); + let name: StringKey = match CStr::from_ptr(name).to_str() { + Ok(name) => name.into(), + Err(_) => return NativeResult::err_from_str("Unable to convert name to string"), + }; let mut flags_set: HashSet = HashSet::with_capacity(flags_length); for flag in flags { - flags_set.insert(CStr::from_ptr(*flag).to_str().unwrap().into()); + let flag = match CStr::from_ptr(*flag).to_str() { + Ok(flag) => flag, + Err(_) => return NativeResult::err_from_str("Unable to convert flag to string"), + }; + flags_set.insert(flag.into()); } let secondary_effect = if secondary_effect.is_null() { None @@ -47,7 +55,7 @@ unsafe extern "C" fn move_data_new( secondary_effect, flags_set, )); - a.into() + NativeResult::ok(a.into()) } /// Drops a reference counted move. @@ -58,9 +66,12 @@ unsafe extern "C" fn move_data_drop(ptr: OwnedPtr>) { /// The name of the move. #[no_mangle] -unsafe extern "C" fn move_data_name(ptr: ExternPointer>) -> OwnedPtr { +unsafe extern "C" fn move_data_name(ptr: ExternPointer>) -> NativeResult> { let name = ptr.as_ref().name(); - CString::new(name.str()).unwrap().into_raw() + match CString::new(name.str()) { + Ok(name) => NativeResult::ok(name.into_raw()), + Err(_) => NativeResult::err_from_str("Unable to convert name to string"), + } } ffi_arc_dyn_getter!(MoveData, move_type, TypeIdentifier); @@ -122,8 +133,16 @@ unsafe extern "C" fn secondary_effect_chance(ptr: ExternPointer>) -> OwnedPtr { - CString::new(ptr.as_ref().effect_name().str()).unwrap().into_raw() +unsafe extern "C" fn secondary_effect_effect_name( + ptr: ExternPointer>, +) -> NativeResult> { + match CString::new(ptr.as_ref().effect_name().str()) { + Ok(name) => NativeResult::ok(name.into_raw()), + Err(_) => NativeResult::err(anyhow!( + "Unable to convert effect name '{}' to CString", + ptr.as_ref().effect_name() + )), + } } /// The length of parameters of the effect. diff --git a/src/ffi/static_data/species.rs b/src/ffi/static_data/species.rs index daff7b6..ffc10b0 100644 --- a/src/ffi/static_data/species.rs +++ b/src/ffi/static_data/species.rs @@ -1,8 +1,9 @@ use crate::ffi::{ - ffi_arc_dyn_getter, ffi_arc_stringkey_getter, BorrowedPtr, ExternPointer, IdentifiablePointer, OwnedPtr, + ffi_arc_dyn_getter, ffi_arc_stringkey_getter, BorrowedPtr, ExternPointer, IdentifiablePointer, NativeResult, + OwnedPtr, }; use crate::static_data::{Form, Gender, Species, SpeciesImpl}; -use crate::{Random, StringKey}; +use crate::{PkmnError, Random, StringKey}; use hashbrown::HashSet; use std::ffi::{c_char, CStr}; use std::ptr::drop_in_place; @@ -19,25 +20,40 @@ unsafe extern "C" fn species_new( default_form: OwnedPtr>, flags: *const *const c_char, flags_length: usize, -) -> IdentifiablePointer> { - let name: StringKey = CStr::from_ptr(name).to_str().unwrap().into(); - let growth_rate: StringKey = CStr::from_ptr(growth_rate).to_str().unwrap().into(); +) -> NativeResult>> { + let name: StringKey = match CStr::from_ptr(name).to_str() { + Ok(name) => name.into(), + Err(_) => return NativeResult::err(PkmnError::InvalidCString.into()), + }; + let growth_rate: StringKey = match CStr::from_ptr(growth_rate).to_str() { + Ok(growth_rate) => growth_rate.into(), + Err(_) => return NativeResult::err(PkmnError::InvalidCString.into()), + }; let flags = std::slice::from_raw_parts(flags, flags_length); let mut flags_set: HashSet = HashSet::with_capacity(flags_length); for flag in flags { - flags_set.insert(CStr::from_ptr(*flag).to_str().unwrap().into()); + let flag = match CStr::from_ptr(*flag).to_str() { + Ok(flag) => flag, + Err(_) => return NativeResult::err(PkmnError::InvalidCString.into()), + }; + flags_set.insert(flag.into()); } + let default_form = match default_form.as_ref() { + Some(default_form) => default_form, + None => return NativeResult::err(PkmnError::NullReference.into()), + }; + let a: Arc = Arc::new(SpeciesImpl::new( id, &name, gender_rate, &growth_rate, capture_rate, - default_form.as_ref().unwrap().clone(), + default_form.clone(), flags_set, )); - a.into() + NativeResult::ok(a.into()) } /// Drop a reference to the species. @@ -58,9 +74,13 @@ unsafe extern "C" fn species_add_form( mut species: ExternPointer>, name: BorrowedPtr, form: OwnedPtr>, -) { - let form = form.as_ref().unwrap().clone(); - species.as_mut().add_form(CStr::from_ptr(name).into(), form) +) -> NativeResult<()> { + let form = match form.as_ref() { + Some(form) => form.clone(), + None => return NativeResult::err(PkmnError::NullReference.into()), + }; + species.as_mut().add_form(CStr::from_ptr(name).into(), form); + NativeResult::ok(()) } /// Gets a form by name. diff --git a/src/lib.rs b/src/lib.rs index d5bcae4..63cc987 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -64,6 +64,9 @@ use thiserror::Error; /// that can fail. For uncommon errors, anyhow is used. #[derive(Error, Debug)] pub enum PkmnError { + /// The given value was null + #[error("The given value was null")] + NullReference, /// The given index is out of bounds for the given length of the array. #[error("The given index {index} is out of bounds for the given len {len}")] IndexOutOfBounds { @@ -94,4 +97,13 @@ pub enum PkmnError { /// The nature that was requested nature: StringKey, }, + /// Unable to get species + #[error("Unable to get species {species}")] + InvalidSpeciesName { + /// The species that was requested + species: StringKey, + }, + /// The given pointer is not a valid CString + #[error("The given pointer is not a valid CString")] + InvalidCString, } diff --git a/src/script_implementations/mod.rs b/src/script_implementations/mod.rs index dfcd6ab..36f74fa 100755 --- a/src/script_implementations/mod.rs +++ b/src/script_implementations/mod.rs @@ -1,3 +1,10 @@ /// The WASM module handles loading dynamic scripts through WebAssembly. #[cfg(feature = "wasm")] +// FIXME: allow these for now until we have a good result interface defined. +#[allow(clippy::unwrap_used)] +#[allow(clippy::expect_used)] +#[allow(clippy::indexing_slicing)] +#[allow(clippy::string_slice)] +#[allow(clippy::exit)] +#[allow(clippy::panic)] pub mod wasm; diff --git a/src/script_implementations/wasm/export_registry/dynamic_data/battle_side.rs b/src/script_implementations/wasm/export_registry/dynamic_data/battle_side.rs index dbcf386..c52fc1b 100755 --- a/src/script_implementations/wasm/export_registry/dynamic_data/battle_side.rs +++ b/src/script_implementations/wasm/export_registry/dynamic_data/battle_side.rs @@ -78,7 +78,7 @@ register! { let c_name = CStr::from_ptr(env.data().data().get_raw_pointer(name_ptr)); let script = side.value_func(&env).unwrap().add_volatile_script(&c_name.as_ref().into()).unwrap(); if let Some(script) = script { - let script = script.get_as::(); + let script = script.get_as::().unwrap(); script.get_wasm_pointer() } else { 0 @@ -101,7 +101,7 @@ register! { if let Some(script) = script { let script = side.add_volatile_script_with_script(script); - let s = script.as_ref().unwrap().as_ref().unwrap().get_as::(); + let s = script.as_ref().unwrap().as_ref().unwrap().get_as::().unwrap(); s.get_wasm_pointer() } else { 0 @@ -129,7 +129,7 @@ register! { let c_name = CStr::from_ptr(env.data().data().get_raw_pointer(name_ptr)); let script = side.value_func(&env).unwrap().get_volatile_script(&c_name.as_ref().into()); if let Some(script) = script { - let script = script.get_as::(); + let script = script.get_as::().unwrap(); script.get_wasm_pointer() } else { 0 @@ -144,7 +144,7 @@ register! { ) { unsafe { let c_name = CStr::from_ptr(env.data().data().get_raw_pointer(name_ptr)); - side.value_func(&env).unwrap().remove_volatile_script(&c_name.as_ref().into()); + side.value_func(&env).unwrap().remove_volatile_script(&c_name.as_ref().into()).unwrap(); } } diff --git a/src/script_implementations/wasm/export_registry/dynamic_data/pokemon.rs b/src/script_implementations/wasm/export_registry/dynamic_data/pokemon.rs index 762b002..47e153d 100755 --- a/src/script_implementations/wasm/export_registry/dynamic_data/pokemon.rs +++ b/src/script_implementations/wasm/export_registry/dynamic_data/pokemon.rs @@ -413,7 +413,7 @@ register! { let c_name = CStr::from_ptr(env.data().data().get_raw_pointer(name_ptr)); let script = pokemon.value_func(&env).unwrap().add_volatile_script(&c_name.as_ref().into()).unwrap(); if let Some(script) = script { - let script = script.get_as::(); + let script = script.get_as::().unwrap(); script.get_wasm_pointer() } else { 0 @@ -436,7 +436,7 @@ register! { if let Some(script) = script { let script = pokemon.add_volatile_script_with_script(script); - let s = script.as_ref().unwrap().as_ref().unwrap().get_as::(); + let s = script.as_ref().unwrap().as_ref().unwrap().get_as::().unwrap(); s.get_wasm_pointer() } else { 0 @@ -464,7 +464,7 @@ register! { let c_name = CStr::from_ptr(env.data().data().get_raw_pointer(name_ptr)); let script = pokemon.value_func(&env).unwrap().get_volatile_script(&c_name.as_ref().into()); if let Some(script) = script { - let script = script.get_as::(); + let script = script.get_as::().unwrap(); script.get_wasm_pointer() } else { 0 @@ -479,7 +479,7 @@ register! { ) { unsafe { let c_name = CStr::from_ptr(env.data().data().get_raw_pointer(name_ptr)); - pokemon.value_func(&env).unwrap().remove_volatile_script(&c_name.as_ref().into()); + pokemon.value_func(&env).unwrap().remove_volatile_script(&c_name.as_ref().into()).unwrap(); } } diff --git a/src/script_implementations/wasm/mod.rs b/src/script_implementations/wasm/mod.rs index 051c163..424fd3a 100755 --- a/src/script_implementations/wasm/mod.rs +++ b/src/script_implementations/wasm/mod.rs @@ -1,6 +1,3 @@ -// allow these for now until we have a good result interface defined. -#[allow(clippy::unwrap_used)] -#[allow(clippy::expect_used)] /// The export registry module deals with registering all functions we require in WebAssembly. mod export_registry; /// A hacky extern ref implementation to ensure the client does not do things it is not allowed to do. diff --git a/src/static_data/growth_rates.rs b/src/static_data/growth_rates.rs index dd035e5..c24d8c9 100755 --- a/src/static_data/growth_rates.rs +++ b/src/static_data/growth_rates.rs @@ -1,4 +1,5 @@ use crate::defines::LevelInt; +use crate::VecExt; use anyhow::Result; use anyhow_ext::ensure; @@ -37,9 +38,12 @@ impl GrowthRate for LookupGrowthRate { fn calculate_experience(&self, level: LevelInt) -> Result { ensure!(level > 0, "Level must be greater than 0, but was {}", level); if level >= self.experience.len() as LevelInt { - Ok(*self.experience.last().unwrap()) + match self.experience.last() { + Some(v) => Ok(*v), + None => anyhow::bail!("No experience values found"), + } } else { - Ok(self.experience[(level - 1) as usize]) + Ok(*self.experience.get_res((level - 1) as usize)?) } } } diff --git a/src/static_data/libraries/data_library.rs b/src/static_data/libraries/data_library.rs index c0441f9..80ed996 100755 --- a/src/static_data/libraries/data_library.rs +++ b/src/static_data/libraries/data_library.rs @@ -1,3 +1,4 @@ +use anyhow_ext::Result; use std::sync::Arc; use indexmap::IndexMap; @@ -48,8 +49,11 @@ pub trait DataLibrary { } /// Gets a random value from the library. - fn random_value(&self, rand: &mut Random) -> &Arc { + fn random_value(&self, rand: &mut Random) -> Result<&Arc> { let i = rand.get_between(0, self.len() as i32); - return self.map().get_index(i as usize).unwrap().1; + match self.map().get_index(i as usize) { + Some(v) => Ok(v.1), + None => anyhow::bail!("No value found for index {}", i), + } } } diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 513efac..7c3c508 100755 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -4,6 +4,8 @@ pub use random::Random; pub use string_key::StringKey; #[doc(inline)] pub use value_identifier::*; +#[doc(inline)] +pub use vec_ext::*; /// The random module defines a RNG implementation used in pkmn_lib mod random; @@ -12,3 +14,5 @@ mod random; mod string_key; /// Helper tool to keep track of moving memory for FFI. mod value_identifier; +/// Helper functions for vecs +mod vec_ext; diff --git a/src/utils/vec_ext.rs b/src/utils/vec_ext.rs new file mode 100644 index 0000000..fc74ee3 --- /dev/null +++ b/src/utils/vec_ext.rs @@ -0,0 +1,36 @@ +use crate::PkmnError; +use anyhow::Result; + +/// Extension trait for `Vec` and `[T]` to provide common helper methods. +pub trait VecExt { + /// Get a mutable reference to an element in the vector, or return an error if the index is out of bounds. + fn get_mut_res(&mut self, index: usize) -> Result<&mut T>; + /// Get a reference to an element in the vector, or return an error if the index is out of bounds. + fn get_res(&self, index: usize) -> Result<&T>; +} + +impl VecExt for [T] { + fn get_mut_res(&mut self, index: usize) -> Result<&mut T> { + let len = self.len(); + self.get_mut(index) + .ok_or(PkmnError::IndexOutOfBounds { index, len }.into()) + } + + fn get_res(&self, index: usize) -> Result<&T> { + self.get(index) + .ok_or(PkmnError::IndexOutOfBounds { index, len: self.len() }.into()) + } +} + +impl VecExt for Vec { + fn get_mut_res(&mut self, index: usize) -> Result<&mut T> { + let len = self.len(); + self.get_mut(index) + .ok_or(PkmnError::IndexOutOfBounds { index, len }.into()) + } + + fn get_res(&self, index: usize) -> Result<&T> { + self.get(index) + .ok_or(PkmnError::IndexOutOfBounds { index, len: self.len() }.into()) + } +} diff --git a/tests/common/test_case.rs b/tests/common/test_case.rs index 378cd1d..aa04961 100755 --- a/tests/common/test_case.rs +++ b/tests/common/test_case.rs @@ -56,7 +56,7 @@ impl TestCase { } let mut battle_parties = Vec::new(); for party in parties { - battle_parties.push(BattleParty::new(party.0.clone(), party.1)); + battle_parties.push(BattleParty::new(party.0.clone(), party.1).unwrap()); } let mut battle = Battle::new( library, diff --git a/tests/integration.rs b/tests/integration.rs index 42d4565..7c00414 100755 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -99,11 +99,13 @@ fn validate_assurance() { let party1 = BattleParty::new( Arc::new(PokemonParty::new_from_vec(vec![Some(p1.clone())])), vec![(0, 0)], - ); + ) + .unwrap(); let party2 = BattleParty::new( Arc::new(PokemonParty::new_from_vec(vec![Some(p2.clone())])), vec![(1, 0)], - ); + ) + .unwrap(); let battle = Battle::new(lib.clone(), vec![party1, party2], false, 2, 1, None);