validate say blocks for empty strings #45060
Conversation
| @@ -36,6 +36,29 @@ export const commands = { | |||
| return result; | |||
| }, | |||
|
|
|||
| // Return true if the specified sprite began speaking | |||
| // and the text was not an empty string. | |||
There was a problem hiding this comment.
super nit: replace "was" with "is" in the new comments in this file. i think it helps clarify at a glance that we only care about the current frame, not anything that previously happened in the program
| let result = false; | ||
| for (let i = 0; i < spriteIds.length; i++) { | ||
| if (commands.strictSpriteSpeechRenderedThisFrame.call(this, i)) { | ||
| result = true; | ||
| } | ||
| } | ||
| return result; |
There was a problem hiding this comment.
(same comment goes for the for loop in strictAnySpriteSpeaking) i think we can simplify this calculation with .filter. you'll see calculations like this commonly expressed as:
spriteIds
.filter((_spriteId, index) => commands.strictSpriteSpeechRenderedThisFrame.call(this, index))
.length > 0;the end result is the same, but it makes the code more functional (as in "functional programming," not related to functionality) and readable
There was a problem hiding this comment.
Love it! I changed both the functions you mentioned, as well as a couple more that were very similar. Reading up on how you can specify both (element, id) led me to realize there was actually an edge case bug that I also fixed.
getSpriteIds() returns an array of ids. In most cases, the index matches the element, but if a sprite is removed, they stop. We needed to be passing in the element, not the id, to be sure we were checking the right sprites.
| strictAnySpriteSpeaks() { | ||
| const spriteIds = this.getSpriteIdsInUse(); | ||
| return ( | ||
| spriteIds.filter(_spriteId => |
There was a problem hiding this comment.
ah, sorry, i used the underscore convention (_spriteId) because i thought it would be an unused variable -- in the previous implementation, i thought we were using index, not ID, but i must've misread it. this variable should be spriteId or just id (when you're iterating over a list of IDs, id is usually sufficient context)
There was a problem hiding this comment.
You didn't misread it; there was a bug (see other comment)! I was wondering about this underscore convention. I'll make one last update to rename to id, then assume you're still good to approve.
Add validation to check that students have entered some text into
sayblocks: