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.

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

Modifed Group.getClosestTo() and Group.getFurthestFrom() to add optional filter callback #2577

Merged
merged 5 commits into from
Jun 21, 2016

Conversation

LoneStranger
Copy link
Contributor

This PR changes the public-facing API.

A pair of new functions for the Group class, getClosestToFilter() and getFurthestFromFilter(), allowing a filter function to be passed and only members of the group who satisfy the filter will be considered for distance to the target object.

@photonstorm
Copy link
Collaborator

Thanks for the PR :)

I think there are a couple of things you could tweak, and then I can happily merge this:

  1. I don't think we need 2 new functions for this. Let's just make 'filter' an optional argument to the existing getFurthest / getClosest - then it doesn't change the API for anyone already using it, but provides the exact same new functionality as you've added here.

  2. Be sure to include the callback context as an argument, and then use callback.call to invoke the callback, rather than just hitting it directly like you do at the moment. That way it'll avoid scoping issues and keep it consistent with the way other similar functions work. Look at Group.iterate for an example of what I mean.

…hat 'kill' will be called if it goes below zero."

This reverts commit 18ae3b1.
…lude optional filter callback. Removed the getClosestToFilter and getFurthestFromFilter.
@LoneStranger
Copy link
Contributor Author

Ok, I took a look at Group.iterate and I modified the original functions instead, adding the optional callback and context.

@LoneStranger LoneStranger changed the title Added Group.getClosestToFilter() and Group.getFurthestFromFilter() Modifed Group.getClosestTo() and Group.getFurthestFrom() to add optional filter callback Jun 20, 2016
@photonstorm photonstorm merged commit 8cde880 into phaserjs:dev Jun 21, 2016
photonstorm added a commit that referenced this pull request Jun 21, 2016
@LoneStranger LoneStranger deleted the dev branch June 22, 2016 01:23
@LoneStranger LoneStranger restored the dev branch June 22, 2016 04:47
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.

None yet

2 participants