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.

Sign up for GitHub

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

Jump to bottom

Variable tempo conjugates #177

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Variable tempo conjugates #177

wants to merge 2 commits into from

Conversation

ticklemepierce
Copy link
Contributor

No description provided.

ticklemepierce
ticklemepierce commented Feb 26, 2022
View reviewed changes
src/cubing/twisty/views/TwistyAlgViewer.ts
@@ -149,6 +154,9 @@ class AlgToDOMTree extends TraversalDownUp<DataDown, DataUp, DataUp> {
public traverseAlg(alg: Alg, dataDown: DataDown): DataUp {
let moveCount = 0;
const element = new TwistyAlgWrapperElem("twisty-alg-alg", alg); // TODO: pick a better class name.
if (dataDown.type) {
element.addClass(dataDown.type);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't love having the state stored in class but it was at least the easiest way I could find for POC. Still figuring out the code base so there may be a better way already that I'm just not aware of.

ticklemepierce
ticklemepierce commented Feb 26, 2022
View reviewed changes
src/cubing/twisty/views/TwistyAlgViewer.ts
@@ -293,6 +301,7 @@ class AlgToDOMTree extends TraversalDownUp<DataDown, DataUp, DataUp> {
earliestMoveIndex: dataDown.earliestMoveIndex + moveCount,
twistyAlgViewer: dataDown.twistyAlgViewer,
direction: dataDown.direction,
type: "setup",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open to alternate names for this and "execution" (unsure if there are technical terms for the different parts of a conjugate)

ticklemepierce
ticklemepierce commented Feb 26, 2022
View reviewed changes
src/cubing/twisty/views/TwistyAlgViewer.ts
@@ -366,13 +376,18 @@ class MoveHighlighter {
this.moveCharIndexMap.set(charIndex, elem);
}

set(move: Parsed<Move> | null): void {
set(move: Parsed<Move> | null, twistyPlayer: TwistyPlayer): void {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find a way to access twistyPlayer from here, but I also didn't try very hard :P
Again, just moving quickly for POC

ticklemepierce
ticklemepierce commented Feb 26, 2022
View reviewed changes
src/cubing/twisty/views/TwistyAlgViewer.ts
const newElem = move
? this.moveCharIndexMap.get(move.startCharIndex) ?? null
: null;
if (this.currentElem === newElem) {
return;
}
if (newElem?.parentElement?.classList.contains('execution')) {
twistyPlayer.tempoScale = 10;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hardcoded for now. This is where I think implementation details would need sorting out. How would one pass in these values? Presumably something on DataDown?

ticklemepierce
ticklemepierce commented Feb 26, 2022
View reviewed changes
src/sites/experiments.cubing.net/cubing.js/twisty/twisty-alg-viewer.html
@@ -28,7 +28,9 @@ <h1 style="text-align: center">
</h1>
<twisty-player
id="main-player"
alg="([R, U])3 (U L)2' [M2': U2]"
alg="[R2: R U' R' U' R U R' F' R U R' U' R' F R]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: revert back.

This alg just made it easier to see my changes being applied.

@lgarron
Copy link
Member

lgarron commented Feb 27, 2022 •
edited
Loading

Thanks for the PR! It definitely matches the style of the existing code, although I'm not sure this is the right place for it.

Is your goal to set the tempo depending on where you start animating, and then keep that tempo for the rest of the alg? Or perhaps that you only plan to be animating one "alg" at a time?

I think my original impression was that you wanted to speed up commutators and conjugates any time they appeared in an alg/solve. The most robust place to do that would be the place where durations are assigned to the alg: https://github.com/cubing/cubing.js/blob/f7f5574f5cdf5937681b09c9381604ef0fc8879e/src/cubing/twisty/controllers/indexer/tree/AlgWalker.ts

Any chance you have a screenshot or two that convey the context for these animations, before I point you too far down a given path?

@ticklemepierce
Copy link
Contributor Author

I think my ultimate goal would be to have one long algorithm so that I could use this algviewer thing now that I know about it but then have different portions of it be time adjustable.

For example (with the context of my project being geared towards blindsolving):

  • Do set up moves for a given letter at tempo 1
  • Do edge swap alg at tempo 10
  • Undo set up moves for a given letter at tempo 1
  • (repeat for all letters)
  • Do parity alg at x tempo
  • Do set up moves for a given letter at tempo 1
  • Do corner swap alg at tempo 10
  • Undo set up moves for a given letter at tempo 1
  • (repeat for all letters).

Right now I'm able to do this behavior but have to treat every setup, swap, and undo setup alg as totally separate. In other words each letter of the memorization sequence involves setting and unsetting the experimentalSetupAlg as well as the alg. This also would make it harder to do any of the highlighting I think. I can take a peak at the duration tomorrow and see if that fits.

image

Here is a screenshot. Also the alg that I used in this PR illustrates it nicely as it does demonstrates what my app would do for a B corner swap follow by D corner swap.

@ticklemepierce
Copy link
Contributor Author

I have been playing around with duration but I don't understand it fully. It seems tempoScale is the only way to actually change the animation speed because if I change duration it will animate at the same speed for a shorter amount of time (so end up with a partial move).

@lgarron lgarron force-pushed the main branch 5 times, most recently from 9d66b71 to e606af4 Compare March 30, 2022 23:08
@lgarron lgarron force-pushed the main branch 2 times, most recently from a0e0af5 to 6c8143d Compare May 15, 2022 20:19
@lgarron lgarron force-pushed the main branch 4 times, most recently from 1c43df3 to cb52a09 Compare July 10, 2022 03:34
@lgarron lgarron force-pushed the main branch 9 times, most recently from 8f6a252 to c46ac2a Compare July 19, 2022 02:00
@lgarron lgarron force-pushed the main branch 16 times, most recently from 2da319f to b0ef9d4 Compare January 19, 2024 11:08
@lgarron lgarron force-pushed the main branch 3 times, most recently from ff5f6eb to 9521f7c Compare August 30, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ticklemepierce @lgarron