Skip to content

Add FormatNumbers Extension #2036

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

DogeisCut
Copy link

This is a simple extension that lets you format large numbers into AD Standard, Fixed Decimal, Comma Separated, or Scientific Notation.

It adds but one block, yet a very useful one imo, especially for incremental games!

msedge_2ZeCztrapG
msedge_b8scWM1ITL
msedge_easzgrLqwB
msedge_bvo6IzMFhC
msedge_d6Rc1Kz7ia
msedge_i5PasinNgF
msedge_SZVAPgfXeq

This is the banner, drawn by Dillon (dillonr on Discord)!
FormatNumbers

@github-actions github-actions bot added the pr: new extension Pull requests that add a new extension label Mar 16, 2025
"No",
"Dc",
];
const unitPrefixes = ["U", "D", "T", "Qa", "Qt", "Sx", "Sp", "O", "N"];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you structure this one vertically like the other arrays above and below it for consistency purposes?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the formatting check is not a fan of this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically extension images aren't supposed to contain text because of translation reasons, but I think this one is okay because it uses Arabic numerals which are (generally speaking) somewhat universal? I don't know. Second opinion would be great

Copy link
Collaborator

@CST1229 CST1229 Mar 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo numbers wouldn't be considered text for thumbnails.
they are the exact same Arabic numberals in all of Scratch's languages and they already exist in several extension thumbnails (base, bitwise). i think it's good

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo numbers aren't considered text for thumbnails. they are Arabic numberals in all of Scratch's languages and they already are in several extension thumbnails (base, bitwise). i think it's good

That's what I was thinking too

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

though ideally this should be an SVG

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would've liked to make this an SVG but unfortunately, I don't believe Dillon has an SVG version. Guess I could try to re-create it if it is really necissary?

- Fixed the comment ID to match the extension ID.
- Structured long arrays in format function vertically for consistency.
- Re-ordered placement of extension (I recall being told not to worry about this?)
Copy link
Contributor

@Brackets-Coder Brackets-Coder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most people like to put their block code definitions beneath the block's opcodes instead of above getInfo(), but I guess if it still works it's fine?

Also, I've asked this multiple times but I don't know if it's just a problem with me. how'd you request my review because I can't seem to request a review from anyone. It might be from me using the beta PR experience instead of the original one?

@@ -0,0 +1,251 @@
// Name: Format Numbers
// ID: formatNumbers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the extension id should generally contain your username.


getInfo() {
return {
id: "formatNumbers",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, should contain username and match the ID at the top

@Brackets-Coder
Copy link
Contributor

!format

@DogeisCut
Copy link
Author

Also the way I requested your review is this little panel at the top right.
image

@Brackets-Coder
Copy link
Contributor

Also the way I requested your review is this little panel at the top right. image

But it only works if I've already reviewed it?

@DogeisCut
Copy link
Author

DogeisCut commented Mar 19, 2025

Also the way I requested your review is this little panel at the top right. image

But it only works if I've already reviewed it?

Yeah, as far as I know.

@Brackets-Coder
Copy link
Contributor

also I guess !format doesn't work for me because I'm not a member, but you could probably run npm run format in the local fork of your repo to fix it

Copy link
Contributor

@Brackets-Coder Brackets-Coder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it looks fine

@@ -0,0 +1,251 @@
// Name: Format Numbers
// ID: dogeiscutformatnumbers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better

wow im glad this completley undid the vertical consistency you asked me to do.......
Copy link
Contributor

@Brackets-Coder Brackets-Coder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any problems

@DogeisCut DogeisCut requested a review from CST1229 March 19, 2025 19:06
@yuri-kiss
Copy link
Member

also I guess !format doesn't work for me because I'm not a member, but you could probably run npm run format in the local fork of your repo to fix it

thats not how the command works

Copy link

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.

@CubesterYT
Copy link
Member

Only the PR owner or a member can use the command

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: new extension Pull requests that add a new extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants