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 keys by variable do not store correctly #41377

Closed
jimbeauphloyd opened this issue Aug 19, 2020 · 5 comments · Fixed by #41983
Closed

Dictionary keys by variable do not store correctly #41377

jimbeauphloyd opened this issue Aug 19, 2020 · 5 comments · Fixed by #41983
Assignees
Milestone

Comments

@jimbeauphloyd
Copy link

jimbeauphloyd commented Aug 19, 2020

Godot version:
Godot Engine v4.0.dev.custom_buld.f97280a01

OS/device including version:
Mac OSX 10.15.5
Vulkan

Issue description:
Passing a variable key to a dictionary does not place values in the correct key.

Result:

{A:[A entry 0, B entry 1, A entry 2, B entry 3, A entry 4, B entry 5], B:[...]}

Expected Result:

{A:[A entry 0, A entry 1, A entry 2], B:[B entry 0, B entry 1, B entry 2]}

Steps to reproduce:

extends Node

var _entries = {}

func _ready():
	var key
	key = "A"
	_entries[key] = []
	key = "B"
	_entries[key] = []
	_add_entry("A")
	_add_entry("B")
	_add_entry("A")
	_add_entry("B")
	_add_entry("A")
	_add_entry("B")
	print(_entries)

func _add_entry(key):
	_entries[key].append(key + " entry " + str(_entries[key].size()))

Minimal reproduction project: DictionaryBug.zip

@Calinou Calinou added this to the 4.0 milestone Aug 19, 2020
@EricEzaM
Copy link
Contributor

EricEzaM commented Aug 20, 2020

Seems to be with the initialization of the values. This gives the expected output:

extends Node

var _entries = {}

func _ready():
	_entries = {
		"A": [],
		"B": []
	}
	_add_entry("A")
	_add_entry("B")
	_add_entry("A")
	_add_entry("B")
	_add_entry("A")
	_add_entry("B")
	print(_entries)

func _add_entry(key):
	_entries[key].append(key + " entry " + str(_entries[key].size()))

{A:[A entry 0, A entry 1, A entry 2], B:[B entry 0, B entry 1, B entry 2]}

@EricEzaM
Copy link
Contributor

EricEzaM commented Aug 20, 2020

Additionally, expanding this to:

	_entries = {
		"A": [],
		"B": []
	}
	_entries["C"] = []
	var key = "D"
	_entries[key] = []
	_add_entry("A")
	_add_entry("B")
	_add_entry("A")
	_add_entry("B")
	_add_entry("A")
	_add_entry("B")
	_add_entry("C")
	_add_entry("D")
	_add_entry("C")
	_add_entry("D")
	_add_entry("A")
	_add_entry("B")
	_add_entry("A")
	_add_entry("B")
	print(_entries["A"]) # [A entry 0, A entry 1, A entry 2, A entry 3, A entry 4]
	print(_entries["B"]) # [B entry 0, B entry 1, B entry 2, B entry 3, B entry 4]
	print(_entries["C"]) # [C entry 0, D entry 1, C entry 2, D entry 3]
	print(_entries["D"]) # [C entry 0, D entry 1, C entry 2, D entry 3]

"A" and "B" remain correct, but "C" and "D" get mixed up. However, they do not get values from "A" and "B" even when values are added to A and B after C and D.

@sednave
Copy link
Contributor

sednave commented Aug 20, 2020

Initializing each array with UNIQUE values seems to give the expected output, but only if it's unique. Otherwise, Godot writes to the same array, e.g.

	var key
	key = "A"
	_entries[key] = [1]
	key = "B"
	_entries[key] = [2]
	key = "C"
	_entries[key] = [2] # Same as B
	var new_array = [2] # Standalone variable, not in dictionary, same as B
	_add_entry("A")
	_add_entry("B")
	_add_entry("A")
	_add_entry("B")
	_add_entry("A")
	_add_entry("B")
	_add_entry("C")
	_add_entry("C")
	_add_entry("C")
	print(_entries["A"]) # [1, A entry 1, A entry 2, A entry 3]
	print(_entries["B"]) # [2, B entry 1, B entry 2, B entry 3, C entry 4, C entry 5, C entry 6]
	print(_entries["C"]) # [2, B entry 1, B entry 2, B entry 3, C entry 4, C entry 5, C entry 6]
	print(new_array) # [2, B entry 1, B entry 2, B entry 3, C entry 4, C entry 5, C entry 6]

"A" points to a unique array, but "B" and "C" appear to reference the same array.

Additionally: If you set _entries["A"][0] = 2 so that it looks like "B" and "C" = [2], "A" is still unique and gives output: [2, A entry 1, A entry 2, A entry 3]

So the problem only seems to occur when you use the '=' operator to assign an array that's been initialized the same as another array to a variable.

@ThakeeNathees
Copy link
Contributor

A simplified version of the issue

func _ready():
	var v1 = []
	var v2 = []  ## both arrays are reduced to Array const and shares the same memory in constant map.
	
	v1.append(42)
	print(v2)   ## prints [42]

In 3.2 the literal array doesn't reduced to variant (even if all elements are compile time const) but it was ArrayNode and constructed at runtime (OPCODE_CONSTRUCT_ARRAY) but in 4.0 it's reduced to Array constant at GDScriptAnalyzer::reduce_array() and since it's a constant value every other similar array will points to the same location in the local constant map.

similar issue : #20436

adding constant to constant_map first search if it already exists, is it okey to ignore that existence search for arrays/ and dictionaries @vnen ?

@vnen
Copy link
Member

vnen commented Sep 10, 2020

adding constant to constant_map first search if it already exists, is it okey to ignore that existence search for arrays/ and dictionaries

Yeah, I was thinking about that. Though I'm not sure if we actually want to keep arrays/dictionaries in the constant map. It is theoretically faster (since it doesn't need to be built at runtime) but the space needed might not be worth it.

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.

6 participants