diff --git a/cxx/src/api/game.cpp b/cxx/src/api/game.cpp index f09079b4..836044b8 100644 --- a/cxx/src/api/game.cpp +++ b/cxx/src/api/game.cpp @@ -189,7 +189,7 @@ const PluginInterface* Game::GetPlugin(const std::string& pluginName) const { } try { - auto plugin = std::make_shared(std::move(pluginOpt->as_ref())); + auto plugin = std::make_shared(std::move(pluginOpt->as_ref().boxed_clone())); const auto result = plugins_.emplace(key, plugin); return result.first->second.get(); diff --git a/cxx/src/api/plugin.cpp b/cxx/src/api/plugin.cpp index ddf6c208..592a1a53 100644 --- a/cxx/src/api/plugin.cpp +++ b/cxx/src/api/plugin.cpp @@ -7,7 +7,7 @@ #include "api/exception.h" namespace loot { -Plugin::Plugin(::rust::Box plugin) : +Plugin::Plugin(::rust::Box plugin) : plugin_(std::move(plugin)) {} std::string Plugin::GetName() const { return std::string(plugin_->name()); } diff --git a/cxx/src/api/plugin.h b/cxx/src/api/plugin.h index b4ecd523..79eb6d40 100644 --- a/cxx/src/api/plugin.h +++ b/cxx/src/api/plugin.h @@ -16,7 +16,7 @@ namespace loot { class Plugin final : public PluginInterface { public: - explicit Plugin(::rust::Box plugin); + explicit Plugin(::rust::Box plugin); std::string GetName() const override; std::optional GetHeaderVersion() const override; @@ -39,7 +39,7 @@ public: bool DoRecordsOverlap(const PluginInterface& plugin) const override; private: - ::rust::Box plugin_; + ::rust::Box plugin_; }; } diff --git a/cxx/src/game.rs b/cxx/src/game.rs index 147ebfa7..6236edaa 100644 --- a/cxx/src/game.rs +++ b/cxx/src/game.rs @@ -3,8 +3,8 @@ use std::path::Path; use delegate::delegate; use crate::{ - PluginRef, UnsupportedEnumValueError, VerboseError, database::Database, ffi::GameType, - plugin::OptionalPluginRef, + Plugin, UnsupportedEnumValueError, VerboseError, database::Database, ffi::GameType, + plugin::OptionalPlugin, }; impl TryFrom for GameType { @@ -150,11 +150,11 @@ impl Game { .map_err(Into::into) } - pub fn plugin(&self, plugin_name: &str) -> Box { + pub fn plugin(&self, plugin_name: &str) -> Box { Box::new(self.0.plugin(plugin_name).into()) } - pub fn loaded_plugins(&self) -> Vec> { + pub fn loaded_plugins(&self) -> Vec { self.0 .loaded_plugins() .into_iter() diff --git a/cxx/src/lib.rs b/cxx/src/lib.rs index ad23b29b..a56c2e5e 100644 --- a/cxx/src/lib.rs +++ b/cxx/src/lib.rs @@ -13,7 +13,7 @@ use metadata::{ new_location, new_message, new_message_content, new_plugin_cleaning_data, new_plugin_metadata, new_tag, select_message_content, }; -use plugin::{OptionalPluginRef, PluginRef}; +use plugin::{OptionalPlugin, Plugin}; use std::{ error::Error, ffi::{CString, c_char, c_uchar, c_uint, c_void}, @@ -407,10 +407,10 @@ mod ffi { pub fn clear_loaded_plugins(&mut self); // The plugin's lifetime is actually less than &Game's, it's only valid so long as the loaded plugin is not overwritten or cleared. - pub fn plugin(&self, plugin_name: &str) -> Box; + pub fn plugin(&self, plugin_name: &str) -> Box; // The plugin's lifetime is actually less than &Game's, it's only valid so long as the loaded plugin is not overwritten or cleared. - pub unsafe fn loaded_plugins<'a>(&'a self) -> Vec>; + pub fn loaded_plugins(&self) -> Vec; pub fn sort_plugins(&self, plugin_names: &[&str]) -> Result>; @@ -541,19 +541,19 @@ mod ffi { } extern "Rust" { - type PluginRef<'a>; + type Plugin; - pub unsafe fn name<'a>(&'a self) -> &'a str; + pub fn name(&self) -> &str; // The None case is signalled by NaN. pub fn header_version(&self) -> f32; // The None case is signalled by an empty string (which is not a valid version). - pub unsafe fn version<'a>(&'a self) -> &'a str; + pub fn version(&self) -> &str; pub fn masters(&self) -> Result>; - pub unsafe fn bash_tags<'a>(&'a self) -> &'a [String]; + pub fn bash_tags(&self) -> &[String]; // The None case is signalled by -1, all other values fit in u32. pub fn crc(&self) -> i64; @@ -578,18 +578,18 @@ mod ffi { pub fn loads_archive(&self) -> bool; - pub unsafe fn do_records_overlap<'a>(&self, plugin: &PluginRef<'a>) -> Result; + pub fn do_records_overlap(&self, plugin: &Plugin) -> Result; - pub unsafe fn boxed_clone<'a>(&'a self) -> Box>; + pub fn boxed_clone(&self) -> Box; } extern "Rust" { - type OptionalPluginRef; + type OptionalPlugin; pub fn is_some(&self) -> bool; // Again, these lifetimes are wrong. - pub unsafe fn as_ref<'a>(&'a self) -> Result>>; + pub unsafe fn as_ref<'a>(&'a self) -> Result<&'a Plugin>; } extern "Rust" { diff --git a/cxx/src/plugin.rs b/cxx/src/plugin.rs index 59b9267d..54365f84 100644 --- a/cxx/src/plugin.rs +++ b/cxx/src/plugin.rs @@ -1,13 +1,15 @@ +use std::sync::Arc; + use delegate::delegate; -use crate::{EmptyOptionalError, OptionalRef, VerboseError}; +use crate::{Optional, VerboseError}; #[derive(Debug)] #[repr(transparent)] -pub struct PluginRef<'a>(&'a libloot::Plugin); +pub struct Plugin(Arc); -impl<'a> PluginRef<'a> { - pub fn new(plugin: &'a libloot::Plugin) -> Self { +impl Plugin { + pub fn new(plugin: Arc) -> Self { Self(plugin) } @@ -40,11 +42,11 @@ impl<'a> PluginRef<'a> { } pub fn do_records_overlap(&self, plugin: &Self) -> Result { - self.0.do_records_overlap(plugin.0).map_err(Into::into) + self.0.do_records_overlap(&plugin.0).map_err(Into::into) } pub fn boxed_clone(&self) -> Box { - Box::new(Self(self.0)) + Box::new(Self(self.0.clone())) } delegate! { @@ -70,23 +72,16 @@ impl<'a> PluginRef<'a> { } } -impl<'a> From<&'a libloot::Plugin> for PluginRef<'a> { - fn from(value: &'a libloot::Plugin) -> Self { - PluginRef(value) +impl From> for Plugin { + fn from(value: Arc) -> Self { + Plugin(value) } } -pub type OptionalPluginRef = OptionalRef; +pub type OptionalPlugin = Optional; -impl OptionalRef { - /// # Safety - /// - /// This is safe as long as the pointer in the OptionalRef is still valid. - pub unsafe fn as_ref(&self) -> Result>, EmptyOptionalError> { - if self.0.is_null() { - Err(EmptyOptionalError) - } else { - unsafe { Ok(Box::new(PluginRef::new(&*self.0))) } - } +impl From>> for Optional { + fn from(value: Option>) -> Self { + Self(value.map(Into::into)) } } diff --git a/pyo3/README.md b/pyo3/README.md index 771b8fcf..806c886b 100644 --- a/pyo3/README.md +++ b/pyo3/README.md @@ -16,7 +16,7 @@ An **incomplete** and **experimental** Python wrapper around the libloot Rust im - [x] `GameType` - [ ] `LogLevel` - [x] `Database` -- [ ] `Game` +- [x] `Game` - [x] `Plugin` - [x] `Vertex` - [x] `File` diff --git a/pyo3/src/game.rs b/pyo3/src/game.rs index 78459fec..66757372 100644 --- a/pyo3/src/game.rs +++ b/pyo3/src/game.rs @@ -120,15 +120,17 @@ impl Game { self.0.clear_loaded_plugins(); } - // TODO: Game::plugin(). - // fn plugin(&self, plugin_name: &str) -> Option<&Plugin> { - // self.0.plugin(plugin_name).map(Plugin::wrap) - // } + fn plugin(&self, plugin_name: &str) -> Option { + self.0.plugin(plugin_name).map(Into::into) + } - // TODO: Game::loaded_plugins(). - // fn loaded_plugins(&self) -> Vec<&Plugin> { - // wrap_plugins(self.0.loaded_plugins()) - // } + fn loaded_plugins(&self) -> Vec { + self.0 + .loaded_plugins() + .into_iter() + .map(Into::into) + .collect() + } fn sort_plugins(&self, plugin_names: Vec) -> Result, VerboseError> { Ok(self.0.sort_plugins(&as_strs(&plugin_names))?) diff --git a/pyo3/src/plugin.rs b/pyo3/src/plugin.rs index 37ed9ff9..b897cedb 100644 --- a/pyo3/src/plugin.rs +++ b/pyo3/src/plugin.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use pyo3::{pyclass, pymethods}; use crate::VerboseError; @@ -5,7 +7,7 @@ use crate::VerboseError; #[pyclass(eq, frozen)] #[derive(Clone, Debug, Eq, PartialEq)] #[repr(transparent)] -pub struct Plugin(libloot::Plugin); +pub struct Plugin(Arc); #[pymethods] impl Plugin { @@ -78,18 +80,8 @@ impl Plugin { } } -impl Plugin { - pub fn wrap(plugin: &libloot::Plugin) -> &Plugin { - let p = plugin as *const libloot::Plugin; - // SAFETY: Plugin is a transparent wrapper, so reinterpreting the pointer is safe. - unsafe { - let p = p as *const Plugin; - &*p - } +impl From> for Plugin { + fn from(value: Arc) -> Self { + Self(value) } } - -pub fn wrap_plugins(mut vec: Vec<&libloot::Plugin>) -> Vec<&Plugin> { - // SAFETY: This is safe because Plugin is a transparent wrapper around the libloot type. - unsafe { Vec::from_raw_parts(vec.as_mut_ptr().cast(), vec.len(), vec.capacity()) } -} diff --git a/src/game.rs b/src/game.rs index 0de70510..1b6e218c 100644 --- a/src/game.rs +++ b/src/game.rs @@ -335,7 +335,7 @@ impl Game { .cache .plugins() .iter() - .map(|(k, v)| (k.clone(), v)) + .map(|(k, v)| (k.clone(), v.as_ref())) .collect(); for plugin in &plugins { @@ -420,13 +420,13 @@ impl Game { } /// Get data for a loaded plugin. - pub fn plugin(&self, plugin_name: &str) -> Option<&Plugin> { - self.cache.plugin(plugin_name) + pub fn plugin(&self, plugin_name: &str) -> Option> { + self.cache.plugin(plugin_name).cloned() } /// Get data for all loaded plugins. - pub fn loaded_plugins(&self) -> Vec<&Plugin> { - self.cache.plugins_iter().collect() + pub fn loaded_plugins(&self) -> Vec> { + self.cache.plugins_iter().cloned().collect() } /// Calculates a new load order for the game's installed plugins (including @@ -444,7 +444,8 @@ impl Game { let plugins = plugin_names .iter() .map(|n| { - self.plugin(n) + self.cache + .plugin(n) .ok_or_else(|| SortPluginsError::PluginNotLoaded(n.to_string())) }) .collect::, _>>()?; @@ -458,7 +459,7 @@ impl Game { let masterlist_metadata = database.plugin_metadata(p.name(), false, true)?; let user_metadata = database.plugin_user_metadata(p.name(), true)?; let plugin = PluginSortingData::new( - p, + p.as_ref(), masterlist_metadata.as_ref(), user_metadata.as_ref(), i, @@ -711,7 +712,7 @@ fn resolve_plugin_path(game_type: GameType, data_path: &Path, plugin_path: &Path fn update_loaded_plugin_state<'a>( state: &mut loot_condition_interpreter::State, - plugins: impl Iterator, + plugins: impl Iterator>, ) { let mut plugin_versions = Vec::new(); let mut plugin_crcs = Vec::new(); @@ -748,7 +749,7 @@ fn update_loaded_plugin_state<'a>( #[derive(Clone, Debug, Default, Eq, PartialEq)] pub(crate) struct GameCache { - plugins: HashMap, + plugins: HashMap>, archive_paths: HashSet, } @@ -761,7 +762,7 @@ impl GameCache { fn insert_plugins(&mut self, plugins: Vec) { for plugin in plugins { self.plugins - .insert(Filename::new(plugin.name().to_string()), plugin); + .insert(Filename::new(plugin.name().to_string()), Arc::new(plugin)); } } @@ -769,15 +770,15 @@ impl GameCache { self.plugins.clear(); } - fn plugins(&self) -> &HashMap { + fn plugins(&self) -> &HashMap> { &self.plugins } - fn plugins_iter(&self) -> impl Iterator { + fn plugins_iter(&self) -> impl Iterator> { self.plugins.values() } - fn plugin(&self, plugin_name: &str) -> Option<&Plugin> { + fn plugin(&self, plugin_name: &str) -> Option<&Arc> { self.plugins.get(&Filename::new(plugin_name.to_string())) }