cubing / cubing.js

🛠 A library for displaying and working with twisty puzzles. Also currently home to the code for Twizzle.
https://js.cubing.net/cubing/
GNU General Public License v3.0
232 stars 42 forks source link

[cubing.js issue] throws error with ClassListManager.ts and Twistyprop.ts #305

Closed GhostShadow0316 closed 6 months ago

GhostShadow0316 commented 6 months ago

Steps to reproduce the issue

I created a cube timer project and I want to show scrambles

Observed behaviour

but today when I'm updating my project I see these error massages and I don't know how to fix it

🖼 Screenshots

brave_E3k23pR6dh

Expected behaviour

no error

Environment

brave browser

Additional info

here's my code

lgarron commented 6 months ago

I've had a chance to look into this, and this stems from:

https://github.com/GhostShadow0316/HAC-Cube/blob/8cd4a8e5ae3779929219fe7c58dd89ed3d2381d8/js/hactool.js#L87-L94

We could (and probably should) update the cubing.js code to avoid relying on in returning an iterator with the right elements, but this definition may also be causing other issues for you. It doesn't seem you're using this function, and I don't really see a reason you'd need to declare it on the Object prototype. But if you truly have to declare it there, I recommend replacing it with something like:

// Adapted from: https://stackoverflow.com/a/10695535
Object.defineProperty(Object.prototype, 'renameKey', {
    value: function (oldName, newName) {
        if (oldName === newName) { return this; }
       if (this.hasOwnProperty(oldName)) {
           this[newName] = this[oldName];
           delete this[oldName];
       }
       return this;
    },
    enumerable: false, // this is actually the default
});

But if you can avoid defining properties on the prototypes of the core JS objects then you'll run into these issues less frequently.

GhostShadow0316 commented 6 months ago

it worked!!! thank you very much

lgarron commented 6 months ago

I took a lot at working around programs that do this, and concluded that it's impractical at this time. three.js has a bunch of built-in assumptions, so we can't work around this in cubing.js alone.