-
-
Notifications
You must be signed in to change notification settings - Fork 272
New extension: Complexity! #2091
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
base: master
Are you sure you want to change the base?
Conversation
A new way to play with Complex Numbers in TurboWarp! Must be loaded Unsandboxed for the [Go To ( )] to run.
throw new Error("We don't like sand :("); | ||
} | ||
|
||
const OGcomplexCode = ` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must you put the library in a constant? Everyone else just includes the minified library straight in the JS code with the license and // prettier-ignore
above it. Also must go inside the extension's function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried to put the library inside the class thing, but it Unexpected token
or smth like that. Me and ClickerTale had tried our best, Can you help?
Oh, and that's the minified version, see source.
extensions/Complexity!.js
Outdated
if (!Scratch.extensions.unsandboxed) { | ||
throw new Error("We don't like sand :("); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this must go in the extension IIFE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what IIFE means but I'll try to put it in the main function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIFE means immediately invoked function expression so eg (function() {})(); // invoked immediately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx @yuri-kiss !
extensions/Complexity!.js
Outdated
this.trig = ['sin', 'cos', 'tan', 'cot', 'sec', 'csc', 'asin', 'acos', 'atan', 'acot', 'asec', 'acsc', 'sinh', 'cosh', 'tanh', 'coth', 'sech', 'csch', 'asinh', 'acosh', 'atanh', 'acoth', 'asech', 'acsch']; | ||
|
||
try { | ||
eval(OGcomplexCode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is used to execute the library code, but no. Eval is a banned function because it can be used to execute malicious JS code, so if you're possibly going to have errors with loading the library then do it inside this try block and not outside the extension's function. See Banned APIs under CONTRUBUTING.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank for the tip! I'll try to see what can I do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when importing the library you must also make sure it does not pollute the global scope, so create some sort of wrapper to grab values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also don't know what a global scope or wrapper for values are, but i'll se what i can do @yuri-kiss!
extensions/Complexity!.js
Outdated
imComplex({ COMPLEX }) | ||
{ | ||
try { | ||
const cInstance = new this.Complex(COMPLEX); | ||
return cInstance.im; | ||
} catch (e) { | ||
return NaN | ||
} | ||
} | ||
|
||
polarComplex({ RADIUS, ANGLE }) | ||
{ | ||
try { | ||
const cInstance = coolCis(Complex(ANGLE)).mul(Complex(RADIUS)); | ||
return cInstance.toString().replace(/\s+/g, ''); | ||
} catch (e) { | ||
return NaN | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This inconsistent formatting with the try blocks are painful to look at. Prettier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When ClickerTale build half the extension and I checked for errors
, I had to delete the try{}catch(e){} thingies. It'll take time to move EVERY single one, but it's not like someone's gonna review my extension looking each and every line and tell me to take a workaround for eval() [i know you will].
Also, a similar extension already has a PR, so this is a duplicate: #1861, so I don't even know if this can be merged |
theres like 5 complex number extensions atp, and almost none of them are different with the only thing ever changing being the library they |
are we going to fix that problem and close the dupes? |
Well, @yuri-kiss , mine has polar and can play with position easier :) And yes, I |
Ok let's see what pettier does here |
A new way to play with Complex Numbers in TurboWarp! Must be loaded Unsandboxed for the [Go To ( )] to run.