godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.16k stars 97 forks source link

Add `merge_with()` and `intersect_with()` methods to the Array and Dictionary classes #1756

Open dalexeev opened 4 years ago

dalexeev commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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

dalexeev commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 https://github.com/godotengine/godot-proposals/issues/867.

okla commented 2 years ago

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.