Skip to content

Gamelab: Accept strings in addition to constants for mouse blocks#9647

Merged
caleybrock merged 5 commits into
stagingfrom
const-to-str
Jul 26, 2016
Merged

Gamelab: Accept strings in addition to constants for mouse blocks#9647
caleybrock merged 5 commits into
stagingfrom
const-to-str

Conversation

@caleybrock

Copy link
Copy Markdown
Contributor

Remove constants as defaults for mouse blocks.

screen shot 2016-07-21 at 1 53 30 pm

Comment thread apps/lib/p5play/p5.play.js Outdated
if(buttonCode === undefined)
buttonCode = this.LEFT;

// Add other acceptable strings

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now only constants are accepted for these blocks so I had to add support for strings (which I ended up mapping back to constants). Not sure if this is the right place for these though since I have to modify p5.play, but I couldn't find another place to do it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to make this follow the pattern used with keys, with a mapping between the string names and the constants and a lookup.

@caleybrock

Copy link
Copy Markdown
Contributor Author

@islemaster can you take a look at this one too when you get a chance? thanks!

@islemaster

Copy link
Copy Markdown
Contributor

Is this something we intend to backport upstream? If not, can we wrap _isMouseButtonInState in GameLabP5 and convert a string argument to the appropriate constant before calling the original method?

@caleybrock

caleybrock commented Jul 25, 2016

Copy link
Copy Markdown
Contributor Author

@islemaster Are these changes what you meant by your comment?

(I don't think we are planning on putting these extra strings upstream into p5.play)

Comment thread apps/src/gamelab/GameLabP5.js Outdated
};

// Overrride p5.play so we can use strings in addition to constants.
window.p5.prototype._isMouseButtonInState = function (buttonCode, state) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would go another step and implement this as a wrapper around the existing _isMouseButtonInState instead of copying the implementation over:

// Overrride p5.play so we can use strings in addition to constants.
const p5IsMouseButtonInState = window.p5.prototype._isMouseButtonInState;
window.p5.prototype._isMouseButtonInState = function (buttonCode, state) {
  return p5IsMouseButtonInState(this._clickKeyFromString(buttonCode), state);
};

@caleybrock

Copy link
Copy Markdown
Contributor Author

@islemaster should be good to go now! thanks for your help :)

@islemaster

Copy link
Copy Markdown
Contributor

LGTM! :shipit:

@caleybrock caleybrock merged commit 326fae7 into staging Jul 26, 2016
@caleybrock caleybrock deleted the const-to-str branch July 26, 2016 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants