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

The return value of KTransformation.toKPattern() is not guaranteed to be immutable

Open Wyverex42 opened this issue 10 months ago • 2 comments

Steps to reproduce the issue

  • Call KPuzzle.algToTransformation(new Alg()).toKPattern()
  • Change some data in the resulting KPattern, e.g. swap some edges
  • Call KPuzzle.defaultPattern()
  • Observe that the default pattern has changed

Observed behaviour

I've written a pseudo-scramble algorithm that scrambles just a part of the cube based on a mask (while maintaining parity) and sometimes the whole cube was messed up after a few scrambles. I found that there is an early out in applyTransformationDataToKPatternData that returns the pattern data for a given orbit as-is if the transformation is the identity transform.

I assume this was done as an optimization.

Now the question is what the expectations around KPatterns are. Is each pattern supposed to be a separate instance? The fact that the library seems to return new KPatterns in various places when it could return an existing one, seems to suggest to me that they are meant to be instanced. If that's the case, then this optimization breaks that assumption.

I don't know if anyone else has tried to change KPatternData directly, so I wouldn't be surprised if that use case is unsupported. However, my assumption is that this behavior wasn't intended?

🖼 Screenshots

No response

Expected behaviour

KPatterns are separate instances

Environment

node v21.6.0

Additional info

No response

Wyverex42 avatar Mar 27 '24 20:03 Wyverex42