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

Use cubing.js for case display. #23

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lgarron
Copy link

@lgarron lgarron commented Nov 24, 2022

https://cubing.net/api/visualcube/ has moved. It should continue to work, but it will always be slower than local rendering. I'm trying to encourage use of cubing.js as a replacement where possible, and it can cover the use cases well (in addition to supporting many more features, like animation and a wide variety of puzzles). This PR includes the minimal changes to switch from VisualCube to cubing.js.

You may want additional options for the <twisty-player>, such as hint-facelets="none" (which hides the hint facelets in the 3D view).

https://cubing.net/api/visualcube/ has moved. It should continue to
work, but it will always be slower than local rendering. I'm trying to
encourage use of `cubing.js` as a replacement where possible, and it can
cover the use cases well (in addition to supporting many more features,
like animation and a wide variety of puzzles). This PR includes the
minimal changes to switch from VisualCube to `cubing.js`.

You may want additional options for the `<twisty-player>`, such as
`hint-facelets="none"` (which hides the hint facelets in the 3D view).
@tao-yu
Copy link
Owner

tao-yu commented Dec 1, 2022

Hi Lucas,

Apologies for the late response. Thank you very much for this PR. Local rendering of images has been something I've wanted for a long time, but I was not aware this functionality had already become available in cubing.js.

I have two issues that need to be fixed before I feel comfortable merging:

  1. The twisty player displays cases an x2 rotation away from the keyboard controlled virtual cube (when "Use Virtual Cube" is selected). I believe this can be fixed by simply removing "x2 " + from line 956 and line 953.
  2. The cube will need to support custom color schemes, according to the color scheme chosen by the user (when "Use custom colour scheme" is selected). This is an important feature, since a portion of users do use custom color schemes and I would not like this update to remove a existing functionality for these users.

For the second issue, I have taken a look at https://js.cubing.net/cubing/twisty/ but was not able to find an option for setting the color scheme. I am wondering if you could confirm whether this feature exists or not and explain how it could be implemented?

Again thank you very much for this PR, this adds a big feature I've always wanted. Please let me know if there is anything I can do to help with resolving these issues.

@lgarron
Copy link
Author

lgarron commented Dec 2, 2022

2. The cube will need to support custom color schemes, according to the color scheme chosen by the user (when "Use custom colour scheme" is selected). This is an important feature, since a portion of users do use custom color schemes and I would not like this update to remove a existing functionality for these users.

I've been waiting for a good use case to implement this; this seems like a good one.

I think it wouldn't be too confusing to add something like an experimentalPreStickeringRotations property, which you could set to something like x2.

However, I've been very carefully avoiding arbitrary custom color schemes, because 1) this makes it hard to choose accessible and themed color palettes, and 2) this creates situations like "the original front face, which is usually the green face, but for this person the orange face", which can cause real communcation issues when combined with other features in certain apps. Do you think you'd be able to offer a limited color palette, such as a subset of https://software.rubikscube.info/AnimCube/#colorscheme#facelets ?

@lgarron
Copy link
Author

lgarron commented Dec 15, 2022

Do you think you'd be able to offer a limited color palette, such as a subset of https://software.rubikscube.info/AnimCube/#colorscheme#facelets ?

poke

@tao-yu
Copy link
Owner

tao-yu commented Dec 19, 2022

Really sorry for the late reply Lucas,

Thinking about it right now, I think my trainer has been desgined with the sprit of allowing as much freedom as possible, rather than catering for a wider audience such as beginners that just want to learn OLL and PLL. This is why there are so many odd and experiemental algsets, and features like virtual cube training which exist for what I assume is a minority people who are looking for an alg trainer. When people have invented new methods, such as Mehta, my trainer has often been the first place people have turned to in order to learn the new algs. There are also algsets on there which are probably used by only one person (EDMARTER, who learned full 1LLL on my trainer comes to mind).

So considering this, I think I would prefer to keep the ability to customize the color scheme to whatever a user wants, just to keep consistent with the spirit of allowing as much freedom as possible, catering for obscure use cases and for the sake of not removing any features with an update.

Based on this, I would only be satisfied with using cubing.js if arbitrary color schemes are available, or if there could be a fallback option when the user selects non approved colors.

I am definitely motivated now to generate images locally instead of through cubing.net, and if cubing.js isn't appropriate for this I most likely will look into another solution over the Christmas break.

Please let me know what your thoughts would be on this. I am very sorry if this decision means that your great work on my project has been in vain, but I feel that I just don't want to compromise on my own vision on what this trainer should be. Your code will certainly be very useful to me for future projects going forward, so I greatly thank you again for that.

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