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

[3.x] Add search methods for pool arrays #60856

Merged
merged 1 commit into from
May 10, 2022
Merged

[3.x] Add search methods for pool arrays #60856

merged 1 commit into from
May 10, 2022

Conversation

timothyqiu
Copy link
Member

3.x version of #60855

Adds the following methods to pool arrays:

  • has()
  • count()
  • find()
  • rfind()

@timothyqiu timothyqiu added this to the 3.5 milestone May 7, 2022
@timothyqiu timothyqiu requested review from a team as code owners May 7, 2022 12:28
@timothyqiu
Add search methods for pool arrays
3d1644f 
* has()
* count()
* find()
* rfind()
@timothyqiu
Copy link
Member Author

Since we're unable to change the behavior of Array.find()/Array.rfind() on 3.x. I simply documented the current start index out of bounds behavior.

CC @kleonc

@kleonc
Copy link
Member

kleonc commented May 9, 2022

Since we're unable to change the behavior of Array.find()/Array.rfind() on 3.x. I simply documented the current start index out of bounds behavior.

Makes sense. No matter how rfind() added in this PR will be made to behave, there will be inconsistencies. So I guess it's acceptable as is.

kleonc
kleonc approved these changes May 9, 2022
View reviewed changes
Copy link
Member

@kleonc kleonc left a comment

Choose a reason for hiding this comment

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

Besides mentioned find()/rfind() inconsistencies it looks good.

@akien-mga akien-mga merged commit 2883706 into godotengine:3.x May 10, 2022
@akien-mga
Copy link
Member

Thanks!

@timothyqiu timothyqiu deleted the pool-array-find branch May 10, 2022 11:45
@timothyqiu timothyqiu mentioned this pull request May 11, 2022
Closed
@kdiduk kdiduk mentioned this pull request Jul 7, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kleonc kleonc kleonc approved these changes

Assignees
No one assigned
Labels
enhancement topic:core
Projects
None yet
Milestone
3.5
Development

Successfully merging this pull request may close these issues.
3 participants
@timothyqiu @kleonc @akien-mga