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

ResourceSaver duplicates items in all arrays #29179

Open
JonathanPicques opened this issue May 25, 2019 · 11 comments
Open

ResourceSaver duplicates items in all arrays #29179

JonathanPicques opened this issue May 25, 2019 · 11 comments

Comments

@JonathanPicques
Copy link

Godot version:
3.1

OS/device including version:
Windows 10

Issue description:
ResourceSaver duplicates items in all arrays upon saving

Minimal reproduction script:

extends Node2D

class Save:
	extends Resource

	export var version : String = "1.0"
	export var save_name : String = "Whatever"
    
	export var arr1: Array = []
	export var arr2: Array = []
	export var arr3: Array = []

func _ready():
	var save := Save.new()
	
	save.arr1.push_back({"name": "Plane"})
	save.arr2.push_back({"name": "Human"})
	save.arr3.push_back({"name": "Cat"})
	
	ResourceSaver.save("res://test.tres", save)

test.tres:

[gd_resource type="Resource" load_steps=2 format=2]

[sub_resource type="GDScript" id=1]

[resource]
script = SubResource( 1 )
version = "1.0"
save_name = "Whatever"
arr1 = [ {
"name": "Plane"
}, {
"name": "Human"
}, {
"name": "Cat"
} ]
arr2 = [ {
"name": "Plane"
}, {
"name": "Human"
}, {
"name": "Cat"
} ]
arr3 = [ {
"name": "Plane"
}, {
"name": "Human"
}, {
"name": "Cat"
} ]
@KoBeWi
Copy link
Member

KoBeWi commented May 25, 2019

The actual problem here is that each exported array is the same instance. I remember it being mentioned/reported somewhere (like this related issue #20436), but not sure if there was some proper fix/workaround.

btw, this makes Resources almost useless for saving game progress, because you often need to use arrays in complex games.

@PetePete1984
Copy link
Contributor

PetePete1984 commented May 26, 2019

The erroneous sharing between typed arrays and dictionaries should be fixed in 3.2, see #24134
In the meantime, if you give your Save class an _init function, you should be able to bypass this (heavy emphasis on should because I haven't tried saving exported arrays since v306):

func _init():
    arr1 = []
    arr2 = []
    arr3 = []

@JonathanPicques
Copy link
Author

Hey, thanks for the heads up @PetePete1984

I ended up using

class Save:
	extends Resource

	export var version : String = "1.0"
	export var save_name : String = "Whatever"
	export var save_data = {
		"nodes": [],
		"connections": []
	}

@nicktgn
Copy link

nicktgn commented Jun 1, 2019

The erroneous sharing between typed arrays and dictionaries should be fixed in 3.2, see #24134
In the meantime, if you give your Save class an _init function, you should be able to bypass this (heavy emphasis on should because I haven't tried saving exported arrays since v306):

func _init():
    arr1 = []
    arr2 = []
    arr3 = []

Stumbled upon the same issue. Can confirm, @PetePete1984's workaround suggestion works.

@KoBeWi
Copy link
Member

KoBeWi commented Jun 2, 2019

The erroneous sharing between typed arrays and dictionaries should be fixed in 3.2, see #24134

Tested in 4493957 and the problem still exists.

@KoBeWi
Copy link
Member

KoBeWi commented Sep 4, 2019

So, another way to avoid this is not initializing the arrays:

export var arr1: Array
export var arr2: Array
export var arr3: Array

They are initialized automatically to unique references.

@ghost
Copy link

ghost commented Sep 6, 2019

The issues related to this mentioned that dictionaries also had this problem; in my own experiments, the dictionaries were fine, but the arrays would still share the same instance. Not initializing them does the trick.

@KoBeWi
Copy link
Member

KoBeWi commented Sep 6, 2019

Eh, if someone is interested, export var arr = [] executes code blocks at these lines:

} else if (tokenizer->get_token() == GDScriptTokenizer::TK_BRACKET_OPEN) {

case Node::TYPE_ARRAY: {

Same operation for dictionaries uses exactly the same code, so not sure where is the part that makes arrays shared (but probably there are some parts I'm not aware of and these might differ. Dunno). I used #28603 to get a clue where to look, but then got stuck :/

@KoBeWi
Copy link
Member

KoBeWi commented Aug 25, 2020

Still valid in a609b30

I'm surprised that GDScript 2.0 still has this problem.

@MGilleronFJ
Copy link

MGilleronFJ commented Jan 18, 2023

Would it be because arrays were made to be compared by value (even though they are ref types), therefore making them under the same key if indexed in a map or hashmap? If dictionaries also get compared by value they might end up with the same problem

@KoBeWi
Copy link
Member

KoBeWi commented Jan 18, 2023

Except property name is the key in hashmap, not its 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

5 participants