-
Notifications
You must be signed in to change notification settings - Fork 10
Can orientate cube when printing #23
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for submitting a PR 🙌 Left some minor suggestions.
My main question is the use case for this change. Can you elaborate a bit on the goal?
magiccube/cube_print.py
Outdated
return result | ||
|
||
def print_cube(self): | ||
def print_cube(self, orientation: str = ''): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency, could we make this support CubeMove/CubeMoveType
object as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, I have corrected the PR accordingly tell me if it's OK for you.
magiccube/cube_print.py
Outdated
cube.rotate([orientation]) | ||
if cube._store_history: | ||
cube._history.pop() | ||
cube._history.pop() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a new undo
method, which seems helpful here.
cube.rotate([orientation]) | |
if cube._store_history: | |
cube._history.pop() | |
cube._history.pop() | |
cube.undo() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, it's implemented !
…wich can be parametred with orientation
Hello, Thank you for your good question. My goal is to enable different visualizations of the cube based on the user's preferred orientation or method of resolution. In my application (I'm developing a timer), I display a scramble - a random sequence of moves - before the user begins solving. Since I personally use the CFOP method, it's more convenient to display the cube with a z2 rotation. I considered alternative approaches, such as modifying the scramble directly or changing the facelets initialization, but applying an optional rotation during display seems like the most elegant and flexible solution. This implementation provides adaptability without altering the underlying scramble logic or cube state. Regards |
I understand the idea of providing a point of view for printing—for example, to render the cube as seen from However, I find the current orientation specification a bit unintuitive, especially when using values like Personally, I’d prefer to leave orientation control to the API caller. If a specific print orientation is needed, they can rotate the object accordingly and then call the |
Indeed, the cube orientation system is not clear. It's similar to the north on a compass, serving as a reference point. After reading your comment, I realize my implementation is limited, as one might want to pass multiple orientation movements like 'x2 y2'. Let me know if it's worth correcting the code accordingly, or if you think it's not worth integrating this feature and should be delegated to the developer. For reference, I've included a screenshot from cstimer that helped me better understand this orientation topic coupled with the faces. Perhaps there's something worth exploring here, or perhaps not :) Feel free to close this PR. Best regards |
Thanks for adding the image - it really clarified your intent. I appreciate the explanation! If I’m interpreting the image correctly (please correct me if I got it wrong), it shows two cube faces (e.g., DL) and the sequence of rotations (like z2y) needed to reach that orientation. In this setup, the first face (D) ends up facing UP and the second face (L) faces FRONT. This definitely helps me see the use case for reorienting the cube based on two specified faces. What do you think about supporting a tuple or string of two Face values as an argument to allow more flexibility when specifying orientation? For example: def print_cube(self, orientation: Union[tuple[Face,Face], str, None] = None) -> str: I realize this might be a bit more involved than the original implementation, so feel free to let me know if this direction makes sense to you. |
Hello, I believe supporting a tuple of two Face strings would be the better option for magiccube. However, in my implementation, rather than using the Face concept, I use movement orientation concepts as this provides more flexibility when transitioning between algorithm logic and cube representation. I can offer a quick solution that requires a reasonable amount of effort from my side: mapping the new orientation parameter of Union[tuple[Face, Face], str] to an orientation move. While not the most efficient approach, it allows the two conceptual worlds to communicate easily without requiring me to rework the cube printing functionality to accommodate the Face tuple. Tell me what do you think about this. Regards |
Love this! |
Hello,
This modification aims to enable printing the cube in different orientations.
I chose not to modify the Cube class to add an orientation parameter, as orientation is closely tied to the cube’s initial state and the sequence of applied moves. Introducing such a parameter directly in the Cube class could lead to misunderstandings or incorrect assumptions about its behavior.
Best regards