cubing.js icon indicating copy to clipboard operation
cubing.js copied to clipboard

Moves repeat indefinitely when using bluetooth input

Open anicolao opened this issue 3 years ago • 2 comments

To Reproduce Steps to reproduce the behavior:

  1. Go to https://alpha.twizzle.net/edit/
  2. Set input to a bluetooth cube (I am using a Gan i2)
  3. Twist a face
  4. That twist repeats indefinitely - e.g. U1 -> U2 -> U3 -> ...

Expected behavior One twist should be recorded.

Browser: Google Chrome 104.0.5112.79 (Official Build) (arm64) Revision 3cf3e8c8a07d104b9e1260c910efb8f383285dc5-refs/branch-heads/5112@{#1307} OS macOS Version 12.3.1 (Build 21E258) JavaScript V8 10.4.132.20

anicolao avatar Aug 09 '22 06:08 anicolao

Thanks for filing an issue.

I'm guessing this is because Gan they switched from their inefficient polling implementation to characteristic subscriptions. Do you have any other cube to test with?

The i2 seems to be discontinued, but I've just ordered an i3 to debug this. If that doesn't work, I might ask you for more help debugging.

lgarron avatar Aug 09 '22 06:08 lgarron

I believe but am not 100% sure that this was working better a few weeks ago when you responded to me about bluetooth support on the software forum on speedcubing.net (I am "Osric").

The GAN i3 doesn't work at all. It shows up in Chrome's pairing UI but after selecting it though the bluetooth symbol appears in the tabstrip, the button does not grey out and rotating the cube does not control the puzzle. So its problems are different than the i2, which almost works.

anicolao avatar Aug 09 '22 16:08 anicolao

In the JS console I get an infinite number of these after connecting:

gan.ts:111 Uncaught (in promise) TypeError: quat.clone(...).inverse is not a function
    at PhysicalState.rotQuat (gan.ts:111:38)
    at GanCube.intervalHandler (gan.ts:367:33)

anicolao avatar Aug 17 '22 22:08 anicolao

Here is a patch that will fix it:

diff --git a/src/cubing/bluetooth/smart-puzzle/gan.ts b/src/cubing/bluetooth/smart-puzzle/gan.ts
index b53dd60c..8e9d8eda 100644
--- a/src/cubing/bluetooth/smart-puzzle/gan.ts
+++ b/src/cubing/bluetooth/smart-puzzle/gan.ts
@@ -108,7 +108,7 @@ class PhysicalState {
     const quat = new Quaternion(x, y, z, w);
 
     if (!homeQuatInverse) {
-      homeQuatInverse = quat.clone().inverse();
+      homeQuatInverse = quat.clone().invert();
     }
 
     return quat.clone().multiply(homeQuatInverse.clone());

anicolao avatar Aug 17 '22 22:08 anicolao

Ooh, nice find!

lgarron avatar Aug 17 '22 22:08 lgarron

Alright, please let me know if it works now!

lgarron avatar Aug 17 '22 23:08 lgarron

Yes, this works. The problem was introduced when three.js dropped support for legacy features in commit a3c25a5418680e94e8860c29d4832cd24bf5c122 and a lot of the quaternion changes that were made in 2020 were gone. These used to log warnings to the console that might be worth looking for if you downgrade to an old three.js to see them in case there are more dependencies. This explains why it was working when I tried it a short while back and recently started failing, must have corresponded with a new deploy including the latest three.js code.

anicolao avatar Aug 17 '22 23:08 anicolao

Sorry that's the wrong (original) commit, the one that drops the feature entirely is https://github.com/mrdoob/three.js/commit/a3c25a5418680e94e8860c29d4832cd24bf5c122

anicolao avatar Aug 17 '22 23:08 anicolao

Yes, this works. The problem was introduced when three.js dropped support for legacy features in commit a3c25a5418680e94e8860c29d4832cd24bf5c122 and a lot of the quaternion changes that were made in 2020 were gone. These used to log warnings to the console that might be worth looking for if you downgrade to an old three.js to see them in case there are more dependencies. This explains why it was working when I tried it a short while back and recently started failing, must have corresponded with a new deploy including the latest three.js code.

Yeah, that definitely make sense. It's unfortunate that the type still exists and is merely marked as deprecated, since that doesn't cause a TypeScript error. :-/

lgarron avatar Aug 17 '22 23:08 lgarron