-
Notifications
You must be signed in to change notification settings - Fork 284
Enhanced extension: Complexify!.js #2113
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 better extension for complex numbers, fixing all the issues with the original Complexity!
What do you think, @Brackets-Coder and @yuri-kiss? |
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.
Generally speaking you should try to follow formatting best practices so your code doesn't look cluttered. Additionally, try to avoid opening new pull requests when it isn't necessary and you could just update the original :)
@@ -0,0 +1,1012 @@ | |||
// Name: Complexify! | |||
// ID: kenayComplexify | |||
// Description: Complex Numbres in TurboWarp! Can you believe it? |
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.
Description has typos and should be more professional, targeted towards users. try something like "Complex Numerical Operations in TurboWarp." or something idk
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.
makes sense
'use strict'; | ||
//Just in case: | ||
if (!Scratch.extensions.unsandboxed) { | ||
alert("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.
Nice allusion to Anakin Skywalker's famous opinion, but generally you should avoid alerts and just throw the error
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.
ok, i'll avoid alerts. also who's Anakin Skywalker?
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.
ok, i'll avoid alerts. also who's Anakin Skywalker?
I'm just going to assume you've never watched the masterpiece of Star Wars
} | ||
} | ||
|
||
function jsCode() { /**Again, thanks Rawify*/ |
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.
First, you're putting the library (which is already inside an Immediately Invoked Function Expression) inside a function that is just executed elsewhere, and you're also re-declaring "use-strict"
again which is totally redundant. Why can't you just minify the library and put it inside the class constructor?
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.
you're saying i can just insert the complex code in the constructor?
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.
you're saying i can just insert the complex code in the constructor?
That's how JS works, if you're just executing the function once inside the try catch
block than might as well not have the function and reduce line count
{ | ||
filter: [Scratch.TargetType.SPRITE], //Just in case | ||
blockType: Scratch.BlockType.LABEL, | ||
text: Scratch.translate("Motion"), | ||
}, |
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.
so this section seems to be restricted to sprites, but let's check to make sure you're also filtering the block code so it doesn't return an error if these blocks are dragged from a sprite into the backdrop
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 how to do that, but i'll try
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 how to do that, but i'll try
You're correctly filtering the block pallet in the stage, but the sprite-exclusive blocks can be dragged into the stage and may cause errors. You should check to see if the block's target is the stage, it works like this:
blockOpcode({ Arg1, Arg2 }, { target }) {
console.log(target);
}
just console log target
and you'll see how to detect the stage
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.
ok i'll if (util.target.isStage) return;
to each movement block
filter: [Scratch.TargetType.SPRITE], | ||
opcode: 'goToPolar', //A block no one asked for, and I fear no one needs | ||
blockType: Scratch.BlockType.COMMAND, | ||
text: 'Go polar [RADII] ∠ [ANGLY]', | ||
arguments: { | ||
RADII: { type: Scratch.ArgumentType.STRING, defaultValue: 50 }, | ||
ANGLY: { type: Scratch.ArgumentType.STRING, defaultValue: '0.9272952180016123' } | ||
}, | ||
}, | ||
{ | ||
filter: [Scratch.TargetType.SPRITE], | ||
opcode: 'glideComplex', //My favourite block [I love it] | ||
blockType: Scratch.BlockType.COMMAND, | ||
text: 'Glide [SECS] secs to [COMPLEX]', | ||
arguments: { | ||
COMPLEX: { type: Scratch.ArgumentType.STRING, defaultValue: '30+40i' }, | ||
SECS: { type: Scratch.ArgumentType.NUMBER, defaultValue: 1 } | ||
}, | ||
}, |
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.
Are the offhand comments here really necessary? I think they just distract from the code
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.
no, just some nice details. i'll delete them
/** | ||
We'll use toString() to return the Complex number with math notation. | ||
If you wonder why, remember Complex is a class, and hence, returns objects. | ||
So, we don't want "{re: -5, im: 1}", "[-5,1]" or "[object Object]". We want "-5+i" as is | ||
Thus, no Scratch.Cast.toString() or anything like that, because toString() will always 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.
May I ask why this is the case? If everything is returned as objects then shouldn't you just parse them and return their properties instead of the whole object? The reason Scratch.Cast.toString() exists is because scratch has weird quirks that it has to account for which the normal javascript toString doesn't
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
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.
thanks for the tip, @yuri-kiss! also @Brackets-Coder, the true reason we used .toString() is because no other function will output the strings we want
glideComplex (args, util) { //Do you recognize this? Answer at the end! | ||
if (util.stackFrame.timer) { | ||
const timeElapsed = util.stackFrame.timer.timeElapsed(); | ||
if (timeElapsed < util.stackFrame.duration * 1000) { | ||
// We've moving! And we'll move again. | ||
const frac = timeElapsed / (util.stackFrame.duration * 1000); | ||
const dx = frac * (util.stackFrame.endX - util.stackFrame.startX); | ||
const dy = frac * (util.stackFrame.endY - util.stackFrame.startY); | ||
util.target.setXY(util.stackFrame.startX + dx, util.stackFrame.startY + dy); | ||
util.yield(); | ||
} else { | ||
// We're done! Now, lets end this | ||
util.target.setXY(util.stackFrame.endX, util.stackFrame.endY); | ||
} | ||
} else { | ||
// We're starting! So, new Timer! | ||
util.stackFrame.timer = new Timer(); | ||
util.stackFrame.timer.start(); | ||
util.stackFrame.duration = args.SECS; | ||
util.stackFrame.startX = util.target.x; | ||
util.stackFrame.startY = util.target.y; | ||
util.stackFrame.endX = Complex(args.COMPLEX).re; //A little edit | ||
util.stackFrame.endY = Complex(args.COMPLEX).im; | ||
if (util.stackFrame.duration <= 0) { | ||
// We can't glide -1 seconds, can we? | ||
util.target.setXY(util.stackFrame.endX, util.stackFrame.endY); | ||
return; | ||
} | ||
util.yield(); | ||
} | ||
} |
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 seems really unoptimized
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.
yea i just copied the code from Scratch-vm because we didn't know how to glide it ourselves
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.
even asked ChatGPT, we couldn't find any optimization
convertComplex({ ANGLE, TOSMTH }) { | ||
try { | ||
if (ANGLE == "") { | ||
return 0; | ||
} | ||
const cInstance = Complex(ANGLE); | ||
switch (TOSMTH) { | ||
case 'degs to rads': if (cInstance.im == 0) return (cInstance.re * 0.017453292519943295) % twoPi; | ||
return cInstance.mul(0.017453292519943295).toString(); break; | ||
case '𝜋': return cInstance.mul(3.141592653589793).toString(); break; | ||
case 'rads to degs': if (cInstance.im == 0) return (cInstance.re * 57.29577951308232) % 360; | ||
return cInstance.mul(57.29577951308232).toString(); break; | ||
default: return NaN | ||
} | ||
} catch (e) { | ||
console.log(e); | ||
return 0; | ||
} | ||
} |
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.
Oh my gosh the formatting is crazy I'll see if I can fix it
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.
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.
Just learn I can !format
!format |
This comment was marked as abuse.
This comment was marked as abuse.
I'll add a better glideComplex, more Scratch.Cast and the minified code later. For now, small changes first
Still working on some thing though. I'll tell you when it's ready.
@Brackets-Coder, I'm new to JS and GitHub and that sort of stuff. I don't know a lot of things. Some things, but not all of them. I didn't knew that you could edit pull-requesting files until recently, after I created this new pull request.
And which are the horrible practices? Now that I know how to edit files, maybe I can correct Complexify!.js |
!format |
Absolutely not trying to be critical, we all were there once and it was only recently (in the past few months) that I really started with Github. It's an understandable situation, I'm just here to try to help you through it. |
!format (heard these fix smth idk) |
The formatting bot didn't find any formatting issues. It currently only checks the extensions folder. The author or a maintainer can run terminal command 'npm run format' manually to format all files. |
Yep. MORE THINGIES. Thanks to every error you've found, it's better than ever. Can't wait to see it at the gallery!
!format (for the update. two done, one to go) |
Basic changes :)
Checked again. what is define? idk
The formatting bot didn't find any formatting issues. It currently only checks the extensions folder. The author or a maintainer can run terminal command 'npm run format' manually to format all files. |
Testing some small changes
Let's C if it works
!format |
The formatting bot didn't find any formatting issues. It currently only checks the extensions folder. The author or a maintainer can run terminal command 'npm run format' manually to format all files. |
After Thanks @Brackets-Coder for telling me where I could be better. Fixing such a wierd extension is truly hard, but enhancing #2091 was really worth it. What now? (also i thought |
The formatting bot didn't find any formatting issues. It currently only checks the extensions folder. The author or a maintainer can run terminal command 'npm run format' manually to format all files. |
For now, we'll go through the next round of reviews before merging just to make sure everything is perfect and add some policing details before it gets merged. It's going to be really nitpicky and specific and detailed so just warning you ahead of time if some requests seem odd. Additionally, we strongly recommend a gallery thumbnail, so if you haven't started making that you should do that now or generate one with a tool of your choice (assuming proper attribution and credit of sources). If you like, you can also write documentation and add sample projects, but these aren't required. |
Also make sure the extension still works the same after all those changes |
It does! Maybe I'll edit it in the future due to silly factorial issues (of course, I set |
[I FORGOT THE LICENCE THINGIE]
A teeny-tiny change just for it to be better [bet you won't notice] but it's not like the !format is wrong |
Don't be surprised if I update this. |
You heard that right, so go and grab your favourite Fibonacci numbers and your couple of wierd constants to set this.devtools to false because WE HAVE DOCUMENTATION
wdym by 'ReduxStore not exist'??
Now to give this !format so it can be accepted |
@Kenay555 I am willing to review this code, but I first need to address some copyright concerns. Did you copy any of your code verbatim from an external source (ChatGPT, someone else's code, etc.)? |
Well... we used Rawify's |
@Kenay555 Your extension adds over 2000 lines of code to the extension library, and at least 53 of those lines do not actually do anything. The whole thing looks like it was over-commented with a slight sense of humor. While we all love funny source code comments, the codebase needs to be sterilized of unprofessionalism before being introduced. |
A better extension for complex numbers, fixing all the issues with the original Complexity! See the first one at #2091
More motion, vectors, decimals and factorials**********!
*[Somehow not working]