Support for new error handling.

src/dynamic_data/flow/choice_queue.rs

@@ -2,6 +2,8 @@ use crate::dynamic_data::choices::TurnChoice;
 use crate::dynamic_data::script_handling::ScriptSource;
 use crate::dynamic_data::Pokemon;
 use crate::{script_hook, ValueIdentifiable, ValueIdentifier};
+use anyhow::Result;
+use anyhow_ext::anyhow;
 use parking_lot::lock_api::MappedRwLockReadGuard;
 use parking_lot::{RawRwLock, RwLock, RwLockReadGuard};
 
@@ -48,12 +50,16 @@ impl ChoiceQueue {
     }
 
     /// This reads what the next choice to execute will be, without modifying state.
-    pub fn peek(&self) -> Option<MappedRwLockReadGuard<'_, RawRwLock, TurnChoice>> {
+    pub fn peek(&self) -> Result<Option<MappedRwLockReadGuard<'_, RawRwLock, TurnChoice>>> {
         let read_lock = self.queue.read();
         if self.current >= read_lock.len() {
-            None
+            Ok(None)
         } else {
-            Some(RwLockReadGuard::map(read_lock, |a| a[self.current].as_ref().unwrap()))
+            let v = RwLockReadGuard::try_map(read_lock, |a| a[self.current].as_ref());
+            match v {
+                Ok(v) => Ok(Some(v)),
+                Err(_) => Err(anyhow!("Could not map choice")),
+            }
         }
     }
 
@@ -81,7 +87,7 @@ impl ChoiceQueue {
     }
 
     /// This moves the choice of a specific Pokemon up to the next choice to be executed.
-    pub fn move_pokemon_choice_next(&self, pokemon: &Pokemon) -> bool {
+    pub fn move_pokemon_choice_next(&self, pokemon: &Pokemon) -> Result<bool> {
         let mut queue_lock = self.queue.write();
         let mut desired_index = None;
         // Find the index for the choice we want to move up.
@@ -93,26 +99,29 @@ impl ChoiceQueue {
                 }
             }
         }
-        // If we couldn't find a choice, we can't execute, return.
-        if desired_index.is_none() {
-            return false;
-        }
-        let desired_index = desired_index.unwrap();
-        // If the choice we want to move up is already the next choice, just return.
-        if desired_index == self.current {
-            return true;
-        }
+        let result = match desired_index {
+            Some(desired_index) => {
+                // If the choice we want to move up is already the next choice, just return.
+                if desired_index == self.current {
+                    return Ok(true);
+                }
 
-        // Take the choice we want to move forward out of it's place.
-        let choice = queue_lock[desired_index].take().unwrap();
-        // Iterate backwards from the spot before the choice we want to move up, push them all back
-        // by 1 spot.
-        for index in (self.current..desired_index).rev() {
-            queue_lock.swap(index, index + 1);
-        }
-        // Place the choice that needs to be next in the next to be executed position.
-        let _ = queue_lock[self.current].insert(choice);
-        true
+                // Take the choice we want to move forward out of it's place.
+                let choice = queue_lock[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
+                // by 1 spot.
+                for index in (self.current..desired_index).rev() {
+                    queue_lock.swap(index, index + 1);
+                }
+                // Place the choice that needs to be next in the next to be executed position.
+                let _ = queue_lock[self.current].insert(choice);
+                true
+            }
+            None => false,
+        };
+        Ok(result)
     }
 
     /// Internal helper function to be easily able to iterate over the yet to be executed choices.
@@ -140,7 +149,7 @@ mod tests {
     fn create_empty_queue() {
         let queue = ChoiceQueue::new(Vec::new());
         assert!(!queue.has_next());
-        assert!(queue.peek().is_none());
+        assert!(queue.peek().unwrap().is_none());
     }
 
     #[test]
@@ -168,6 +177,7 @@ mod tests {
             0,
             &"test_nature".into(),
         )
+        .unwrap()
     }
 
     #[test]
@@ -176,8 +186,8 @@ mod tests {
 
         let queue = ChoiceQueue::new(vec![Some(TurnChoice::Pass(PassChoice::new(user)))]);
         assert!(queue.has_next());
-        assert!(queue.peek().is_some());
-        assert_eq!(7, queue.peek().unwrap().speed());
+        assert!(queue.peek().unwrap().is_some());
+        assert_eq!(7, queue.peek().unwrap().unwrap().speed());
     }
 
     #[test]
@@ -188,7 +198,7 @@ mod tests {
         assert!(queue.has_next());
         assert_eq!(7, queue.dequeue().unwrap().speed());
         assert!(!queue.has_next());
-        assert!(queue.peek().is_none());
+        assert!(queue.peek().unwrap().is_none());
     }
 
     #[test]
@@ -201,8 +211,8 @@ mod tests {
             Some(TurnChoice::Pass(PassChoice::new(user2))),
         ]);
         assert!(queue.has_next());
-        assert!(queue.peek().is_some());
-        assert_eq!(7, queue.peek().unwrap().speed());
+        assert!(queue.peek().unwrap().is_some());
+        assert_eq!(7, queue.peek().unwrap().unwrap().speed());
     }
 
     #[test]
@@ -247,12 +257,12 @@ mod tests {
         user2.change_level_by(60);
         assert_eq!(
             user1.value_identifier(),
-            queue.peek().unwrap().user().value_identifier()
+            queue.peek().unwrap().unwrap().user().value_identifier()
         );
         queue.resort();
         assert_eq!(
             user2.value_identifier(),
-            queue.peek().unwrap().user().value_identifier()
+            queue.peek().unwrap().unwrap().user().value_identifier()
         );
     }
 
@@ -267,9 +277,9 @@ mod tests {
         ]);
         assert_eq!(
             user1.value_identifier(),
-            queue.peek().unwrap().user().value_identifier()
+            queue.peek().unwrap().unwrap().user().value_identifier()
         );
-        assert!(queue.move_pokemon_choice_next(user2.as_ref()));
+        assert!(queue.move_pokemon_choice_next(user2.as_ref()).unwrap());
 
         assert_eq!(
             user2.value_identifier(),
@@ -294,13 +304,13 @@ mod tests {
             user2.value_identifier(),
             queue.dequeue().unwrap().user().value_identifier()
         );
-        assert!(!queue.move_pokemon_choice_next(user2.as_ref()));
+        assert!(!queue.move_pokemon_choice_next(user2.as_ref()).unwrap());
 
         assert_eq!(
             user1.value_identifier(),
             queue.dequeue().unwrap().user().value_identifier()
         );
-        assert!(queue.peek().is_none())
+        assert!(queue.peek().unwrap().is_none())
     }
 
     #[test]
@@ -314,12 +324,12 @@ mod tests {
         ]);
         assert_eq!(
             user1.value_identifier(),
-            queue.peek().unwrap().user().value_identifier()
+            queue.peek().unwrap().unwrap().user().value_identifier()
         );
-        assert!(queue.move_pokemon_choice_next(user1.as_ref()));
+        assert!(queue.move_pokemon_choice_next(user1.as_ref()).unwrap());
         assert_eq!(
             user1.value_identifier(),
-            queue.peek().unwrap().user().value_identifier()
+            queue.peek().unwrap().unwrap().user().value_identifier()
         );
     }
 
@@ -346,9 +356,9 @@ mod tests {
         ]);
         assert_eq!(
             users[0].value_identifier(),
-            queue.peek().unwrap().user().value_identifier()
+            queue.peek().unwrap().unwrap().user().value_identifier()
         );
-        assert!(queue.move_pokemon_choice_next(users[4].as_ref()));
+        assert!(queue.move_pokemon_choice_next(users[4].as_ref()).unwrap());
 
         assert_eq!(
             users[4].value_identifier(),

src/dynamic_data/flow/target_resolver.rs

@@ -135,45 +135,48 @@ pub fn resolve_targets(side: u8, index: u8, target: MoveTarget, battle: &Battle)
 
 /// Checks whether a given side and index are valid for a given move target and user.
 pub fn is_valid_target(side: u8, index: u8, target: MoveTarget, user: &Pokemon) -> bool {
-    let user_side = user.get_battle_side_index();
-    let user_index = user.get_battle_index();
     // If the user is not on the field, nothing is a valid target
-    if user_side.is_none() || user_index.is_none() {
-        return false;
-    }
+    let user_side = match user.get_battle_side_index() {
+        Some(side) => side,
+        None => return false,
+    };
+    let user_index = match user.get_battle_index() {
+        Some(index) => index,
+        None => return false,
+    };
 
     match target {
         MoveTarget::Adjacent | MoveTarget::AllAdjacent => {
-            let diff = abs(index as i32 - user_index.unwrap() as i32);
+            let diff = abs(index as i32 - user_index as i32);
             if diff == 0 {
-                return user_side.unwrap() != side;
+                return user_side != side;
             }
             diff <= 1
         }
         MoveTarget::AdjacentAlly => {
-            if user_side.unwrap() != side {
+            if user_side != side {
                 false
             } else {
-                abs(index as i32 - user_index.unwrap() as i32) == 1
+                abs(index as i32 - user_index as i32) == 1
             }
         }
         MoveTarget::AdjacentAllySelf => {
-            if user_side.unwrap() != side {
+            if user_side != side {
                 false
             } else {
-                abs(index as i32 - user_index.unwrap() as i32) <= 1
+                abs(index as i32 - user_index as i32) <= 1
             }
         }
         MoveTarget::AdjacentOpponent | MoveTarget::AllAdjacentOpponent => {
-            if user_side.unwrap() == side {
+            if user_side == side {
                 false
             } else {
-                abs(index as i32 - user_index.unwrap() as i32) <= 1
+                abs(index as i32 - user_index as i32) <= 1
             }
         }
         MoveTarget::All | MoveTarget::Any | MoveTarget::RandomOpponent => true,
-        MoveTarget::AllAlly => user_side.unwrap() == side,
-        MoveTarget::AllOpponent => user_side.unwrap() != side,
-        MoveTarget::SelfUse => user_side.unwrap() == side && user_index.unwrap() == index,
+        MoveTarget::AllAlly => user_side == side,
+        MoveTarget::AllOpponent => user_side != side,
+        MoveTarget::SelfUse => user_side == side && user_index == index,
     }
 }

src/dynamic_data/flow/turn_runner.rs

@@ -1,3 +1,5 @@
+use anyhow::Result;
+use anyhow_ext::anyhow;
 use std::ops::Deref;
 use std::sync::Arc;
 
@@ -10,11 +12,21 @@ use crate::dynamic_data::DamageSource;
 use crate::dynamic_data::ExecutingMove;
 use crate::dynamic_data::Pokemon;
 use crate::static_data::MoveCategory;
-use crate::{run_scripts, script_hook, PkmnResult};
+use crate::{run_scripts, script_hook};
+
+/// Simple macro to get a read lock on the choice queue.
+macro_rules! read_choice_queue {
+    ($choice_queue:ident) => {
+        $choice_queue
+            .read()
+            .as_ref()
+            .ok_or(anyhow!("Could not get a lock on choice queue"))?
+    };
+}
 
 impl Battle {
     /// Execute the entire turn after the choices are all set.
-    pub(crate) fn run_turn(&self) -> PkmnResult<()> {
+    pub(crate) fn run_turn(&self) -> Result<()> {
         let choice_queue = self.current_turn_queue();
 
         // We are now at the very beginning of a turn. We have assigned speeds and priorities to all
@@ -24,15 +36,19 @@ impl Battle {
         // is primarily intended to be used to reset variables on a script (for example scripts that need
         // to check whether a pokemon was hit this turn. By resetting here, and setting a variable to true
         // they can then know this later on.)
-        for choice in choice_queue.read().as_ref().unwrap().get_queue().iter().flatten() {
+        for choice in read_choice_queue!(choice_queue).get_queue().iter().flatten() {
             script_hook!(on_before_turn, choice, choice);
         }
 
         // Now we can properly begin executing choices.
         // One by one dequeue the turns, and run them. If the battle has ended we do not want to
         // continue running however.
-        while choice_queue.read().as_ref().unwrap().has_next() && !self.has_ended() {
-            let choice = choice_queue.write().as_mut().unwrap().dequeue();
+        while read_choice_queue!(choice_queue).has_next() && !self.has_ended() {
+            let choice = choice_queue
+                .write()
+                .as_mut()
+                .ok_or(anyhow!("Failed to get a write lock on choice queue"))?
+                .dequeue();
             if let Some(choice) = choice {
                 self.execute_choice(&choice)?;
             }
@@ -64,7 +80,7 @@ impl Battle {
     }
 
     /// Executes a single choice.
-    fn execute_choice(&self, choice: &TurnChoice) -> PkmnResult<()> {
+    fn execute_choice(&self, choice: &TurnChoice) -> Result<()> {
         // A pass turn choice means the user does not intend to do anything. As such, return.
         if let TurnChoice::Pass(..) = choice {
             return Ok(());
@@ -93,7 +109,7 @@ impl Battle {
     }
 
     /// Executes a move choice.
-    fn execute_move_choice<'func>(&'func self, choice: &'func TurnChoice) -> PkmnResult<()> {
+    fn execute_move_choice<'func>(&'func self, choice: &'func TurnChoice) -> Result<()> {
         let move_choice = choice.get_move_turn_data();
         let used_move = move_choice.used_move();
         let move_data = {
@@ -101,7 +117,11 @@ impl Battle {
             let move_data = move_data_lock.move_data();
             let mut move_name = move_data.name().clone();
             script_hook!(change_move, choice, choice, &mut move_name);
-            self.library().static_data().moves().get(&move_name).unwrap()
+            self.library()
+                .static_data()
+                .moves()
+                .get(&move_name)
+                .ok_or(anyhow!("Move not found"))?
         };
         // FIXME: also change the script on the choice if changed;
         let target_type = move_data.target();
@@ -150,7 +170,7 @@ impl Battle {
     }
 
     /// Executes a move turn choice on a single target.
-    fn handle_move_for_target(&self, executing_move: &mut ExecutingMove, target: &Arc<Pokemon>) -> PkmnResult<()> {
+    fn handle_move_for_target(&self, executing_move: &mut ExecutingMove, target: &Arc<Pokemon>) -> Result<()> {
         {
             let mut fail = false;
             script_hook!(fail_incoming_move, target, executing_move, target, &mut fail);
@@ -228,7 +248,7 @@ impl Battle {
                 let is_critical =
                     self.library()
                         .damage_calculator()
-                        .is_critical(self, executing_move, target, hit_index);
+                        .is_critical(self, executing_move, target, hit_index)?;
                 hit_data.set_critical(is_critical);
             }
             let base_power = self.library().damage_calculator().get_base_power(
@@ -243,7 +263,7 @@ impl Battle {
                 target,
                 hit_index,
                 executing_move.get_hit_data(target, hit_index)?,
-            );
+            )?;
             hit_data.set_damage(damage);
 
             let mut accuracy = executing_move.use_move().accuracy();
@@ -259,7 +279,9 @@ impl Battle {
                     &mut accuracy
                 );
             }
-            if accuracy < 100 && self.random().get_max(100) as u8 >= accuracy {
+
+            // TODO: Deal with accuracy/evasion stats.
+            if accuracy < 100 && self.random().get_max(100)? as u8 >= accuracy {
                 script_hook!(on_move_miss, target, executing_move, target);
                 self.event_hook().trigger(Event::Miss {
                     user: executing_move.user().deref(),
@@ -273,7 +295,7 @@ impl Battle {
                     if secondary_effect_chance == -1.0
                         || self
                             .random()
-                            .effect_chance(secondary_effect_chance, executing_move, target, hit_index)
+                            .effect_chance(secondary_effect_chance, executing_move, target, hit_index)?
                     {
                         script_hook!(on_secondary_effect, executing_move, executing_move, target, hit_index);
                         // TODO: on fail
@@ -287,7 +309,7 @@ impl Battle {
                     hit_data.set_damage(damage);
                 }
                 if damage > 0 {
-                    target.damage(damage, DamageSource::MoveDamage);
+                    target.damage(damage, DamageSource::MoveDamage)?;
                     if !target.is_fainted() {
                         script_hook!(on_incoming_hit, target, executing_move, target, hit_index);
                     } else {
@@ -313,7 +335,7 @@ impl Battle {
                                         executing_move,
                                         target,
                                         hit_index,
-                                    )
+                                    )?
                                 {
                                     script_hook!(
                                         on_secondary_effect,

src/dynamic_data/libraries/damage_library.rs

@@ -1,3 +1,5 @@
+use anyhow::Result;
+use anyhow_ext::anyhow;
 use std::sync::Arc;
 
 use crate::dynamic_data::script_handling::ScriptSource;
@@ -16,7 +18,7 @@ pub trait DamageLibrary: std::fmt::Debug + ValueIdentifiable {
         target: &Arc<Pokemon>,
         hit_number: u8,
         hit_data: &HitData,
-    ) -> u32;
+    ) -> Result<u32>;
 
     /// Calculate the base power for a given hit on a Pokemon.
     fn get_base_power(
@@ -34,7 +36,7 @@ pub trait DamageLibrary: std::fmt::Debug + ValueIdentifiable {
         executing_move: &ExecutingMove,
         target: &Arc<Pokemon>,
         hit_number: u8,
-    ) -> bool;
+    ) -> Result<bool>;
 }
 
 /// The implementation of a Damage Library for generation 7.
@@ -174,9 +176,9 @@ impl DamageLibrary for Gen7DamageLibrary {
         target: &Arc<Pokemon>,
         hit_number: u8,
         hit_data: &HitData,
-    ) -> u32 {
+    ) -> Result<u32> {
         if executing_move.use_move().category() == MoveCategory::Status {
-            return 0;
+            return Ok(0);
         }
 
         let level_modifier = ((2.0 * executing_move.user().level() as f32) / 5.0).floor() + 2.0;
@@ -205,7 +207,13 @@ impl DamageLibrary for Gen7DamageLibrary {
         }
 
         if self.has_randomness {
-            let random_percentage = 85 + executing_move.user().get_battle().unwrap().random().get_between(0, 16);
+            let random_percentage = 85
+                + executing_move
+                    .user()
+                    .get_battle()
+                    .ok_or(anyhow!("Damage calculation was done for a Pokemon not in battle"))?
+                    .random()
+                    .get_between(0, 16)?;
             float_damage = (float_damage * (random_percentage as f32 / 100.0)).floor();
         }
 
@@ -247,7 +255,7 @@ impl DamageLibrary for Gen7DamageLibrary {
             hit_number,
             &mut damage
         );
-        damage
+        Ok(damage)
     }
 
     fn get_base_power(
@@ -279,10 +287,10 @@ impl DamageLibrary for Gen7DamageLibrary {
         executing_move: &ExecutingMove,
         target: &Arc<Pokemon>,
         hit_number: u8,
-    ) -> bool {
+    ) -> Result<bool> {
         // Status moves can't be critical.
         if executing_move.use_move().category() == MoveCategory::Status {
-            return false;
+            return Ok(false);
         }
         // Get the critical stage from scripts.
         let mut crit_stage = 0;
@@ -296,12 +304,12 @@ impl DamageLibrary for Gen7DamageLibrary {
         );
         // Crit stage is an unsigned byte, so we only care about values of 0 or higher.
         // For a critical stage of 3+ we always return true.
-        match crit_stage {
-            0 => battle.random().get_max(24) == 0,
-            1 => battle.random().get_max(8) == 0,
-            2 => battle.random().get_max(2) == 0,
+        Ok(match crit_stage {
+            0 => battle.random().get_max(24)? == 0,
+            1 => battle.random().get_max(8)? == 0,
+            2 => battle.random().get_max(2)? == 0,
             _ => true,
-        }
+        })
     }
 }
 

src/dynamic_data/libraries/dynamic_library.rs

@@ -1,3 +1,4 @@
+use anyhow::Result;
 use std::fmt::Debug;
 use std::sync::Arc;
 
@@ -9,7 +10,7 @@ use crate::dynamic_data::{ItemScript, ScriptResolver};
 use crate::dynamic_data::{Script, ScriptOwnerData};
 use crate::static_data::Item;
 use crate::static_data::StaticData;
-use crate::{PkmnResult, StringKey, ValueIdentifiable, ValueIdentifier};
+use crate::{StringKey, ValueIdentifiable, ValueIdentifier};
 
 /// The dynamic library stores a static data library, as well as holding different libraries and
 /// calculators that might be customized between different generations and implementations.
@@ -32,11 +33,11 @@ pub trait DynamicLibrary: Debug + ValueIdentifiable {
         owner: ScriptOwnerData,
         _category: ScriptCategory,
         _key: &StringKey,
-    ) -> PkmnResult<Option<Arc<dyn Script>>>;
+    ) -> Result<Option<Arc<dyn Script>>>;
     /// Loads an item script with the given unique key. If no script can be created with this
     /// combinations, returns None. Note that ItemScripts are immutable, as their script should be
     /// shared between all different usages.
-    fn load_item_script(&self, _key: &Arc<dyn Item>) -> PkmnResult<Option<Arc<dyn ItemScript>>>;
+    fn load_item_script(&self, _key: &Arc<dyn Item>) -> Result<Option<Arc<dyn ItemScript>>>;
 }
 
 /// The dynamic library stores a static data library, as well as holding different libraries and
@@ -107,13 +108,13 @@ impl DynamicLibrary for DynamicLibraryImpl {
         owner: ScriptOwnerData,
         _category: ScriptCategory,
         _key: &StringKey,
-    ) -> PkmnResult<Option<Arc<dyn Script>>> {
+    ) -> Result<Option<Arc<dyn Script>>> {
         self.script_resolver.load_script(owner, _category, _key)
     }
     /// Loads an item script with the given unique key. If no script can be created with this
     /// combinations, returns None. Note that ItemScripts are immutable, as their script should be
     /// shared between all different usages.
-    fn load_item_script(&self, _key: &Arc<dyn Item>) -> PkmnResult<Option<Arc<dyn ItemScript>>> {
+    fn load_item_script(&self, _key: &Arc<dyn Item>) -> Result<Option<Arc<dyn ItemScript>>> {
         todo!()
     }
 }
@@ -145,8 +146,8 @@ pub mod test {
                 owner: ScriptOwnerData,
                 _category: ScriptCategory,
                 _key: &StringKey,
-            ) -> PkmnResult<Option<Arc<dyn Script>>>;
-            fn load_item_script(&self, _key: &Arc<dyn Item>) -> PkmnResult<Option<Arc<dyn ItemScript>>>;
+            ) -> Result<Option<Arc<dyn Script>>>;
+            fn load_item_script(&self, _key: &Arc<dyn Item>) -> Result<Option<Arc<dyn ItemScript>>>;
         }
         impl ValueIdentifiable for DynamicLibrary{
             fn value_identifier(&self) -> ValueIdentifier{

src/dynamic_data/libraries/script_resolver.rs

@@ -1,9 +1,10 @@
+use anyhow::Result;
 use std::fmt::Debug;
 use std::sync::Arc;
 
 use crate::dynamic_data::{ItemScript, Script, ScriptOwnerData};
 use crate::static_data::Item;
-use crate::{PkmnResult, StringKey, ValueIdentifiable, ValueIdentifier};
+use crate::{StringKey, ValueIdentifiable, ValueIdentifier};
 
 /// A script resolver deals with the resolving of scripts. These scripts are non-hardcoded
 /// implementations of different effects in Pokemon. This allows for things such as generational
@@ -16,12 +17,12 @@ pub trait ScriptResolver: Debug + ValueIdentifiable {
         owner: ScriptOwnerData,
         category: ScriptCategory,
         script_key: &StringKey,
-    ) -> PkmnResult<Option<Arc<dyn Script>>>;
+    ) -> Result<Option<Arc<dyn Script>>>;
 
     /// Loads an item script with the given unique key. If no script can be created with this
     /// combinations, returns None. Note that ItemScripts are immutable, as their script should be
     /// shared between all different usages.
-    fn load_item_script(&self, _key: &dyn Item) -> PkmnResult<Option<Arc<dyn ItemScript>>>;
+    fn load_item_script(&self, _key: &dyn Item) -> Result<Option<Arc<dyn ItemScript>>>;
 }
 
 use std::fmt::Display;
@@ -73,11 +74,11 @@ impl ScriptResolver for EmptyScriptResolver {
         _owner: ScriptOwnerData,
         _category: ScriptCategory,
         _script_key: &StringKey,
-    ) -> PkmnResult<Option<Arc<dyn Script>>> {
+    ) -> Result<Option<Arc<dyn Script>>> {
         Ok(None)
     }
 
-    fn load_item_script(&self, _key: &dyn Item) -> PkmnResult<Option<Arc<dyn ItemScript>>> {
+    fn load_item_script(&self, _key: &dyn Item) -> Result<Option<Arc<dyn ItemScript>>> {
         Ok(None)
     }
 }

src/dynamic_data/models/battle.rs

@@ -2,6 +2,8 @@ use std::ops::{Deref, DerefMut};
 use std::sync::atomic::{AtomicBool, AtomicU32, Ordering};
 use std::sync::Arc;
 
+use anyhow::Result;
+use anyhow_ext::anyhow;
 use atomig::Atomic;
 use parking_lot::RwLock;
 
@@ -18,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, PkmnResult, StringKey, ValueIdentifiable, ValueIdentifier};
+use crate::{script_hook, StringKey, ValueIdentifiable, ValueIdentifier};
 
 /// A pokemon battle, with any amount of sides and pokemon per side.
 #[derive(Debug)]
@@ -169,11 +171,9 @@ impl Battle {
     /// Get a Pokemon on the battlefield, on a specific side and an index on that side.
     pub fn get_pokemon(&self, side: u8, index: u8) -> Option<Arc<Pokemon>> {
         let side = self.sides.get(side as usize);
-        side?;
-        let pokemon_read_lock = side.unwrap().pokemon();
+        let pokemon_read_lock = side?.pokemon();
         let pokemon = pokemon_read_lock.get(index as usize);
-        pokemon?;
-        pokemon.unwrap().clone()
+        pokemon?.clone()
     }
 
     /// Returns whether a slot on the battlefield can still be filled. If no party is responsible
@@ -190,10 +190,10 @@ impl Battle {
 
     /// Validates whether the battle is still in a non-ended state. If the battle has ended, this
     /// properly sets who has won etc.
-    pub fn validate_battle_state(&self) {
+    pub fn validate_battle_state(&self) -> Result<()> {
         // If we've already ended, we dont need to run this.
         if self.has_ended() {
-            return;
+            return Ok(());
         }
         let mut surviving_side_exists = false;
         let mut winning_side = None;
@@ -205,13 +205,13 @@ impl Battle {
                     self.result.data_ptr().replace(BattleResult::Inconclusive);
                 }
                 self.has_ended.store(true, Ordering::SeqCst);
-                return;
+                return Ok(());
             }
             // If the side is not defeated
             if !side.is_defeated() {
                 // More than 1 surviving side. Battle is not ended
                 if surviving_side_exists {
-                    return;
+                    return Ok(());
                 }
                 surviving_side_exists = true;
                 winning_side = Some(side_index as u8);
@@ -228,12 +228,13 @@ impl Battle {
         else {
             let _w = self.result.write();
             unsafe {
-                self.result
-                    .data_ptr()
-                    .replace(BattleResult::Conclusive(winning_side.unwrap()));
+                self.result.data_ptr().replace(BattleResult::Conclusive(
+                    winning_side.ok_or(anyhow!("Winning side was not set"))?,
+                ));
             }
         }
         self.has_ended.store(true, Ordering::SeqCst);
+        Ok(())
     }
 
     /// Checks whether a choice is actually possible.
@@ -260,7 +261,7 @@ impl Battle {
     }
 
     /// Try and set the choice for the battle. If the choice is not valid, this returns false.
-    pub fn try_set_choice(&self, choice: TurnChoice) -> PkmnResult<bool> {
+    pub fn try_set_choice(&self, choice: TurnChoice) -> Result<bool> {
         if !self.can_use(&choice) {
             return Ok(false);
         }
@@ -268,17 +269,19 @@ impl Battle {
             return Ok(false);
         }
         let side = choice.user().get_battle_side_index();
-        if side.is_none() {
-            return Ok(false);
+        match side {
+            Some(side) => {
+                self.sides[side as usize].set_choice(choice);
+                self.check_choices_set_and_run()?;
+                Ok(true)
+            }
+            None => Ok(false),
         }
-        self.sides[side.unwrap() as usize].set_choice(choice);
-        self.check_choices_set_and_run()?;
-        Ok(true)
     }
 
     /// Checks to see whether all Pokemon on the field have set their choices. If so, we then run
     /// the turn.
-    fn check_choices_set_and_run(&self) -> PkmnResult<()> {
+    fn check_choices_set_and_run(&self) -> Result<()> {
         for side in &self.sides {
             if !side.all_choices_set() {
                 return Ok(());
@@ -292,10 +295,9 @@ impl Battle {
         for side in &self.sides {
             let mut side_choices = side.choices().write();
             for choice_opt in side_choices.deref_mut() {
-                if choice_opt.is_none() {
-                    panic!("Choice was none, but all choices were set? Logic error.");
-                }
-                let mut choice = choice_opt.as_mut().unwrap();
+                let mut choice = choice_opt
+                    .as_mut()
+                    .ok_or(anyhow!("Choice was none, but all choices were set? Logic error."))?;
                 let c = choice.deref();
                 if let TurnChoice::Move(data) = c {
                     let mut change_priority = data.priority();
@@ -312,7 +314,7 @@ impl Battle {
                 script_hook!(change_speed, c, c, &mut speed);
                 *choice.speed_mut() = speed;
 
-                choice.set_random_value(self.random.get() as u32);
+                choice.set_random_value(self.random.get()? as u32);
                 choices.push(choice_opt.take());
             }
             // Drop the lock guard, as we need to write into it in reset_choices.
@@ -331,32 +333,41 @@ impl Battle {
         self.event_hook.trigger(Event::EndTurn);
         let end_time = chrono::Utc::now();
         let time = end_time - start_time;
-        self.last_turn_time
-            .store(time.num_nanoseconds().unwrap() as u64, Ordering::SeqCst);
+        match time.num_nanoseconds() {
+            None => {}
+            Some(v) => {
+                self.last_turn_time.store(v as u64, Ordering::SeqCst);
+            }
+        }
         Ok(())
     }
 
     /// Sets the current weather for the battle. If None is passed, this clears the weather.
-    pub fn set_weather(&self, weather: Option<StringKey>) {
+    pub fn set_weather(&self, weather: Option<StringKey>) -> Result<()> {
         if let Some(weather) = weather {
             let script = self
                 .library()
-                .load_script(self.into(), ScriptCategory::Weather, &weather)
-                .unwrap()
-                .unwrap_or_else(|| panic!("Couldn't find weather script by name {}", weather));
+                .load_script(self.into(), ScriptCategory::Weather, &weather)?
+                .ok_or(anyhow!("Couldn't find weather script by name {}", weather))?;
             self.weather.set(script);
         } else {
             self.weather.clear();
         }
+        Ok(())
     }
 
     /// Gets the current weather of the battle. If no weather is present, this returns None.
-    pub fn weather_name(&self) -> Option<StringKey> {
+    pub fn weather_name(&self) -> Result<Option<StringKey>> {
         if let Some(script) = self.weather.get() {
             let lock = script.read();
-            Some(lock.as_ref().unwrap().name().clone())
+            Ok(Some(
+                lock.as_ref()
+                    .ok_or(anyhow!("Failed to get a lock on weather"))?
+                    .name()
+                    .clone(),
+            ))
         } else {
-            None
+            Ok(None)
         }
     }
 }
@@ -366,7 +377,7 @@ impl VolatileScriptsOwner for Battle {
         &self.volatile_scripts
     }
 
-    fn load_volatile_script(&self, key: &StringKey) -> PkmnResult<Option<Arc<dyn Script>>> {
+    fn load_volatile_script(&self, key: &StringKey) -> Result<Option<Arc<dyn Script>>> {
         self.library.load_script(self.into(), ScriptCategory::Battle, key)
     }
 }

src/dynamic_data/models/battle_random.rs

@@ -1,3 +1,5 @@
+use anyhow::Result;
+use anyhow_ext::anyhow;
 use std::fmt::{Debug, Formatter};
 use std::sync::{Arc, Mutex};
 
@@ -32,16 +34,25 @@ impl BattleRandom {
     }
 
     /// Get a random 32 bit integer. Can be any value between min int and max int.
-    pub fn get(&self) -> i32 {
-        return self.get_rng().lock().unwrap().get();
+    pub fn get(&self) -> Result<i32> {
+        match self.get_rng().lock() {
+            Ok(mut l) => Ok(l.get()),
+            Err(_) => Err(anyhow!("Failed to get a RNG lock")),
+        }
     }
     /// Get a random 32 bit integer between 0 and max.
-    pub fn get_max(&self, max: i32) -> i32 {
-        return self.get_rng().lock().unwrap().get_max(max);
+    pub fn get_max(&self, max: i32) -> Result<i32> {
+        match self.get_rng().lock() {
+            Ok(mut l) => Ok(l.get_max(max)),
+            Err(_) => Err(anyhow!("Failed to get a RNG lock")),
+        }
     }
     /// Get a random 32 bit integer between min and max.
-    pub fn get_between(&self, min: i32, max: i32) -> i32 {
-        return self.get_rng().lock().unwrap().get_between(min, max);
+    pub fn get_between(&self, min: i32, max: i32) -> Result<i32> {
+        match self.get_rng().lock() {
+            Ok(mut l) => Ok(l.get_between(min, max)),
+            Err(_) => Err(anyhow!("Failed to get a RNG lock")),
+        }
     }
 
     /// Gets whether or not a move triggers its secondary effect. This takes its chance, and
@@ -53,7 +64,7 @@ impl BattleRandom {
         executing_move: &ExecutingMove,
         target: &Arc<Pokemon>,
         hit_number: u8,
-    ) -> bool {
+    ) -> Result<bool> {
         script_hook!(
             change_effect_chance,
             executing_move,
@@ -72,12 +83,15 @@ impl BattleRandom {
         );
         if chance < 100.0 {
             if chance > 0.0 {
-                self.get_rng().lock().unwrap().get_float() < (chance / 100.0)
+                match self.get_rng().lock() {
+                    Ok(mut l) => Ok(l.get_float() < (chance / 100.0)),
+                    Err(_) => Err(anyhow!("Failed to get a RNG lock")),
+                }
             } else {
-                false
+                Ok(false)
             }
         } else {
-            true
+            Ok(true)
         }
     }
 }
@@ -92,6 +106,10 @@ impl Clone for BattleRandom {
     fn clone(&self) -> Self {
         Self {
             identifier: Default::default(),
+
+            // As cloning when we can't get a lock on the randomness is completely impossible, we
+            // should unwrap here.
+            #[allow(clippy::unwrap_used)]
             random: Mutex::new(self.random.lock().unwrap().clone()),
         }
     }

src/dynamic_data/models/battle_side.rs

@@ -2,6 +2,7 @@ use std::ops::Deref;
 use std::sync::atomic::{AtomicBool, AtomicU8, Ordering};
 use std::sync::Arc;
 
+use anyhow::Result;
 use parking_lot::lock_api::RwLockReadGuard;
 use parking_lot::{RawRwLock, RwLock};
 
@@ -14,7 +15,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, PkmnResult, StringKey, ValueIdentifiable, ValueIdentifier};
+use crate::{script_hook, StringKey, ValueIdentifiable, ValueIdentifier};
 
 /// A side on a battle.
 #[derive(Debug)]
@@ -248,9 +249,9 @@ impl BattleSide {
     }
 
     /// Gets a random Pokemon on the given side.
-    pub fn get_random_creature_index(&self) -> u8 {
+    pub fn get_random_creature_index(&self) -> Result<u8> {
         // TODO: Consider adding parameter to only get index for available creatures.
-        self.battle().random().get_max(self.pokemon_per_side as i32) as u8
+        Ok(self.battle().random().get_max(self.pokemon_per_side as i32)? as u8)
     }
 
     /// Swap two Pokemon on a single side around.
@@ -299,7 +300,7 @@ impl VolatileScriptsOwner for BattleSide {
         &self.volatile_scripts
     }
 
-    fn load_volatile_script(&self, key: &StringKey) -> PkmnResult<Option<Arc<dyn Script>>> {
+    fn load_volatile_script(&self, key: &StringKey) -> Result<Option<Arc<dyn Script>>> {
         self.battle()
             .library()
             .load_script(self.into(), crate::ScriptCategory::Side, key)

src/dynamic_data/models/executing_move.rs

@@ -2,6 +2,8 @@ use std::ops::Deref;
 use std::sync::atomic::{AtomicBool, AtomicU32, AtomicU8, Ordering};
 use std::sync::Arc;
 
+use anyhow::Result;
+use anyhow_ext::bail;
 use atomig::Atomic;
 use parking_lot::RwLock;
 
@@ -11,7 +13,7 @@ use crate::dynamic_data::script_handling::{ScriptSource, ScriptSourceData, Scrip
 use crate::dynamic_data::ScriptContainer;
 use crate::dynamic_data::TargetList;
 use crate::static_data::{MoveData, TypeIdentifier};
-use crate::{PkmnResult, PokemonError, ValueIdentifiable, ValueIdentifier};
+use crate::{ValueIdentifiable, ValueIdentifier};
 
 /// A hit data is the data for a single hit, on a single target.
 #[derive(Default, Debug)]
@@ -164,7 +166,7 @@ impl ExecutingMove {
     }
 
     /// Gets a hit data for a target, with a specific index.
-    pub fn get_hit_data(&self, for_target: &Pokemon, hit: u8) -> PkmnResult<&HitData> {
+    pub fn get_hit_data(&self, for_target: &Pokemon, hit: u8) -> Result<&HitData> {
         for (index, target) in self.targets.iter().enumerate() {
             if let Some(target) = target {
                 if std::ptr::eq(target.deref().deref(), for_target.deref().deref()) {
@@ -173,7 +175,7 @@ impl ExecutingMove {
                 }
             }
         }
-        Err(PokemonError::InvalidTargetRequested)
+        bail!("Invalid target requested");
     }
 
     /// Checks whether a Pokemon is a target for this move.
@@ -187,7 +189,7 @@ impl ExecutingMove {
     }
 
     /// Gets the index of the hits in this move where the hits for a specific target start.
-    pub(crate) fn get_index_of_target(&self, for_target: &Arc<Pokemon>) -> PkmnResult<usize> {
+    pub(crate) fn get_index_of_target(&self, for_target: &Arc<Pokemon>) -> Result<usize> {
         for (index, target) in self.targets.iter().enumerate() {
             if let Some(target) = target {
                 if std::ptr::eq(target.deref().deref(), for_target.deref().deref()) {
@@ -196,7 +198,7 @@ impl ExecutingMove {
                 }
             }
         }
-        Err(PokemonError::InvalidTargetRequested)
+        bail!("Invalid target requested");
     }
 
     /// Gets a hit based on its raw index.

src/dynamic_data/models/pokemon.rs

@@ -22,7 +22,8 @@ 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, PkmnResult, StringKey, ValueIdentifiable, ValueIdentifier};
+use crate::{script_hook, StringKey, ValueIdentifiable, ValueIdentifier};
+use anyhow::Result;
 
 /// An individual Pokemon as we know and love them.
 #[derive(Debug)]
@@ -129,12 +130,12 @@ impl Pokemon {
         gender: Gender,
         coloring: u8,
         nature: &StringKey,
-    ) -> Self {
+    ) -> Result<Self> {
         // Calculate experience from the level for the specified growth rate.
         let experience = library
             .static_data()
             .growth_rates()
-            .calculate_experience(species.growth_rate(), level);
+            .calculate_experience(species.growth_rate(), level)?;
         let weight = form.weight();
         let height = form.height();
         let nature = library
@@ -183,7 +184,7 @@ impl Pokemon {
         let health = pokemon.flat_stats().hp();
         pokemon.current_health = AtomicU32::new(health);
 
-        pokemon
+        Ok(pokemon)
     }
 
     /// The library data of the Pokemon.
@@ -617,12 +618,12 @@ impl Pokemon {
     }
 
     /// Damages the Pokemon by a certain amount of damage, from a damage source.
-    pub fn damage(&self, mut damage: u32, source: DamageSource) {
+    pub fn damage(&self, mut damage: u32, source: DamageSource) -> Result<()> {
         if damage > self.current_health() {
             damage = self.current_health();
         }
         if damage == 0 {
-            return;
+            return Ok(());
         }
         let new_health = self.current_health() - damage;
         if let Some(battle_data) = &self.battle_data.read().deref() {
@@ -641,12 +642,13 @@ impl Pokemon {
 
         self.current_health.store(new_health, Ordering::SeqCst);
         if self.is_fainted() && damage > 0 {
-            self.on_faint(source);
+            self.on_faint(source)?;
         }
+        Ok(())
     }
 
     /// Triggers when the Pokemon faints.
-    fn on_faint(&self, source: DamageSource) {
+    fn on_faint(&self, source: DamageSource) -> Result<()> {
         let r = self.battle_data.read();
         if let Some(battle_data) = r.deref() {
             if let Some(battle) = battle_data.battle() {
@@ -659,9 +661,10 @@ impl Pokemon {
                         .mark_slot_as_unfillable(battle_data.index());
                 }
 
-                battle.validate_battle_state();
+                battle.validate_battle_state()?;
             }
         }
+        Ok(())
     }
 
     /// Heals the Pokemon by a specific amount. Unless allow_revive is set to true, this will not
@@ -803,7 +806,7 @@ impl VolatileScriptsOwner for Pokemon {
         &self.volatile
     }
 
-    fn load_volatile_script(&self, key: &StringKey) -> PkmnResult<Option<Arc<dyn Script>>> {
+    fn load_volatile_script(&self, key: &StringKey) -> Result<Option<Arc<dyn Script>>> {
         self.library.load_script(self.into(), ScriptCategory::Pokemon, key)
     }
 }
@@ -866,7 +869,9 @@ pub mod test {
         static_lib.expect_species().return_const(Box::new(species_lib));
 
         let mut growth_rate_lib = MockGrowthRateLibrary::new();
-        growth_rate_lib.expect_calculate_experience().return_const(1000u32);
+        growth_rate_lib
+            .expect_calculate_experience()
+            .returning(|_, _| Ok(1000u32));
 
         let mut nature_lib = MockNatureLibrary::new();
         nature_lib.expect_get_nature().returning(|_| {
@@ -907,7 +912,8 @@ pub mod test {
             Gender::Male,
             0,
             &"test_nature".into(),
-        );
+        )
+        .unwrap();
         assert_eq!(pokemon.species().name(), &"test_species".into());
         assert_eq!(pokemon.form().name(), &"default".into());
     }

src/dynamic_data/models/pokemon_builder.rs

@@ -6,6 +6,7 @@ use crate::dynamic_data::models::pokemon::Pokemon;
 use crate::dynamic_data::DynamicLibrary;
 use crate::static_data::{AbilityIndex, Gender};
 use crate::{Random, StringKey};
+use anyhow::Result;
 
 /// This allows for the easy chain building of a Pokemon.
 pub struct PokemonBuilder {
@@ -39,7 +40,7 @@ impl PokemonBuilder {
     }
 
     /// Finally turn the builder into an actual Pokemon.
-    pub fn build(self) -> Pokemon {
+    pub fn build(self) -> Result<Pokemon> {
         let mut random = if let Some(seed) = self.random_seed {
             Random::new(seed)
         } else {
@@ -61,11 +62,11 @@ impl PokemonBuilder {
             Gender::Male,
             0,
             &"hardy".into(),
-        );
+        )?;
         for learned_move in self.learned_moves {
             p.learn_move(&learned_move, MoveLearnMethod::Unknown);
         }
 
-        p
+        Ok(p)
     }
 }

src/dynamic_data/script_handling/script_set.rs

@@ -1,11 +1,12 @@
 use std::ops::Deref;
 use std::sync::Arc;
 
+use anyhow::Result;
 use indexmap::IndexMap;
 use parking_lot::RwLock;
 
 use crate::dynamic_data::script_handling::script::{Script, ScriptContainer};
-use crate::{PkmnResult, StringKey};
+use crate::StringKey;
 
 /// A collection of unique scripts.
 #[derive(Debug, Default)]
@@ -36,9 +37,9 @@ impl ScriptSet {
 
     /// Adds a script with a name to the set. If the script with that name already exists in this
     /// set, this makes that script stack instead. The return value here is that script.
-    pub fn stack_or_add<F>(&self, key: &StringKey, instantiation: &F) -> PkmnResult<Option<ScriptContainer>>
+    pub fn stack_or_add<F>(&self, key: &StringKey, instantiation: &F) -> Result<Option<ScriptContainer>>
     where
-        F: Fn() -> PkmnResult<Option<Arc<dyn Script>>>,
+        F: Fn() -> Result<Option<Arc<dyn Script>>>,
     {
         if let Some(lock) = self.scripts.read().get(key) {
             if let Some(existing) = lock.get() {

src/dynamic_data/script_handling/volatile_scripts_owner.rs

@@ -1,15 +1,16 @@
+use anyhow::Result;
 use std::sync::Arc;
 
 use crate::dynamic_data::script_handling::script::{Script, ScriptContainer};
 use crate::dynamic_data::script_handling::script_set::ScriptSet;
-use crate::{PkmnResult, StringKey};
+use crate::StringKey;
 
 /// This trait adds a bunch of helper functions to deal with volatile scripts on a struct.
 pub trait VolatileScriptsOwner {
     /// Return the [`ScriptSet`] that are our volatile scripts.
     fn volatile_scripts(&self) -> &Arc<ScriptSet>;
     /// Loads a volatile script by name.
-    fn load_volatile_script(&self, key: &StringKey) -> PkmnResult<Option<Arc<dyn Script>>>;
+    fn load_volatile_script(&self, key: &StringKey) -> Result<Option<Arc<dyn Script>>>;
 
     /// Check if a volatile script with given name exists.
     fn has_volatile_script(&self, key: &StringKey) -> bool {
@@ -22,13 +23,13 @@ pub trait VolatileScriptsOwner {
     }
 
     /// Adds a volatile script by name.
-    fn add_volatile_script(&self, key: &StringKey) -> PkmnResult<Option<ScriptContainer>> {
+    fn add_volatile_script(&self, key: &StringKey) -> Result<Option<ScriptContainer>> {
         self.volatile_scripts()
             .stack_or_add(key, &|| self.load_volatile_script(key))
     }
 
     /// Adds a volatile script by name.
-    fn add_volatile_script_with_script(&self, script: Arc<dyn Script>) -> PkmnResult<Option<ScriptContainer>> {
+    fn add_volatile_script_with_script(&self, script: Arc<dyn Script>) -> Result<Option<ScriptContainer>> {
         self.volatile_scripts()
             .stack_or_add(&script.name().clone(), &|| Ok(Some(script.clone())))
     }