diff --git a/src/dynamic_data/libraries/dynamic_library.rs b/src/dynamic_data/libraries/dynamic_library.rs index 17d10f1..b0284d2 100644 --- a/src/dynamic_data/libraries/dynamic_library.rs +++ b/src/dynamic_data/libraries/dynamic_library.rs @@ -8,6 +8,7 @@ use crate::static_data::items::item::Item; use crate::static_data::libraries::static_data::StaticData; use crate::{PkmnResult, StringKey}; use std::ops::Deref; +use std::sync::Arc; #[derive(Debug)] pub struct DynamicLibrary { @@ -49,7 +50,7 @@ impl DynamicLibrary { self.misc_library.deref() } - pub fn load_script(&self, _category: ScriptCategory, _key: &StringKey) -> PkmnResult>> { + pub fn load_script(&self, _category: ScriptCategory, _key: &StringKey) -> PkmnResult>> { todo!() } pub fn load_item_script(&self, _key: &Item) -> PkmnResult>> { diff --git a/src/dynamic_data/models/battle.rs b/src/dynamic_data/models/battle.rs index 4b97177..568ac38 100644 --- a/src/dynamic_data/models/battle.rs +++ b/src/dynamic_data/models/battle.rs @@ -292,7 +292,7 @@ impl<'own, 'library> VolatileScripts<'own> for Battle<'own, 'library> { &self.volatile_scripts } - fn load_volatile_script(&self, key: &StringKey) -> PkmnResult>> { + fn load_volatile_script(&self, key: &StringKey) -> PkmnResult>> { self.library.load_script(ScriptCategory::Battle, key) } } diff --git a/src/dynamic_data/models/battle_side.rs b/src/dynamic_data/models/battle_side.rs index da3a824..fd866cd 100644 --- a/src/dynamic_data/models/battle_side.rs +++ b/src/dynamic_data/models/battle_side.rs @@ -258,7 +258,7 @@ impl<'own, 'library> VolatileScripts<'own> for BattleSide<'own, 'library> { &self.volatile_scripts } - fn load_volatile_script(&self, key: &StringKey) -> PkmnResult>> { + fn load_volatile_script(&self, key: &StringKey) -> PkmnResult>> { self.battle().library().load_script(crate::ScriptCategory::Side, key) } } diff --git a/src/dynamic_data/models/pokemon.rs b/src/dynamic_data/models/pokemon.rs index f245b13..993fc11 100644 --- a/src/dynamic_data/models/pokemon.rs +++ b/src/dynamic_data/models/pokemon.rs @@ -395,15 +395,10 @@ impl<'own, 'library> Pokemon<'own, 'library> { .load_script(ScriptCategory::Ability, self.active_ability().name()) .unwrap(); if let Some(ability_script) = ability_script { - self.ability_script.set(ability_script); - // Ensure the ability script gets initialized with the parameters for the ability. - self.ability_script() - .get() - .unwrap() - .read() - .as_ref() - .unwrap() + self.ability_script + .set(ability_script) .as_ref() + // Ensure the ability script gets initialized with the parameters for the ability. .on_initialize(self.active_ability().parameters()) } else { self.ability_script.clear(); @@ -581,7 +576,7 @@ impl<'own, 'library> VolatileScripts<'own> for Pokemon<'own, 'library> { &self.volatile } - fn load_volatile_script(&self, key: &StringKey) -> PkmnResult>> { + fn load_volatile_script(&self, key: &StringKey) -> PkmnResult>> { self.library.load_script(ScriptCategory::Pokemon, key) } } diff --git a/src/dynamic_data/script_handling/mod.rs b/src/dynamic_data/script_handling/mod.rs index 157a237..22fdab2 100644 --- a/src/dynamic_data/script_handling/mod.rs +++ b/src/dynamic_data/script_handling/mod.rs @@ -99,7 +99,7 @@ pub trait ScriptSource<'a> { #[derive(Debug)] pub enum ScriptWrapper { - Script(Weak>>>), + Script(Weak>>>), Set(Weak>), } @@ -263,7 +263,7 @@ mod tests { #[test] fn script_aggregator_property_iterates_single_script() { - let script = ScriptContainer::new(Box::new(TestScript::new())); + let script = ScriptContainer::new(Arc::new(TestScript::new())); let scripts = vec![ScriptWrapper::from(&script)]; let mut aggregator = ScriptAggregator::new(&scripts as *const Vec); while let Some(v) = aggregator.get_next() { @@ -275,7 +275,7 @@ mod tests { #[test] fn script_aggregator_property_iterates_single_script_with_resets() { - let script = ScriptContainer::new(Box::new(TestScript::new())); + let script = ScriptContainer::new(Arc::new(TestScript::new())); let scripts = vec![ScriptWrapper::from(&script)]; let mut aggregator = ScriptAggregator::new(&scripts as *const Vec); for i in 1..11 { @@ -290,9 +290,9 @@ mod tests { #[test] fn script_aggregator_property_iterates_three_script() { - let script1 = ScriptContainer::new(Box::new(TestScript::new())); - let script2 = ScriptContainer::new(Box::new(TestScript::new())); - let script3 = ScriptContainer::new(Box::new(TestScript::new())); + let script1 = ScriptContainer::new(Arc::new(TestScript::new())); + let script2 = ScriptContainer::new(Arc::new(TestScript::new())); + let script3 = ScriptContainer::new(Arc::new(TestScript::new())); let scripts = vec![ ScriptWrapper::from(&script1), ScriptWrapper::from(&script2), @@ -312,9 +312,9 @@ mod tests { #[test] fn script_aggregator_property_iterates_three_script_with_resets() { - let script1 = ScriptContainer::new(Box::new(TestScript::new())); - let script2 = ScriptContainer::new(Box::new(TestScript::new())); - let script3 = ScriptContainer::new(Box::new(TestScript::new())); + let script1 = ScriptContainer::new(Arc::new(TestScript::new())); + let script2 = ScriptContainer::new(Arc::new(TestScript::new())); + let script3 = ScriptContainer::new(Arc::new(TestScript::new())); let scripts = vec![ ScriptWrapper::from(&script1), ScriptWrapper::from(&script2), @@ -339,9 +339,9 @@ mod tests { fn script_aggregator_property_iterates_script_set() { let set = Arc::new(RwLock::new(ScriptSet::default())); let mut mut_set = set.write(); - mut_set.add(Box::new(TestScript::new_with_name("test_a"))); - mut_set.add(Box::new(TestScript::new_with_name("test_b"))); - mut_set.add(Box::new(TestScript::new_with_name("test_c"))); + mut_set.add(Arc::new(TestScript::new_with_name("test_a"))); + mut_set.add(Arc::new(TestScript::new_with_name("test_b"))); + mut_set.add(Arc::new(TestScript::new_with_name("test_c"))); // Drop so we don't have a lock on it anymore. drop(mut_set); @@ -369,9 +369,9 @@ mod tests { fn script_aggregator_property_iterates_script_set_when_removing_last() { let set = Arc::new(RwLock::new(ScriptSet::default())); let mut mut_set = set.write(); - mut_set.add(Box::new(TestScript::new_with_name("test_a"))); - mut_set.add(Box::new(TestScript::new_with_name("test_b"))); - mut_set.add(Box::new(TestScript::new_with_name("test_c"))); + mut_set.add(Arc::new(TestScript::new_with_name("test_a"))); + mut_set.add(Arc::new(TestScript::new_with_name("test_b"))); + mut_set.add(Arc::new(TestScript::new_with_name("test_c"))); // Drop so we don't have a lock on it anymore. drop(mut_set); @@ -415,9 +415,9 @@ mod tests { fn script_aggregator_property_iterates_script_set_when_removing_middle() { let set = Arc::new(RwLock::new(ScriptSet::default())); let mut mut_set = set.write(); - mut_set.add(Box::new(TestScript::new_with_name("test_a"))); - mut_set.add(Box::new(TestScript::new_with_name("test_b"))); - mut_set.add(Box::new(TestScript::new_with_name("test_c"))); + mut_set.add(Arc::new(TestScript::new_with_name("test_a"))); + mut_set.add(Arc::new(TestScript::new_with_name("test_b"))); + mut_set.add(Arc::new(TestScript::new_with_name("test_c"))); // Drop so we don't have a lock on it anymore. drop(mut_set); @@ -491,7 +491,7 @@ mod tests { aggregator.reset(); assert!(aggregator.get_next().is_none()); aggregator.reset(); - source.script.set(Box::new(TestScript::new())); + source.script.set(Arc::new(TestScript::new())); assert!(aggregator.get_next().is_some()); } @@ -503,7 +503,7 @@ mod tests { }; let mut aggregator = source.get_script_iterator(); - source.script.set(Box::new(TestScript::new())); + source.script.set(Arc::new(TestScript::new())); assert!(aggregator.get_next().is_some()); aggregator.reset(); source.script.clear(); diff --git a/src/dynamic_data/script_handling/script.rs b/src/dynamic_data/script_handling/script.rs index 6b8ba17..6ac579e 100644 --- a/src/dynamic_data/script_handling/script.rs +++ b/src/dynamic_data/script_handling/script.rs @@ -142,7 +142,7 @@ impl Debug for dyn Script { } } -type ScriptHolder = Arc>>>; +type ScriptHolder = Arc>>>; #[derive(Default, Debug, Clone)] pub struct ScriptContainer { @@ -150,7 +150,7 @@ pub struct ScriptContainer { } impl ScriptContainer { - pub fn new(script: Box) -> ScriptContainer { + pub fn new(script: Arc) -> ScriptContainer { Self { script: Arc::new(RwLock::new(Some(script))), } @@ -167,21 +167,20 @@ impl ScriptContainer { } } - pub fn set(&self, script: Box) { + pub fn set(&self, script: Arc) -> Arc { 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); - } + let holder = self.script.clone(); + let new = script.clone(); + std::thread::spawn(move || { + holder.write().replace(new); + }); } else { - self.script.write().replace(script); - } + self.script.write().replace(script.clone()); + }; + script } pub fn clear(&self) { @@ -215,11 +214,13 @@ impl From for ScriptContainer { #[cfg(test)] mod tests { use super::*; - use std::sync::atomic::AtomicBool; + use std::sync::atomic::{AtomicBool, AtomicPtr}; + use std::thread::sleep; + use std::time::Duration; pub struct TestScript { name: StringKey, - container: *mut ScriptContainer, + container: AtomicPtr, suppressed_count: AtomicUsize, marked_for_deletion: AtomicBool, } @@ -228,7 +229,7 @@ mod tests { fn new() -> Self { Self { name: StringKey::new("test"), - container: std::ptr::null_mut::(), + container: AtomicPtr::::default(), suppressed_count: AtomicUsize::new(0), marked_for_deletion: Default::default(), } @@ -253,7 +254,7 @@ mod tests { } fn on_initialize(&self, _pars: &[EffectParameter]) { - unsafe { self.container.as_mut().unwrap().clear() } + unsafe { self.container.load(Ordering::Relaxed).as_ref().unwrap().clear() } } fn as_any(&self) -> &dyn Any { @@ -270,11 +271,13 @@ mod tests { // 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; + let script = Arc::new(TestScript::new()); + let mut container = ScriptContainer::new(script); + let c = &mut container as *mut ScriptContainer; + + let w = container.script.read(); + let script: &TestScript = w.as_ref().unwrap().as_any().downcast_ref().unwrap(); + script.container.store(c, Ordering::SeqCst); 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. @@ -290,7 +293,7 @@ mod tests { pub struct ReplaceTestScript { name: StringKey, - container: *mut ScriptContainer, + container: AtomicPtr, suppressed_count: AtomicUsize, marked_for_deletion: AtomicBool, } @@ -299,15 +302,15 @@ mod tests { fn new(name: StringKey) -> Self { Self { name, - container: std::ptr::null_mut::(), + container: AtomicPtr::::default(), suppressed_count: AtomicUsize::new(0), marked_for_deletion: Default::default(), } } - fn test(&self, script: Box) { + fn test(&self, script: Arc) { unsafe { - self.container.as_mut().unwrap().set(script); + self.container.load(Ordering::Relaxed).as_ref().unwrap().set(script); } } } @@ -339,14 +342,16 @@ mod tests { #[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; + let script = Arc::new(ReplaceTestScript::new("script1".into())); + let mut container = ScriptContainer::new(script); + let c = &mut container as *mut ScriptContainer; + + let w = container.script.read(); + let script: &ReplaceTestScript = w.as_ref().unwrap().as_any().downcast_ref().unwrap(); + script.container.store(c, Ordering::SeqCst); drop(w); - let script2 = Box::new(ReplaceTestScript::new("script2".into())); + let script2 = Arc::new(ReplaceTestScript::new("script2".into())); container .script .read() @@ -356,6 +361,7 @@ mod tests { .downcast_ref::() .unwrap() .test(script2); + sleep(Duration::from_micros(500)); 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 5920634..17d973f 100644 --- a/src/dynamic_data/script_handling/script_set.rs +++ b/src/dynamic_data/script_handling/script_set.rs @@ -1,6 +1,7 @@ use crate::dynamic_data::script_handling::script::{Script, ScriptContainer}; use crate::{PkmnResult, StringKey}; use indexmap::IndexMap; +use std::sync::Arc; #[derive(Debug, Default)] pub struct ScriptSet { @@ -8,7 +9,7 @@ pub struct ScriptSet { } impl ScriptSet { - pub fn add(&mut self, script: Box) -> ScriptContainer { + pub fn add(&mut self, script: Arc) -> ScriptContainer { if let Some(lock) = self.scripts.get(script.name()) { if let Some(existing) = lock.get() { let existing = existing.read(); @@ -24,7 +25,7 @@ impl ScriptSet { pub fn stack_or_add<'b, F>(&mut self, key: &StringKey, instantiation: &'b F) -> PkmnResult> where - F: Fn() -> PkmnResult>>, + F: Fn() -> PkmnResult>>, { if let Some(lock) = self.scripts.get(key) { if let Some(existing) = lock.get() { diff --git a/src/dynamic_data/script_handling/volatile_scripts.rs b/src/dynamic_data/script_handling/volatile_scripts.rs index f7dbcfa..b54407d 100644 --- a/src/dynamic_data/script_handling/volatile_scripts.rs +++ b/src/dynamic_data/script_handling/volatile_scripts.rs @@ -6,7 +6,7 @@ use std::sync::Arc; pub trait VolatileScripts<'a> { fn volatile_scripts(&self) -> &Arc>; - fn load_volatile_script(&self, key: &StringKey) -> PkmnResult>>; + fn load_volatile_script(&self, key: &StringKey) -> PkmnResult>>; fn has_volatile_script(&self, key: &StringKey) -> bool { self.volatile_scripts().read().has(key)