Fix for field_of_movement algorithm

CHANGELOG.md

@@ -6,6 +6,7 @@
 
 * (**BREAKING**) `a_star` `cost` function parameter now takes two adjacent `Hex`
 nodes instead of one, allowing for more use cases (#130, #128)
+* Fixed `field_of_movement` algorithm (#142, #127)
 
 ### Dependencies
 

src/algorithms/field_of_movement.rs

@@ -1,5 +1,8 @@
 use crate::Hex;
-use std::collections::{HashMap, HashSet};
+use std::{
+    collections::{HashMap, HashSet},
+    usize,
+};
 
 /// Computes a field of movement around `coord` given a `budget`.
 /// This algorithm takes a `cost` function, which calculates and
@@ -13,6 +16,11 @@ use std::collections::{HashMap, HashSet};
 /// in order to avoid the possibility of unlimited movement range (i.e. a `Hex`
 /// instance will always have a minimum movement `cost` of 1).
 ///
+/// # Warning
+///
+/// The implementation of this function is pretty naive and has a high
+/// complexity. It is not suitable for production use
+///
 /// # Examples
 ///
 /// - Compute field of movement with no boundaries and some wall tiles that
@@ -56,24 +64,37 @@ pub fn field_of_movement(
     budget: u32,
     cost: impl Fn(Hex) -> Option<u32>,
 ) -> HashSet<Hex> {
-    let mut computed_costs = HashMap::new();
+    let mut computed_costs = HashMap::with_capacity(Hex::range_count(budget) as usize);
     computed_costs.insert(coord, 0);
-    (1..=budget)
-        .flat_map(|i| coord.ring(i))
-        .filter_map(|coord| {
-            let coord_cost = cost(coord)?;
-            let neighbor_cost = coord
+
+    // We cache the rings and costs
+    let rings: Vec<(Hex, u32)> = coord
+        .rings(1..=budget)
+        .flatten()
+        .filter_map(|h| cost(h).map(|c| (h, c)))
+        .collect();
+
+    let mut loop_again = true;
+    while loop_again {
+        loop_again = false;
+        for (coord, coord_cost) in &rings {
+            let Some(neighbor_cost) = coord
                 .all_neighbors()
                 .into_iter()
                 .filter_map(|n| computed_costs.get(&n))
-                .min()?;
+                .min()
+            else {
+                continue;
+            };
             let computed_cost = coord_cost + 1 + neighbor_cost;
-            if computed_cost <= budget {
-                computed_costs.insert(coord, computed_cost);
-                Some(coord)
-            } else {
-                None
+            let res = computed_costs.insert(*coord, computed_cost);
+            if !loop_again && (res.is_none() || res != Some(computed_cost)) {
+                loop_again = true;
             }
-        })
+        }
+    }
+    computed_costs
+        .into_iter()
+        .filter_map(|(coord, cost)| (cost <= budget).then_some(coord))
         .collect()
 }