search

muodov / kociemba

A pure Python and pure C ports of Kociemba's algorithm for solving Rubik's cube
GNU General Public License v2.0
566 stars 126 forks source link

Add ability to solve() to a given pattern #8

Closed ansiwen closed 8 years ago

ansiwen commented 8 years ago

As described in #6

muodov commented 8 years ago

Looking good! Did you consider making the same changes to the C version or shall I do it myself? Should be very easy since the implementations are virtually equivalent.

ansiwen commented 8 years ago

I'm not 100% sure what you mean. The code where I implemented this, the wrapper function for both the C and Python solver in __init__.py, doesn't have an equivalent in the c-code. I could implement it in the CLI tool (ckociemba/solve.c), but maybe it would be better then to move the code into the solution() functions, which exist in both the C and Python versions. Another problem is, that toFaceCube() in ckociemba/include/cubiecube.h is commented out. Is there a problem with that code?

muodov commented 8 years ago

Sorry for that. The idea is, I'd like the C version to be full-featured so that it can be used on its own without losing features. The wrapper function is basically choosing between the C and Python versions of the same function (Search.solution()). Since we are adding a new feature here, I just want it to be present both in C and Python versions of this library, and keep __init__.py to be a very thin wrapper.

Regarding the commented code, there shouldn't be problems with it. It is probably commented out because it is not needed for solving. Initially I started this project because I needed python implementation for our FAC cube solver. But now I'm going to make a proper library out of it.

So my suggestion is: let's merge your changes to develop, since it is fine for Python use-case. And I'll create an issue for implementing this in C part as well. Is this fine by you?

muodov commented 8 years ago

This PR is merged manually into develop branch. Will be pushed to master with the next release.