Skip to content

Fix collision bug and setup tests for GameLabP5#9714

Merged
caleybrock merged 4 commits into
stagingfrom
test-collision
Jul 27, 2016
Merged

Fix collision bug and setup tests for GameLabP5#9714
caleybrock merged 4 commits into
stagingfrom
test-collision

Conversation

@caleybrock

Copy link
Copy Markdown
Contributor

No description provided.


if (sprite.collider instanceof this.CircleCollider) {
return window.p5.dist(mousePosition.x, mousePosition.y, sprite.collider.center.x, sprite.collider.center.y) < sprite.collider.radius;
return this.dist(mousePosition.x, mousePosition.y, sprite.collider.center.x, sprite.collider.center.y) < sprite.collider.radius;

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.

This is the actual fix of the collision bug.

Comment thread apps/Gruntfile.js Outdated
{pattern: 'build/**/*', watched: false, included: false, nocache: true},
{pattern: 'static/**/*', watched: false, included: false, nocache: true},
{pattern: 'lib/p5play/p5.js'},
{pattern: 'lib/p5play/p5.play.js'},

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.

@pcardune I'm pretty sure we did this correctly, can you confirm? p5.js and p5.play.js absolutely insist on being included as script tags so they can load stuff onto window (they are not requirejs-compatible) so we put them here to include them in tests ahead of any of the actual test files.

It's not great that this means they'll be included for any/every test run of any kind, but I didn't see a better way to do this and use them in our gamelab unit tests. I also doubt they'll slow the build down very much, as they're both prebuilt libraries, leaf nodes in the dependency tree.

Thoughts?

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.

You could import them where you need them using the webpack script-loader. See https://github.com/webpack/script-loader.

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.

updated - thanks for the suggestion @pcardune

@balderdash

Copy link
Copy Markdown
Contributor

LGTM

@caleybrock caleybrock merged commit fa45ba4 into staging Jul 27, 2016
@caleybrock caleybrock deleted the test-collision branch July 27, 2016 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants