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 merge_with() and intersect_with() methods to the Array and Dictionary classes #1756

Open
dalexeev opened this issue Nov 1, 2020 · 13 comments

Comments

@dalexeev
Copy link
Member

dalexeev commented Nov 1, 2020

Describe the project you are working on:
A game

Describe the problem or limitation you are having in your project:
Right now, no. But I may need it in the future or someone else needs it now.
See also: godotengine/godot#11073, #1718

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
We can add merge_with() and intersect_with() methods (maybe we need other names) for Array and Dictionary classes. We can also add operators | and & (as shorthand) for these data types.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:

var arr1 = [1, 2]
var arr2 = [2, 3]

# print(arr1 + arr2)
print(arr1.append_array(arr2)) # [1, 2, 2, 3]

# print(arr2 + arr1)
print(arr2.append_array(arr1)) # [2, 3, 1, 2]

# print(arr1 | arr2)
print(arr1.merge_with(arr2)) # [1, 2, 3]

# print(arr2 | arr1)
print(arr2.merge_with(arr1)) # [2, 3, 1]

# print(arr1 & arr2)
print(arr1.intersect_with(arr2)) # [2]

var dict1 = {a = 1, b = 2}
var dict2 = {b = 3, c = 4}
var dict3 = {b = 2, d = 5}

# print(dict1 | dict2)
print(dict1.merge_with(dict2)) # {a: 1, b: 2, c: 4}

# print(dict2 | dict1)
print(dict2.merge_with(dict1)) # {b: 3, c: 4, a: 1}

# print(dict1 & dict2)
print(dict1.intersect_with(dict2)) # {}

# print(dict1 & dict3)
print(dict1.intersect_with(dict3)) # {b: 2}

If this enhancement will not be used often, can it be worked around with a few lines of script?:
Maybe. But I think anyone who needs these functions will be happy if these functions are already implemented out of the box.

Is there a reason why this should be core and not an add-on in the asset library?:
This is basic functionality that is also available in some programming languages.

@Xrayez
Copy link
Contributor

Xrayez commented Nov 1, 2020

I once had a somewhat crazy idea of adding these boolean operation methods to GDScript functions (preferably as an extension to GDScript godotengine/godot#15639, of course). What I say is that merge(), intersect() etc. could in theory work with more datatypes (compatible with Variant).

PoolVector2Array, PoolVector3Array

Like with merge(polygon_a, polygon_b) godotengine/godot#28987. Intersection can also be done for lines.

Rect2, AABB

merge(rect_a, rect_b), intersect(rect_a, rect_b) as in Rect2.merge(), Rect2.intersect(), same for AABB.

Color

merge(color_a, color_b) for Color.blend()?

And more types...

But for this to be general-purpose, those methods has to be named as union(), intersection(), difference(), xor().

And by the way, xor() method could also help bool operations: godotengine/godot#18816. 😄


This likely won't be implemented in Godot like that, but having those for built-in containers like Array and Dictionary make sense to me.

@Error7Studios
Copy link

Error7Studios commented Nov 1, 2020

I think matching_keys() and matching_values() should also be added for dictionaries.
That way, you can make sure you don't have any duplicate keys/values before you merge them.
This would be different from intersect(), which would return only a matching key-value pair.

var dict1 = {a = 1, b = 2}
var dict2 = {b = 3, c = 4}
var dict3 = {b = 2, d = 5}
var dict4 = {c = 4, d = 6}

print(dict1.matching_keys(dict2)) # ["b"]
print(dict1.matching_keys(dict4)) # []

print(dict1.matching_values(dict2)) # []
print(dict1.matching_values(dict3)) # [2]


func get_safely_merged_dict(__dict_a: Dictionary, __dict_b: Dictionary, __matching_values_allowed: bool) -> Dictionary:
	var __matching_keys := __dict_a.matching_keys(__dict_b)
	if !__matching_keys.empty():
		assert(false, str("Dictionaries have matching keys: ", __matching_keys))

	var __matching_values := __dict_a.matching_values(__dict_b)
	if !__matching_values.empty():
		if !__matching_values_allowed:
			assert(false, str("Dictionaries have matching values: ", __matching_values))
		else:	
			var __matching_entries := __dict_a.intersect_with(__dict_b)
			if !__matching_entries.empty():
				assert(false, str("Dictionaries have matching entries: ", __matching_entries))
		
	return __dict_a.merge_with(__dict_b))

@dalexeev
Copy link
Member Author

dalexeev commented Nov 1, 2020

@Error7Studios You may be right, but remember that Dictionary has keys() and values() methods.

var dict1 = {a = 1, b = 2}
var dict2 = {b = 3, c = 4}
var dict3 = {b = 2, d = 5}

print(dict1 & dict2) # {}
print(dict1.keys() & dict2.keys()) # ["b"]

print(dict1 & dict3) # {b: 2}
print(dict1.keys() & dict3.keys()) # ["b"]

@Error7Studios
Copy link

Ah, now I see what you mean by:

We can also add operators | and & (as shorthand) for these data types.

That would work, too.

@Calinou
Copy link
Member

Calinou commented Nov 1, 2020

If we have map/filter/reduce methods implemented in Array and Dictionary, this can probably be achieved with such methods too.

@Calinou Calinou changed the title Add merge_with() and intersect_with() methods for Array and Dictionary classes Add merge_with() and intersect_with() methods to the Array and Dictionary classes Nov 1, 2020
@dalexeev
Copy link
Member Author

dalexeev commented Nov 1, 2020

Since dictionaries in GDScript preserve the order of elements, it is possible to add an optional overwrite_keys parameter to the merge_with() method:

var dict1 = {a = 1, b = 2}
var dict2 = {b = 3, c = 4}

print(dict1.merge_with(dict2))        # {a: 1, b: 2, c: 4}
print(dict1.merge_with(dict2, true))  # {a: 1, b: 3, c: 4}
print(dict2.merge_with(dict1))        # {b: 3, c: 4, a: 1}
print(dict2.merge_with(dict1, true))  # {b: 2, c: 4, a: 1}

Also, it's not entirely clear what to do if the original arrays contain duplicate elements.

[2, 2, 3] | [2, 5] # `[2, 2, 3, 5]` or `[2, 3, 5]`?
[2, 2, 3, 5] & [2, 2] # `[2, 2]` or `[2]`?

@vnen
Copy link
Member

vnen commented Nov 3, 2020

Also, it's not entirely clear what to do if the original arrays contain duplicate elements.

This is my main gripe with this. Those methods make sense in a set, but an array is not a set (that is, it may contain duplicate elements unlike a set). So it's not clear what any of those methods do with duplicates (when the duplicate is on the original).

I also think we should have external libraries for those things as these small functions can pile up quite quickly and start becoming bloat if we don't draw a line.

@dalexeev
Copy link
Member Author

dalexeev commented Nov 3, 2020

Those methods make sense in a set, but an array is not a set

In such a case, these methods should take this into account. That is, the following option is correct:

[2, 2, 3] | [2, 5] # [2, 2, 3, 5]
[2, 2, 3, 5] & [2, 2] # [2, 2]

For example, it can be used like this:

func gcd(a: int, b: int) -> int:
    return product(factor(a) & factor(b))

func lcm(a: int, b: int) -> int:
    return product(factor(a) | factor(b))

In addition, for dictionaries, everything remains in effect, since they do not have the same keys.

can pile up quite quickly and start becoming bloat

Until extensions appear, it will be much less convenient. Also, operator overloading is unlikely to appear in GDScript.

@KoBeWi
Copy link
Member

KoBeWi commented Nov 4, 2020

Related: #867

I personally like the idea of Dictionary.merge if it was something like in Ruby. It allows you to easily create a Dictionary with default values, e.g. var dict = defaults.merge(other_dict) will make a copy of defaults dictionary with matching keys being replaced by values from other_dict.

@okla
Copy link

okla commented Jul 14, 2022

I think that Set type should be added to GDScript and then merge/intersect/xor/etc operations on Dictionaries would simply become operations on their keys (which will be of this new Set type). Not sure about Arrays though, I've never seen bool operations on Arrays in other languages.

@mnn
Copy link

mnn commented Jul 14, 2022

Not sure about Arrays though, I've never seen bool operations on Arrays in other languages.

Set-like operations on arrays (lists) are for example in Haskell (string in Haskell is "array of characters"):

>>> "Hello World!" \\ "ell W"
"Hoorld!"
>>> "dog" `union` "cow"
"dogcw"
>>> [1,2,3,4] `intersect` [2,4,6,8]
[2,4]

source: https://hackage.haskell.org/package/base-4.16.2.0/docs/Data-List.html#g:20

@Calinou
Copy link
Member

Calinou commented Jul 14, 2022

I think that Set type should be added to GDScript and then merge/intersect/xor/etc operations on Dictionaries would simply become operations on their keys (which will be of this new Set type).

This is already being tracked in #867.

@okla
Copy link

okla commented Jul 14, 2022

This is already being tracked in #867.

Yeah, I've seen this proposal.

Set-like operations on arrays (lists) are for example in Haskell (string in Haskell is "array of characters"):

This may be just a free side effect of Haskell array/string implementation, not a deliberate addition to the language.

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

No branches or pull requests

8 participants