Skip to content

assets: Update getSpriteValue to include target selection in sprite export#2561

Closed
ghost wants to merge 4 commits into
masterfrom
unknown repository
Closed

assets: Update getSpriteValue to include target selection in sprite export#2561
ghost wants to merge 4 commits into
masterfrom
unknown repository

Conversation

@ghost

@ghost ghost commented Jul 5, 2026

Copy link
Copy Markdown

Adds a new target identifier in the "sprite [name/dataURI]" reporter block, to allow one sprite to query other sprites' names or data URIs.

@ghost
ghost self-requested a review as a code owner July 5, 2026 00:46
@ghost ghost changed the title assets: Update getSpriteValue to include target selection in sprite export assets: Update getSpriteValue to include target selection in sprite export Jul 5, 2026
@github-actions github-actions Bot added the pr: change existing extension Pull requests that change an existing extension label Jul 5, 2026
@SharkPool-SP

Copy link
Copy Markdown
Collaborator

You have to hide the old block and add your changes as a separate new block.

You have to do this since you're mutating the block arguments, which will result in broken blocks for old projects using this

@ghost

ghost commented Jul 5, 2026

Copy link
Copy Markdown
Author

You have to hide the old block and add your changes as a separate new block.

You have to do this since you're mutating the block arguments, which will result in broken blocks for old projects using this

Whoops, sorry. I'm working on that now.

@ghost

ghost commented Jul 5, 2026

Copy link
Copy Markdown
Author

Fixed in commit 7ca35c6.

@SharkPool-SP

Copy link
Copy Markdown
Collaborator

The old blocks behaviour can just call the new one for simplicity but with the sprite hardcoded

@ghost

ghost commented Jul 5, 2026

Copy link
Copy Markdown
Author

The old blocks behaviour can just call the new one for simplicity but with the sprite hardcoded

Fixed in commit 34d8668. The legacy method now calls the new method, with the target hard-coded to _myself_ (the caller sprite).

Comment thread extensions/Lily/Assets.js Outdated
disableMonitor: true,
opcode: "getSpriteValue2",
blockType: Scratch.BlockType.REPORTER,
text: Scratch.translate("sprite [TARGET] [EXPORT]"),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the old string probably need to give translator context but this one even more so needs to do that

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in commit ebc96a3.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Do you want me to resolve this conversation?

@GarboMuffin GarboMuffin Jul 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no, that wasn't really what i meant

today: translators just see this

image

what is [EXPORT]? they have no idea from here. we're adding another completely ambiguous input. you should tell them what [TARGET] and [EXPORT] are. are they an input? are they an image? are they a dropdown? what's in that dropdown? context matters

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

for best results email thomas@turbowarp.org to get a quick review once you've addressed that

@GarboMuffin

Copy link
Copy Markdown
Member

can you disclose to what extent you wrote anything here vs. claude writing everything here?

you're not in trouble. it's actually pretty ok at it. but we would like to know

@ghost

ghost commented Jul 5, 2026

Copy link
Copy Markdown
Author

can you disclose to what extent you wrote anything here vs. claude writing everything here?

you're not in trouble. it's actually pretty ok at it. but we would like to know

These changes were made with Google Gemini 3.1 Pro, however all changes were manually tested by me, and I understand how the new code works. Some of the quick fixes were manually made by me.

@ghost

ghost commented Jul 7, 2026

Copy link
Copy Markdown
Author

Just checking in to see if there is anything else I need to adjust here!

@ghost ghost closed this by deleting the head repository Jul 11, 2026
@SharkPool-SP

Copy link
Copy Markdown
Collaborator

Bruh

@GarboMuffin

Copy link
Copy Markdown
Member

yeah that's crazy

@SharkPool-SP

Copy link
Copy Markdown
Collaborator

yeah that's crazy

What's not crazy is that I finished your requested changes to Messages+ :trollface:

@Brackets-Coder

Copy link
Copy Markdown
Contributor

So why did we delete the repo and close this right after it got an approval?

@SharkPool-SP

Copy link
Copy Markdown
Collaborator

So why did we delete the repo and close this right after it got an approval?

Account got deleted. So everything gets closed

@Brackets-Coder

Brackets-Coder commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

So why did we delete the repo and close this right after it got an approval?

Account got deleted. So everything gets closed

You would think bro wouldn't delete his account just a few hours after getting an approval 💀

GarboMuffin pushed a commit that referenced this pull request Jul 13, 2026
… export

Adds a new target identifier in the "sprite [name/dataURI]" reporter
block, to allow one sprite to query other sprites' names or data URIs.

The original `getSpriteValue` block is kept and hidden from the palette
so existing projects keep working; it now delegates to the new
`getSpriteValue2` implementation with the target hardcoded to `_myself_`.

Recreated from #2561, which was closed when the original author deleted
their account. Original authorship preserved.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GarboMuffin added a commit that referenced this pull request Jul 13, 2026
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: change existing extension Pull requests that change an existing extension

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants