Modify Struct variable in a Dictionary

I have a struct like this:

public struct MapTile
{
    public int bgAnimation;
    public int bgFrame;
}

But when I loop over it with foreach to change animation frame I can't do it...

Here's the code:

foreach (KeyValuePair<string, MapTile> tile in tilesData)
{
        if (tilesData[tile.Key].bgFrame >= tilesData[tile.Key].bgAnimation)
        {
            tilesData[tile.Key].bgFrame = 0;
        }
        else
        {
            tilesData[tile.Key].bgFrame++;
        }
}

It gives me compile arror:

Error 1 Cannot modify the return value of 'System.Collections.Generic.Dictionary<string,Warudo.MapTile>.this[string]' because it is not a variable
Error 2 Cannot modify the return value of 'System.Collections.Generic.Dictionary<string,Warudo.MapTile>.this[string]' because it is not a variable

Why can't I change a value inside a struct which is inside a dictionary?

Solution 1:

The indexer will return a copy of the value. Making a change to that copy won't do anything to the value within the dictionary... the compiler is stopping you from writing buggy code. If you want to do modify the value in the dictionary, you'll need to use something like:

// Note: copying the contents to start with as you can't modify a collection
// while iterating over it
foreach (KeyValuePair<string, MapTile> pair in tilesData.ToList())
{
    MapTile tile = pair.Value;
    tile.bgFrame = tile.bgFrame >= tile.bgAnimation ? 0 : tile.bgFrame + 1;
    tilesData[pair.Key] = tile;
}

Note that this is also avoiding doing multiple lookups for no good reason, which your original code was doing.

Personally I'd strongly advise against having a mutable struct to start with, mind you...

Of course, another alternative is to make it a reference type, at which point you could use:

// If MapTile is a reference type...
// No need to copy anything this time; we're not changing the value in the
// dictionary, which is just a reference. Also, we don't care about the
// key this time.
foreach (MapTile tile in tilesData.Values)
{
    tile.bgFrame = tile.bgFrame >= tile.bgAnimation ? 0 : tile.bgFrame + 1;
}

Solution 2:

tilesData[tile.Key] is not a storage location (i.e., it's not a variable). It's a copy of the instance of MapTile associated with the key tile.Key in the dictionary tilesData. This is what happens with struct. Copies of their instances get passed around and returned everywhere (and is a large part of why mutable struct are considered evil).

What you need to do is:

    MapTile tile = tilesData[tile.Key];
    if (tile.bgFrame >= tile.bgAnimation)
    {
        tile.bgFrame = 0;
    }
    else
    {
        tile.bgFrame++;
    }
    tilesData[tile.Key] = tile;

Solution 3:

I would suggest creating a utility class:

public class MutableHolder<T>
{
    public T Value;
    public MutableHolder(T value)
    {
        this.Value = value;
    }
}

Then store a newly-created MutableHolder<MapTile> into each dictionary slot rather than storing a MapTile directly. This will allow you to easily update the map tile associated with any particular key, without having to modify the dictionary itself (an act which would otherwise, at minimum, invalidate the enumerator used by your foreach loop).