Implement pen blocks #384

cwillisf · 2017-01-19T19:55:43Z

Resolves

Along with scratchfoundation/scratch-render#72, this resolves scratchfoundation/scratch-render#6

Proposed Changes

These blocks implement pen features as found in Scratch 2.0

Supporting changes include:

Target is now an event emitter
RenderedTarget now emits an event when it moves
Target can now store additional arbitrary data, called "custom state" in the code, using a Target's setCustomState and getCustomState methods. This is used to store per-target pen state without requiring Target or RenderedTarget to know anything about the pen.
Cast can now cast to an RGB color object.
Color now has functions to convert between RGB and HSV, constants for for black & white, and a mixRgb function to lerp between two colors.

Reason for Changes

Implementing pen blocks brings us closer to Scratch 2.0 compatibility.

Test Coverage

All newly-added util functions are covered:

toRbgColorObject from util/cast.js is covered in test/unit/util_cast.js
hsvToRgb, rgbToHsv, and mixRgb from util/color.js are covered in test/unit/util_color.js

All pen blocks are covered by the new pen.js integration test. Unfortunately our current integration test setup makes it very difficult (impossible?) to hook to the renderer, so the blocks which actually draw something will skip parts of their functionality during this test. The three affected pen blocks are clear, pen down, and stamp.

These blocks implement pen features as found in Scratch 2.0 Supporting changes include: - `Target` is now an event emitter - `RenderedTarget` now emits an event when it moves - `Target` can now store arbitrary "extra" data, called "custom state" in the code, using a `Target`'s `setCustomState` and `getCustomState` methods. This is used to store per-target pen state without requiring `Target` or `RenderedTarget` to know anything about the pen. - `Cast` can now cast to an RGB color object. - `Color` now has functions to convert between RGB and HSV, constants for for black & white, and a `mixRgb` function to lerp between two colors.

src/util/color.js

+        g = p;
+        b = q;
+        break;
+    }


Also, fix a math error in `Color.rgbToHsv`. Newly covered functions: - `toRbgColorObject` from `util/cast.js` - `hsvToRgb` from `util/colors.js` - `rgbToHsv` from `util/colors.js` - `mixRgb` from `util/colors.js`

See `pen.sb2` for details

thisandagain

This looks great overall. Thanks for expanding the test coverage. Some fairly minor notes for the most part.

src/blocks/scratch3_pen.js

+// Place the pen layer in front of the backdrop but behind everything else
+// We should probably handle this somewhere else... somewhere central that knows about pen, backdrop, video, etc.
+// Maybe it should be in the GUI?
+var penOrder = 1;


src/blocks/scratch3_pen.js

+     */
+    this.runtime = runtime;
+
+    this._penSkinId = -1;


src/blocks/scratch3_pen.js

+ * @private
+ */
+Scratch3PenBlocks.prototype._clampPenSize = function (requestedSize) {
+    return MathUtil.clamp(requestedSize, 1, 255);


src/blocks/scratch3_pen.js

+    return MathUtil.clamp(requestedSize, 1, 255);
+};
+
+Scratch3PenBlocks.prototype._getPenLayerID = function () {


src/blocks/scratch3_pen.js

+Scratch3PenBlocks.prototype._getPenState = function (target) {
+    var penState = target.getCustomState(stateKey);
+    if (!penState) {
+        penState = JSON.parse(JSON.stringify(defaultPenState));


src/blocks/scratch3_pen.js

+
+Scratch3PenBlocks.prototype.clear = function () {
+    var penSkinId = this._getPenLayerID();
+    this.runtime.renderer.penClear(penSkinId);


src/engine/target.js

@@ -112,10 +129,20 @@ Target.prototype.lookupOrCreateList = function (name) {
 */
 Target.prototype.postSpriteInfo = function () {};

+Target.prototype.getCustomState = function (stateId) {


src/engine/target.js

+    return this._customState[stateId];
+};
+
+Target.prototype.setCustomState = function (stateId, newValue) {


Changes include: - Added missing JSDoc for items in `scratch3_pen.js` and `target.js`. - Moved constants used by `Scratch3PenBlocks` into the class. - Created a constant for minimum and maximum pen size. - Added `util/clone.js` to host cloning functionality. - Pen blocks now check for the renderer before trying to use it. - The pen integration test covers all blocks, though `clear`, `stamp`, and `pen down` will skip some of their functionality when there is no renderer.

SillyInventor · 2017-01-21T23:21:17Z

For pen clear to work you need to set the skin to dirty:
this.runtime.renderer._allSkins[penSkinId]._canvasDirty=true;

cwillisf added the pr - changes requested label Jan 19, 2017

cwillisf added this to the February 16 milestone Jan 19, 2017

cwillisf self-assigned this Jan 19, 2017

Lint fixes and related cleanup

a6190da

towerofnix reviewed Jan 19, 2017

View reviewed changes

src/util/color.js

g = p;

b = q;

break;

}

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

Christopher Willis-Ford added 2 commits January 19, 2017 15:24

Add unit tests for new color-related functionality

d084572

Also, fix a math error in `Color.rgbToHsv`. Newly covered functions: - `toRbgColorObject` from `util/cast.js` - `hsvToRgb` from `util/colors.js` - `rgbToHsv` from `util/colors.js` - `mixRgb` from `util/colors.js`

Add pen integration test

72f17d6

See `pen.sb2` for details

cwillisf added pr - needs review and removed pr - changes requested labels Jan 20, 2017

cwillisf requested review from thisandagain and rschamp January 20, 2017 00:07

cwillisf assigned thisandagain and rschamp Jan 20, 2017

thisandagain suggested changes Jan 20, 2017

View reviewed changes

thisandagain removed their assignment Jan 20, 2017

thisandagain added pr - changes requested and removed pr - needs review labels Jan 20, 2017

cwillisf added pr - needs review and removed pr - changes requested labels Jan 20, 2017

cwillisf assigned thisandagain Jan 20, 2017

Undo accidental harmless but unnecessary change

f8088be

thisandagain approved these changes Jan 20, 2017

View reviewed changes

thisandagain added pr - good to merge and removed pr - needs review labels Jan 20, 2017

thisandagain unassigned thisandagain and rschamp Jan 20, 2017

cwillisf merged commit bdabe2e into scratchfoundation:develop Jan 20, 2017

cwillisf deleted the pen branch January 20, 2017 21:22

SillyInventor mentioned this pull request Jan 22, 2017

Clear Block Doesn't Work #390

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement pen blocks #384

Implement pen blocks #384

cwillisf commented Jan 19, 2017 •

edited

This comment was marked as abuse.

This comment was marked as abuse.

thisandagain left a comment

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

SillyInventor commented Jan 21, 2017

Implement pen blocks #384

Implement pen blocks #384

Conversation

cwillisf commented Jan 19, 2017 • edited

Resolves

Proposed Changes

Reason for Changes

Test Coverage

This comment was marked as abuse.

This comment was marked as abuse.

thisandagain left a comment

Choose a reason for hiding this comment

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

SillyInventor commented Jan 21, 2017

cwillisf commented Jan 19, 2017 •

edited