Skip to content

Conversation

Fantomas42
Copy link
Contributor

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

Copy link
Owner

@trincaog trincaog left a 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?

return result

def print_cube(self):
def print_cube(self, orientation: str = ''):
Copy link
Owner

@trincaog trincaog Apr 17, 2025

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?

Copy link
Contributor Author

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.

Comment on lines 84 to 87
cube.rotate([orientation])
if cube._store_history:
cube._history.pop()
cube._history.pop()
Copy link
Owner

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.

Suggested change
cube.rotate([orientation])
if cube._store_history:
cube._history.pop()
cube._history.pop()
cube.undo()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, it's implemented !

@Fantomas42
Copy link
Contributor Author

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

@trincaog
Copy link
Owner

I understand the idea of providing a point of view for printing—for example, to render the cube as seen from above or from the left side.

However, I find the current orientation specification a bit unintuitive, especially when using values like bottom or back. In my opinion, specifying only a single face introduces ambiguity, since orientation generally requires two axes (e.g., the face pointing forward and the one pointing upward). With just one face provided, it's not always clear what the final orientation should be.

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 undo method to revert the rotations afterward.

@Fantomas42
Copy link
Contributor Author

Indeed, the cube orientation system is not clear.
From what I've understood, the standard is green face in front, and white face on top.
See issue #18.

It's similar to the north on a compass, serving as a reference point.
In reality, the concepts of 'up', 'down', 'left', 'right' don't truly make sense on a cube. They depend on the orientation and the observer's viewpoint.
That's why we need a compass. :)
And finally that's why I wanted to make this independent of the cube's state.

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.
I understand, that the current PR is just a convenient helper for 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 :)

image

Feel free to close this PR.

Best regards

@trincaog
Copy link
Owner

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.

@Fantomas42
Copy link
Contributor Author

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

@trincaog
Copy link
Owner

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.

Love this!
I think a conversion map between the orientation faces and the cube movements would work perfectly here.
Thank you so much for taking on this improvement — really appreciate it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants