Use enums instead of bools in public API

Replace the boolean parameters for including user metadata and evaluating conditions with MergeMode and EvalMode enums.

The C++ and Python wrappers still use booleans because they're more constrained than enums in those languages, which can be given invalid values.
This commit is contained in:
Oliver Hamlet
2025-08-02 16:05:36 +01:00
parent 4e74a174c5
commit d4d6893491
6 changed files with 210 additions and 75 deletions
+25 -5
View File
@@ -4,7 +4,7 @@ use std::{
};
use delegate::delegate;
use libloot::{WriteMode, error::DatabaseLockPoisonError};
use libloot::{EvalMode, MergeMode, WriteMode, error::DatabaseLockPoisonError};
use libloot_ffi_errors::UnsupportedEnumValueError;
use crate::{
@@ -109,7 +109,7 @@ impl Database {
self.0
.write()
.map_err(DatabaseLockPoisonError::from)?
.general_messages(evaluate_conditions)
.general_messages(to_eval_mode(evaluate_conditions))
.map(|v| v.into_iter().map(Into::into).collect())
.map_err(Into::into)
}
@@ -119,7 +119,7 @@ impl Database {
.0
.read()
.map_err(DatabaseLockPoisonError::from)?
.groups(include_user_metadata)
.groups(to_merge_mode(include_user_metadata))
.into_iter()
.map(Into::into)
.collect())
@@ -170,7 +170,11 @@ impl Database {
self.0
.read()
.map_err(DatabaseLockPoisonError::from)?
.plugin_metadata(plugin_name, include_user_metadata, evaluate_conditions)
.plugin_metadata(
plugin_name,
to_merge_mode(include_user_metadata),
to_eval_mode(evaluate_conditions),
)
.map(|p| Box::new(p.map(Into::into).into()))
.map_err(Into::into)
}
@@ -183,7 +187,7 @@ impl Database {
self.0
.read()
.map_err(DatabaseLockPoisonError::from)?
.plugin_user_metadata(plugin_name, evaluate_conditions)
.plugin_user_metadata(plugin_name, to_eval_mode(evaluate_conditions))
.map(|p| Box::new(p.map(Into::into).into()))
.map_err(Into::into)
}
@@ -216,6 +220,22 @@ impl Database {
}
}
fn to_eval_mode(value: bool) -> EvalMode {
if value {
EvalMode::Evaluate
} else {
EvalMode::DoNotEvaluate
}
}
fn to_merge_mode(value: bool) -> MergeMode {
if value {
MergeMode::WithUserMetadata
} else {
MergeMode::WithoutUserMetadata
}
}
#[derive(Clone, Debug)]
#[repr(transparent)]
pub struct Vertex(libloot::Vertex);
+45 -9
View File
@@ -12,6 +12,38 @@ use crate::{
metadata::{Group, Message, PluginMetadata},
};
#[napi]
#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
pub enum EvalMode {
DoNotEvaluate,
Evaluate,
}
impl From<EvalMode> for libloot::EvalMode {
fn from(value: EvalMode) -> Self {
match value {
EvalMode::DoNotEvaluate => libloot::EvalMode::DoNotEvaluate,
EvalMode::Evaluate => libloot::EvalMode::Evaluate,
}
}
}
#[napi]
#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
pub enum MergeMode {
WithoutUserMetadata,
WithUserMetadata,
}
impl From<MergeMode> for libloot::MergeMode {
fn from(value: MergeMode) -> Self {
match value {
MergeMode::WithoutUserMetadata => libloot::MergeMode::WithoutUserMetadata,
MergeMode::WithUserMetadata => libloot::MergeMode::WithUserMetadata,
}
}
}
#[napi]
#[derive(Clone, Debug)]
pub struct Database(Arc<RwLock<libloot::Database>>);
@@ -108,23 +140,23 @@ impl Database {
#[napi]
pub fn general_messages(
&self,
evaluate_conditions: bool,
evaluate_conditions: EvalMode,
) -> Result<Vec<Message>, VerboseError> {
self.0
.write()
.map_err(DatabaseLockPoisonError::from)?
.general_messages(evaluate_conditions)
.general_messages(evaluate_conditions.into())
.map(|v| v.into_iter().map(Into::into).collect())
.map_err(Into::into)
}
#[napi]
pub fn groups(&self, include_user_metadata: bool) -> Result<Vec<Group>, VerboseError> {
pub fn groups(&self, include_user_metadata: MergeMode) -> Result<Vec<Group>, VerboseError> {
Ok(self
.0
.read()
.map_err(DatabaseLockPoisonError::from)?
.groups(include_user_metadata)
.groups(include_user_metadata.into())
.into_iter()
.map(Into::into)
.collect())
@@ -171,13 +203,17 @@ impl Database {
pub fn plugin_metadata(
&self,
plugin_name: String,
include_user_metadata: bool,
evaluate_conditions: bool,
include_user_metadata: MergeMode,
evaluate_conditions: EvalMode,
) -> Result<Option<PluginMetadata>, VerboseError> {
self.0
.read()
.map_err(DatabaseLockPoisonError::from)?
.plugin_metadata(&plugin_name, include_user_metadata, evaluate_conditions)
.plugin_metadata(
&plugin_name,
include_user_metadata.into(),
evaluate_conditions.into(),
)
.map(|p| p.map(Into::into))
.map_err(Into::into)
}
@@ -186,12 +222,12 @@ impl Database {
pub fn plugin_user_metadata(
&self,
plugin_name: String,
evaluate_conditions: bool,
evaluate_conditions: EvalMode,
) -> Result<Option<PluginMetadata>, VerboseError> {
self.0
.read()
.map_err(DatabaseLockPoisonError::from)?
.plugin_user_metadata(&plugin_name, evaluate_conditions)
.plugin_user_metadata(&plugin_name, evaluate_conditions.into())
.map(|p| p.map(Into::into))
.map_err(Into::into)
}
+25 -5
View File
@@ -3,7 +3,7 @@ use std::{
sync::{Arc, RwLock},
};
use libloot::{WriteMode, error::DatabaseLockPoisonError};
use libloot::{EvalMode, MergeMode, WriteMode, error::DatabaseLockPoisonError};
use libloot_ffi_errors::UnsupportedEnumValueError;
use pyo3::{
Bound, PyResult, pyclass, pymethods,
@@ -113,7 +113,7 @@ impl Database {
self.0
.write()
.map_err(DatabaseLockPoisonError::from)?
.general_messages(evaluate_conditions)
.general_messages(to_eval_mode(evaluate_conditions))
.map(|v| v.into_iter().map(Into::into).collect())
.map_err(Into::into)
}
@@ -123,7 +123,7 @@ impl Database {
.0
.read()
.map_err(DatabaseLockPoisonError::from)?
.groups(include_user_metadata)
.groups(to_merge_mode(include_user_metadata))
.into_iter()
.map(Into::into)
.collect())
@@ -172,7 +172,11 @@ impl Database {
self.0
.read()
.map_err(DatabaseLockPoisonError::from)?
.plugin_metadata(plugin_name, include_user_metadata, evaluate_conditions)
.plugin_metadata(
plugin_name,
to_merge_mode(include_user_metadata),
to_eval_mode(evaluate_conditions),
)
.map(|p| p.map(Into::into))
.map_err(Into::into)
}
@@ -185,7 +189,7 @@ impl Database {
self.0
.read()
.map_err(DatabaseLockPoisonError::from)?
.plugin_user_metadata(plugin_name, evaluate_conditions)
.plugin_user_metadata(plugin_name, to_eval_mode(evaluate_conditions))
.map(|p| p.map(Into::into))
.map_err(Into::into)
}
@@ -224,6 +228,22 @@ impl From<Arc<RwLock<libloot::Database>>> for Database {
}
}
fn to_eval_mode(value: bool) -> EvalMode {
if value {
EvalMode::Evaluate
} else {
EvalMode::DoNotEvaluate
}
}
fn to_merge_mode(value: bool) -> MergeMode {
if value {
MergeMode::WithUserMetadata
} else {
MergeMode::WithoutUserMetadata
}
}
#[pyclass(eq, ord, frozen, hash, str = "{0:?}")]
#[derive(Clone, Debug, Default, Eq, PartialEq, Ord, PartialOrd, Hash)]
#[repr(transparent)]
+101 -49
View File
@@ -30,6 +30,27 @@ pub enum WriteMode {
CreateOrTruncate,
}
/// Control whether user metadata is included or not when retrieving metadata.
#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
#[expect(clippy::exhaustive_enums, reason = "It's effectively a boolean")]
pub enum MergeMode {
/// Do not include user metadata in the return value.
WithoutUserMetadata,
/// Include user metadata in the return value.
WithUserMetadata,
}
/// Control whether or not conditions are evaluated when retrieving metadata.
#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
#[expect(clippy::exhaustive_enums, reason = "It's effectively a boolean")]
pub enum EvalMode {
/// Do not evaluate metadata conditions when retrieving metadata.
DoNotEvaluate,
/// Evaluate metadata conditions when retrieving metadata, filtering out
/// metadata with conditions that evaluate to false.
Evaluate,
}
/// The interface through which metadata can be accessed.
#[derive(Debug)]
pub struct Database {
@@ -145,15 +166,13 @@ impl Database {
/// Get all general messages listed in the loaded metadata lists.
///
/// If `evaluate_conditions` is `true`, any metadata conditions are
/// evaluated before the metadata is returned, otherwise unevaluated
/// metadata is returned. Evaluating general message conditions also clears
/// the condition cache before evaluating conditions.
/// Evaluating general message conditions also clears the condition cache
/// before evaluating conditions.
pub fn general_messages(
&mut self,
evaluate_conditions: bool,
evaluate_conditions: EvalMode,
) -> Result<Vec<Message>, ConditionEvaluationError> {
if evaluate_conditions {
if evaluate_conditions == EvalMode::Evaluate {
self.clear_condition_cache();
}
@@ -163,7 +182,7 @@ impl Database {
.iter()
.chain(self.userlist.messages());
if evaluate_conditions {
if evaluate_conditions == EvalMode::Evaluate {
let messages = messages_iter
.filter_map(|m| {
filter_map_on_condition(m, m.condition(), &self.condition_evaluator_state)
@@ -177,12 +196,8 @@ impl Database {
}
/// Gets the groups that are defined in the loaded metadata lists.
///
/// If `include_user_metadata` is `true`, any group metadata present in the
/// userlist is included in the returned metadata, otherwise the metadata
/// returned only includes metadata from the masterlist.
pub fn groups(&self, include_user_metadata: bool) -> Vec<Group> {
if include_user_metadata {
pub fn groups(&self, include_user_metadata: MergeMode) -> Vec<Group> {
if include_user_metadata == MergeMode::WithUserMetadata {
merge_groups(self.masterlist.groups(), self.userlist.groups())
} else {
self.masterlist.groups().to_vec()
@@ -223,23 +238,17 @@ impl Database {
/// Get all of a plugin's loaded metadata.
///
/// If `include_user_metadata` is `true`, any user metadata the plugin has
/// is included in the returned metadata, otherwise the metadata returned
/// only includes metadata from the masterlist.
///
/// If `evaluateConditions` is `true`, any metadata conditions are evaluated
/// before the metadata otherwise unevaluated metadata is returned.
/// Evaluating plugin metadata conditions does **not** clear the condition
/// cache.
pub fn plugin_metadata(
&self,
plugin_name: &str,
include_user_metadata: bool,
evaluate_conditions: bool,
include_user_metadata: MergeMode,
evaluate_conditions: EvalMode,
) -> Result<Option<PluginMetadata>, MetadataRetrievalError> {
let mut metadata = self.masterlist.find_plugin(plugin_name)?;
if include_user_metadata {
if include_user_metadata == MergeMode::WithUserMetadata {
if let Some(mut user_metadata) = self.userlist.find_plugin(plugin_name)? {
if let Some(metadata) = metadata {
user_metadata.merge_metadata(&metadata);
@@ -248,7 +257,7 @@ impl Database {
}
}
if evaluate_conditions {
if evaluate_conditions == EvalMode::Evaluate {
if let Some(metadata) = metadata {
return evaluate_all_conditions(metadata, &self.condition_evaluator_state)
.map_err(Into::into);
@@ -260,18 +269,16 @@ impl Database {
/// Get a plugin's metadata loaded from the given userlist.
///
/// If `evaluateConditions` is `true`, any metadata conditions are evaluated
/// before the metadata otherwise unevaluated metadata is returned.
/// Evaluating plugin metadata conditions does **not** clear the condition
/// cache.
pub fn plugin_user_metadata(
&self,
plugin_name: &str,
evaluate_conditions: bool,
evaluate_conditions: EvalMode,
) -> Result<Option<PluginMetadata>, MetadataRetrievalError> {
let metadata = self.userlist.find_plugin(plugin_name)?;
if evaluate_conditions {
if evaluate_conditions == EvalMode::Evaluate {
if let Some(metadata) = metadata {
return evaluate_all_conditions(metadata, &self.condition_evaluator_state)
.map_err(Into::into);
@@ -780,7 +787,10 @@ plugins:
.with_condition("file(\"missing.esp\")".into()),
Message::new(MessageType::Say, "A user message".into())
],
database.general_messages(false).unwrap().as_slice()
database
.general_messages(EvalMode::DoNotEvaluate)
.unwrap()
.as_slice()
);
}
@@ -802,7 +812,10 @@ plugins:
assert_eq!(
&[Message::new(MessageType::Say, "A user message".into())],
database.general_messages(true).unwrap().as_slice()
database
.general_messages(EvalMode::Evaluate)
.unwrap()
.as_slice()
);
}
}
@@ -835,7 +848,10 @@ plugins:
let fixture = Fixture::new(GameType::Oblivion);
let database = fixture.database();
assert_eq!(&[Group::default(),], database.groups(true).as_slice());
assert_eq!(
&[Group::default(),],
database.groups(MergeMode::WithUserMetadata).as_slice()
);
}
#[test]
@@ -860,7 +876,7 @@ plugins:
Group::new("group1".into()),
Group::new("group2".into()).with_after_groups(vec!["group1".into()])
],
database.groups(false).as_slice()
database.groups(MergeMode::WithoutUserMetadata).as_slice()
);
}
@@ -888,7 +904,7 @@ plugins:
.with_after_groups(vec!["group1".into(), "default".into()]),
Group::new("group3".into()).with_after_groups(vec!["group1".into()])
],
database.groups(true).as_slice()
database.groups(MergeMode::WithUserMetadata).as_slice()
);
}
}
@@ -927,7 +943,7 @@ plugins:
Group::new("group1".into()),
Group::new("group2".into()).with_after_groups(vec!["group1".into()])
],
database.groups(false).as_slice()
database.groups(MergeMode::WithoutUserMetadata).as_slice()
);
assert_eq!(
@@ -969,7 +985,11 @@ plugins:
assert!(
database
.plugin_metadata(BLANK_ESM, true, false)
.plugin_metadata(
BLANK_ESM,
MergeMode::WithUserMetadata,
EvalMode::DoNotEvaluate
)
.unwrap()
.is_none()
);
@@ -984,7 +1004,11 @@ plugins:
assert!(
database
.plugin_metadata(BLANK_ESM, true, false)
.plugin_metadata(
BLANK_ESM,
MergeMode::WithUserMetadata,
EvalMode::DoNotEvaluate
)
.unwrap()
.is_none()
);
@@ -1008,7 +1032,11 @@ plugins:
File::new("Oblivion.esm".into())
],
database
.plugin_metadata(BLANK_ESM, true, false)
.plugin_metadata(
BLANK_ESM,
MergeMode::WithUserMetadata,
EvalMode::DoNotEvaluate
)
.unwrap()
.unwrap()
.load_after_files()
@@ -1030,7 +1058,11 @@ plugins:
assert_eq!(
&[File::new("Oblivion.esm".into())],
database
.plugin_metadata(BLANK_ESM, false, false)
.plugin_metadata(
BLANK_ESM,
MergeMode::WithoutUserMetadata,
EvalMode::DoNotEvaluate
)
.unwrap()
.unwrap()
.load_after_files()
@@ -1054,7 +1086,7 @@ plugins:
assert!(
database
.plugin_metadata(BLANK_ESM, true, true)
.plugin_metadata(BLANK_ESM, MergeMode::WithUserMetadata, EvalMode::Evaluate)
.unwrap()
.unwrap()
.messages()
@@ -1073,7 +1105,7 @@ plugins:
assert!(
database
.plugin_user_metadata(BLANK_ESM, false)
.plugin_user_metadata(BLANK_ESM, EvalMode::DoNotEvaluate)
.unwrap()
.is_none()
);
@@ -1088,7 +1120,7 @@ plugins:
assert!(
database
.plugin_user_metadata(BLANK_ESM, false)
.plugin_user_metadata(BLANK_ESM, EvalMode::DoNotEvaluate)
.unwrap()
.is_none()
);
@@ -1109,7 +1141,7 @@ plugins:
assert_eq!(
&[File::new(BLANK_DIFFERENT_ESM.into())],
database
.plugin_user_metadata(BLANK_ESM, false)
.plugin_user_metadata(BLANK_ESM, EvalMode::DoNotEvaluate)
.unwrap()
.unwrap()
.load_after_files()
@@ -1133,7 +1165,7 @@ plugins:
assert!(
database
.plugin_user_metadata(BLANK_ESM, true)
.plugin_user_metadata(BLANK_ESM, EvalMode::Evaluate)
.unwrap()
.is_none()
);
@@ -1162,7 +1194,7 @@ plugins:
assert_eq!(
&[File::new(BLANK_MASTER_DEPENDENT_ESM.into())],
database
.plugin_user_metadata(BLANK_ESM, false)
.plugin_user_metadata(BLANK_ESM, EvalMode::DoNotEvaluate)
.unwrap()
.unwrap()
.load_after_files()
@@ -1187,7 +1219,11 @@ plugins:
File::new("Oblivion.esm".into()),
],
database
.plugin_metadata(BLANK_ESM, true, false)
.plugin_metadata(
BLANK_ESM,
MergeMode::WithUserMetadata,
EvalMode::DoNotEvaluate
)
.unwrap()
.unwrap()
.load_after_files()
@@ -1216,7 +1252,11 @@ plugins:
assert_eq!(
&[File::new("Oblivion.esm".into())],
database
.plugin_metadata(BLANK_ESM, true, false)
.plugin_metadata(
BLANK_ESM,
MergeMode::WithUserMetadata,
EvalMode::DoNotEvaluate
)
.unwrap()
.unwrap()
.load_after_files()
@@ -1227,7 +1267,11 @@ plugins:
File::new(BLANK_MASTER_DEPENDENT_ESM.into()),
],
database
.plugin_metadata(BLANK_DIFFERENT_ESM, true, false)
.plugin_metadata(
BLANK_DIFFERENT_ESM,
MergeMode::WithUserMetadata,
EvalMode::DoNotEvaluate
)
.unwrap()
.unwrap()
.load_after_files()
@@ -1259,12 +1303,16 @@ plugins:
Group::new("group1".into()),
Group::new("group2".into()).with_after_groups(vec!["group1".into()])
],
database.groups(true).as_slice()
database.groups(MergeMode::WithUserMetadata).as_slice()
);
assert_eq!(
&[File::new("Oblivion.esm".into())],
database
.plugin_metadata(BLANK_ESM, true, false)
.plugin_metadata(
BLANK_ESM,
MergeMode::WithUserMetadata,
EvalMode::DoNotEvaluate
)
.unwrap()
.unwrap()
.load_after_files()
@@ -1272,7 +1320,11 @@ plugins:
assert_eq!(
&[File::new(BLANK_MASTER_DEPENDENT_ESM.into()),],
database
.plugin_metadata(BLANK_DIFFERENT_ESM, true, false)
.plugin_metadata(
BLANK_DIFFERENT_ESM,
MergeMode::WithUserMetadata,
EvalMode::DoNotEvaluate
)
.unwrap()
.unwrap()
.load_after_files()
+13 -6
View File
@@ -9,7 +9,7 @@ use loadorder::WritableLoadOrder;
use rayon::iter::{IntoParallelRefIterator, ParallelIterator};
use crate::{
LogLevel,
EvalMode, LogLevel, MergeMode,
database::Database,
error::{
DatabaseLockPoisonError, GameHandleCreationError, LoadOrderError, LoadOrderStateError,
@@ -467,7 +467,10 @@ impl Game {
}
}
let groups_graph = build_groups_graph(&database.groups(false), database.user_groups())?;
let groups_graph = build_groups_graph(
&database.groups(MergeMode::WithoutUserMetadata),
database.user_groups(),
)?;
let new_load_order = sort_plugins(
plugins_sorting_data,
@@ -748,12 +751,16 @@ fn to_plugin_sorting_data<'a>(
load_order_index: usize,
) -> Result<PluginSortingData<'a, Plugin>, SortPluginsError> {
let masterlist_metadata = database
.plugin_metadata(plugin.name(), false, true)?
.plugin_metadata(
plugin.name(),
MergeMode::WithoutUserMetadata,
EvalMode::Evaluate,
)?
.map(|m| m.filter_by_constraints(database))
.transpose()?;
let user_metadata = database
.plugin_user_metadata(plugin.name(), true)?
.plugin_user_metadata(plugin.name(), EvalMode::Evaluate)?
.map(|m| m.filter_by_constraints(database))
.transpose()?;
@@ -1211,7 +1218,7 @@ mod tests {
.database()
.read()
.unwrap()
.plugin_user_metadata(BLANK_ESM, true)
.plugin_user_metadata(BLANK_ESM, EvalMode::Evaluate)
.unwrap();
assert!(evaluated_metadata.is_none());
@@ -1223,7 +1230,7 @@ mod tests {
.database()
.read()
.unwrap()
.plugin_user_metadata(BLANK_ESM, true)
.plugin_user_metadata(BLANK_ESM, EvalMode::Evaluate)
.unwrap()
.unwrap();
assert!(!evaluated_metadata.load_after_files().is_empty());
+1 -1
View File
@@ -32,7 +32,7 @@ use std::{path::Path, slice::EscapeAscii};
use regress::{Error as RegexImplError, Regex};
pub use database::{Database, WriteMode};
pub use database::{Database, EvalMode, MergeMode, WriteMode};
pub use game::{Game, GameType};
pub use logging::{LogLevel, set_log_level, set_logging_callback};
pub use plugin::Plugin;