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

Change raycastAll sorting default to false #5181

Merged
merged 4 commits into from Mar 22, 2023

Conversation

MushAsterion
Copy link
Collaborator

@MushAsterion MushAsterion commented Mar 21, 2023

As requested by @LeXXik in #5179, added an options parameter to raycastAll to allow non-sorting of results.

Changes to RigidBodyComponentSystem public API:

  • raycastFirst(start, end, options)
  • raycastAll(start, end, options)

Options declares as follow for not sorting:

{
    sort: false
}

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

@LeXXik
Copy link
Contributor

LeXXik commented Mar 21, 2023

Hey, thank you for this one! However, I would like to reiterate, that my view on this still stands as the default option (when no options specified) should be non-sorted result. And if a developer wants a sorting, it would be enabled via options or do own sorting on the results that would match their game logic.

@MushAsterion MushAsterion changed the title Make an option to not sort raycast results [BREAKING] Change raycastAll sorting default to false Mar 21, 2023
@MushAsterion
Copy link
Collaborator Author

Then it becomes breaking change, but there you go. Placing #5180 back on draft as if this change gets merged the code will need to be updated.

@LeXXik
Copy link
Contributor

LeXXik commented Mar 22, 2023

Thank you! I don't think it is a breaking change, unless it changes the code that was already released. I will leave it up to @willeastcott to decide.

@mvaligursky
Copy link
Contributor

Correct, this is not a breaking change, as it matches the currently released engine.

@mvaligursky mvaligursky changed the title [BREAKING] Change raycastAll sorting default to false Change raycastAll sorting default to false Mar 22, 2023
@MushAsterion
Copy link
Collaborator Author

MushAsterion commented Mar 22, 2023

Oh alright, I was referring at the current repo state but yeah that makes sense if that's from the released state!

@MushAsterion MushAsterion changed the title Change raycastAll sorting default to false Change raycastAll sorting default to false Mar 22, 2023
@willeastcott willeastcott merged commit 776f3c4 into playcanvas:main Mar 22, 2023
7 checks passed
@MushAsterion MushAsterion deleted the raycast-sort-as-option branch March 22, 2023 11:00
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

4 participants