-
Notifications
You must be signed in to change notification settings - Fork 479
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
[Maker][CP] - Use blank array param for buzzer #44877
Conversation
just read through the thread and it looks like some updates might be coming through, so i'll hold off until then. in the meantime, it looks like code-dot-org/apps/src/lib/kits/maker/dropletConfig.js Lines 295 to 301 in e262b1c
do we want to update/change the default for that block as well? |
@madelynkasula - I had the same thought and made a ticket to track it! https://codedotorg.atlassian.net/jira/software/c/projects/STAR/boards/26?modal=detail&selectedIssue=STAR-2103 I wanted to split it out because I want to get this change out ASAP for curriculum and want to make sure I'm deprecating carefully. We only use buzzer.play internally and it does the same things as PlaySong, so we should just remove Play (and stringifySong). |
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.
left a nit comment/question, but it's not a merge-blocker
paramButtons: {minArgs: 1, maxArgs: 2} | ||
}, | ||
{ | ||
func: 'buzzer.playSong', | ||
category: CIRCUIT_CATEGORY, | ||
paletteParams: ['notes', 'tempo'], | ||
params: [stringifySong(SONG_CHARGE), 120], | ||
params: [`[${stringifySong(SONG_CHARGE[0])}]`, 120], |
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.
nit: SONG_CHARGE[0]
is the same as SONG_SINGLE_NOTE
-- should we use that instead?
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.
SONG_CHARGE[0] is ['G3', 0.25] and SONG_SINGLE_NOTE is ['G3']. PlayNotes takes a one dimensional value (note) and PlaySong takes two dimensional (note, length).
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.
ahh, my bad!
Curriculum request to style the block parameters for buzzer.playSong/playNotes. Old notes were displayed in a vertical array to minimize impact of a really long array. This change removes the default values placed in the array. PlayNotes uses an empty array and PlaySong uses an array with spaces - discussion here: https://codedotorg.slack.com/archives/C0T0EGE8L/p1644507117916229
Before / After in Blocks:
Before/After in Text:
PR Checklist: