From 88cc50aa18bacf9f97d5f8f9cd8deba997b453b8 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Fri, 20 Jan 2017 11:26:18 -0800 Subject: [PATCH] Code review: more docs, move constants, clone util 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. --- src/blocks/scratch3_pen.js | 172 +++++++++++++++++++++++++++++-------- src/engine/target.js | 10 +++ src/util/clone.js | 17 ++++ test/fixtures/pen.sb2 | Bin 55038 -> 54853 bytes 4 files changed, 164 insertions(+), 35 deletions(-) create mode 100644 src/util/clone.js diff --git a/src/blocks/scratch3_pen.js b/src/blocks/scratch3_pen.js index 7f1f6a28..5d652531 100644 --- a/src/blocks/scratch3_pen.js +++ b/src/blocks/scratch3_pen.js @@ -1,15 +1,9 @@ var Cast = require('../util/cast'); +var Clone = require('../util/clone'); var Color = require('../util/color'); var MathUtil = require('../util/math-util'); var RenderedTarget = require('../sprites/rendered-target'); -// 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; - -var stateKey = 'Scratch.pen'; - /** * @typedef {object} PenState - the pen state associated with a particular target. * @property {Boolean} penDown - tracks whether the pen should draw for this target. @@ -19,11 +13,40 @@ var stateKey = 'Scratch.pen'; * diameter but not for pen color. */ +/** + * Host for the Pen-related blocks in Scratch 3.0 + * @param {Runtime} runtime - the runtime instantiating this block package. + * @constructor + */ +var Scratch3PenBlocks = function (runtime) { + /** + * The runtime instantiating this block package. + * @type {Runtime} + */ + this.runtime = runtime; + + /** + * The ID of the renderer Drawable corresponding to the pen layer. + * @type {int} + * @private + */ + this._penDrawableId = -1; + + /** + * The ID of the renderer Skin corresponding to the pen layer. + * @type {int} + * @private + */ + this._penSkinId = -1; + + this._onTargetMoved = this._onTargetMoved.bind(this); +}; + /** * The default pen state, to be used when a target has no existing pen state. * @type {PenState} */ -var defaultPenState = { +Scratch3PenBlocks.DEFAULT_PEN_STATE = { penDown: false, hue: 120, shade: 50, @@ -33,17 +56,25 @@ var defaultPenState = { } }; -var Scratch3PenBlocks = function (runtime) { - /** - * The runtime instantiating this block package. - * @type {Runtime} - */ - this.runtime = runtime; +/** + * 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? + * @type {int} + */ +Scratch3PenBlocks.PEN_ORDER = 1; - this._penSkinId = -1; +/** + * The minimum and maximum allowed pen size. + * @type {{min: number, max: number}} + */ +Scratch3PenBlocks.PEN_SIZE_RANGE = {min: 1, max: 255}; - this._onTargetMoved = this._onTargetMoved.bind(this); -}; +/** + * The key to load & store a target's pen-related state. + * @type {string} + */ +Scratch3PenBlocks.STATE_KEY = 'Scratch.pen'; /** * Clamp a pen size value to the range allowed by the pen. @@ -52,14 +83,19 @@ var Scratch3PenBlocks = function (runtime) { * @private */ Scratch3PenBlocks.prototype._clampPenSize = function (requestedSize) { - return MathUtil.clamp(requestedSize, 1, 255); + return MathUtil.clamp(requestedSize, Scratch3PenBlocks.PEN_SIZE_RANGE.min, Scratch3PenBlocks.PEN_SIZE_RANGE.max); }; +/** + * Retrieve the ID of the renderer "Skin" corresponding to the pen layer. If the pen Skin doesn't yet exist, create it. + * @returns {int} the Skin ID of the pen layer, or -1 on failure. + * @private + */ Scratch3PenBlocks.prototype._getPenLayerID = function () { - if (this._penSkinId < 0) { + if (this._penSkinId < 0 && this.runtime.renderer) { this._penSkinId = this.runtime.renderer.createPenSkin(); this._penDrawableId = this.runtime.renderer.createDrawable(); - this.runtime.renderer.setDrawableOrder(this._penDrawableId, penOrder); + this.runtime.renderer.setDrawableOrder(this._penDrawableId, Scratch3PenBlocks.PEN_ORDER); this.runtime.renderer.updateDrawableProperties(this._penDrawableId, {skinId: this._penSkinId}); } return this._penSkinId; @@ -71,10 +107,10 @@ Scratch3PenBlocks.prototype._getPenLayerID = function () { * @private */ Scratch3PenBlocks.prototype._getPenState = function (target) { - var penState = target.getCustomState(stateKey); + var penState = target.getCustomState(Scratch3PenBlocks.STATE_KEY); if (!penState) { - penState = JSON.parse(JSON.stringify(defaultPenState)); - target.setCustomState(stateKey, penState); + penState = Clone.simple(Scratch3PenBlocks.DEFAULT_PEN_STATE); + target.setCustomState(Scratch3PenBlocks.STATE_KEY, penState); } return penState; }; @@ -87,10 +123,12 @@ Scratch3PenBlocks.prototype._getPenState = function (target) { * @private */ Scratch3PenBlocks.prototype._onTargetMoved = function (target, oldX, oldY) { - var penState = this._getPenState(target); var penSkinId = this._getPenLayerID(); - this.runtime.renderer.penLine(penSkinId, penState.penAttributes, oldX, oldY, target.x, target.y); - this.runtime.requestRedraw(); + if (penSkinId >= 0) { + var penState = this._getPenState(target); + this.runtime.renderer.penLine(penSkinId, penState.penAttributes, oldX, oldY, target.x, target.y); + this.runtime.requestRedraw(); + } }; /** @@ -143,19 +181,36 @@ Scratch3PenBlocks.prototype.getPrimitives = function () { }; }; +/** + * The pen "clear" block clears the pen layer's contents. + */ Scratch3PenBlocks.prototype.clear = function () { var penSkinId = this._getPenLayerID(); - this.runtime.renderer.penClear(penSkinId); - this.runtime.requestRedraw(); + if (penSkinId >= 0) { + this.runtime.renderer.penClear(penSkinId); + this.runtime.requestRedraw(); + } }; +/** + * The pen "stamp" block stamps the current drawable's image onto the pen layer. + * @param {object} args - the block arguments. + * @param {object} util - utility object provided by the runtime. + */ Scratch3PenBlocks.prototype.stamp = function (args, util) { var penSkinId = this._getPenLayerID(); - var target = util.target; - this.runtime.renderer.penStamp(penSkinId, target.drawableID); - this.runtime.requestRedraw(); + if (penSkinId >= 0) { + var target = util.target; + this.runtime.renderer.penStamp(penSkinId, target.drawableID); + this.runtime.requestRedraw(); + } }; +/** + * The pen "pen down" block causes the target to leave pen trails on future motion. + * @param {object} args - the block arguments. + * @param {object} util - utility object provided by the runtime. + */ Scratch3PenBlocks.prototype.penDown = function (args, util) { var target = util.target; var penState = this._getPenState(target); @@ -166,10 +221,17 @@ Scratch3PenBlocks.prototype.penDown = function (args, util) { } var penSkinId = this._getPenLayerID(); - this.runtime.renderer.penPoint(penSkinId, penState.penAttributes, target.x, target.y); - this.runtime.requestRedraw(); + if (penSkinId >= 0) { + this.runtime.renderer.penPoint(penSkinId, penState.penAttributes, target.x, target.y); + this.runtime.requestRedraw(); + } }; +/** + * The pen "pen up" block stops the target from leaving pen trails. + * @param {object} args - the block arguments. + * @param {object} util - utility object provided by the runtime. + */ Scratch3PenBlocks.prototype.penUp = function (args, util) { var target = util.target; var penState = this._getPenState(target); @@ -180,6 +242,12 @@ Scratch3PenBlocks.prototype.penUp = function (args, util) { } }; +/** + * The pen "set pen color to {color}" block sets the pen to a particular RGB color. + * @param {object} args - the block arguments. + * @property {int} COLOR - the color to set, expressed as a 24-bit RGB value (0xRRGGBB). + * @param {object} util - utility object provided by the runtime. + */ Scratch3PenBlocks.prototype.setPenColorToColor = function (args, util) { var penState = this._getPenState(util.target); var rgb = Cast.toRgbColorObject(args.COLOR); @@ -190,39 +258,73 @@ Scratch3PenBlocks.prototype.setPenColorToColor = function (args, util) { penState.penAttributes.color4f[0] = rgb.r / 255.0; penState.penAttributes.color4f[1] = rgb.g / 255.0; penState.penAttributes.color4f[2] = rgb.b / 255.0; - - return rgb; }; +/** + * The pen "change pen color by {number}" block rotates the hue of the pen by the given amount. + * @param {object} args - the block arguments. + * @property {number} COLOR - the amount of desired hue rotation. + * @param {object} util - utility object provided by the runtime. + */ Scratch3PenBlocks.prototype.changePenHueBy = function (args, util) { var penState = this._getPenState(util.target); penState.hue = this._wrapHueOrShade(penState.hue + Cast.toNumber(args.COLOR)); this._updatePenColor(penState); }; +/** + * The pen "set pen color to {number}" block sets the hue of the pen. + * @param {object} args - the block arguments. + * @property {number} COLOR - the desired hue. + * @param {object} util - utility object provided by the runtime. + */ Scratch3PenBlocks.prototype.setPenHueToNumber = function (args, util) { var penState = this._getPenState(util.target); penState.hue = this._wrapHueOrShade(Cast.toNumber(args.COLOR)); this._updatePenColor(penState); }; +/** + * The pen "change pen shade by {number}" block changes the "shade" of the pen, related to the HSV value. + * @param {object} args - the block arguments. + * @property {number} SHADE - the amount of desired shade change. + * @param {object} util - utility object provided by the runtime. + */ Scratch3PenBlocks.prototype.changePenShadeBy = function (args, util) { var penState = this._getPenState(util.target); penState.shade = this._wrapHueOrShade(penState.shade + Cast.toNumber(args.SHADE)); this._updatePenColor(penState); }; +/** + * The pen "set pen shade to {number}" block sets the "shade" of the pen, related to the HSV value. + * @param {object} args - the block arguments. + * @property {number} SHADE - the amount of desired shade change. + * @param {object} util - utility object provided by the runtime. + */ Scratch3PenBlocks.prototype.setPenShadeToNumber = function (args, util) { var penState = this._getPenState(util.target); penState.shade = this._wrapHueOrShade(Cast.toNumber(args.SHADE)); this._updatePenColor(penState); }; +/** + * The pen "change pen size by {number}" block changes the pen size by the given amount. + * @param {object} args - the block arguments. + * @property {number} SIZE - the amount of desired size change. + * @param {object} util - utility object provided by the runtime. + */ Scratch3PenBlocks.prototype.changePenSizeBy = function (args, util) { var penAttributes = this._getPenState(util.target).penAttributes; penAttributes.diameter = this._clampPenSize(penAttributes.diameter + Cast.toNumber(args.SIZE)); }; +/** + * The pen "set pen size to {number}" block sets the pen size to the given amount. + * @param {object} args - the block arguments. + * @property {number} SIZE - the amount of desired size change. + * @param {object} util - utility object provided by the runtime. + */ Scratch3PenBlocks.prototype.setPenSizeTo = function (args, util) { var penAttributes = this._getPenState(util.target).penAttributes; penAttributes.diameter = this._clampPenSize(Cast.toNumber(args.SIZE)); diff --git a/src/engine/target.js b/src/engine/target.js index d5e37941..947df53d 100644 --- a/src/engine/target.js +++ b/src/engine/target.js @@ -129,10 +129,20 @@ Target.prototype.lookupOrCreateList = function (name) { */ Target.prototype.postSpriteInfo = function () {}; +/** + * Retrieve custom state associated with this target and the provided state ID. + * @param {string} stateId - specify which piece of state to retrieve. + * @returns {*} the associated state, if any was found. + */ Target.prototype.getCustomState = function (stateId) { return this._customState[stateId]; }; +/** + * Store custom state associated with this target and the provided state ID. + * @param {string} stateId - specify which piece of state to store on this target. + * @param {*} newValue - the state value to store. + */ Target.prototype.setCustomState = function (stateId, newValue) { this._customState[stateId] = newValue; }; diff --git a/src/util/clone.js b/src/util/clone.js new file mode 100644 index 00000000..5e1011a2 --- /dev/null +++ b/src/util/clone.js @@ -0,0 +1,17 @@ +/** + * Methods for cloning JavaScript objects. + * @type {object} + */ +var Clone = {}; + +/** + * Deep-clone a "simple" object: one which can be fully expressed with JSON. + * Non-JSON values, such as functions, will be stripped from the clone. + * @param {object} original - the object to be cloned. + * @returns {object} a deep clone of the original object. + */ +Clone.simple = function (original) { + return JSON.parse(JSON.stringify(original)); +}; + +module.exports = Clone; diff --git a/test/fixtures/pen.sb2 b/test/fixtures/pen.sb2 index ac4c780d71592eed1cf52c78ae8e0add4ea8df4e..5d68aa0c54a0eae94f28c9e35415ef6a1f855162 100644 GIT binary patch delta 1218 zcmeyjmig!!=K26{W)?061_lm>;7C<3=Y6p<{>%&vO&kmiJV4QcqWrAXjl0shSPCqbbJ#Wv^H#G6e^Q;XiazQAdVJKHMFl>Z#6vlragZ56wJ*jUSb)|>`; zIhks=NmG(I5?wOnk{cpt)T^|pI!d*wbUbnm&Tp@4UAN$8(^{SHyC!`7p*H18!o=z| zrmNgn>weO2-FH*!UH`0aUNaBWs;5M9|H(7TS$RFd_Nk|_9D`yZpQisrBY~OzLJtI% zwWLhovFR`M+wywJ#;hl|&&!{&DmxIY7_d)sw(N~5tff9>u9`g4Kfd)#Um957dhFre zZ8No`-kwX8Eip>p{P=LyTiLRR|H}kkm#gON^_8EocJCUOfcYE$ty-XQJ9CL^?uYx| ze}vwv++JKSWYqL7Rdec5&Hf~}8<7vQ*ca(9Ro}Y$QT&z)gT~fBO}w)hr58wk?eBMU z*w$RtExU00`E^xqUbXOWZmXVOsCv+7_l^3+H>01`o7~eb(cW6Kr}$@fv0;|t>T<8J z@>`w1kDi+{yzK0@rp|S+GS3a5`D=@ zriVB7q)na}QD83?DKu%qQcfd#uZCkL-Is0T&ptbo$0S#5*HU$w)A@hJC-r5Q<1gN1 zeQddXUVWXU%=Wsswg)Be+|RNXlzeXUHCXMF!pWKX6%Ll{DJopGFO#auGgH=O=a@B2{H_o1n;ieHjb@(0d>i2{C1=OTGb6sNS>DA`9|F31RnuXlCzTb8MNeZ}6^H#fZ%3lcxQFlQrwgwg3= zwG*`h&Oa9Oca1JN8TIz}ooklwH=SGBc-L{SjJ@2^j&GI61GgW&x6k!!S~uUvu(<~+ zcYJt0|I;3wtU3R+xOAs3e!aE*kAFz6`i1H)jh7$xw3L~5{@MB`!{*^N^ZyO5wQGMX z?74TRHreiK^C#)Uzn|Lk!tx&jDF1D?XW?gM1~CFTxuw92c;zAqFyo4~07UA!gU=+e zh}q>JCO*_$J9*kwolKC>hKoXv-?K0w%57-O+)^2BS>riuMs-QSrO6MkiL)KP0&+9}hcp2( delta 1407 zcmX@QhWX!G=K26{W)?061_llWfqE6M{x|z>a%it@8klS}lniu3c{ zhMg^5Y$jrFUC(-WsrY(sj>Z$Y<}bI+ySe^m&h57rmK} z?O#Hw-!$I;{mb^{OWWH05kHjve`d}-xc|!^Xk9jC-Fft2&I|r*=O)YLabC(hq=X`i+3F>EvtN}T@UMK-)FQHQ$r8(xjLWBp zGM>^{7}yu!xViKB#dE4}Z89^gKR&x${dt~6Lb$QO`q_EjYfo`mtKHs}*mk;p&#vWL zFP)4!yk*_038l;L+T|QlJbU!fj}13}{MP+2UzMT$_JvdXrCGas*E`fR{qyFJKeBZH zmnPwf3u{%S6m|PN=Pc2vO{&kD(Rag{@|r<#j#1R zl744huiBbf-#WD<_u}ORThh1fe{ZAa@KJt?x!KWY4_5Ab-Et*veO8>MzS{wZCKJPL zTR-ufe{@^YS@*1eh0mqE?mrK{_>}eZ?y2aVdinKR{+_(u>@F!S8}&WdKv_e>ylLIS zzTbxD_FaoU?o{xhDC0=oQ+e@c4+Uy%+@b}3uk`a*dl|C+vyN_kz#)$b8^cX(IRlMhJ{?GyK(6phi>EIuP;+%;}0jlXkTnv!_@Dr zeImk<<7BfWWklNwCw7e&pSU>l^rve07jbD6icr0eq?R@~}-_O5f&8^*NQyZ7O*Ol1B-W6?pw$xz5?KQt2|6MWfV&MCTci!!9Pv3V~)wp?ow_(wlKbsb2xmPRw zto?iN%YoE{eTh>1R(qL#?qYg=cE9AbgJ_)yE6 z$#<{lfaE6gT$PqE0v16AdIfpu0p5&ETnwP{32e(``>WFFE}RSuhI++i=#pM_T|bP1 zBp4V#m=&xAMg%D{Fc@LeQlVY2>I2X+5JuH