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

Add Dictionary.merge() #59883

Merged
merged 1 commit into from
Jun 6, 2022
Merged

Add Dictionary.merge() #59883

merged 1 commit into from
Jun 6, 2022

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Apr 4, 2022

Implements godotengine/godot-proposals#3739, based on my comment here: godotengine/godot-proposals#1756 (comment)
with 2 differences:

  • it modifies the Dictionary instead of creating a new one
  • I added overwrite parameter

@KoBeWi KoBeWi added this to the 4.0 milestone Apr 4, 2022
@KoBeWi KoBeWi requested review from a team as code owners April 4, 2022 16:53
@YuriSizov
Copy link
Contributor

You seem to have a tiny style issue 👀

@akien-mga akien-mga merged commit 83421cd into godotengine:master Jun 6, 2022
@akien-mga
Copy link
Member

Thanks!

@KoBeWi KoBeWi deleted the merge_this branch June 6, 2022 21:40
@bend-n
Copy link
Contributor

bend-n commented Jun 7, 2022

In the array edition, it's append_array. Why is it merge in this instance? Wouldn't it be better to be consistent with naming?

@akien-mga
Copy link
Member

@bend-n "append" means "add at the end", so it makes sense for an ordered list like an array. Dictionaries aren't ordered and so the new (key, value) sets are not added at the end, but merged with the existing keys. So if this were a real-life language dictionary, "append" would be adding an appendix of new words, while "merge" creates a new edition of the dictionary with the words interspersed.

@Mickeon
Copy link
Contributor

Mickeon commented Jun 7, 2022

Can this be backported to 3.x?

@KoBeWi KoBeWi added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Jun 7, 2022
@bend-n
Copy link
Contributor

bend-n commented Jun 7, 2022

@bend-n "append" means "add at the end", so it makes sense for an ordered list like an array. Dictionaries aren't ordered and so the new (key, > value) sets are not added at the end, but merged with the existing keys. So if this were a real-life language dictionary, "append" would be adding an appendix of new words, while "merge" creates a new edition of the dictionary with the words interspersed.

Thanks, I see. Its more of a "makes sense" thing than a consistency thing.

@akien-mga
Copy link
Member

akien-mga commented Jun 9, 2022

Cherry-picked for 3.5.

Note that I didn't bother adding it to the GDNative header - if someone wants to, feel free.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants