I see some inconsistencies in tense (present and past) in some doc comments (currently detected in generic_cube.rs).
E.g.

/// Maps over the pieces of the cube, replacing each piece
/// according to the given mask function. 
fn mask(&self, mask: &dyn Fn(i32, Face) -> Face) -> Self;

/// Apply a move to a cube.
/// ...
fn apply_move(&self, mv: Move) -> Self;

The implementation of geometrics moves currently features a lot of duplicate code. This was originally done as a temporary solution to support arbitrary sized cubes. A more elegant solution with less duplicate code would be preferred here.

Code readability can be improved by more intuitive variable names and/or comments.

If a move's n value was passed when checking if a sticker satisfies the predicate, the predicate field can become
pub predicate: fn(&Sticker, u32) -> bool>, which is potentially cleaner/faster than the current boxed dyn implementation.

The mask method in the Cube trait is intended to ignore certain stickers which can be useful for defining solvers of certain states (e.g. cross, first block, etc). The method however currently takes in a slice of indices within the current cube state. It would be much more useful if it were to take a slice of indices with respect to the original solved cube state.

parse_scramble should:

• trim the provided scramble
• split by whitespace (use split_whitespace instead of split(' ')
• return an empty Vec for a string with no moves, as it currently panics on this case

The pruning tables inside each phase of the Thislethwaite solver are constant and do not vary from call to call. At the moment, the tables are regenerated at each function call. This is inefficient and caching the tables would be preferable.

By setting MoveVariant's 'Standard' value to 1, it improves semantics, as

• Standard is equivalent to 1 clockwise turn,
• Double becomes 2 turns,
• Inverse becomes 3 turns, equivalent to 1 anticlockwise turn

This allows reduction/simplification of summed MoveVariants, and can simplify modify_move (geometric_cube/moves.rs) to

fn modify_move(mv: GeometricMove, variant: MoveVariant) -> GeometricMove {
    GeometricMove { angle: mv.angle * variant as f32, ..mv }
}

We currently have tests for 2x2x2, 3x3x3 and 4x4x4 cubes. It would be good to have tests that can generalise up to any NxNxN size cube.

In cube.rs, the new function initializes as:

            .enumerate()
            .map(|(i, s)| (*s, i as u16))

Effectively adding the number of the position of the original vec inside as extra info.

While on the from function:

            faces: faces.iter().map(|f| (*f, 0)).collect(),

Sets all indices to zero.

As a result, calling cubesim::solve(&cube) will always return None for cubes that are created with the from function instead of new.

This makes impossible to use this library for arbitrary cubes where we don't know the rotations made to get into that position.

Add #![forbid(unsafe_code)] to ensure everything is implemented in 100% safe Rust, and mention this in the README.

Now that this is a single argument function, I believe you can extract this function and implement it as part of the From trait.

Originally posted by @V-Wong in #63 (comment)

The current FaceletCube implementation is not as performant as I would like it to be. Since the results are cached, actual computations are rarely required. I suspect the bottleneck to lie in the actual caching mechanism.

Leads to doc.rs non-existent resource, in both docs.rs and rust-analyzer (VSCode Extension).

Fix typo in

The implementation of GeoCube currently features many i32s, where u32s would be more appropriate, such as the size field in the GeoCube struct, and other mentions of size as a parameter.
A better semantically-typed solution would be preferred here.

e.g. pub fn get_variant(&self) -> MoveVariant.
It will be handy, and there are already instances in the codebase where it could be applied.