Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cubes created from FaceletCube::from(faces) are invalid #108

Open
deavid opened this issue Nov 27, 2022 · 2 comments
Open

Cubes created from FaceletCube::from(faces) are invalid #108

deavid opened this issue Nov 27, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@deavid
Copy link

deavid commented Nov 27, 2022

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.

@V-Wong V-Wong added the bug Something isn't working label Nov 27, 2022
@V-Wong
Copy link
Owner

V-Wong commented Nov 27, 2022

Yes, you're completely right about this. Thank you for spotting this.

I'll admit this is a bit of bad API moment. I think my original intention was to only have geometric to facelet cube conversions. So implementing FaceletCube::from(faces) instead of FaceletCube::from(geometric_cube) was a poor design decision on my part.

But you also make a valid point, a correct implementation of FaceletCube::from(faces) would be necessary to allow solving of arbitrary cubes. So I will try fixing this.

Thank you again.

@vmiklos
Copy link

vmiklos commented Jul 30, 2023

I just ran into the very same problem. :-) If I have a Vec of moves, then the solver works fine, but if I init the facelet cube from a state I have in the real world, then the solver doesn't work.

How do you imagine implementing the conversion from a GeoCube for FaceletCube? Also, once we have that, then GeoCube would also need a way to be initialized from a real world state to resolve this issue, I guess. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants