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

Export arrays in singleton end up shared if initialized to same value (fixed in master) #20436

Open
masmartyg opened this issue Jul 25, 2018 · 21 comments

Comments

@masmartyg
Copy link

masmartyg commented Jul 25, 2018


Bugsquad note: This issue has been confirmed several times already. No need to confirm it further.


Godot version:
Godot v3.0.5.stable.official.6a88e22 win64

OS/device including version:
Windows 10 Pro v.1803 (on 2 different pc)

Issue description:
Loading values in a two-dimensional array has different results if the array is local or global.

On a new project, I added a script (g.gd) as autoload-singleton enabled with the following code:
extends Node

extends Node
export var gridTiles = []
export var gridStat = []

I create a new scene assigned as main scene on project setting, on the root node added a script with the following code:

extends Node
func _ready():
	var x
	var y
	var gridTiles = []
	var gridStat = []

	for y in range(3):
		gridTiles.append([])
		gridStat.append([])
		for x in range(4):
			gridTiles[y].append(null)
			gridStat[y].append(0)

	gridStat[0][0] = 1	
	print(gridStat)
	print(gridStat.size(), " x ", gridStat[0].size())
	print(gridTiles)
	print(gridTiles.size(), " x ", gridTiles[0].size())
	
	for y in range(3):
		g.gridTiles.append([])
		g.gridStat.append([])
		for x in range(4):
			g.gridTiles[y].append(null)
			g.gridStat[y].append(0)

	g.gridStat[0][0] = 2
	print(g.gridStat)
	print(g.gridStat.size(), " x ", g.gridStat[0].size())
	print(g.gridTiles)
	print(g.gridTiles.size(), " x ", g.gridTiles[0].size())

The resulting output is:

[[1, 0, 0, 0], [0, 0, 0, 0], [0, 0, 0, 0]]
3 x 4
[[Null, Null, Null, Null], [Null, Null, Null, Null], [Null, Null, Null, Null]]
3 x 4
[[2, 0, Null, 0, Null, 0, Null, 0], [Null, 0, Null, 0, Null, 0, Null, 0], [Null, 0, Null, 0, Null, 0, Null, 0], [], [], []]
6 x 8
[[2, 0, Null, 0, Null, 0, Null, 0], [Null, 0, Null, 0, Null, 0, Null, 0], [Null, 0, Null, 0, Null, 0, Null, 0], [], [], []]
6 x 8

Is it a bug or my poor knowledge of gdscript (python)?
Thanks for the attention.

@akien-mga akien-mga changed the title Unexpected behavior with two-dimensional global arrays Two empty export arrays in singleton end up shared Jul 25, 2018
@akien-mga akien-mga changed the title Two empty export arrays in singleton end up shared Export arrays in singleton end up shared if initialized to same value Jul 25, 2018
@akien-mga
Copy link
Member

akien-mga commented Jul 25, 2018

I could reproduce the issue, here's a simpler example:

# g.gd, defined as "g" singleton
extends Node
export var arr1 = []
export var arr2 = []
# main script
extends Node
func _ready():
	print(g.arr1, " ", g.arr2)
	g.arr1.append(1)
	g.arr2.append(2)
	print(g.arr1, " ", g.arr2)

Output:

[] []
[1, 2] [1, 2]

If I remove export from the arrays in the singleton, or define them with a different starting value, the error goes away.

So it seems like export arrays defined in a singleton with the same value will end up shared (so they're actually the same array, modifying one modifies the other).

The bug also happens if both are initialized to the same non-null value, e.g.:

# g.gd, defined as "g" singleton
extends Node
export var arr1 = [3]
export var arr2 = [3]

Output:

[3] [3]
[3, 1, 2] [3, 1, 2]

If the export keyword is removed from one or both of them, the issue disappears.

@akien-mga
Copy link
Member

I could reproduce the issue on 3.0.5-stable and in the current master branch (f778bd8).

@akien-mga akien-mga added this to the 3.1 milestone Jul 25, 2018
@masmartyg
Copy link
Author

I removed the keyword export because it's not for my purposes (I just looked at the documentation of godot - my lack of knowledge of gdscript as I wanted to prove) and it works.
I hope the bug does not create problems for others and is easy to solve.
Thanks akien-mga for the prompt reply, it's great to see how godot is supported by the community.

@akien-mga
Copy link
Member

Just tested when asked by @bojidar-bg: if g.gd is not a singleton, but e.g. a script attached to a child node, then there is no bug.

So it's buggy only in the singleton + export + same initialization case.

@akien-mga
Copy link
Member

Can you still reproduce this issue in the current master branch?

@masmartyg
Copy link
Author

Just downloaded Godot v3.1.beta2.official and tested, nothing has changed:
with "export" the bug is present, without "export" no bug.

@caburet
Copy link

caburet commented Jan 18, 2020

Same issue in Godot v 3.1.2-stable in linux. Remove "export" solve the issue.

Godot version:
Godot_v3.1.2-stable_x11.64 for linux.

OS/device including version:
Ubuntu 18.04

@psychobeans
Copy link

Similar issue in Godot_v3.2.1-stable_win64. However, it applies to non-singleton scripts as well.

@nezvers
Copy link

nezvers commented May 22, 2020

extends Node2D
export (Array) var arr_A = []
export(Array) var arr_B = []

func _ready():
		for i in range(10):
			arr_A.append("A")
			#arr_B.append("B") # even bypassed it appends
		print(arr_B)

Output:

[A, A, A, A, A, A, A, A, A, A]

3.2.2 beta3

@nonunknown
Copy link
Contributor

nonunknown commented May 22, 2020

I tried using:

Godot: 3.2.2 beta 3 - Launched Today
OS: Ubuntu 20.04

This is a script attached to a Node

Screenshot from 2020-05-22 09-37-47

got the same issue

There is a workaround?

yes, there is, just attach different initial values

Screenshot from 2020-05-22 09-46-59

@boruok
Copy link
Contributor

boruok commented May 22, 2020

  1. export var a: Array = Array() # broken
  2. export var b: Array = [] # broken
  3. export var c: Array # works
  4. export var b := [] # works

@RevoluPowered
Copy link
Contributor

I will look into this on my time off from FBX over the weekend.

@Anutrix
Copy link
Contributor

Anutrix commented May 22, 2020

1. export var a: Array = Array()  # broken

2. export var b: Array = []  # broken

3. export var c: Array # works

4. export var b := []  # works

Based on this info, it seems like an issue with type inference where it has two sources of type. Here both are Array but that seems inconsequential.
In cases 1 and 2, we have type Array specified on both sides of =. Hence, fails.
In cases 3 and 4, we have type Array specified on only 1 side of =. Hence, successes.
I am just guessing right now. Will test it soon.

Update: @boruok I couldn't get case 4 to work if both are like that.
But, it works if they are different.

#failed
export var arr1:= []
export var arr2:= []
#works
export var arr1:Array
export var arr2:= []

@bojidar-bg
Copy link
Contributor

bojidar-bg commented May 22, 2020

Quick look at the code:
The main place where constant values might get merged together and mixed up is the GDScript compiler. It has the following code, when processing a ConstantNode:

const GDScriptParser::ConstantNode *cn = static_cast<const GDScriptParser::ConstantNode *>(p_expression);
int idx;
if (!codegen.constant_map.has(cn->value)) {
idx = codegen.constant_map.size();
codegen.constant_map[cn->value] = idx;
} else {
idx = codegen.constant_map[cn->value];
}

Now, this does not happen for all empty arrays. This is so because _reduce_expression in the parser converts an ArrayNode to a ConstantNode only when its elements are constant and when it is in a place where it should try to convert an expression to a constant (p_to_const).

if (all_constants && p_to_const) {
//reduce constant array expression
ConstantNode *cn = alloc_node<ConstantNode>();
Array arr;
arr.resize(an->elements.size());
for (int i = 0; i < an->elements.size(); i++) {
ConstantNode *acn = static_cast<ConstantNode *>(an->elements[i]);
arr[i] = acn->value;
}
cn->value = arr;
cn->datatype = _type_from_variant(cn->value);
return cn;
}

This function is called through the place parsing TK_PR_VAR. p_to_const is set to true only when exporting the variable (code is a bit more convulted, but that's what autoexport || member._export.type != Variant::NIL means)

Node *subexpr = _parse_and_reduce_expression(p_class, false, autoexport || member._export.type != Variant::NIL);

This does not happen in case 3 and 4 above, since when there is no assigned value _parse_and_reduce_expression does not get called. Instead, the code tries to generate a valid default value for the type, and that piece of code returns a proper ArrayNode for arrays.

expr = _get_default_value_for_type(member.data_type);

ThakeeNathees added a commit to ThakeeNathees/godot that referenced this issue May 23, 2020
@akien-mga akien-mga changed the title Export arrays in singleton end up shared if initialized to same value Export arrays and dictionaries in singleton or global class end up shared if initialized to same value Sep 10, 2020
@akien-mga akien-mga changed the title Export arrays and dictionaries in singleton or global class end up shared if initialized to same value Export arrays in singleton end up shared if initialized to same value Sep 10, 2020
@vnen vnen modified the milestones: 4.0, 3.2 Oct 20, 2020
@vnen vnen closed this as completed in 3886a2f Oct 20, 2020
@akien-mga
Copy link
Member

This is fixed by #41983 in 4.0, but a fix is still needed for 3.2, so reopening.

@akien-mga akien-mga reopened this Oct 20, 2020
@akien-mga akien-mga modified the milestones: 3.2, 3.3 Mar 17, 2021
@akien-mga akien-mga modified the milestones: 3.3, 3.5 Oct 26, 2021
@Calinou Calinou changed the title Export arrays in singleton end up shared if initialized to same value Export arrays in singleton end up shared if initialized to same value (fixed in master) Mar 14, 2022
@Calinou Calinou modified the milestones: 3.5, 3.x Jul 15, 2022
@TheCulprit
Copy link

Just ran in to this with v3.5.stable.official [991bb6a] although it was in a Resource, not a singleton.
Two arrays initialized as the following were sharing data, although I only noticed after saving/loading the resource

var arr1: Array = []
var arr2: Array = []

Simply removing the assignment was enough to make it work correctly.

var arr1: Array
var arr2: Array

@Exerionius
Copy link

It's been 4 years since this issue was discovered. And it is still there.

Godot v3.5.stable.official [991bb6a]:

extends Node2D

export (Array) var arr_a := []
export (Array) var arr_b := []


func _ready() -> void:
	for _i in range(10):
		arr_a.append("A")
		arr_b.append("B")
	print(arr_a)

Prints:

[A, B, A, B, A, B, A, B, A, B, A, B, A, B, A, B, A, B, A, B]

If it was really fixed in Godot 4, is there any plans on backporting it to 3.5?

@kleonc
Copy link
Member

kleonc commented Nov 13, 2022

If it was really fixed in Godot 4, is there any plans on backporting it to 3.5?

@Exerionius I don't think it's a matter of backporting. For 4.0 GDScript was totally rewritten (parser, analyzer, compiler etc.), meaning in 3.x it works way differently. Hence it likely needs a dedicated fix, not a backport.

@yikescloud
Copy link

I found that if you really need a default value, store in another var and pass it like this, will fix the issue

var CONFIG_DEFAULT = {
	"grid_subd": 5,
	"color_bg":Color(0.5,0.5,0.5,1.0)
}
export var config:Dictionary = CONFIG_DEFAULT

But if you change var to const, then the issue came back,,, very strange

const CONFIG_DEFAULT = {
	"grid_subd": 5,
	"color_bg":Color(0.5,0.5,0.5,1.0)
}

export var config:Dictionary = CONFIG_DEFAULT

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

Successfully merging a pull request may close this issue.