Skip to content

Fix tunneling issue in isTouching#9702

Merged
caleybrock merged 4 commits into
stagingfrom
touch-tunneling-fix
Jul 29, 2016
Merged

Fix tunneling issue in isTouching#9702
caleybrock merged 4 commits into
stagingfrom
touch-tunneling-fix

Conversation

@caleybrock

Copy link
Copy Markdown
Contributor

Issue example here: https://studio.code.org/projects/gamelab/BTI2WO6es1CTvJ7oJ5Vubw

Instead of keeping a copy of a portion of AABBops for isTouching, add it to AABBops with a modifyPosition parameter so that it does a check of if it's touching but does not modify the sprites position.

Comment thread apps/src/gamelab/GameLabSprite.js Outdated

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 think you can also pull the calculations here of newVel1 and newVel2 into your if (modifyPosition) { condition.

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.

udpated

@caleybrock caleybrock force-pushed the touch-tunneling-fix branch from b849989 to 401e1b2 Compare July 26, 2016 23:35
@islemaster

Copy link
Copy Markdown
Contributor

LGTM!

@caleybrock caleybrock force-pushed the touch-tunneling-fix branch from 401e1b2 to 99f39e1 Compare July 28, 2016 22:06
@caleybrock caleybrock force-pushed the touch-tunneling-fix branch from 99f39e1 to 86a4ebe Compare July 28, 2016 22:09
@caleybrock

Copy link
Copy Markdown
Contributor Author

@islemaster I added some tests for this.

@@ -0,0 +1,63 @@
var P5 = require("script!@cdo/apps/../lib/p5play/p5");
var P5Play = require("script!@cdo/apps/../lib/p5play/p5.play");

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.

Since you don't ever call these directly in this scope, you can have the require statements without assigning their result:

require("script!@cdo/apps/../lib/p5play/p5");
require("script!@cdo/apps/../lib/p5play/p5.play");

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 think you can do this as import statements too, like so:

import "script!@cdo/apps/../lib/p5play/p5";
import "script!@cdo/apps/../lib/p5play/p5.play";

@islemaster

Copy link
Copy Markdown
Contributor

Nice tests!

@caleybrock

Copy link
Copy Markdown
Contributor Author

updated :D

@islemaster

Copy link
Copy Markdown
Contributor

LGTM!

@caleybrock caleybrock merged commit 169026f into staging Jul 29, 2016
@caleybrock caleybrock deleted the touch-tunneling-fix branch July 29, 2016 17:22
deploy-code-org added a commit that referenced this pull request Jul 29, 2016
26780fa Merge pull request #9721 from code-dot-org/netsim-teacher-view (Brad Buchanan)
751ca8a Automatically built. (Continuous Integration)
da1d1b8 Merge pull request #9748 from code-dot-org/bump-block-svg-framed (Elijah Hamovitz)
f646303 Fix tests by passing locale into component (Brad Buchanan)
5bf6038 Merge pull request #9750 from code-dot-org/fix-topinstructions-feature-test (Elijah Hamovitz)
4de1a32 Removes last .new-router-log-modal style (Brad Buchanan)
2821f6e Move most styles into JSX (Brad Buchanan)
5f3e688 Merge pull request #9739 from code-dot-org/hideStagesNonAuthorized2 (Brent Van Minnen)
d327da6 move uitest-specific className from custom Component which does not support classnames to standard paragraph tag which does (Elijah Hamovitz)
c53f0ac Use px for filters marginBottom (Brad Buchanan)
31be25a Move sort methods onto the component (Brad Buchanan)
952a54e Indent conditional component (Brad Buchanan)
50cd059 Restore localized log browser header (Brad Buchanan)
e38b4f8 Manually specify blockSvgFramed clippath width (Elijah Hamovitz)
cf3c84e Merge pull request #9742 from code-dot-org/topInstructions-fix-unwanted-dashed-border (Elijah Hamovitz)
06d30ae Merge pull request #9744 from code-dot-org/fix-survey-view (Brendan Reville)
9b03d61 Merge pull request #9745 from code-dot-org/levelbuilder (Josh Lory)
26501e2 Level builder changes (Continuous Integration)
0cbfc1e Content changes (Continuous Integration)
3d4ef73 Merge branch 'staging' into fix-survey-view (Brendan Reville)
169026f Merge pull request #9702 from code-dot-org/touch-tunneling-fix (Caley Brock)
c863139 Merge pull request #9740 from code-dot-org/prevent-auto-starting-animations (Brad Buchanan)
12ee7ac Merge pull request #9729 from code-dot-org/survey-fixes (Brendan Reville)
c3af7a1 Merge branch 'staging' into survey-fixes (Brendan Reville)
98ace8a Teacher dashboard: Fix teacher view default course (Brendan Reville)
564d930 chat bubble tip stroke should default to none rather than white (Elijah Hamovitz)
9fba967 Clean up test syntax from code review (Caley Brock)
@islemaster islemaster mentioned this pull request Oct 20, 2016
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