obviousAlexC/newgroundsIO.js -- "The fix-it update!" #2254
Conversation
github-actions
bot
added
the
pr: change existing extension
|
!Format |
|
!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. |
|
Remind me to look this weekend |
Will do once it is finished. |
|
!format |
|
Nah this isn't the fix it update, this is the weight loss update 🗣️ |
A lot of it is reminifying an api prettier thought shouldn't be minifed. |
|
@David-Orangemoon do you want this reviewed or is it still being worked on? |
I'm waiting for reviews. |
|
Though I know I should add a can fetch when I get home. |
|
!format |
|
@Brackets-Coder ok it's ready |
|
I'll review when I can |
There was a problem hiding this comment.
I'm not approving or requesting changes, just wanna ask a few questions
I'm not familiar with new grounds or their API or anything so this is a very quick skim review of things that jump out at me more than a critique of the actual functionality/best practices...
I also haven't tested this yet but I normally like to before I approve
It looks like the biggest thing is just a minification of whatever library you're using but other than that it looks like you added some things related to medals and scoreboards? like I said, not very familiar...
|
I am going to wait for #2267 to finish up (soon - it has a real deadline), then this will be the first extension to use the new importing system |
One thing that concerns me about this is the fact that certain penguinmod projects (Since I know some use this extension) will break if Scratch.import is used due to it not existing in the PM. |
|
We can give them sufficient notice to copy and paste our implementations of those if we end up adding anything required at run time |
|
okay, changed my mind, dependency management still a bit away so i'll get this in sooner i figure it can't be any more broken than the old version |
| //eslint-disable-next-line no-async-promise-executor | ||
| return new Promise(async (resolve, reject) => { | ||
| //Just do it once | ||
| const canFetch = await Scratch.canFetch("https://www.newgrounds.io/"); |
There was a problem hiding this comment.
was going to ask about this. thanks for thinking about it
There was a problem hiding this comment.
i made two changes
- one is just copying and pasting the built library from the original github again to ensure that there was no tampering (don't think there was)
- one is to remove innerhtml usage
if you can check that i didn't break anything bad this can merge whenever
I'll check as well but we can merge whenever you like |
|
Alright we're all good I suppose the only thing that's keeping this from officially ready is @David-Orangemoon's approval |
|
Looks good @Brackets-Coder |
5 participants
Fixes, and updates the NGIO extension which has been degrading due to back-end updates, and my general unthoughtfulness from years ago.
It's ready