From a1051d059eae32df0d7d6c5b1b4baa1561969de9 Mon Sep 17 00:00:00 2001 From: Deukhoofd Date: Sat, 18 Jun 2022 18:08:25 +0200 Subject: [PATCH] We need to be able to remove and change scripts while they're currently being active. --- src/dynamic_data/flow/turn_runner.rs | 2 +- src/dynamic_data/models/pokemon.rs | 6 +- src/dynamic_data/script_handling/mod.rs | 136 +++++-- src/dynamic_data/script_handling/script.rs | 378 ++++++++++++------ .../script_handling/script_set.rs | 32 +- 5 files changed, 380 insertions(+), 174 deletions(-) diff --git a/src/dynamic_data/flow/turn_runner.rs b/src/dynamic_data/flow/turn_runner.rs index 7b807a3..a18d337 100644 --- a/src/dynamic_data/flow/turn_runner.rs +++ b/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> { diff --git a/src/dynamic_data/models/pokemon.rs b/src/dynamic_data/models/pokemon.rs index 3c6daa8..f245b13 100644 --- a/src/dynamic_data/models/pokemon.rs +++ b/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(); diff --git a/src/dynamic_data/script_handling/mod.rs b/src/dynamic_data/script_handling/mod.rs index 4b2af2d..157a237 100644 --- a/src/dynamic_data/script_handling/mod.rs +++ b/src/dynamic_data/script_handling/mod.rs @@ -13,12 +13,14 @@ 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 = lock.as_mut().unwrap(); - if script.is_suppressed() { - continue; + let script = script.get(); + if let Some(script) = script { + if let Some(script) = script.read().as_deref() { + if !script.is_suppressed() { + script.$hook_name($($parameters),*); + } + } } - 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,10 +58,12 @@ 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.deref_mut() { - if !s.is_suppressed() { - s.$hook_name($($parameters),*); + if let Some(s) = s.1.get() { + let s = s.read(); + if let Some(s) = s.deref() { + if !s.is_suppressed() { + s.$hook_name($($parameters),*); + } } } } @@ -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 { - 0 + fn get_marked_for_deletion(&self) -> &AtomicBool { + &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]) { - self.test_count += 1; + fn remove_suppression(&self) {} + + 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); 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::(); - 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::(); - 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); 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::(); - assert_eq!(a.test_count, 1); + assert_eq!(a.test_count.load(Ordering::Relaxed), 1); let a = script2.get_as::(); - assert_eq!(a.test_count, 1); + assert_eq!(a.test_count.load(Ordering::Relaxed), 1); let a = script3.get_as::(); - 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::(); - assert_eq!(a.test_count, i); + assert_eq!(a.test_count.load(Ordering::Relaxed), i); let a = script2.get_as::(); - assert_eq!(a.test_count, i); + assert_eq!(a.test_count.load(Ordering::Relaxed), i); let a = script3.get_as::(); - 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::(); - 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::(); - 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::(); - 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); 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); 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()); diff --git a/src/dynamic_data/script_handling/script.rs b/src/dynamic_data/script_handling/script.rs index 0a17670..6b8ba17 100644 --- a/src/dynamic_data/script_handling/script.rs +++ b/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 on_remove(&mut self) {} - fn on_initialize(&mut self, _pars: &[EffectParameter]) {} - fn on_before_turn(&mut self, _choice: &TurnChoice) {} - fn change_speed(&mut self, _choice: &TurnChoice, _speed: &mut u32) {} - fn change_priority(&mut self, _choice: &TurnChoice, _priority: &mut i8) {} - fn change_move(&mut self, _choice: &MoveChoice, _move_name: &mut StringKey) {} - fn change_number_of_hits(&mut self, _choice: &MoveChoice, _number_of_hits: &mut u8) {} - fn prevent_move(&mut self, _move: &ExecutingMove, _prevent: &mut bool) {} - fn fail_move(&mut self, _move: &ExecutingMove, _fail: &mut bool) {} - fn stop_before_move(&mut self, _move: &ExecutingMove, _stop: &mut bool) {} - fn on_before_move(&mut self, _move: &ExecutingMove) {} - fn fail_incoming_move(&mut self, _move: &ExecutingMove, _target: &Arc, _fail: &mut bool) {} - fn is_invulnerable(&mut self, _move: &ExecutingMove, _target: &Arc, _invulnerable: &mut bool) {} - fn on_move_miss(&mut self, _move: &ExecutingMove, _target: &Arc) {} - fn change_move_type(&mut self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _move_type: &mut u8) {} - fn change_effectiveness( - &mut self, - _move: &ExecutingMove, - _target: &Arc, - _hit: u8, - _effectiveness: &mut f32, - ) { - } - fn block_critical(&mut self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _block_critical: &mut bool) {} + fn stack(&self) {} + fn on_remove(&self) {} + fn on_initialize(&self, _pars: &[EffectParameter]) {} + fn on_before_turn(&self, _choice: &TurnChoice) {} + fn change_speed(&self, _choice: &TurnChoice, _speed: &mut u32) {} + fn change_priority(&self, _choice: &TurnChoice, _priority: &mut i8) {} + fn change_move(&self, _choice: &MoveChoice, _move_name: &mut StringKey) {} + fn change_number_of_hits(&self, _choice: &MoveChoice, _number_of_hits: &mut u8) {} + fn prevent_move(&self, _move: &ExecutingMove, _prevent: &mut bool) {} + fn fail_move(&self, _move: &ExecutingMove, _fail: &mut bool) {} + fn stop_before_move(&self, _move: &ExecutingMove, _stop: &mut bool) {} + fn on_before_move(&self, _move: &ExecutingMove) {} + fn fail_incoming_move(&self, _move: &ExecutingMove, _target: &Arc, _fail: &mut bool) {} + fn is_invulnerable(&self, _move: &ExecutingMove, _target: &Arc, _invulnerable: &mut bool) {} + fn on_move_miss(&self, _move: &ExecutingMove, _target: &Arc) {} + fn change_move_type(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _move_type: &mut u8) {} + fn change_effectiveness(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _effectiveness: &mut f32) {} + fn block_critical(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _block_critical: &mut bool) {} fn block_incoming_critical( - &mut self, + &self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _block_critical: &mut bool, ) { } - fn change_critical_stage(&mut self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _stage: &mut u8) {} - fn change_critical_modifier( - &mut self, - _move: &ExecutingMove, - _target: &Arc, - _hit: u8, - _modifier: &mut f32, - ) { - } - fn change_stab_modifier(&mut self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _modifier: &mut f32) {} + fn change_critical_stage(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _stage: &mut u8) {} + fn change_critical_modifier(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _modifier: &mut f32) {} + fn change_stab_modifier(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _modifier: &mut f32) {} - fn change_base_power(&mut self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _base_power: &mut u8) {} + fn change_base_power(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _base_power: &mut u8) {} fn change_damage_stats_user( - &mut self, + &self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _stats_user: &mut Arc, ) { } - fn bypass_defensive_stat_boost( - &mut self, - _move: &ExecutingMove, - _target: &Arc, - _hit: u8, - _bypass: &mut bool, - ) { + fn bypass_defensive_stat_boost(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _bypass: &mut bool) { } - fn bypass_offensive_stat_boost( - &mut self, - _move: &ExecutingMove, - _target: &Arc, - _hit: u8, - _bypass: &mut bool, - ) { - } - fn change_offensive_stat_value( - &mut self, - _move: &ExecutingMove, - _target: &Arc, - _hit: u8, - _amount: &mut u32, - ) { - } - fn change_defensive_stat_value( - &mut self, - _move: &ExecutingMove, - _target: &Arc, - _hit: u8, - _amount: &mut u32, - ) { + fn bypass_offensive_stat_boost(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _bypass: &mut bool) { } + fn change_offensive_stat_value(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _amount: &mut u32) {} + fn change_defensive_stat_value(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _amount: &mut u32) {} fn change_damage_stat_modifier( - &mut self, + &self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _modifier: &mut f32, ) { } - fn change_damage_modifier(&mut self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _modifier: &mut f32) { - } - fn change_damage(&mut self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _damage: &mut u32) {} - fn change_incoming_damage(&mut self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _damage: &mut u32) {} - fn on_incoming_hit(&mut self, _move: &ExecutingMove, _target: &Arc, _hit: u8) {} - fn on_opponent_faints(&mut self, _move: &ExecutingMove, _target: &Arc, _hit: u8) {} + fn change_damage_modifier(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _modifier: &mut f32) {} + fn change_damage(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _damage: &mut u32) {} + fn change_incoming_damage(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _damage: &mut u32) {} + fn on_incoming_hit(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8) {} + fn on_opponent_faints(&self, _move: &ExecutingMove, _target: &Arc, _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( - &mut self, - _target: &Pokemon, - _stat: Statistic, - _self_inflicted: bool, - _amount: *mut i8, - ) { - } - fn prevent_secondary_effect( - &mut self, - _move: &ExecutingMove, - _target: &Arc, - _hit: u8, - _prevent: &mut bool, - ) { - } - fn change_effect_chance(&mut self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _chance: &mut f32) {} + fn change_stat_boost_change(&self, _target: &Pokemon, _stat: Statistic, _self_inflicted: bool, _amount: *mut i8) {} + fn prevent_secondary_effect(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _prevent: &mut bool) {} + fn change_effect_chance(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _chance: &mut f32) {} fn change_incoming_effect_chance( - &mut self, + &self, _move: &ExecutingMove, _target: &Arc, _hit: u8, _chance: &mut f32, ) { } - fn on_secondary_effect(&mut self, _move: &ExecutingMove, _target: &Arc, _hit: u8) {} - fn on_after_hits(&mut self, _move: &ExecutingMove, _target: &Arc) {} - fn prevent_self_switch(&mut self, _choice: &TurnChoice, _prevent: &mut bool) {} - fn prevent_opponent_switch(&mut self, _choice: &TurnChoice, _prevent: &mut bool) {} - fn on_fail(&mut self, _target: &Pokemon) {} - fn on_opponent_fail(&mut self, _target: &Pokemon) {} - fn prevent_self_run_away(&mut self, _choice: &TurnChoice, _prevent: &mut bool) {} - fn prevent_opponent_run_away(&mut self, _choice: &TurnChoice, _prevent: &mut bool) {} - fn on_end_turn(&mut self) {} - fn on_damage(&mut self, _pokemon: &Pokemon, _source: DamageSource, _old_health: u32, _new_health: u32) {} - fn on_faint(&mut self, _pokemon: &Pokemon, _source: DamageSource) {} - fn on_switch_in(&mut self, _pokemon: &Pokemon) {} - fn on_after_held_item_consume(&mut self, _pokemon: &Pokemon, _item: &Item) {} - fn change_experience_gained(&mut 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 block_weather(&mut self, _battle: &Battle, _blocked: &mut bool) {} - fn change_capture_rate_bonus(&mut self, _target: &Pokemon, _pokeball: &Item, _modifier: &mut u8) {} + fn on_secondary_effect(&self, _move: &ExecutingMove, _target: &Arc, _hit: u8) {} + fn on_after_hits(&self, _move: &ExecutingMove, _target: &Arc) {} + fn prevent_self_switch(&self, _choice: &TurnChoice, _prevent: &mut bool) {} + fn prevent_opponent_switch(&self, _choice: &TurnChoice, _prevent: &mut bool) {} + fn on_fail(&self, _target: &Pokemon) {} + fn on_opponent_fail(&self, _target: &Pokemon) {} + fn prevent_self_run_away(&self, _choice: &TurnChoice, _prevent: &mut bool) {} + fn prevent_opponent_run_away(&self, _choice: &TurnChoice, _prevent: &mut bool) {} + fn on_end_turn(&self) {} + fn on_damage(&self, _pokemon: &Pokemon, _source: DamageSource, _old_health: u32, _new_health: u32) {} + fn on_faint(&self, _pokemon: &Pokemon, _source: DamageSource) {} + fn on_switch_in(&self, _pokemon: &Pokemon) {} + fn on_after_held_item_consume(&self, _pokemon: &Pokemon, _item: &Item) {} + fn change_experience_gained(&self, _fainted_mon: &Pokemon, _winning_mon: &Pokemon, _amount: &mut u32) {} + fn share_experience(&self, _fainted_mon: &Pokemon, _winning_mon: &Pokemon, _shares: &mut bool) {} + fn block_weather(&self, _battle: &Battle, _blocked: &mut bool) {} + 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,16 +156,41 @@ impl ScriptContainer { } } - pub fn get(&self) -> RwLockWriteGuard<'_, Option>> { - self.script.write() + pub fn get(&self) -> Option<&ScriptHolder> { + 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) { - self.script.write().replace(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) { - self.script.write().take(); + 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 { @@ -226,3 +211,154 @@ impl From 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::(), + 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::(), + suppressed_count: AtomicUsize::new(0), + marked_for_deletion: Default::default(), + } + } + + fn test(&self, script: Box) { + 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::() + .unwrap() + .test(script2); + assert_eq!( + container.script.read().as_ref().unwrap().name(), + &StringKey::new("script2") + ); + } +} diff --git a/src/dynamic_data/script_handling/script_set.rs b/src/dynamic_data/script_handling/script_set.rs index 031ca79..5920634 100644 --- a/src/dynamic_data/script_handling/script_set.rs +++ b/src/dynamic_data/script_handling/script_set.rs @@ -10,10 +10,12 @@ pub struct ScriptSet { impl ScriptSet { pub fn add(&mut self, script: Box) -> ScriptContainer { if let Some(lock) = self.scripts.get(script.name()) { - let mut existing = lock.get(); - if let Some(v) = &mut *existing { - v.stack(); - return lock.clone(); + if let Some(existing) = lock.get() { + let existing = existing.read(); + if let Some(v) = &*existing { + v.stack(); + return lock.clone(); + } } } self.scripts.insert(script.name().clone(), ScriptContainer::new(script)); @@ -25,10 +27,12 @@ impl ScriptSet { F: Fn() -> PkmnResult>>, { if let Some(lock) = self.scripts.get(key) { - let mut existing = lock.get(); - if let Some(v) = &mut *existing { - v.stack(); - return Ok(Some(lock.clone())); + if let Some(existing) = lock.get() { + let existing = existing.read(); + if let Some(v) = &*existing { + v.stack(); + return Ok(Some(lock.clone())); + } } } let script = instantiation()?; @@ -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(); }