Skip to content

remove garbage and bubbling from tracked-controls #3589

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 21, 2018
Merged

remove garbage and bubbling from tracked-controls #3589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 31 additions & 12 deletions src/components/tracked-controls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,29 @@ var EYES_TO_ELBOW = {x: 0.175, y: -0.3, z: -0.03};
// Vector from eyes to elbow (divided by user height).
var FOREARM = {x: 0, y: 0, z: -0.175};

// Due to unfortunate name collision, add empty touches array to avoid Daydream error.
var EMPTY_DAYDREAM_TOUCHES = {touches: []};

var EVENTS = {
AXISMOVE: 'axismove',
BUTTONCHANGED: 'buttonchanged',
BUTTONDOWN: 'buttondown',
BUTTONUP: 'buttonup',
TOUCHSTART: 'touchstart',
TOUCHEND: 'touchend'
};

/**
* Tracked controls component.
* Wrap the gamepad API for pose and button states.
* Select the appropriate controller and apply pose to the entity.
* Observe button states and emit appropriate events.
*
* @property {number} controller - Index of controller in array returned by Gamepad API. Only used if hand property is not set.
* @property {number} controller - Index of controller in array returned by Gamepad API.
* Only used if hand property is not set.
* @property {string} id - Selected controller among those returned by Gamepad API.
* @property {number} hand - If multiple controllers found with id, choose the one with the given value for hand. If set, we ignore 'controller' property
* @property {number} hand - If multiple controllers found with id, choose the one with the
* given value for hand. If set, we ignore 'controller' property
*/
module.exports.Component = registerComponent('tracked-controls', {
schema: {
Expand All @@ -43,6 +57,8 @@ module.exports.Component = registerComponent('tracked-controls', {
this.controllerEuler = new THREE.Euler();

this.updateGamepad();

this.buttonEventDetails = {};
},

tick: function (time, delta) {
Expand Down Expand Up @@ -200,6 +216,9 @@ module.exports.Component = registerComponent('tracked-controls', {
if (!this.buttonStates[id]) {
this.buttonStates[id] = {pressed: false, touched: false, value: 0};
}
if (!this.buttonEventDetails[id]) {
this.buttonEventDetails[id] = {id: id, state: this.buttonStates[id]};
}

buttonState = controller.buttons[id];
this.handleButton(id, buttonState);
Expand All @@ -216,11 +235,12 @@ module.exports.Component = registerComponent('tracked-controls', {
* @returns {boolean} Whether button has changed in any way.
*/
handleButton: function (id, buttonState) {
var changed = this.handlePress(id, buttonState) |
this.handleTouch(id, buttonState) |
this.handleValue(id, buttonState);
var changed;
changed = this.handlePress(id, buttonState) |
this.handleTouch(id, buttonState) |
this.handleValue(id, buttonState);
if (!changed) { return false; }
this.el.emit('buttonchanged', {id: id, state: buttonState});
this.el.emit(EVENTS.BUTTONCHANGED, this.buttonEventDetails[id], false);
return true;
},

Expand Down Expand Up @@ -249,7 +269,7 @@ module.exports.Component = registerComponent('tracked-controls', {
for (i = 0; i < controllerAxes.length; i++) {
this.axis.push(controllerAxes[i]);
}
this.el.emit('axismove', this.axisMoveEventDetail);
this.el.emit(EVENTS.AXISMOVE, this.axisMoveEventDetail, false);
Copy link
Member

@dmarcos dmarcos May 19, 2018

Choose a reason for hiding this comment

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

I’m ok with the PR but I believe these strings should be allocated on stack and not on the heap so no subjects for GC

Copy link
Member Author

@ngokevin ngokevin May 19, 2018

Choose a reason for hiding this comment

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

I read strings get collected. But this PR also removes the string concatenations which do allocate and are messy anyways, constants are cleaner.

return true;
},

Expand All @@ -267,8 +287,8 @@ module.exports.Component = registerComponent('tracked-controls', {
// Not changed.
if (buttonState.pressed === previousButtonState.pressed) { return false; }

evtName = buttonState.pressed ? 'down' : 'up';
this.el.emit('button' + evtName, {id: id, state: buttonState});
evtName = buttonState.pressed ? EVENTS.BUTTONDOWN : EVENTS.BUTTONUP;
this.el.emit(evtName, this.buttonEventDetails[id], false);
previousButtonState.pressed = buttonState.pressed;
return true;
},
Expand All @@ -287,9 +307,8 @@ module.exports.Component = registerComponent('tracked-controls', {
// Not changed.
if (buttonState.touched === previousButtonState.touched) { return false; }

evtName = buttonState.touched ? 'start' : 'end';
// Due to unfortunate name collision, add empty touches array to avoid Daydream error.
this.el.emit('touch' + evtName, {id: id, state: buttonState}, true, {touches: []});
evtName = buttonState.touched ? EVENTS.TOUCHSTART : EVENTS.TOUCHEND;
this.el.emit(evtName, this.buttonEventDetails[id], false, EMPTY_DAYDREAM_TOUCHES);
previousButtonState.touched = buttonState.touched;
return true;
},
Expand Down
2 changes: 2 additions & 0 deletions tests/components/tracked-controls.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,7 @@ suite('tracked-controls', function () {
test('emits buttonup if button released', function () {
const emitSpy = sinon.spy(el, 'emit');
component.buttonStates[1] = {pressed: true, touched: false, value: 1};
component.buttonEventDetails[1] = {id: 1, state: component.buttonStates[1]};
controller.buttons[1].pressed = false;
controller.buttons[1].value = 0;
component.tick();
Expand Down Expand Up @@ -477,6 +478,7 @@ suite('tracked-controls', function () {
test('emits touchend if button no longer touched', function () {
const emitSpy = sinon.spy(el, 'emit');
component.buttonStates[1] = {pressed: false, touched: true, value: 1};
component.buttonEventDetails[1] = {id: 1, state: component.buttonStates[1]};
controller.buttons[1].touched = false;
controller.buttons[1].value = 0;
component.tick();
Expand Down