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

Array::insert consistent with Pool*Array::insert #47406

Merged
merged 1 commit into from
Aug 28, 2021
Merged

Array::insert consistent with Pool*Array::insert #47406

merged 1 commit into from
Aug 28, 2021

Conversation

mashumafi
Copy link
Contributor

@mashumafi mashumafi commented Mar 27, 2021 •
edited
Loading

There were some inconsistencies with Array and Pool*Array that were resolved a few month sago such as exposing new methods. Another difference is the return value of the insert function. This change adds an Error response to the Array::insert function similar to how Pool*Array have Error on their insert..

@mashumafi mashumafi requested a review from a team as a code owner March 27, 2021 04:55
qarmin
qarmin requested changes Mar 27, 2021
View reviewed changes
core/variant/array.cpp Outdated Show resolved Hide resolved
@mashumafi mashumafi requested review from qarmin and removed request for a team March 27, 2021 20:17
@mashumafi mashumafi mentioned this pull request Mar 27, 2021
Closed
@Calinou Calinou added this to the 4.0 milestone Mar 27, 2021
qarmin
qarmin requested changes Mar 27, 2021
View reviewed changes
core/variant/array.cpp Outdated Show resolved Hide resolved
core/variant/array.cpp Outdated Show resolved Hide resolved
@akien-mga akien-mga requested a review from a team March 28, 2021 12:09
@akien-mga
Copy link
Member

Sorry for the delay reviewing. This looks fine to me, but it should likely be rebased to make sure that it still compiles fine. And I believe the class reference might need to be updated too to take the changes into account in doc/classes/Array.xml.

mhilbrunner
mhilbrunner approved these changes Aug 28, 2021
View reviewed changes
Copy link
Member

@mhilbrunner mhilbrunner left a comment

Choose a reason for hiding this comment

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

Rebased, built & tested locally, this indeed still works with current master. I'll see if any of the docs need changing in a bit.

@mhilbrunner mhilbrunner merged commit 04c64b5 into godotengine:master Aug 28, 2021
@mhilbrunner
Copy link
Member

Thank you, and sorry again for the delay!

mhilbrunner added a commit to mhilbrunner/godot that referenced this pull request Aug 28, 2021
@mhilbrunner
Fix Array class docs after godotengine#47406
a2473d7
@mhilbrunner mhilbrunner mentioned this pull request Aug 28, 2021
Merged
curious-broccoli added a commit to curious-broccoli/godot that referenced this pull request Aug 24, 2023
@curious-broccoli
Clarify Array class methods that return error
d8e7ce5 
related to godotengine#47406
mandryskowski pushed a commit to mandryskowski/godot that referenced this pull request Oct 11, 2023
@curious-broccoli @mandryskowski
Clarify Array class methods that return error
761a3a9 
related to godotengine#47406
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhilbrunner mhilbrunner mhilbrunner approved these changes

@qarmin qarmin qarmin approved these changes

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

Successfully merging this pull request may close these issues.
5 participants
@mashumafi @akien-mga @mhilbrunner @qarmin @Calinou