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

Dictionary.duplicate(true) still does not make a recursive deep copy #96627

Open
BlakeKessler opened this issue Sep 5, 2024 · 1 comment
Open

Comments

@BlakeKessler
Copy link

Tested versions

Personal experience: 4.3
I have also seen documentation of this bug from over 4 years ago, as well as some more recent reports.

System information

Windows 11

Issue description

The documentation for the Dictionary class states "If deep is true, inner Dictionary and Array keys and values are also copied, recursively." for the duplicate method. However, the objects in dictionaries created via the Dictionary.duplicate(true) function still act as references to those in the duplicated dictionary, implying either that a shallow copy or a non-recursive deep copy was made, instead of the recursive deep copy proscribed by the documentation.

Steps to reproduce

Create a dictionary. Put an array of objects passed by reference into the dictionary. Call duplicate(true) on the dictionary. Alter an object in the array in the result of the duplicate function. Print or otherwise display the object it corresponds to in the original dictionary. It will appear as if a shallow copy had been made, despite described behavior specifying a deep copy.

Minimal reproduction project (MRP)

I do not currently have access to a Godot editor to make a MRP. I hope the issue is simple enough fort my description to be sufficient.

@dalexeev
Copy link
Member

dalexeev commented Sep 6, 2024

Duplicating objects is tricky because it's not always obvious whether it's needed or not. Resources can often be considered shared. Nodes can have children and connections. Objects other than Nodes and Resources don't even have a built-in duplicate() method. Object properties are divided into those exported with PROPERTY_USAGE_STORAGE and those not essential for serialization/duplication.

Try the following function as a workaround:

Code
static func recursive_duplicate(value: Variant, recursion_count: int = 0) -> Variant:
    const MAX_RECURSION: int = 100

    if value is Array:
        var array: Array = value
        var copy: Array = Array(
            [],
            array.get_typed_builtin(),
            array.get_typed_class_name(),
            array.get_typed_script(),
        )

        if recursion_count > MAX_RECURSION:
            push_error("Max recursion reached.")
            return copy
        recursion_count += 1

        for element: Variant in array:
            copy.append(recursive_duplicate(element, recursion_count))

        return copy

    if value is Dictionary:
        var dictionary: Dictionary = value
        var copy: Dictionary = {}

        if recursion_count > MAX_RECURSION:
            push_error("Max recursion reached.")
            return copy
        recursion_count += 1

        for key: Variant in dictionary:
            copy[recursive_duplicate(key, recursion_count)] \
                    = recursive_duplicate(dictionary[key], recursion_count)

        return copy

    if value is Object:
        if recursion_count > MAX_RECURSION:
            push_error("Max recursion reached.")
            return null
        recursion_count += 1

        var object: Object = value

        if not is_instance_valid(object):
            return null # TODO `return object`?

        var object_class: StringName = object.get_class()

        if not ClassDB.can_instantiate(object_class):
            push_error('Cannot instantiate "%s".' % object_class)
            return null

        var copy: Object = ClassDB.instantiate(object_class)

        for property: Dictionary in object.get_property_list():
            var property_name: String = property.name
            var property_usage: int = property.usage

            # TODO: Check `PROPERTY_USAGE_ALWAYS_DUPLICATE`?
            if not (property_usage & PROPERTY_USAGE_STORAGE):
                continue

            if property_usage & PROPERTY_USAGE_NEVER_DUPLICATE:
                copy.set(property_name, object.get(property_name))
            else:
                copy.set(
                    property_name,
                    recursive_duplicate(object.get(property_name), recursion_count),
                )

        return copy

    # `Packed*Array`s are pass-by-reference types, but they cannot contain
    # pass-by-reference elements, so we can use the standard method.
    if typeof(value) >= TYPE_PACKED_BYTE_ARRAY:
        @warning_ignore("unsafe_method_access")
        return value.duplicate()

    # A pass-by-value type.
    return value

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

2 participants