minor code cleanup particularly in TransTable

fish/src/bot/node.rs

@@ -99,27 +99,26 @@ impl Node {
     where
         E: FnMut(&Mat, Queue<'_>, &Range<i16>) -> (bool, i32) + 'a,
     {
+        let mut matrix = MatBuf::new();
+
         let placements = self.queue().reachable().flat_map(|ty| {
             let locs = find_locations(self.matrix(), ty);
             locs.map(move |loc| Piece { ty, loc })
         });
 
-        let mut matrix = MatBuf::new();
-
-        placements.filter_map(move |placement| {
+        let children = placements.map(move |placement| {
+            // compute new board state from placement
             matrix.copy_from(self.matrix());
             placement.cells().fill(&mut matrix);
             let cleared = matrix.clear_lines();
             let queue = self.queue().remove(placement.ty);
 
-            trans
-                .get_or_insert(&matrix, queue, || {
+            // allocate new node unless this state already exists
+            trans.get_or_insert(&matrix, queue, || {
                 Node::alloc(
                     arena,
                     copy_matrix(arena, &matrix),
-                        // optimization: copy_queue() not necessary! self.queue is already
-                        // allocated on the arena, and this queue just aliases pointers
-                        // into self.queue.next
+                    // `queue` is already allocated on the arena so don't need to copy it
                     queue,
                     evaluate(&matrix, queue, &cleared),
                     Some(Edge {
@@ -128,12 +127,18 @@ impl Node {
                     }),
                 )
             })
-                // ignore duplicates found in the transposition table
-                .map(|n| tracing::trace!("duplicate: {n:?}"))
-                .err()
+        });
 
-            // TODO: when a duplicate node is encountered, reparent it so that it has
-            // a more preferable initial path.
+        // exclude duplicate children to avoid redundant search sub-trees
+        children.filter_map(|child| {
+            match child {
+                Ok(new) => Some(new),
+                Err(dup) => {
+                    // TODO: reparent dup so that it has a more preferable initial path.
+                    tracing::trace!("ignoring duplicate child {dup:?}");
+                    None
+                }
+            }
         })
     }
 }

fish/src/bot/trans.rs

@@ -6,14 +6,12 @@ use mino::srs::{PieceType, Queue};
 use mino::Mat;
 
 use crate::bot::node::{Node, RawNodePtr};
+use crate::HashMap;
 
 pub(crate) struct TransTable {
     lookup: HashMap<QueueKey, HashMap<RawMatPtr, RawNodePtr>>,
 }
 
-type HashMap<K, V> = hashbrown::HashMap<K, V, HashBuilder>;
-type HashBuilder = core::hash::BuildHasherDefault<ahash::AHasher>;
-
 impl TransTable {
     /// Constructs a new empty transposition table.
     pub fn new() -> Self {
@@ -23,14 +21,14 @@ impl TransTable {
     }
 
     /// Looks up a node in the transposition table matching the given state. If one is
-    /// found, returns `Ok(existing_node_ptr)`. If not found, a node is created and
-    /// inserted by calling the given closure, and returns `Err(newly_inserted_node)`.
+    /// found, returns `Err(existing_node_ptr)`. If not found, a node is created and
+    /// inserted by calling the given closure, and returns `Ok(newly_inserted_node)`.
     pub fn get_or_insert<'a>(
         &mut self,
         matrix: &Mat,
         queue: Queue<'_>,
         mut alloc: impl FnMut() -> &'a Node,
-    ) -> Result<RawNodePtr, &'a Node> {
+    ) -> Result<&'a Node, RawNodePtr> {
         // two-phase lookup: first key off queue info, then matrix info
 
         let map = self
@@ -44,19 +42,19 @@ impl TransTable {
         let mat_ptr = unsafe { RawMatPtr::new(matrix) };
 
         if let Some(&node_ptr) = map.get(&mat_ptr) {
-            return Ok(node_ptr);
+            return Err(node_ptr);
         }
 
         let node = alloc();
-        debug_assert!(*node.matrix() == *matrix);
-        debug_assert!(node.queue() == queue);
+        debug_assert_eq!(*node.matrix(), *matrix);
+        debug_assert_eq!(node.queue(), queue);
 
         // SAFETY: as noted above, this matrix ptr is OK to use for insert, since it comes
         // from the newly allocated node.
         let mat_ptr = unsafe { RawMatPtr::new(node.matrix()) };
 
         map.insert(mat_ptr, RawNodePtr::from(node));
-        Err(node)
+        Ok(node)
     }
 }
 

fish/src/find.rs

@@ -1,12 +1,13 @@
 //! Find locations.
 
 use alloc::vec::Vec;
-use hashbrown::hash_set::HashSet;
 use mino::input::Kicks;
 use mino::piece::{Shape, Spawn};
 use mino::srs::PieceType;
 use mino::{input, Loc, Mat, Movement, Piece};
 
+use crate::HashSet;
+
 static ALL_INPUTS: &[Movement] = &[
     Movement::LEFT,
     Movement::RIGHT,
@@ -31,7 +32,7 @@ pub struct FindLocations<'m, T> {
     piece_ty: T,
     // TODO: allow these two collections to be extracted and reused
     stack: Vec<Loc>,
-    visited: HashSet<Loc, core::hash::BuildHasherDefault<ahash::AHasher>>,
+    visited: HashSet<Loc>,
 }
 
 impl<'m, 'c, 'k, T> FindLocations<'m, T>

fish/src/lib.rs

@@ -5,3 +5,7 @@ extern crate alloc;
 pub mod bot;
 pub mod eval;
 pub mod find;
+
+type HashMap<K, V> = hashbrown::HashMap<K, V, HashBuilder>;
+type HashSet<T> = hashbrown::HashSet<T, HashBuilder>;
+type HashBuilder = core::hash::BuildHasherDefault<ahash::AHasher>;

mino/src/queue.rs

@@ -47,7 +47,7 @@ impl<'a, T: Copy> Queue<'a, T> {
     /// be empty, and may yield at most 2 elements.
     pub fn reachable(&self) -> impl Iterator<Item = T> {
         let mut hold = self.hold;
-        let mut front = self.next.get(0).copied();
+        let mut front = self.next.first().copied();
         core::iter::from_fn(move || hold.take().or_else(|| front.take()))
     }
 }
@@ -107,14 +107,11 @@ mod test {
             let idx = ts
                 .iter()
                 .enumerate()
-                .find_map(|(i, &u)| (t == u).then_some(i));
-            if let Some(idx) = idx {
+                .find_map(|(i, &u)| (t == u).then_some(i))
+                .unwrap_or_else(|| panic!("{t:?} found but not expected"));
             ts.remove(idx);
-            } else {
-                panic!("{t:?} found but not expected");
         }
-        }
-        for t in ts {
+        if let Some(t) = ts.first() {
             panic!("{t:?} expected but not found");
         }
     }