Skip to content
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

Jump to bottom

feat: expose platform effector #413

Merged
merged 9 commits into from
Oct 5, 2024
Merged

feat: expose platform effector #413

merged 9 commits into from
Oct 5, 2024

Conversation

dragoncoder047
Copy link
Contributor

idk why i didn't make a PR for this 2 weeks ago...

@pkg-pr-new pkg.pr.new
Copy link

pkg-pr-new bot commented Sep 19, 2024 •
edited
Loading

Open in Stackblitz

pnpm add https://pkg.pr.new/kaplayjs/kaplay@413

commit: d5781b1

@mflerackers
Copy link
Member

Did you test the platform effector? It wasn't exported because it is completely untested.

@dragoncoder047
Copy link
Contributor Author

Did you test the platform effector? It wasn't exported because it is completely untested.

Can't test it without it being exported!

@mflerackers
Copy link
Member

Yes, but you can export it locally to test it.

@dragoncoder047
Copy link
Contributor Author

OK, I tested it and worked on it to make it work. Added 2 new examples on how to use it too.

@dragoncoder047
Copy link
Contributor Author

@mflerackers

mflerackers
mflerackers reviewed Oct 4, 2024
View reviewed changes
Copy link
Member

@mflerackers mflerackers left a comment

Choose a reason for hiding this comment

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

shouldCollide is set as optional, but later it is said to exist, is that correct?
Shouldn't there be a default shouldCollide using the collisionSides?

@dragoncoder047
Copy link
Contributor Author

shouldCollide is set as optional, but later it is said to exist, is that correct? Shouldn't there be a default shouldCollide using the collisionSides?

Yes, it is lines 185-188

@lajbel lajbel changed the title expose platform effector feat: expose platform effector Oct 4, 2024
@lajbel lajbel merged commit 96c2e0b into kaplayjs:master Oct 5, 2024
2 checks passed
@dragoncoder047 dragoncoder047 deleted the platformEffector branch October 5, 2024 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mflerackers mflerackers mflerackers approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dragoncoder047 @mflerackers @lajbel