From 167c9b21135bacb2d777581efb912c7e7e0282c9 Mon Sep 17 00:00:00 2001 From: Maveth Date: Thu, 13 Jan 2022 11:51:17 -0600 Subject: [PATCH 1/4] Test for cyclic skill dependencies --- common/Cargo.toml | 1 + common/src/comp/skillset/mod.rs | 2 ++ common/src/comp/skillset/test.rs | 29 +++++++++++++++++++++++++++++ 3 files changed, 32 insertions(+) create mode 100644 common/src/comp/skillset/test.rs diff --git a/common/Cargo.toml b/common/Cargo.toml index 095d169e7f..e0c023cb93 100644 --- a/common/Cargo.toml +++ b/common/Cargo.toml @@ -88,6 +88,7 @@ criterion = "0.3" #test tracing-subscriber = { version = "0.3.2", default-features = false, features = ["fmt", "time", "ansi", "smallvec", "env-filter"] } +petgraph = "0.5.1" [[bench]] name = "chonk_benchmark" diff --git a/common/src/comp/skillset/mod.rs b/common/src/comp/skillset/mod.rs index 5ef2e4b325..aa7861620e 100644 --- a/common/src/comp/skillset/mod.rs +++ b/common/src/comp/skillset/mod.rs @@ -16,6 +16,8 @@ use tracing::{trace, warn}; pub mod skills; +#[cfg(test)] mod test; + /// BTreeSet is used here to ensure that skills are ordered. This is important /// to ensure that the hash created from it is consistent so that we don't /// needlessly force a respec when loading skills from persistence. diff --git a/common/src/comp/skillset/test.rs b/common/src/comp/skillset/test.rs new file mode 100644 index 0000000000..a9ced178f1 --- /dev/null +++ b/common/src/comp/skillset/test.rs @@ -0,0 +1,29 @@ +use super::*; +use crate::comp::{skillset::SkillPrerequisitesMap, Skill}; +use hashbrown::HashMap; + +#[cfg(test)] +use petgraph::{algo::is_cyclic_undirected, graph::UnGraph}; + +#[test] +fn check_cyclic_skill_deps() { + let skill_prereqs = + SkillPrerequisitesMap::load_expect_cloned("common.skill_trees.skill_prerequisites").0; + let mut graph = UnGraph::new_undirected(); + let mut nodes = HashMap::::new(); + let mut add_node = |graph: &mut UnGraph, node: Skill| { + *nodes + .entry(node.clone()) + .or_insert_with(|| graph.add_node(node.clone())) + }; + + for (skill, prereqs) in skill_prereqs.iter() { + let skill_node = add_node(&mut graph, *skill); + for (prereq, _) in prereqs.iter() { + let prereq_node = add_node(&mut graph, *prereq); + graph.add_edge(prereq_node, skill_node, ()); + } + } + + assert!(!is_cyclic_undirected(&graph)); +} From c8f083aa4484f6e51f5e91e3be1612de8ef068d4 Mon Sep 17 00:00:00 2001 From: Maveth Date: Thu, 13 Jan 2022 12:31:51 -0600 Subject: [PATCH 2/4] Thank you clippy, very cool --- common/src/comp/skillset/test.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/common/src/comp/skillset/test.rs b/common/src/comp/skillset/test.rs index a9ced178f1..def109c608 100644 --- a/common/src/comp/skillset/test.rs +++ b/common/src/comp/skillset/test.rs @@ -12,9 +12,7 @@ fn check_cyclic_skill_deps() { let mut graph = UnGraph::new_undirected(); let mut nodes = HashMap::::new(); let mut add_node = |graph: &mut UnGraph, node: Skill| { - *nodes - .entry(node.clone()) - .or_insert_with(|| graph.add_node(node.clone())) + *nodes.entry(node).or_insert_with(|| graph.add_node(node)) }; for (skill, prereqs) in skill_prereqs.iter() { From b8a12b582dfcc9b541dd3479b6d97d761fcab98b Mon Sep 17 00:00:00 2001 From: Maveth Date: Mon, 17 Jan 2022 10:18:32 -0600 Subject: [PATCH 3/4] Revert "Thank you clippy, very cool" This reverts commit c8f083aa4484f6e51f5e91e3be1612de8ef068d4. --- common/src/comp/skillset/test.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 common/src/comp/skillset/test.rs diff --git a/common/src/comp/skillset/test.rs b/common/src/comp/skillset/test.rs new file mode 100644 index 0000000000..a9ced178f1 --- /dev/null +++ b/common/src/comp/skillset/test.rs @@ -0,0 +1,29 @@ +use super::*; +use crate::comp::{skillset::SkillPrerequisitesMap, Skill}; +use hashbrown::HashMap; + +#[cfg(test)] +use petgraph::{algo::is_cyclic_undirected, graph::UnGraph}; + +#[test] +fn check_cyclic_skill_deps() { + let skill_prereqs = + SkillPrerequisitesMap::load_expect_cloned("common.skill_trees.skill_prerequisites").0; + let mut graph = UnGraph::new_undirected(); + let mut nodes = HashMap::::new(); + let mut add_node = |graph: &mut UnGraph, node: Skill| { + *nodes + .entry(node.clone()) + .or_insert_with(|| graph.add_node(node.clone())) + }; + + for (skill, prereqs) in skill_prereqs.iter() { + let skill_node = add_node(&mut graph, *skill); + for (prereq, _) in prereqs.iter() { + let prereq_node = add_node(&mut graph, *prereq); + graph.add_edge(prereq_node, skill_node, ()); + } + } + + assert!(!is_cyclic_undirected(&graph)); +} From fa6e5f2cec9884d1ac8cd71927e0528cf130e246 Mon Sep 17 00:00:00 2001 From: Maveth Date: Mon, 17 Jan 2022 10:39:58 -0600 Subject: [PATCH 4/4] Add a comment explaining something unintuitive --- common/src/comp/skillset/test.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/common/src/comp/skillset/test.rs b/common/src/comp/skillset/test.rs index def109c608..bda4849ad6 100644 --- a/common/src/comp/skillset/test.rs +++ b/common/src/comp/skillset/test.rs @@ -2,6 +2,7 @@ use super::*; use crate::comp::{skillset::SkillPrerequisitesMap, Skill}; use hashbrown::HashMap; +// Unneeded cfg(test) here keeps rust-analyzer happy #[cfg(test)] use petgraph::{algo::is_cyclic_undirected, graph::UnGraph};