godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.55k stars 21.08k forks source link

Typed Arrays and Dictionaries without initial value are shared #24134

Closed qarmin closed 5 years ago

qarmin commented 5 years ago

Godot version: 3.1 8dd00ed

OS/device including version: Windows 10

Issue description: When I append value to array, other array have this value inside. This is broken code

extends Node2D

var offerts_parameters_array : Array
var offerts_array : Array

func _ready() -> void:
    offerts_parameters_array.resize(0)
    offerts_array.resize(0)

    offerts_parameters_array.append(["OPA"])
    offerts_parameters_array.append(["OPA"])
    offerts_parameters_array.append(["OPA"])
    offerts_parameters_array.append("SOPA")
    offerts_array.append(["OA"])
    offerts_array.append(["OA"])
    offerts_array.append("SOA")
    print(offerts_parameters_array)
    print(offerts_array)

and this is Output

[[OPA], [OPA], [OPA], SOPA, [OA], [OA], SOA]
[[OPA], [OPA], [OPA], SOPA, [OA], [OA], SOA]

When I didn't use typed GDScript then it seems to work correct:

extends Node2D

var offerts_parameters_array = []
var offerts_array = []

func _ready() -> void:
    offerts_parameters_array.resize(0)
    offerts_array.resize(0)

    offerts_parameters_array.append(["OPA"])
    offerts_parameters_array.append(["OPA"])
    offerts_parameters_array.append(["OPA"])
    offerts_parameters_array.append("SOPA")
    offerts_array.append(["OA"])
    offerts_array.append(["OA"])
    offerts_array.append("SOA")
    print(offerts_parameters_array)
    print(offerts_array)

and this is Output

[[OPA], [OPA], [OPA], SOPA]
[[OA], [OA], SOA]
bojidar-bg commented 5 years ago

Flagging as bug since we should either enforce giving an initial value or make sure it is initialized to new, unshared arrays.

remorse107 commented 5 years ago

Looks like decorating with export (Array) also causes the break to occur even when the variable isn't Typed.

akien-mga commented 5 years ago

Confirmed in 6e600706.

TheCire commented 5 years ago

Hmm, Im wondering if the default value of

var array: Array should actually be null and cause a crash if not initialized. it would help to enforce giving a default value and would avoid the bug all together.

MidZik commented 5 years ago

That might require more changes, since you aren't allowed to assign null to variables statically typed as built-in types that don't extend Object (like Array or Dictionary).

LeonardMeagher2 commented 5 years ago

I think typed variables should require an initial value, that's really what "var array: Array should actually be null and cause a crash if not initialized." is trying to get you to infer, but I think it's better to be explicit and tell someone "This isn't going to work as expected without an initial value".

MidZik commented 5 years ago

Yeah, I misunderstood.

Like you said, better to have an explicit warning.

akien-mga commented 5 years ago

As seen in #27997, it seems to be the same for Dictionary.

starcin commented 3 years ago

Looks like decorating with export (Array) also causes the break to occur even when the variable isn't Typed.

This still seems to be the case in v3.3.3.stable.official [b973f997f]

bncastle commented 1 year ago

This still appears to be occurring in v3.5.2.stable.official [170ba337a] as well.