From 715f16e2b8084bc1f6fb6cff2604a2f639670e8e Mon Sep 17 00:00:00 2001 From: Deukhoofd Date: Sun, 19 Jun 2022 12:07:54 +0200 Subject: [PATCH] More work on switching battle data to interior mutability, instead of exterior mutability. --- src/dynamic_data/flow/target_resolver.rs | 5 +- src/dynamic_data/flow/turn_runner.rs | 194 ++++++++---------- src/dynamic_data/libraries/misc_library.rs | 2 +- src/dynamic_data/models/battle.rs | 19 +- src/dynamic_data/models/battle_side.rs | 58 +++--- src/dynamic_data/models/executing_move.rs | 53 +++-- src/dynamic_data/models/pokemon.rs | 34 +-- src/dynamic_data/script_handling/mod.rs | 76 +++---- .../script_handling/script_set.rs | 53 +++-- .../script_handling/volatile_scripts.rs | 12 +- src/static_data/libraries/move_library.rs | 3 +- src/static_data/moves/move_data.rs | 10 +- src/static_data/moves/secondary_effect.rs | 9 +- tests/common/library_loader.rs | 8 +- 14 files changed, 256 insertions(+), 280 deletions(-) diff --git a/src/dynamic_data/flow/target_resolver.rs b/src/dynamic_data/flow/target_resolver.rs index a4568a2..5004ed3 100644 --- a/src/dynamic_data/flow/target_resolver.rs +++ b/src/dynamic_data/flow/target_resolver.rs @@ -2,6 +2,7 @@ use crate::dynamic_data::models::battle::Battle; use crate::dynamic_data::models::pokemon::Pokemon; use crate::static_data::MoveTarget; use num_traits::abs; +use std::ops::Deref; use std::sync::Arc; pub type TargetList<'own, 'library> = Vec>>>; @@ -9,7 +10,7 @@ pub type TargetList<'own, 'library> = Vec>>>; fn get_all_targets<'b, 'library>(battle: &Battle<'b, 'library>) -> TargetList<'b, 'library> { let mut v = Vec::with_capacity(battle.pokemon_per_side() as usize * battle.number_of_sides() as usize); for side in battle.sides() { - for pokemon in side.pokemon() { + for pokemon in side.pokemon().deref() { v.push(pokemon.as_ref().cloned()); } } @@ -106,7 +107,7 @@ pub fn resolve_targets<'b, 'library>( MoveTarget::AllAdjacentOpponent => get_all_adjacent_opponent(side, index, battle), MoveTarget::AllAlly | MoveTarget::AllOpponent => { let mut v = Vec::new(); - for pokemon in battle.sides()[side as usize].pokemon() { + for pokemon in battle.sides()[side as usize].pokemon().deref() { v.push(pokemon.as_ref().cloned()); } v diff --git a/src/dynamic_data/flow/turn_runner.rs b/src/dynamic_data/flow/turn_runner.rs index a18d337..f495769 100644 --- a/src/dynamic_data/flow/turn_runner.rs +++ b/src/dynamic_data/flow/turn_runner.rs @@ -185,113 +185,101 @@ impl<'own, 'library> Battle<'own, 'library> { if target.is_fainted() { break; } - { - let mut hit_type = executing_move.use_move().move_type(); - script_hook!( - change_move_type, - executing_move, - executing_move, - target, - hit_index, - &mut hit_type - ); - executing_move - .get_hit_from_raw_index_mut(target_hit_stat + hit_index as usize) - .set_move_type(hit_type); - let mut effectiveness = self + let used_move = executing_move.use_move(); + let mut hit_type = used_move.move_type(); + script_hook!( + change_move_type, + executing_move, + executing_move, + target, + hit_index, + &mut hit_type + ); + let hit_data = executing_move.get_hit_from_raw_index(target_hit_stat + hit_index as usize); + hit_data.set_move_type(hit_type); + let mut effectiveness = self + .library() + .static_data() + .types() + .get_effectiveness(hit_type, target.types()); + script_hook!( + change_effectiveness, + executing_move, + executing_move, + target, + hit_index, + &mut effectiveness + ); + hit_data.set_effectiveness(effectiveness); + let mut block_critical = false; + script_hook!( + block_critical, + executing_move, + executing_move, + target, + hit_index, + &mut block_critical + ); + script_hook!( + block_incoming_critical, + target, + executing_move, + target, + hit_index, + &mut block_critical + ); + + if !block_critical { + let is_critical = self .library() - .static_data() - .types() - .get_effectiveness(hit_type, target.types()); - script_hook!( - change_effectiveness, - executing_move, - executing_move, - target, - hit_index, - &mut effectiveness - ); - executing_move - .get_hit_from_raw_index_mut(target_hit_stat + hit_index as usize) - .set_effectiveness(effectiveness); - let mut block_critical = false; - script_hook!( - block_critical, - executing_move, - executing_move, - target, - hit_index, - &mut block_critical - ); - script_hook!( - block_incoming_critical, - target, - executing_move, - target, - hit_index, - &mut block_critical - ); + .misc_library() + .is_critical(self, executing_move, target, hit_index); + hit_data.set_critical(is_critical); + } + let base_power = self.library().damage_calculator().get_base_power( + executing_move, + target, + hit_index, + executing_move.get_hit_data(target, hit_index)?, + ); + hit_data.set_base_power(base_power); + let damage = self.library().damage_calculator().get_damage( + executing_move, + target, + hit_index, + executing_move.get_hit_data(target, hit_index)?, + ); + hit_data.set_damage(damage); - if !block_critical { - let is_critical = - self.library() - .misc_library() - .is_critical(self, executing_move, target, hit_index); - executing_move - .get_hit_from_raw_index_mut(target_hit_stat + hit_index as usize) - .set_critical(is_critical); + if used_move.category() == MoveCategory::Status { + if let Some(secondary_effect) = used_move.secondary_effect() { + let secondary_effect_chance = secondary_effect.chance(); + if secondary_effect_chance == -1.0 + || self + .random() + .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 + } } - let base_power = self.library().damage_calculator().get_base_power( - executing_move, - target, - hit_index, - executing_move.get_hit_data(target, hit_index)?, - ); - executing_move - .get_hit_from_raw_index_mut(target_hit_stat + hit_index as usize) - .set_base_power(base_power); - let damage = self.library().damage_calculator().get_damage( - executing_move, - target, - hit_index, - executing_move.get_hit_data(target, hit_index)?, - ); - executing_move - .get_hit_from_raw_index_mut(target_hit_stat + hit_index as usize) - .set_damage(damage); - - if executing_move.use_move().category() == MoveCategory::Status { - if executing_move.use_move().has_secondary_effect() { - let secondary_effect_chance = executing_move.use_move().secondary_effect().chance(); - if secondary_effect_chance == -1.0 - || self - .random() - .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 - } + } else { + let mut damage = hit_data.damage(); + let current_health = target.current_health(); + if damage > current_health { + damage = current_health; + hit_data.set_damage(damage); + } + if damage > 0 { + target.damage(damage, DamageSource::AttackDamage); + if !target.is_fainted() { + script_hook!(on_incoming_hit, target, executing_move, target, hit_index); + } else { + script_hook!(on_opponent_faints, executing_move, executing_move, target, hit_index); } - } else { - let mut damage = executing_move - .get_hit_from_raw_index(target_hit_stat + hit_index as usize) - .damage(); - let current_health = target.current_health(); - if damage > current_health { - damage = current_health; - executing_move - .get_hit_from_raw_index_mut(target_hit_stat + hit_index as usize) - .set_damage(damage); - } - if damage > 0 { - target.damage(damage, DamageSource::AttackDamage); - if !target.is_fainted() { - script_hook!(on_incoming_hit, target, executing_move, target, hit_index); - } else { - script_hook!(on_opponent_faints, executing_move, executing_move, target, hit_index); - } - if executing_move.use_move().has_secondary_effect() && !target.is_fainted() { + if !target.is_fainted() { + if let Some(secondary_effect) = used_move.secondary_effect() { let mut prevent_secondary = false; script_hook!( prevent_secondary_effect, @@ -302,7 +290,7 @@ impl<'own, 'library> Battle<'own, 'library> { &mut prevent_secondary ); if !prevent_secondary { - let secondary_effect_chance = executing_move.use_move().secondary_effect().chance(); + let secondary_effect_chance = secondary_effect.chance(); if secondary_effect_chance == -1.0 || self.random().effect_chance( secondary_effect_chance, diff --git a/src/dynamic_data/libraries/misc_library.rs b/src/dynamic_data/libraries/misc_library.rs index b82ffa9..5f5a4d9 100644 --- a/src/dynamic_data/libraries/misc_library.rs +++ b/src/dynamic_data/libraries/misc_library.rs @@ -52,7 +52,7 @@ impl<'library> Gen7MiscLibrary<'library> { 255, MoveTarget::Any, 0, - SecondaryEffect::new(-1.0, StringKey::new("struggle"), vec![]), + Some(SecondaryEffect::new(-1.0, StringKey::new("struggle"), vec![])), HashSet::new(), )); let struggle_ptr = Box::into_raw(struggle_data); diff --git a/src/dynamic_data/models/battle.rs b/src/dynamic_data/models/battle.rs index 568ac38..c0d3225 100644 --- a/src/dynamic_data/models/battle.rs +++ b/src/dynamic_data/models/battle.rs @@ -35,7 +35,7 @@ pub struct Battle<'own, 'library> { event_hook: EventHook, history_holder: Box, current_turn: AtomicU32, - volatile_scripts: Arc>, + volatile_scripts: Arc, last_turn_time: Atomic, script_source_data: RwLock, @@ -134,16 +134,13 @@ impl<'own, 'library> Battle<'own, 'library> { &self.current_turn_queue } - pub fn get_pokemon(&self, side: u8, index: u8) -> &Option>> { + pub fn get_pokemon(&self, side: u8, index: u8) -> Option>> { let side = self.sides.get(side as usize); - if side.is_none() { - return &None; - } - let pokemon = side.unwrap().pokemon().get(index as usize); - if pokemon.is_none() { - return &None; - } - pokemon.unwrap() + side?; + let pokemon_read_lock = side.unwrap().pokemon(); + let pokemon = pokemon_read_lock.get(index as usize); + pokemon?; + pokemon.unwrap().clone() } pub fn can_slot_be_filled(&self, side: u8, index: u8) -> bool { @@ -288,7 +285,7 @@ impl<'own, 'library> Battle<'own, 'library> { } impl<'own, 'library> VolatileScripts<'own> for Battle<'own, 'library> { - fn volatile_scripts(&self) -> &Arc> { + fn volatile_scripts(&self) -> &Arc { &self.volatile_scripts } diff --git a/src/dynamic_data/models/battle_side.rs b/src/dynamic_data/models/battle_side.rs index fd866cd..19b65e9 100644 --- a/src/dynamic_data/models/battle_side.rs +++ b/src/dynamic_data/models/battle_side.rs @@ -8,22 +8,23 @@ use crate::dynamic_data::script_handling::script_set::ScriptSet; use crate::dynamic_data::script_handling::volatile_scripts::VolatileScripts; use crate::dynamic_data::script_handling::{ScriptSource, ScriptSourceData, ScriptWrapper}; use crate::{script_hook, PkmnResult, StringKey}; -use parking_lot::RwLock; +use parking_lot::lock_api::RwLockReadGuard; +use parking_lot::{RawRwLock, RwLock}; use std::ops::Deref; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU8, Ordering}; use std::sync::Arc; #[derive(Debug)] pub struct BattleSide<'own, 'library> { index: u8, pokemon_per_side: u8, - pokemon: Vec>>>, + pokemon: RwLock>>>>, choices: RwLock>>>, fillable_slots: Vec, - choices_set: u8, + choices_set: AtomicU8, battle: *mut Battle<'own, 'library>, has_fled_battle: bool, - volatile_scripts: Arc>, + volatile_scripts: Arc, script_source_data: RwLock, } @@ -40,6 +41,7 @@ impl<'own, 'library> BattleSide<'own, 'library> { fillable_slots.push(AtomicBool::new(false)); } let choices = RwLock::new(choices); + let pokemon = RwLock::new(pokemon); Self { index, @@ -47,7 +49,7 @@ impl<'own, 'library> BattleSide<'own, 'library> { pokemon, choices, fillable_slots, - choices_set: 0, + choices_set: AtomicU8::new(0), battle: std::ptr::null_mut::(), has_fled_battle: false, volatile_scripts: Default::default(), @@ -65,8 +67,8 @@ impl<'own, 'library> BattleSide<'own, 'library> { pub fn pokemon_per_side(&self) -> u8 { self.pokemon_per_side } - pub fn pokemon(&self) -> &Vec>>> { - &self.pokemon + pub fn pokemon(&self) -> RwLockReadGuard<'_, RawRwLock, Vec>>>> { + self.pokemon.read() } pub fn choices(&self) -> &RwLock>>> { &self.choices @@ -76,7 +78,7 @@ impl<'own, 'library> BattleSide<'own, 'library> { &self.fillable_slots } pub fn choices_set(&self) -> u8 { - self.choices_set + self.choices_set.load(Ordering::SeqCst) } pub fn battle(&self) -> &Battle<'own, 'library> { unsafe { self.battle.as_ref().unwrap() } @@ -84,19 +86,19 @@ impl<'own, 'library> BattleSide<'own, 'library> { pub fn has_fled_battle(&self) -> bool { self.has_fled_battle } - pub fn volatile_scripts(&self) -> &Arc> { + pub fn volatile_scripts(&self) -> &Arc { &self.volatile_scripts } pub fn all_choices_set(&self) -> bool { - self.choices_set == self.pokemon_per_side + self.choices_set() == self.pokemon_per_side } /// Returns true if there are slots that need to be filled with a new pokemon, that have parties /// responsible for them. Returns false if all slots are filled with usable pokemon, or slots are /// empty, but can't be filled by any party anymore. pub fn all_slots_filled(&self) -> bool { - for (i, pokemon) in self.pokemon.iter().enumerate() { + for (i, pokemon) in self.pokemon.read().iter().enumerate() { if (!pokemon.is_none() || !pokemon.as_ref().unwrap().is_usable()) && self.battle().can_slot_be_filled(self.index, i as u8) { @@ -106,12 +108,12 @@ impl<'own, 'library> BattleSide<'own, 'library> { true } - pub fn set_choice(&mut self, choice: TurnChoice<'own, 'library>) { - for (index, pokemon_slot) in self.pokemon.iter().enumerate() { + pub fn set_choice(&self, choice: TurnChoice<'own, 'library>) { + for (index, pokemon_slot) in self.pokemon.read().iter().enumerate() { if let Some(pokemon) = pokemon_slot { if std::ptr::eq(pokemon.deref(), choice.user().deref()) { self.choices.write()[index] = Some(choice); - self.choices_set += 1; + self.choices_set.fetch_add(1, Ordering::SeqCst); return; } } @@ -126,17 +128,19 @@ impl<'own, 'library> BattleSide<'own, 'library> { } pub fn force_clear_pokemon(&mut self, index: u8) { - self.pokemon[index as usize] = None; + self.pokemon.write()[index as usize] = None; } - pub fn set_pokemon(&mut self, index: u8, pokemon: Option>>) { - let old = &mut self.pokemon[index as usize]; - if let Some(old_pokemon) = old { - script_hook!(on_remove, old_pokemon,); - old_pokemon.set_on_battlefield(false); + pub fn set_pokemon(&self, index: u8, pokemon: Option>>) { + { + let old = &self.pokemon.read()[index as usize]; + if let Some(old_pokemon) = old { + script_hook!(on_remove, old_pokemon,); + old_pokemon.set_on_battlefield(false); + } } - self.pokemon[index as usize] = pokemon; - let pokemon = &self.pokemon[index as usize]; + self.pokemon.write()[index as usize] = pokemon; + let pokemon = &self.pokemon.read()[index as usize]; if let Some(pokemon) = pokemon { pokemon.set_battle_data(self.battle, self.index); pokemon.set_on_battlefield(true); @@ -168,7 +172,7 @@ impl<'own, 'library> BattleSide<'own, 'library> { } pub fn is_pokemon_on_side(&self, pokemon: Arc>) -> bool { - for p in self.pokemon.iter().flatten() { + for p in self.pokemon.read().iter().flatten() { if std::ptr::eq(p.deref().deref(), pokemon.deref()) { return true; } @@ -181,7 +185,7 @@ impl<'own, 'library> BattleSide<'own, 'library> { } pub fn is_slot_unfillable(&self, pokemon: Arc>) -> bool { - for (i, slot) in self.pokemon.iter().enumerate() { + for (i, slot) in self.pokemon.read().iter().enumerate() { if let Some(p) = slot { if std::ptr::eq(p.deref().deref(), pokemon.deref()) { return self.fillable_slots[i].load(Ordering::Relaxed); @@ -243,7 +247,7 @@ impl<'own, 'library> BattleSide<'own, 'library> { return false; } - self.pokemon.swap(a as usize, b as usize); + self.pokemon.write().swap(a as usize, b as usize); self.battle().event_hook().trigger(Event::Swap { side_index: self.index, index_a: a, @@ -254,7 +258,7 @@ impl<'own, 'library> BattleSide<'own, 'library> { } impl<'own, 'library> VolatileScripts<'own> for BattleSide<'own, 'library> { - fn volatile_scripts(&self) -> &Arc> { + fn volatile_scripts(&self) -> &Arc { &self.volatile_scripts } diff --git a/src/dynamic_data/models/executing_move.rs b/src/dynamic_data/models/executing_move.rs index 3b1f41c..ba7ef8f 100644 --- a/src/dynamic_data/models/executing_move.rs +++ b/src/dynamic_data/models/executing_move.rs @@ -5,57 +5,59 @@ use crate::dynamic_data::script_handling::script::ScriptContainer; use crate::dynamic_data::script_handling::{ScriptSource, ScriptSourceData, ScriptWrapper}; use crate::static_data::MoveData; use crate::{PkmnResult, PokemonError}; +use atomic::Atomic; use parking_lot::RwLock; use std::ops::Deref; +use std::sync::atomic::{AtomicBool, AtomicU32, AtomicU8, Ordering}; use std::sync::Arc; #[derive(Default, Debug)] pub struct HitData { - critical: bool, - base_power: u8, - effectiveness: f32, - damage: u32, - move_type: u8, - has_failed: bool, + critical: AtomicBool, + base_power: AtomicU8, + effectiveness: Atomic, + damage: AtomicU32, + move_type: AtomicU8, + has_failed: AtomicBool, } impl HitData { pub fn is_critical(&self) -> bool { - self.critical + self.critical.load(Ordering::Relaxed) } pub fn base_power(&self) -> u8 { - self.base_power + self.base_power.load(Ordering::Relaxed) } pub fn effectiveness(&self) -> f32 { - self.effectiveness + self.effectiveness.load(Ordering::Relaxed) } pub fn damage(&self) -> u32 { - self.damage + self.damage.load(Ordering::Relaxed) } pub fn move_type(&self) -> u8 { - self.move_type + self.move_type.load(Ordering::Relaxed) } pub fn has_failed(&self) -> bool { - self.has_failed + self.has_failed.load(Ordering::Relaxed) } - pub fn set_critical(&mut self, value: bool) { - self.critical = value; + pub fn set_critical(&self, value: bool) { + self.critical.store(value, Ordering::SeqCst); } - pub fn set_base_power(&mut self, value: u8) { - self.base_power = value; + pub fn set_base_power(&self, value: u8) { + self.base_power.store(value, Ordering::SeqCst); } - pub fn set_effectiveness(&mut self, value: f32) { - self.effectiveness = value; + pub fn set_effectiveness(&self, value: f32) { + self.effectiveness.store(value, Ordering::SeqCst); } - pub fn set_damage(&mut self, value: u32) { - self.damage = value; + pub fn set_damage(&self, value: u32) { + self.damage.store(value, Ordering::SeqCst); } - pub fn set_move_type(&mut self, value: u8) { - self.move_type = value; + pub fn set_move_type(&self, value: u8) { + self.move_type.store(value, Ordering::SeqCst); } - pub fn fail(&mut self) { - self.has_failed = true; + pub fn fail(&self) { + self.has_failed.store(true, Ordering::SeqCst); } } @@ -156,9 +158,6 @@ impl<'own, 'battle, 'library> ExecutingMove<'own, 'battle, 'library> { pub(crate) fn get_hit_from_raw_index(&self, index: usize) -> &HitData { &self.hits[index] } - pub(crate) fn get_hit_from_raw_index_mut(&mut self, index: usize) -> &mut HitData { - &mut self.hits[index] - } } impl<'own, 'battle, 'library> ScriptSource<'own> for ExecutingMove<'own, 'battle, 'library> { diff --git a/src/dynamic_data/models/pokemon.rs b/src/dynamic_data/models/pokemon.rs index 993fc11..f1d31a2 100644 --- a/src/dynamic_data/models/pokemon.rs +++ b/src/dynamic_data/models/pokemon.rs @@ -73,7 +73,7 @@ where unique_identifier: u32, gender: Gender, coloring: u8, - held_item: Option<&'own Item>, + held_item: RwLock>, current_health: AtomicU32, weight: Atomic, @@ -104,7 +104,7 @@ where held_item_trigger_script: ScriptContainer, ability_script: ScriptContainer, status_script: ScriptContainer, - volatile: Arc>, + volatile: Arc, script_source_data: RwLock, } @@ -144,7 +144,7 @@ impl<'own, 'library> Pokemon<'own, 'library> { unique_identifier, gender, coloring, - held_item: None, + held_item: RwLock::new(None), current_health: AtomicU32::new(1), weight: Atomic::new(weight), height: Atomic::new(height), @@ -216,27 +216,27 @@ impl<'own, 'library> Pokemon<'own, 'library> { pub fn coloring(&self) -> u8 { self.coloring } - pub fn held_item(&self) -> Option<&'own Item> { - self.held_item + pub fn held_item(&self) -> &RwLock> { + &self.held_item } pub fn has_held_item(&self, name: &StringKey) -> bool { // Only true if we have an item, and the item name is the same as the requested item. - if let Some(v) = self.held_item { + if let Some(v) = self.held_item.read().deref() { return v.name() == name; } false } - pub fn set_held_item(&mut self, item: &'own Item) { - self.held_item = Some(item); + pub fn set_held_item(&self, item: &'own Item) -> Option<&'own Item> { + self.held_item.write().replace(item) } - pub fn remove_held_item(&mut self) { - self.held_item = None; + pub fn remove_held_item(&self) -> Option<&'own Item> { + self.held_item.write().take() } - pub fn consume_held_item(&mut self) -> bool { - if self.held_item.is_none() { + pub fn consume_held_item(&self) -> bool { + if self.held_item.read().is_none() { return false; } - let script = self.library.load_item_script(self.held_item.unwrap()).unwrap(); + let script = self.library.load_item_script(self.held_item.read().unwrap()).unwrap(); if script.is_none() { return false; } @@ -408,8 +408,10 @@ impl<'own, 'library> Pokemon<'own, 'library> { let diff_health = (self.max_health() - old_health) as i32; if self.current_health() == 0 && (self.current_health() as i32) < -diff_health { self.current_health.store(0, Ordering::SeqCst); + } else if diff_health < 0 { + self.current_health.fetch_sub(-diff_health as u32, Ordering::SeqCst); } else { - self.current_health.fetch_add(diff_health as u32, Ordering::Acquire); + self.current_health.fetch_add(diff_health as u32, Ordering::SeqCst); } // TODO: consider form specific attacks? @@ -450,7 +452,7 @@ impl<'own, 'library> Pokemon<'own, 'library> { if let Some(data) = &mut r.deref() { data.on_battle_field.store(value, Ordering::SeqCst); if !value { - self.volatile.write().clear(); + self.volatile.clear(); self.weight.store(self.form.weight(), Ordering::SeqCst); self.height.store(self.form.height(), Ordering::SeqCst); } @@ -572,7 +574,7 @@ impl<'own, 'library> ScriptSource<'own> for Pokemon<'own, 'library> { } impl<'own, 'library> VolatileScripts<'own> for Pokemon<'own, 'library> { - fn volatile_scripts(&self) -> &Arc> { + fn volatile_scripts(&self) -> &Arc { &self.volatile } diff --git a/src/dynamic_data/script_handling/mod.rs b/src/dynamic_data/script_handling/mod.rs index e5e63f3..ae4608f 100644 --- a/src/dynamic_data/script_handling/mod.rs +++ b/src/dynamic_data/script_handling/mod.rs @@ -56,12 +56,13 @@ macro_rules! run_scripts { } } ScriptWrapper::Set(s) => { - if let Some(s) = s.upgrade() { - for s in s.read().get_underlying() { - if let Some(s) = s.1.get() { + if let Some(set) = s.upgrade() { + let current_scripts = set.get_owning_iterator(); + for s in current_scripts { + if let Some(s) = s.get() { let s = s.read(); if let Some(s) = s.deref() { - if !s.is_suppressed() { + if !s.is_suppressed() && set.has(s.name()) { s.$hook_name($($parameters),*); } } @@ -100,7 +101,7 @@ pub trait ScriptSource<'a> { #[derive(Debug)] pub enum ScriptWrapper { Script(Weak>>>), - Set(Weak>), + Set(Weak), } impl From<&ScriptContainer> for ScriptWrapper { @@ -109,8 +110,8 @@ impl From<&ScriptContainer> for ScriptWrapper { } } -impl From<&Arc>> for ScriptWrapper { - fn from(c: &Arc>) -> Self { +impl From<&Arc> for ScriptWrapper { + fn from(c: &Arc) -> Self { ScriptWrapper::Set(Arc::downgrade(c)) } } @@ -141,7 +142,7 @@ impl ScriptAggregator { if let ScriptWrapper::Set(set) = wrapper { if let Some(set) = set.upgrade() { self.set_index += 1; - if self.set_index as usize >= set.read().count() { + if self.set_index as usize >= set.count() { self.set_index = -1; } else { return true; @@ -155,7 +156,7 @@ impl ScriptAggregator { let wrapper = unsafe { &self.scripts.as_ref().unwrap()[self.index as usize] }; if let ScriptWrapper::Set(s) = wrapper { if let Some(set) = s.upgrade() { - if set.read().count() > 0 { + if set.count() > 0 { self.set_index = 0; return true; } @@ -181,9 +182,8 @@ impl ScriptAggregator { // increment_to_next_value ScriptWrapper::Script(script) => Some(script.upgrade().unwrap().into()), ScriptWrapper::Set(set) => { - let lock = set.as_ptr().as_ref().unwrap(); - let l = lock.read(); - let sc = l.at(self.set_index as usize); + let set = set.upgrade().unwrap(); + let sc = set.at(self.set_index as usize); return Some(sc.clone()); } }; @@ -337,13 +337,10 @@ mod tests { #[test] fn script_aggregator_property_iterates_script_set() { - let set = Arc::new(RwLock::new(ScriptSet::default())); - let mut mut_set = set.write(); - 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); + let set = Arc::new(ScriptSet::default()); + set.add(Arc::new(TestScript::new_with_name("test_a"))); + set.add(Arc::new(TestScript::new_with_name("test_b"))); + set.add(Arc::new(TestScript::new_with_name("test_c"))); let scripts = vec![ScriptWrapper::from(&set)]; let mut aggregator = ScriptAggregator::new(&scripts as *const Vec); @@ -352,14 +349,16 @@ mod tests { while let Some(v) = aggregator.get_next() { v.get().unwrap().read().as_ref().unwrap().on_initialize(&[]); } - let set = set.read(); - let s = set.at(0).get_as::(); + let s = set.at(0); + let s = s.get_as::(); assert_eq!(s.test_count.load(Ordering::Relaxed), i); assert_eq!(s.name().str(), "test_a"); - let s = set.at(1).get_as::(); + let s = set.at(1); + let s = s.get_as::(); assert_eq!(s.test_count.load(Ordering::Relaxed), i); assert_eq!(s.name().str(), "test_b"); - let s = set.at(2).get_as::(); + let s = set.at(2); + let s = s.get_as::(); assert_eq!(s.test_count.load(Ordering::Relaxed), i); assert_eq!(s.name().str(), "test_c"); } @@ -367,13 +366,10 @@ mod tests { #[test] 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(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); + let set = Arc::new(ScriptSet::default()); + set.add(Arc::new(TestScript::new_with_name("test_a"))); + set.add(Arc::new(TestScript::new_with_name("test_b"))); + set.add(Arc::new(TestScript::new_with_name("test_c"))); let scripts = vec![ScriptWrapper::from(&set)]; let mut aggregator = ScriptAggregator::new(&scripts as *const Vec); @@ -404,22 +400,16 @@ mod tests { "test_b" ); - let mut mut_set = set.write(); - mut_set.remove(&StringKey::new("test_c")); - drop(mut_set); - + set.remove(&StringKey::new("test_c")); assert!(aggregator.get_next().is_none()); } #[test] 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(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); + let set = Arc::new(ScriptSet::default()); + set.add(Arc::new(TestScript::new_with_name("test_a"))); + set.add(Arc::new(TestScript::new_with_name("test_b"))); + set.add(Arc::new(TestScript::new_with_name("test_c"))); let scripts = vec![ScriptWrapper::from(&set)]; let mut aggregator = ScriptAggregator::new(&scripts as *const Vec); @@ -437,9 +427,7 @@ mod tests { "test_a" ); - let mut mut_set = set.write(); - mut_set.remove(&StringKey::new("test_b")); - drop(mut_set); + set.remove(&StringKey::new("test_b")); assert_eq!( aggregator diff --git a/src/dynamic_data/script_handling/script_set.rs b/src/dynamic_data/script_handling/script_set.rs index 17d973f..9deea2f 100644 --- a/src/dynamic_data/script_handling/script_set.rs +++ b/src/dynamic_data/script_handling/script_set.rs @@ -1,16 +1,18 @@ use crate::dynamic_data::script_handling::script::{Script, ScriptContainer}; use crate::{PkmnResult, StringKey}; use indexmap::IndexMap; +use parking_lot::RwLock; +use std::ops::Deref; use std::sync::Arc; #[derive(Debug, Default)] pub struct ScriptSet { - scripts: IndexMap, + scripts: RwLock>, } impl ScriptSet { - pub fn add(&mut self, script: Arc) -> ScriptContainer { - if let Some(lock) = self.scripts.get(script.name()) { + pub fn add(&self, script: Arc) -> ScriptContainer { + if let Some(lock) = self.scripts.read().get(script.name()) { if let Some(existing) = lock.get() { let existing = existing.read(); if let Some(v) = &*existing { @@ -19,15 +21,17 @@ impl ScriptSet { } } } - self.scripts.insert(script.name().clone(), ScriptContainer::new(script)); - self.scripts.last().unwrap().1.clone() + self.scripts + .write() + .insert(script.name().clone(), ScriptContainer::new(script)); + self.scripts.read().last().unwrap().1.clone() } - pub fn stack_or_add<'b, F>(&mut self, key: &StringKey, instantiation: &'b F) -> PkmnResult> + pub fn stack_or_add<'b, F>(&self, key: &StringKey, instantiation: &'b F) -> PkmnResult> where F: Fn() -> PkmnResult>>, { - if let Some(lock) = self.scripts.get(key) { + if let Some(lock) = self.scripts.read().get(key) { if let Some(existing) = lock.get() { let existing = existing.read(); if let Some(v) = &*existing { @@ -40,19 +44,19 @@ impl ScriptSet { if let Some(script) = script { let name = script.name().clone(); let arc = ScriptContainer::new(script); - self.scripts.insert(name, arc); - Ok(Some(self.scripts.last().unwrap().1.clone())) + self.scripts.write().insert(name, arc); + Ok(Some(self.scripts.read().last().unwrap().1.clone())) } else { Ok(None) } } - pub fn get(&self, key: &StringKey) -> Option<&ScriptContainer> { - self.scripts.get(key) + pub fn get(&self, key: &StringKey) -> Option { + self.scripts.read().get(key).cloned() } - pub fn remove(&mut self, key: &StringKey) { - let value = self.scripts.shift_remove(key); + pub fn remove(&self, key: &StringKey) { + let value = self.scripts.write().shift_remove(key); if let Some(script) = value { if let Some(script) = script.get() { let script = script.read(); @@ -62,30 +66,35 @@ impl ScriptSet { } } - pub fn clear(&mut self) { - for script in &self.scripts { + pub fn clear(&self) { + for script in self.scripts.read().deref() { if let Some(script) = script.1.get() { let script = script.read(); script.as_ref().unwrap().on_remove(); script.as_ref().unwrap().mark_for_deletion(); } } - self.scripts.clear(); + self.scripts.write().clear(); } pub fn has(&self, key: &StringKey) -> bool { - self.scripts.contains_key(key) + self.scripts.read().contains_key(key) } - pub fn at(&self, index: usize) -> &ScriptContainer { - &self.scripts[index] + pub fn at(&self, index: usize) -> ScriptContainer { + self.scripts.read()[index].clone() } pub fn count(&self) -> usize { - self.scripts.len() + self.scripts.read().len() } - pub(crate) fn get_underlying(&self) -> &IndexMap { - &self.scripts + pub(crate) fn get_owning_iterator(&self) -> Vec { + let s = self.scripts.read(); + let mut v = Vec::with_capacity(s.deref().len()); + for script in s.deref().values() { + v.push(script.clone()); + } + v } } diff --git a/src/dynamic_data/script_handling/volatile_scripts.rs b/src/dynamic_data/script_handling/volatile_scripts.rs index b54407d..2de6d2e 100644 --- a/src/dynamic_data/script_handling/volatile_scripts.rs +++ b/src/dynamic_data/script_handling/volatile_scripts.rs @@ -1,30 +1,26 @@ use crate::dynamic_data::script_handling::script::{Script, ScriptContainer}; use crate::dynamic_data::script_handling::script_set::ScriptSet; use crate::{PkmnResult, StringKey}; -use parking_lot::RwLock; use std::sync::Arc; pub trait VolatileScripts<'a> { - fn volatile_scripts(&self) -> &Arc>; + fn volatile_scripts(&self) -> &Arc; fn load_volatile_script(&self, key: &StringKey) -> PkmnResult>>; fn has_volatile_script(&self, key: &StringKey) -> bool { - self.volatile_scripts().read().has(key) + self.volatile_scripts().has(key) } fn get_volatile_script(&self, key: &StringKey) -> Option { - let scripts = self.volatile_scripts().read(); - let s = scripts.get(key); - s.cloned() + self.volatile_scripts().get(key) } fn add_volatile_script(&mut self, key: &StringKey) -> PkmnResult> { self.volatile_scripts() - .write() .stack_or_add(key, &|| self.load_volatile_script(key)) } fn remove_volatile_script(&mut self, key: &StringKey) { - self.volatile_scripts().write().remove(key) + self.volatile_scripts().remove(key) } } diff --git a/src/static_data/libraries/move_library.rs b/src/static_data/libraries/move_library.rs index 2b8d3d6..6b91ede 100644 --- a/src/static_data/libraries/move_library.rs +++ b/src/static_data/libraries/move_library.rs @@ -37,7 +37,6 @@ pub mod tests { use crate::static_data::libraries::data_library::DataLibrary; use crate::static_data::libraries::move_library::MoveLibrary; use crate::static_data::moves::move_data::{MoveCategory, MoveData, MoveTarget}; - use crate::static_data::moves::secondary_effect::SecondaryEffect; use crate::StringKey; use hashbrown::HashSet; @@ -51,7 +50,7 @@ pub mod tests { 30, MoveTarget::Any, 0, - SecondaryEffect::empty(), + None, HashSet::new(), ) } diff --git a/src/static_data/moves/move_data.rs b/src/static_data/moves/move_data.rs index 946e501..a3e63a9 100644 --- a/src/static_data/moves/move_data.rs +++ b/src/static_data/moves/move_data.rs @@ -45,7 +45,7 @@ pub struct MoveData { base_usages: u8, target: MoveTarget, priority: i8, - secondary_effect: SecondaryEffect, + secondary_effect: Option, flags: HashSet, } @@ -59,7 +59,7 @@ impl MoveData { base_usages: u8, target: MoveTarget, priority: i8, - secondary_effect: SecondaryEffect, + secondary_effect: Option, flags: HashSet, ) -> MoveData { MoveData { @@ -102,14 +102,10 @@ impl MoveData { self.priority } - pub fn secondary_effect(&self) -> &SecondaryEffect { + pub fn secondary_effect(&self) -> &Option { &self.secondary_effect } - pub fn has_secondary_effect(&self) -> bool { - self.secondary_effect.effect_name() != &StringKey::empty() - } - pub fn has_flag(&self, key: &StringKey) -> bool { self.flags.contains(key) } diff --git a/src/static_data/moves/secondary_effect.rs b/src/static_data/moves/secondary_effect.rs index a511a59..90588c9 100644 --- a/src/static_data/moves/secondary_effect.rs +++ b/src/static_data/moves/secondary_effect.rs @@ -16,13 +16,6 @@ pub struct SecondaryEffect { } impl SecondaryEffect { - pub fn empty() -> SecondaryEffect { - SecondaryEffect { - chance: 0.0, - effect_name: StringKey::empty(), - parameters: vec![], - } - } pub fn new(chance: f32, effect_name: StringKey, parameters: Vec) -> SecondaryEffect { SecondaryEffect { chance, @@ -49,7 +42,7 @@ mod tests { #[test] fn create_secondary_effect() { - let empty = SecondaryEffect::empty(); + let empty = SecondaryEffect::new(0.0, "".into(), vec![]); assert_approx_eq!(empty.chance(), 0.0); assert_eq!(empty.effect_name(), &"".into()); assert_eq!(empty.parameters().len(), 0); diff --git a/tests/common/library_loader.rs b/tests/common/library_loader.rs index ea92d57..d3f8649 100644 --- a/tests/common/library_loader.rs +++ b/tests/common/library_loader.rs @@ -188,9 +188,13 @@ pub fn load_moves(path: &String, lib: &mut StaticData) { } } - SecondaryEffect::new(chance, StringKey::new(v["name"].as_str().unwrap()), parameters) + Some(SecondaryEffect::new( + chance, + StringKey::new(v["name"].as_str().unwrap()), + parameters, + )) } else { - SecondaryEffect::empty() + None }; let mut flags = HashSet::new();