-
Notifications
You must be signed in to change notification settings - Fork 3
Neighborhood: add show/hide buckets #74
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
Conversation
hannahbergam
left a comment
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.
Just trying to fully understand how the paint count works in a couple questions in reset. Otherwise, looks good!
| * Calls resetTile for each tile in the grid, clearing all paint. | ||
| */ | ||
| resetTiles() { | ||
| this.showBuckets = true; |
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.
Does a reset show the buckets again because a student is starting over, so paint is restored to the bucket? Does the paint count (cell.getCurrentValue) change somewhere else?
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.
yep! reset returns the map to its original state. It eventually calls subtype.getCell() which gets the original value for the cell.
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 okay awesome. Thanks!
| if (cell.getOriginalValue() > 0) { | ||
| const newValue = cell.getCurrentValue() > 0 ? cell.getCurrentValue() : ""; | ||
| // drawImage_ calls getAsset. If currentValue() is 0, getAsset will return | ||
| // The new value is the number of paint units to show on the screen. If we have > 0 units and we |
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.
Can you help me understand when a reset wouldn't restore the value to the originalValue?
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.
reset should always restore the value to originalValue. This function (updateItemImage) is called when we change the cell mid-run
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.
🤦🏻♀️I still had reset() in my brain. Perfect:)
hannahbergam
left a comment
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.
LGTM!
Add the option to show or hide paint buckets on the screen. The client will call
neighborhood.setBucketVisiblity(boolean)to show or hide buckets on the screen. When we see that call, we will iterate over all the cells and redraw any that contain active paint buckets. In addition, when drawing a cell we check the bucket visibility boolean to decide whether to show or hide the bucket.The asset name we were already using was
paintCan, but I named the functions withbucketbecause that is what we are using on the code-dot-org and javabuilder side.I also updated the README to include instructions for running maze with local Code Studio.