Adding dictionaries to arrays

[Jerry_Happymeal]

Godot Version

4.6.3

Question

I’m new to godot so let me know if it is a redundant question or if I’m thinking about things the wrong way, but I have been working with an itemList and a tileMap to create a turn based game. I’m storing data via the metadata of the item in the itemList and want to dynamically add the clicked tile to an array to later loop through and action. All of this works well except whenever I add to the array, it is overriding all elements of the array.

This is my function and I am declaring a new dictionary variable inside the loop so I feel it shouldn’t be a reference issue.

func insertIntoQueue(itemsToAdd : Array, mousePos : Vector2i) -> void:
	for items : int in itemsToAdd :
		var metadata : Dictionary = item_list.get_item_metadata(items)
		metadata["tilePosition"] = mousePos
		GameGlobals.constructionQueue.append(metadata)
        print(GameGlobals.constructionQueue)

The output:

#First click
[
   {
      "time":1,
      "cost":0,
      "atlasPos":(3,0),
      "tilePosition":(5,22)
   }
]

#Second click
[
   {
      "time":1,
      "cost":0,
      "atlasPos":(3,0),
      "tilePosition":(4,21)
   },
   {
      "time":1,
      "cost":0,
      "atlasPos":(3,0),
      "tilePosition":(4,21)
   }
]

#Third click
[
   {
      "time":1,
      "cost":0,
      "atlasPos":(3,0),
      "tilePosition":(4,20)
   },
   {
      "time":1,
      "cost":0,
      "atlasPos":(3,0),
      "tilePosition":(4,20)
   },
   {
      "time":1,
      "cost":0,
      "atlasPos":(3,0),
      "tilePosition":(4,20)
   }
]

Just for my testing I added a .duplicate_deep() to the get get metadata call and this works as expected but it feels wrong.

func insertIntoQueue(itemsToAdd : Array, mousePos : Vector2i) -> void:
	for items : int in itemsToAdd :
		var metadata : Dictionary = item_list.get_item_metadata(items).duplicate_deep()
		metadata["tilePosition"] = mousePos
		GameGlobals.constructionQueue.append(metadata)

Output with duplicat_deep():

#First click
[
   {
      "time":1,
      "cost":0,
      "atlasPos":(3,0),
      "tilePosition":(5,22)
   }
]

#Second click
[
   {
      "time":1,
      "cost":0,
      "atlasPos":(3,0),
      "tilePosition":(5,22)
   },
   {
      "time":1,
      "cost":0,
      "atlasPos":(3,0),
      "tilePosition":(4,21)
   }
]

#Third click
[
   {
      "time":1,
      "cost":0,
      "atlasPos":(3,0),
      "tilePosition":(5,22)
   },
   {
      "time":1,
      "cost":0,
      "atlasPos":(3,0),
      "tilePosition":(4,21)
   },
   {
      "time":1,
      "cost":0,
      "atlasPos":(3,0),
      "tilePosition":(4,20)
   }
]

Any pointers are greatly appreciated.

[wchc]

Why does it feel wrong? It works exactly as expected. Dictionaries are stored and passed by reference, so you need to duplicate it to store a different version of the Dictionary.

[Jerry_Happymeal]

I guess, maybe it’s my lack of knowledge, it just feels slightly longwinded to be duplicating dictionaries so often but I can understand the reason behind it, I just wondered if there was a better way of doing it.

[dragonforge-dev]

@Jerry_Happymeal

All of this works well except whenever I add to the array, it is overriding all elements of the array.

This is my function and I am declaring a new dictionary variable inside the loop so I feel it shouldn’t be a reference issue.

func insertIntoQueue(itemsToAdd : Array, mousePos : Vector2i) -> void:
	for items : int in itemsToAdd :
		var metadata : Dictionary = item_list.get_item_metadata(items)
		metadata["tilePosition"] = mousePos
		GameGlobals.constructionQueue.append(metadata)
        print(GameGlobals.constructionQueue)

To be clear you are overriding all elements to the Dictionary. You are not creating a new one Dictionary. Dictionaries are passed by reference. Which means metadata is being used to overwrite the original Dictionary, item by item in your loop.

func insertIntoQueue(itemsToAdd : Array, mousePos : Vector2i) -> void:
	for items : int in itemsToAdd :
		var metadata : Dictionary = item_list.get_item_metadata(items)
		metadata["tilePosition"] = mousePos
		GameGlobals.constructionQueue.append(metadata)
        print(GameGlobals.constructionQueue)

You haven’t given us enough code to help you fix it though.

[normalized]

What do you mean by “longwinded”? If you’re making a snapshot of the state of the metadata dictionary - you need to copy it.

That said, copying the whole metadata dictionary is a dodgy way to do this. Metadata is general purpose. As the game grows, you might find that you need to store some unrelated data there.

[Jerry_Happymeal]

I’m just not used to references so I wasn’t sure if there was a better/cleaner way to manipulate the data within them.

I’ll have a look at storing the data in a nested dictionary or something to that effect so I don’t have to copy the whole metadata every time, thanks

[dragonforge-dev]

If you are using a Resource there’s no need for a Dictionary.

class_name MyResource extends Resource


@export var time: int
@export var cost: int
@export var atlas_position: Vector2
@export var tile_position: Vector2

Although, again, you haven’t really given you enough information to help you. This is a classic XY Problem. The fact that you are storing data about a TileMap means you could probably just add data to it in the cells and be done.

[Jerry_Happymeal]

To give the gist of what I’m trying to do, I’m making a turn based city builder-type game, where the user can select an option from an ItemList (which contains metadata such as cost, time to build, etc.) and can then click on a tile on the tile map. This is where I have to duplicate the dictionary to add the coordinates of where the user clicked and then store the dictionary in an array that gets looped every turn to see if the tile should be changed from the construction tile to what the user initially selected based on the time to build in the dictionary.

I’m open to changing the architecture if there is a better way of doing it, but if not I’m happy to duplicate the dictionary, I just wanted to make sure I was following best practices for performance and whatnot.

[normalized]

Store a new dictionary that contains click data and a reference to metadata dictionary. Alternatively, you can use a custom class instead but it’d boil down to pretty much the same functionality so I’d go with just a dictionary as a simpler solution.

[dragonforge-dev]

Yes, there’s a much better way of doing it. @normalized, just so you know, prefers not to use objects and likes low-level coding solutions. If that kind of solution appeals to you, stick with what you’ve got. But if not, here’s an alternative…

TileMapLayers support Scenes as tiles. What this means is that instead of playing with low level layer info inside tiles, you can construct tiles to do the work for you.

1. Create a scene that has a Sprite2D.
2. Assign an AtlasTexture to it, and have it store the various states of a building.
3. Create a Building node and attach it to the Scene.
4. Every turn, have the building process whether or not it needs to go to the next phase.
5. When the building gets a click, you can process that input directly in the Building script. The TileMapLayer doesn’t need to know anything about the buildings. It literally only needs to knwo what building is there.

Your current solution is basically a Manager Anti-Pattern. You’re trying to manage everything through your TileMapLayer, instead of leveraging the way Godot works - and it’s going to make everything harder. This is not the last problem you will encounter with your current solution.

[normalized]

@dragonforge-dev

@normalized, just so you know, prefers not to use objects and likes low-level coding solutions

That’s a strange thing to say. What do you mean by “low-level” exactly? Some anti-java shit?

I prefer simple solutions. Introducing a class where a dictionary could suffice equals adding unnecessary complexities. Even Godot’s apis follows this. I also never force my architectural preferences onto people by re-doing their whole approach, however flawed it may seem.

Don’t listen to @dragonforge-dev. He’s an object-oriented over-complicator and likes to abuse nodes by keeping grocery lists inside them, while calling it “software architecture”.

[dragonforge-dev]

@normalized

That’s a strange thing to say. What do you mean by “low-level” exactly? Some anti-java shit?

Wait, what’s wrong with Java? I like Java.

It wasn’t meant as a criticism, just an observation. What I meant was exactly what you said: You prefer using a Dictionary when that will work instead of a more complex solution.

@normalized

I prefer simple solutions. Introducing a class where a dictionary could suffice equals adding unnecessary complexities. Even Godot’s apis follows this. I also never force my architectural preferences onto people by re-doing their whole approach, however flawed it may seem.

The OP said any pointers appreciated. I’m just giving an alternate viewpoint.

I wasn’t saying you force your viewpoint. Maybe you’re inferring I force mine? I dunno. I really was just trying to communicate that you have a particular approach. Not that it is good or bad, just different. Your code is quite brilliant and you are a very talented developer. I am quite often in awe of the things you create on here.

@normalized

Don’t listen to @dragonforge-dev. He’s an object-oriented over-complicator and likes to abuse nodes by keeping grocery lists inside them, while calling it “software architecture”.

This gave me a good chuckle. And I totally agree. But I think that Godot lends itself to these kinds of solutions.