We need to be able to remove and change scripts while they're currently being active.

src/dynamic_data/flow/turn_runner.rs

@@ -8,7 +8,7 @@ use crate::dynamic_data::models::pokemon::Pokemon;
 use crate::dynamic_data::script_handling::{ScriptSource, ScriptWrapper};
 use crate::static_data::{DataLibrary, MoveCategory};
 use crate::{run_scripts, script_hook, PkmnResult};
-use std::ops::{Deref, DerefMut};
+use std::ops::Deref;
 use std::sync::Arc;
 
 impl<'own, 'library> Battle<'own, 'library> {

src/dynamic_data/models/pokemon.rs

@@ -399,9 +399,11 @@ impl<'own, 'library> Pokemon<'own, 'library> {
             // Ensure the ability script gets initialized with the parameters for the ability.
             self.ability_script()
                 .get()
-                .as_mut()
                 .unwrap()
-                .as_mut()
+                .read()
+                .as_ref()
+                .unwrap()
+                .as_ref()
                 .on_initialize(self.active_ability().parameters())
         } else {
             self.ability_script.clear();

src/dynamic_data/script_handling/mod.rs

@@ -13,13 +13,15 @@ macro_rules! script_hook {
     ($hook_name: ident, $source: ident, $($parameters: expr),*) => {
         let mut aggregator = $source.get_script_iterator();
         while let Some(script) = aggregator.get_next() {
-            let lock = &mut script.get();
+            let script = script.get();
-            let script = lock.as_mut().unwrap();
+            if let Some(script) = script {
-            if script.is_suppressed() {
+                if let Some(script) = script.read().as_deref() {
-                continue;
+                    if !script.is_suppressed() {
-            }
                         script.$hook_name($($parameters),*);
                     }
+                }
+            }
+        }
     };
 }
 
@@ -28,7 +30,7 @@ macro_rules! script_hook_on_lock {
     ($hook_name: ident, $source: ident, $($parameters: expr),*) => {
         let mut aggregator = $source.read().get_script_iterator();
         while let Some(script) = aggregator.get_next() {
-            let lock = &mut script.get();
+            let lock = &mut script.get().read();
             let script = lock.as_mut().unwrap();
             if script.is_suppressed() {
                 continue;
@@ -45,7 +47,8 @@ macro_rules! run_scripts {
             match script {
                 ScriptWrapper::Script(s) => {
                     if let Some(s) = s.upgrade() {
-                        if let Some(s) = s.write().deref_mut() {
+                        let s = s.read();
+                        if let Some(s) = s.deref() {
                             if !s.is_suppressed() {
                                 s.$hook_name($($parameters),*);
                             }
@@ -55,8 +58,9 @@ macro_rules! run_scripts {
                 ScriptWrapper::Set(s) => {
                     if let Some(s) = s.upgrade() {
                         for s in s.read().get_underlying() {
-                            let mut s = s.1.get();
+                            if let Some(s) = s.1.get() {
-                            if let Some(s) = s.deref_mut() {
+                                let s = s.read();
+                                if let Some(s) = s.deref() {
                                     if !s.is_suppressed() {
                                         s.$hook_name($($parameters),*);
                                     }
@@ -66,6 +70,7 @@ macro_rules! run_scripts {
                     }
                 }
             }
+        }
     };
 }
 
@@ -198,23 +203,30 @@ mod tests {
     use crate::static_data::EffectParameter;
     use crate::StringKey;
     use std::any::Any;
+    use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 
     pub struct TestScript {
         name: StringKey,
-        test_count: usize,
+        is_marked_for_deletion: AtomicBool,
+        suppressed_count: AtomicUsize,
+        test_count: AtomicUsize,
     }
 
     impl TestScript {
         fn new() -> Self {
             Self {
                 name: StringKey::new("test"),
-                test_count: 0,
+                is_marked_for_deletion: Default::default(),
+                suppressed_count: AtomicUsize::new(0),
+                test_count: AtomicUsize::new(0),
             }
         }
         fn new_with_name(name: &str) -> Self {
             Self {
                 name: StringKey::new(name),
-                test_count: 0,
+                is_marked_for_deletion: Default::default(),
+                suppressed_count: AtomicUsize::new(0),
+                test_count: AtomicUsize::new(0),
             }
         }
     }
@@ -224,21 +236,29 @@ mod tests {
             &self.name
         }
 
-        fn get_suppressed_count(&self) -> usize {
+        fn get_marked_for_deletion(&self) -> &AtomicBool {
-            0
+            &self.is_marked_for_deletion
         }
 
-        fn add_suppression(&mut self) {}
+        fn get_suppressed_count(&self) -> &AtomicUsize {
+            &self.suppressed_count
+        }
 
-        fn remove_suppression(&mut self) {}
+        fn add_suppression(&self) {}
 
-        fn on_initialize(&mut self, _pars: &[EffectParameter]) {
+        fn remove_suppression(&self) {}
-            self.test_count += 1;
+
+        fn on_initialize(&self, _pars: &[EffectParameter]) {
+            self.test_count.fetch_add(1, Ordering::SeqCst);
         }
 
         fn as_any(&self) -> &dyn Any {
             self
         }
+
+        fn as_any_mut(&mut self) -> &mut dyn Any {
+            self
+        }
     }
 
     #[test]
@@ -247,10 +267,10 @@ mod tests {
         let scripts = vec![ScriptWrapper::from(&script)];
         let mut aggregator = ScriptAggregator::new(&scripts as *const Vec<ScriptWrapper>);
         while let Some(v) = aggregator.get_next() {
-            v.get().as_mut().unwrap().on_initialize(&[]);
+            v.get().unwrap().read().as_ref().unwrap().on_initialize(&[]);
         }
         let a = script.get_as::<TestScript>();
-        assert_eq!(a.test_count, 1);
+        assert_eq!(a.test_count.load(Ordering::Relaxed), 1);
     }
 
     #[test]
@@ -261,10 +281,10 @@ mod tests {
         for i in 1..11 {
             aggregator.reset();
             while let Some(v) = aggregator.get_next() {
-                v.get().as_mut().unwrap().on_initialize(&[]);
+                v.get().unwrap().read().as_ref().unwrap().on_initialize(&[]);
             }
             let a = script.get_as::<TestScript>();
-            assert_eq!(a.test_count, i);
+            assert_eq!(a.test_count.load(Ordering::Relaxed), i);
         }
     }
 
@@ -280,14 +300,14 @@ mod tests {
         ];
         let mut aggregator = ScriptAggregator::new(&scripts as *const Vec<ScriptWrapper>);
         while let Some(v) = aggregator.get_next() {
-            v.get().as_mut().unwrap().on_initialize(&[]);
+            v.get().unwrap().read().as_ref().unwrap().on_initialize(&[]);
         }
         let a = script1.get_as::<TestScript>();
-        assert_eq!(a.test_count, 1);
+        assert_eq!(a.test_count.load(Ordering::Relaxed), 1);
         let a = script2.get_as::<TestScript>();
-        assert_eq!(a.test_count, 1);
+        assert_eq!(a.test_count.load(Ordering::Relaxed), 1);
         let a = script3.get_as::<TestScript>();
-        assert_eq!(a.test_count, 1);
+        assert_eq!(a.test_count.load(Ordering::Relaxed), 1);
     }
 
     #[test]
@@ -304,14 +324,14 @@ mod tests {
         for i in 1..11 {
             aggregator.reset();
             while let Some(v) = aggregator.get_next() {
-                v.get().as_mut().unwrap().on_initialize(&[]);
+                v.get().unwrap().read().as_ref().unwrap().on_initialize(&[]);
             }
             let a = script1.get_as::<TestScript>();
-            assert_eq!(a.test_count, i);
+            assert_eq!(a.test_count.load(Ordering::Relaxed), i);
             let a = script2.get_as::<TestScript>();
-            assert_eq!(a.test_count, i);
+            assert_eq!(a.test_count.load(Ordering::Relaxed), i);
             let a = script3.get_as::<TestScript>();
-            assert_eq!(a.test_count, i);
+            assert_eq!(a.test_count.load(Ordering::Relaxed), i);
         }
     }
 
@@ -330,17 +350,17 @@ mod tests {
         for i in 1..11 {
             aggregator.reset();
             while let Some(v) = aggregator.get_next() {
-                v.get().as_mut().unwrap().on_initialize(&[]);
+                v.get().unwrap().read().as_ref().unwrap().on_initialize(&[]);
             }
             let set = set.read();
             let s = set.at(0).get_as::<TestScript>();
-            assert_eq!(s.test_count, i);
+            assert_eq!(s.test_count.load(Ordering::Relaxed), i);
             assert_eq!(s.name().str(), "test_a");
             let s = set.at(1).get_as::<TestScript>();
-            assert_eq!(s.test_count, i);
+            assert_eq!(s.test_count.load(Ordering::Relaxed), i);
             assert_eq!(s.name().str(), "test_b");
             let s = set.at(2).get_as::<TestScript>();
-            assert_eq!(s.test_count, i);
+            assert_eq!(s.test_count.load(Ordering::Relaxed), i);
             assert_eq!(s.name().str(), "test_c");
         }
     }
@@ -358,11 +378,29 @@ mod tests {
         let scripts = vec![ScriptWrapper::from(&set)];
         let mut aggregator = ScriptAggregator::new(&scripts as *const Vec<ScriptWrapper>);
         assert_eq!(
-            aggregator.get_next().unwrap().get().as_mut().unwrap().name().str(),
+            aggregator
+                .get_next()
+                .unwrap()
+                .get()
+                .unwrap()
+                .read()
+                .as_ref()
+                .unwrap()
+                .name()
+                .str(),
             "test_a"
         );
         assert_eq!(
-            aggregator.get_next().unwrap().get().as_mut().unwrap().name().str(),
+            aggregator
+                .get_next()
+                .unwrap()
+                .get()
+                .unwrap()
+                .read()
+                .as_ref()
+                .unwrap()
+                .name()
+                .str(),
             "test_b"
         );
 
@@ -386,7 +424,16 @@ mod tests {
         let scripts = vec![ScriptWrapper::from(&set)];
         let mut aggregator = ScriptAggregator::new(&scripts as *const Vec<ScriptWrapper>);
         assert_eq!(
-            aggregator.get_next().unwrap().get().as_mut().unwrap().name().str(),
+            aggregator
+                .get_next()
+                .unwrap()
+                .get()
+                .unwrap()
+                .read()
+                .as_ref()
+                .unwrap()
+                .name()
+                .str(),
             "test_a"
         );
 
@@ -395,7 +442,16 @@ mod tests {
         drop(mut_set);
 
         assert_eq!(
-            aggregator.get_next().unwrap().get().as_mut().unwrap().name().str(),
+            aggregator
+                .get_next()
+                .unwrap()
+                .get()
+                .unwrap()
+                .read()
+                .as_ref()
+                .unwrap()
+                .name()
+                .str(),
             "test_c"
         );
         assert!(aggregator.get_next().is_none());

src/dynamic_data/script_handling/script.rs

@@ -6,123 +6,96 @@ use crate::dynamic_data::models::pokemon::Pokemon;
 use crate::static_data::moves::secondary_effect::EffectParameter;
 use crate::static_data::{Item, Statistic};
 use crate::StringKey;
-use parking_lot::{MappedRwLockReadGuard, RwLock, RwLockReadGuard, RwLockWriteGuard};
+use parking_lot::{MappedRwLockReadGuard, RwLock, RwLockReadGuard};
 use std::any::Any;
 use std::fmt::{Debug, Formatter};
+use std::ops::Deref;
+use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 use std::sync::Arc;
 
-pub trait Script {
+pub trait Script: Send + Sync {
     fn name(&self) -> &StringKey;
+    fn get_marked_for_deletion(&self) -> &AtomicBool;
 
+    fn mark_for_deletion(&self) {
+        self.get_marked_for_deletion().store(true, Ordering::SeqCst);
+    }
+    fn is_marked_for_deletion(&self) -> bool {
+        self.get_marked_for_deletion().load(Ordering::SeqCst)
+    }
+
+    fn get_suppressed_count(&self) -> &AtomicUsize;
     fn is_suppressed(&self) -> bool {
-        self.get_suppressed_count() > 0
+        self.get_suppressed_count().load(Ordering::SeqCst) > 0
+    }
+    fn add_suppression(&self) {
+        self.get_suppressed_count().fetch_add(1, Ordering::SeqCst);
+    }
+    fn remove_suppression(&self) {
+        self.get_suppressed_count().fetch_sub(1, Ordering::SeqCst);
     }
-    fn get_suppressed_count(&self) -> usize;
-    fn add_suppression(&mut self);
-    fn remove_suppression(&mut self);
 
-    fn stack(&mut self) {}
+    fn stack(&self) {}
-    fn on_remove(&mut self) {}
+    fn on_remove(&self) {}
-    fn on_initialize(&mut self, _pars: &[EffectParameter]) {}
+    fn on_initialize(&self, _pars: &[EffectParameter]) {}
-    fn on_before_turn(&mut self, _choice: &TurnChoice) {}
+    fn on_before_turn(&self, _choice: &TurnChoice) {}
-    fn change_speed(&mut self, _choice: &TurnChoice, _speed: &mut u32) {}
+    fn change_speed(&self, _choice: &TurnChoice, _speed: &mut u32) {}
-    fn change_priority(&mut self, _choice: &TurnChoice, _priority: &mut i8) {}
+    fn change_priority(&self, _choice: &TurnChoice, _priority: &mut i8) {}
-    fn change_move(&mut self, _choice: &MoveChoice, _move_name: &mut StringKey) {}
+    fn change_move(&self, _choice: &MoveChoice, _move_name: &mut StringKey) {}
-    fn change_number_of_hits(&mut self, _choice: &MoveChoice, _number_of_hits: &mut u8) {}
+    fn change_number_of_hits(&self, _choice: &MoveChoice, _number_of_hits: &mut u8) {}
-    fn prevent_move(&mut self, _move: &ExecutingMove, _prevent: &mut bool) {}
+    fn prevent_move(&self, _move: &ExecutingMove, _prevent: &mut bool) {}
-    fn fail_move(&mut self, _move: &ExecutingMove, _fail: &mut bool) {}
+    fn fail_move(&self, _move: &ExecutingMove, _fail: &mut bool) {}
-    fn stop_before_move(&mut self, _move: &ExecutingMove, _stop: &mut bool) {}
+    fn stop_before_move(&self, _move: &ExecutingMove, _stop: &mut bool) {}
-    fn on_before_move(&mut self, _move: &ExecutingMove) {}
+    fn on_before_move(&self, _move: &ExecutingMove) {}
-    fn fail_incoming_move(&mut self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _fail: &mut bool) {}
+    fn fail_incoming_move(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _fail: &mut bool) {}
-    fn is_invulnerable(&mut self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _invulnerable: &mut bool) {}
+    fn is_invulnerable(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _invulnerable: &mut bool) {}
-    fn on_move_miss(&mut self, _move: &ExecutingMove, _target: &Arc<Pokemon>) {}
+    fn on_move_miss(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>) {}
-    fn change_move_type(&mut self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _move_type: &mut u8) {}
+    fn change_move_type(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _move_type: &mut u8) {}
-    fn change_effectiveness(
+    fn change_effectiveness(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _effectiveness: &mut f32) {}
-        &mut self,
+    fn block_critical(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _block_critical: &mut bool) {}
-        _move: &ExecutingMove,
-        _target: &Arc<Pokemon>,
-        _hit: u8,
-        _effectiveness: &mut f32,
-    ) {
-    }
-    fn block_critical(&mut self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _block_critical: &mut bool) {}
     fn block_incoming_critical(
-        &mut self,
+        &self,
         _move: &ExecutingMove,
         _target: &Arc<Pokemon>,
         _hit: u8,
         _block_critical: &mut bool,
     ) {
     }
-    fn change_critical_stage(&mut self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _stage: &mut u8) {}
+    fn change_critical_stage(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _stage: &mut u8) {}
-    fn change_critical_modifier(
+    fn change_critical_modifier(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _modifier: &mut f32) {}
-        &mut self,
+    fn change_stab_modifier(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _modifier: &mut f32) {}
-        _move: &ExecutingMove,
-        _target: &Arc<Pokemon>,
-        _hit: u8,
-        _modifier: &mut f32,
-    ) {
-    }
-    fn change_stab_modifier(&mut self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _modifier: &mut f32) {}
 
-    fn change_base_power(&mut self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _base_power: &mut u8) {}
+    fn change_base_power(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _base_power: &mut u8) {}
     fn change_damage_stats_user(
-        &mut self,
+        &self,
         _move: &ExecutingMove,
         _target: &Arc<Pokemon>,
         _hit: u8,
         _stats_user: &mut Arc<Pokemon>,
     ) {
     }
-    fn bypass_defensive_stat_boost(
+    fn bypass_defensive_stat_boost(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _bypass: &mut bool) {
-        &mut self,
-        _move: &ExecutingMove,
-        _target: &Arc<Pokemon>,
-        _hit: u8,
-        _bypass: &mut bool,
-    ) {
     }
-    fn bypass_offensive_stat_boost(
+    fn bypass_offensive_stat_boost(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _bypass: &mut bool) {
-        &mut self,
-        _move: &ExecutingMove,
-        _target: &Arc<Pokemon>,
-        _hit: u8,
-        _bypass: &mut bool,
-    ) {
-    }
-    fn change_offensive_stat_value(
-        &mut self,
-        _move: &ExecutingMove,
-        _target: &Arc<Pokemon>,
-        _hit: u8,
-        _amount: &mut u32,
-    ) {
-    }
-    fn change_defensive_stat_value(
-        &mut self,
-        _move: &ExecutingMove,
-        _target: &Arc<Pokemon>,
-        _hit: u8,
-        _amount: &mut u32,
-    ) {
     }
+    fn change_offensive_stat_value(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _amount: &mut u32) {}
+    fn change_defensive_stat_value(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _amount: &mut u32) {}
 
     fn change_damage_stat_modifier(
-        &mut self,
+        &self,
         _move: &ExecutingMove,
         _target: &Arc<Pokemon>,
         _hit: u8,
         _modifier: &mut f32,
     ) {
     }
-    fn change_damage_modifier(&mut self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _modifier: &mut f32) {
+    fn change_damage_modifier(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _modifier: &mut f32) {}
-    }
+    fn change_damage(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _damage: &mut u32) {}
-    fn change_damage(&mut self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _damage: &mut u32) {}
+    fn change_incoming_damage(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _damage: &mut u32) {}
-    fn change_incoming_damage(&mut self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _damage: &mut u32) {}
+    fn on_incoming_hit(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8) {}
-    fn on_incoming_hit(&mut self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8) {}
+    fn on_opponent_faints(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8) {}
-    fn on_opponent_faints(&mut self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8) {}
     fn prevent_stat_boost_change(
-        &mut self,
+        &self,
         _target: &Pokemon,
         _stat: Statistic,
         _amount: i8,
@@ -130,50 +103,37 @@ pub trait Script {
         _prevent: &mut bool,
     ) {
     }
-    fn change_stat_boost_change(
+    fn change_stat_boost_change(&self, _target: &Pokemon, _stat: Statistic, _self_inflicted: bool, _amount: *mut i8) {}
-        &mut self,
+    fn prevent_secondary_effect(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _prevent: &mut bool) {}
-        _target: &Pokemon,
+    fn change_effect_chance(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _chance: &mut f32) {}
-        _stat: Statistic,
-        _self_inflicted: bool,
-        _amount: *mut i8,
-    ) {
-    }
-    fn prevent_secondary_effect(
-        &mut self,
-        _move: &ExecutingMove,
-        _target: &Arc<Pokemon>,
-        _hit: u8,
-        _prevent: &mut bool,
-    ) {
-    }
-    fn change_effect_chance(&mut self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8, _chance: &mut f32) {}
     fn change_incoming_effect_chance(
-        &mut self,
+        &self,
         _move: &ExecutingMove,
         _target: &Arc<Pokemon>,
         _hit: u8,
         _chance: &mut f32,
     ) {
     }
-    fn on_secondary_effect(&mut self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8) {}
+    fn on_secondary_effect(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>, _hit: u8) {}
-    fn on_after_hits(&mut self, _move: &ExecutingMove, _target: &Arc<Pokemon>) {}
+    fn on_after_hits(&self, _move: &ExecutingMove, _target: &Arc<Pokemon>) {}
-    fn prevent_self_switch(&mut self, _choice: &TurnChoice, _prevent: &mut bool) {}
+    fn prevent_self_switch(&self, _choice: &TurnChoice, _prevent: &mut bool) {}
-    fn prevent_opponent_switch(&mut self, _choice: &TurnChoice, _prevent: &mut bool) {}
+    fn prevent_opponent_switch(&self, _choice: &TurnChoice, _prevent: &mut bool) {}
-    fn on_fail(&mut self, _target: &Pokemon) {}
+    fn on_fail(&self, _target: &Pokemon) {}
-    fn on_opponent_fail(&mut self, _target: &Pokemon) {}
+    fn on_opponent_fail(&self, _target: &Pokemon) {}
-    fn prevent_self_run_away(&mut self, _choice: &TurnChoice, _prevent: &mut bool) {}
+    fn prevent_self_run_away(&self, _choice: &TurnChoice, _prevent: &mut bool) {}
-    fn prevent_opponent_run_away(&mut self, _choice: &TurnChoice, _prevent: &mut bool) {}
+    fn prevent_opponent_run_away(&self, _choice: &TurnChoice, _prevent: &mut bool) {}
-    fn on_end_turn(&mut self) {}
+    fn on_end_turn(&self) {}
-    fn on_damage(&mut self, _pokemon: &Pokemon, _source: DamageSource, _old_health: u32, _new_health: u32) {}
+    fn on_damage(&self, _pokemon: &Pokemon, _source: DamageSource, _old_health: u32, _new_health: u32) {}
-    fn on_faint(&mut self, _pokemon: &Pokemon, _source: DamageSource) {}
+    fn on_faint(&self, _pokemon: &Pokemon, _source: DamageSource) {}
-    fn on_switch_in(&mut self, _pokemon: &Pokemon) {}
+    fn on_switch_in(&self, _pokemon: &Pokemon) {}
-    fn on_after_held_item_consume(&mut self, _pokemon: &Pokemon, _item: &Item) {}
+    fn on_after_held_item_consume(&self, _pokemon: &Pokemon, _item: &Item) {}
-    fn change_experience_gained(&mut self, _fainted_mon: &Pokemon, _winning_mon: &Pokemon, _amount: &mut u32) {}
+    fn change_experience_gained(&self, _fainted_mon: &Pokemon, _winning_mon: &Pokemon, _amount: &mut u32) {}
-    fn share_experience(&mut self, _fainted_mon: &Pokemon, _winning_mon: &Pokemon, _shares: &mut bool) {}
+    fn share_experience(&self, _fainted_mon: &Pokemon, _winning_mon: &Pokemon, _shares: &mut bool) {}
-    fn block_weather(&mut self, _battle: &Battle, _blocked: &mut bool) {}
+    fn block_weather(&self, _battle: &Battle, _blocked: &mut bool) {}
-    fn change_capture_rate_bonus(&mut self, _target: &Pokemon, _pokeball: &Item, _modifier: &mut u8) {}
+    fn change_capture_rate_bonus(&self, _target: &Pokemon, _pokeball: &Item, _modifier: &mut u8) {}
 
     fn as_any(&self) -> &dyn Any;
+    fn as_any_mut(&mut self) -> &mut dyn Any;
 }
 
 impl Debug for dyn Script {
@@ -196,17 +156,42 @@ impl ScriptContainer {
         }
     }
 
-    pub fn get(&self) -> RwLockWriteGuard<'_, Option<Box<dyn Script>>> {
+    pub fn get(&self) -> Option<&ScriptHolder> {
-        self.script.write()
+        if self.script.read().is_some_and(|a| a.is_marked_for_deletion()) {
+            if !self.script.is_locked() {
+                self.script.write().take();
+            }
+            None
+        } else {
+            Some(&self.script)
+        }
     }
 
     pub fn set(&self, script: Box<dyn Script>) {
+        if let Some(v) = self.script.read().deref() {
+            v.mark_for_deletion();
+        }
+        if self.script.is_locked() {
+            // This is unfortunate, but when we're replacing this script from itself, we can not delay
+            // it, and as the script is locked, we run into a deadlock here. As such, we force update
+            // it, bypassing the RwLock entirely. A better solution might be possible, but I currently
+            // cannot think of one.
+            unsafe {
+                self.script.data_ptr().as_mut().unwrap().replace(script);
+            }
+        } else {
             self.script.write().replace(script);
         }
+    }
 
     pub fn clear(&self) {
+        if let Some(v) = self.script.read().deref() {
+            v.mark_for_deletion();
+        }
+        if !self.script.is_locked() {
             self.script.write().take();
         }
+    }
 
     pub fn arc(&self) -> &ScriptHolder {
         &self.script
@@ -226,3 +211,154 @@ impl From<ScriptHolder> for ScriptContainer {
         Self { script: a }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::sync::atomic::AtomicBool;
+
+    pub struct TestScript {
+        name: StringKey,
+        container: *mut ScriptContainer,
+        suppressed_count: AtomicUsize,
+        marked_for_deletion: AtomicBool,
+    }
+
+    impl TestScript {
+        fn new() -> Self {
+            Self {
+                name: StringKey::new("test"),
+                container: std::ptr::null_mut::<ScriptContainer>(),
+                suppressed_count: AtomicUsize::new(0),
+                marked_for_deletion: Default::default(),
+            }
+        }
+    }
+
+    unsafe impl Sync for TestScript {}
+
+    unsafe impl Send for TestScript {}
+
+    impl Script for TestScript {
+        fn name(&self) -> &StringKey {
+            &self.name
+        }
+
+        fn get_marked_for_deletion(&self) -> &AtomicBool {
+            &self.marked_for_deletion
+        }
+
+        fn get_suppressed_count(&self) -> &AtomicUsize {
+            &self.suppressed_count
+        }
+
+        fn on_initialize(&self, _pars: &[EffectParameter]) {
+            unsafe { self.container.as_mut().unwrap().clear() }
+        }
+
+        fn as_any(&self) -> &dyn Any {
+            self
+        }
+        fn as_any_mut(&mut self) -> &mut dyn Any {
+            self
+        }
+    }
+
+    // Removing yourself while active should be completely valid for a script. Consider for example
+    // a status effect such as sleep clearing itself in a script hook. As the script is actively
+    // being read from at that time, we should wait until the script hook is finished reading, and
+    // then dispose of the script.
+    #[test]
+    fn clear_self_while_active() {
+        let script = Box::new(TestScript::new());
+        let container = ScriptContainer::new(script);
+        let mut w = container.script.write();
+        let script: &mut TestScript = w.as_mut().unwrap().as_any_mut().downcast_mut().unwrap();
+        script.container = &container as *const ScriptContainer as *mut ScriptContainer;
+        drop(w);
+        // Initialize with the script being taken as read lock. This prevents the script from actually
+        // removing itself, as it's still doing things.
+        container.script.read().as_ref().unwrap().on_initialize(&[]);
+        // If we now try and get the script, it will be none the first time. This has the side effect
+        // of actually disposing of the script.
+        assert!(container.get().is_none());
+        // As we've properly disposed of the script now, the next fetch will return something.
+        assert!(container.get().is_some());
+        // But the value it returns shows that it is empty.
+        assert!(container.get().unwrap().read().is_none());
+    }
+
+    pub struct ReplaceTestScript {
+        name: StringKey,
+        container: *mut ScriptContainer,
+        suppressed_count: AtomicUsize,
+        marked_for_deletion: AtomicBool,
+    }
+
+    impl ReplaceTestScript {
+        fn new(name: StringKey) -> Self {
+            Self {
+                name,
+                container: std::ptr::null_mut::<ScriptContainer>(),
+                suppressed_count: AtomicUsize::new(0),
+                marked_for_deletion: Default::default(),
+            }
+        }
+
+        fn test(&self, script: Box<dyn Script>) {
+            unsafe {
+                self.container.as_mut().unwrap().set(script);
+            }
+        }
+    }
+
+    unsafe impl Sync for ReplaceTestScript {}
+
+    unsafe impl Send for ReplaceTestScript {}
+
+    impl Script for ReplaceTestScript {
+        fn name(&self) -> &StringKey {
+            &self.name
+        }
+
+        fn get_marked_for_deletion(&self) -> &AtomicBool {
+            &self.marked_for_deletion
+        }
+
+        fn get_suppressed_count(&self) -> &AtomicUsize {
+            &self.suppressed_count
+        }
+
+        fn as_any(&self) -> &dyn Any {
+            self
+        }
+        fn as_any_mut(&mut self) -> &mut dyn Any {
+            self
+        }
+    }
+
+    #[test]
+    fn replace_self_while_active() {
+        let script = Box::new(ReplaceTestScript::new("script1".into()));
+        let container = ScriptContainer::new(script);
+        let mut w = container.script.write();
+        let script: &mut ReplaceTestScript = w.as_mut().unwrap().as_any_mut().downcast_mut().unwrap();
+        script.container = &container as *const ScriptContainer as *mut ScriptContainer;
+        drop(w);
+
+        let script2 = Box::new(ReplaceTestScript::new("script2".into()));
+        container
+            .script
+            .read()
+            .as_ref()
+            .unwrap()
+            .as_any()
+            .downcast_ref::<ReplaceTestScript>()
+            .unwrap()
+            .test(script2);
+        assert_eq!(
+            container.script.read().as_ref().unwrap().name(),
+            &StringKey::new("script2")
+        );
+    }
+}

src/dynamic_data/script_handling/script_set.rs

@@ -10,12 +10,14 @@ pub struct ScriptSet {
 impl ScriptSet {
     pub fn add(&mut self, script: Box<dyn Script>) -> ScriptContainer {
         if let Some(lock) = self.scripts.get(script.name()) {
-            let mut existing = lock.get();
+            if let Some(existing) = lock.get() {
-            if let Some(v) = &mut *existing {
+                let existing = existing.read();
+                if let Some(v) = &*existing {
                     v.stack();
                     return lock.clone();
                 }
             }
+        }
         self.scripts.insert(script.name().clone(), ScriptContainer::new(script));
         self.scripts.last().unwrap().1.clone()
     }
@@ -25,12 +27,14 @@ impl ScriptSet {
         F: Fn() -> PkmnResult<Option<Box<dyn Script>>>,
     {
         if let Some(lock) = self.scripts.get(key) {
-            let mut existing = lock.get();
+            if let Some(existing) = lock.get() {
-            if let Some(v) = &mut *existing {
+                let existing = existing.read();
+                if let Some(v) = &*existing {
                     v.stack();
                     return Ok(Some(lock.clone()));
                 }
             }
+        }
         let script = instantiation()?;
         if let Some(script) = script {
             let name = script.name().clone();
@@ -49,13 +53,21 @@ impl ScriptSet {
     pub fn remove(&mut self, key: &StringKey) {
         let value = self.scripts.shift_remove(key);
         if let Some(script) = value {
-            script.get().as_mut().unwrap().on_remove();
+            if let Some(script) = script.get() {
+                let script = script.read();
+                script.as_ref().unwrap().on_remove();
+                script.as_ref().unwrap().mark_for_deletion();
+            }
         }
     }
 
     pub fn clear(&mut self) {
         for script in &self.scripts {
-            script.1.get().as_mut().unwrap().on_remove();
+            if let Some(script) = script.1.get() {
+                let script = script.read();
+                script.as_ref().unwrap().on_remove();
+                script.as_ref().unwrap().mark_for_deletion();
+            }
         }
         self.scripts.clear();
     }