Can't modify struct members because it is a foreach iteration variable

Unity Engine
1

Looks like I’m reaching too far for this one. I could use some help.

So I’m trying to create a system to store a list of what specific items (gold coins) have been collected in each level. That is, I need to be able say if GoldCoin number X has been collected from LevelName Y.
Rather than have a list within a list (my gut tells me that would be problematic) I’ve created a struct to store gold coin data, and made a list of said struct. The struct simply contains a string for a level name and an int that serves as a composite value to store all the boolean values of what coins are collected.

[Serializable]
public struct GoldCoinData
{
    public string Level;
    public int Coins;

    public GoldCoinData(string level, int coinage)
    {
        Level = level;
        Coins = coinage;
    }
}

[..]
    public List<GoldCoinData> CollectedGoldCoins = new List<GoldCoinData>();

I don’t recall ever working with structs before. Maybe I’ve already done something wrong.

Since I have a list of these goldGoinData’s, when I need a value I use a foreach loop to find the item in the list with the matching level name. I have honestly no idea how else to find such a value within the list.

public bool IsThisGoldCoinCollected(string level, int CoinNumber)
    {
        foreach (GoldCoinData gcd in CollectedGoldCoins)
        {
            if (gcd.Level == level)
            {
                if ((gcd.Coins & (2 ^ CoinNumber)) > 0)
                    return true;
            }
        }
        return false;
    }

So now I’m trying to create the function that will actually mark one of these coins as collected. But I get an error that reads “Cannot modify member of ‘gcd’ because it is a ‘foreach interation variable’” (line 8 in this section below)

public void SaveThisGoldCoin(string level, int CoinNumber)
    {

        if (!IsThisGoldCoinCollected(level, CoinNumber))
        {
            foreach(GoldCoinData gcd in CollectedGoldCoins)
            {
                if (gcd.Level == level)
                {
                    gcd.Coins += 2 ^ CoinNumber;
                    return;
                }
            }

            // no data for this level already exists.
            CollectedGoldCoins.Add(new GoldCoinData(level, 2 ^ CoinNumber));
        }
    }

So, how do I get around this problem? If I already have a struct in my list for this particular level, how do I modify the coins value within that given struct?

Or should I be using a class instead of a struct? Or am I going about this completely the wrong way altogether?

2

structs are value types, classes are reference types… hie thee to google for details.

Index it:

for (int i = 0; i < mycoll.Length; i++)  // obviously use .Count field for List<> types
{
  if (mycoll[i].name == theNameIWant)
  {
    mycoll[i].dostuff = 1;  // for instance
  }
}

Or if you want to do a LOT of stuff, at the inner loop above do:

var item = mycoll[i];

item.dostuff = 1;
item.domorestuff = 2;

// PUT IT BACK... it's a struct, not a class!
mycoll[i] = item;

Or… just make it a class, not a struct.

3

The metric I use when deciding between using a class and using a struct is generally: Am I repeatedly making and destroying a ton of these things? In other words, is the garbage collection cost of this object causing me problems? If so, I’ll probably use a struct to avoid the garbage collection cost. If not, I’ll usually use a class, since it’s generally more convenient (as you’re noticing.)

In your case, it seems as if you’re probably not making that many of these objects. (A few hundred maybe?) And maybe you’re not destroying them until you switch to another scene, (or maybe you never destroy them) in which case garbage collection isn’t really a concern?

So, in your case, I’d probably not bother with structs at all. I also don’t think there’s necessarily anything wrong with keeping these things in a List, unless you’re serializing it, in which case your array approach is probably slightly simpler.

4

For the record, there is absolutely nothing wrong with lists of lists.

5

Honestly, I’m only planning on using these within my RecordsMaster script, which is attached to the same game object as my GameManager script, which never gets destroyed. And even within the RecordsMaster, the only time anything on this list would ever get removed is when a new game is started or a save file is loaded. Otherwise it only gets items added or modified.
So I guess it sounds like I should change this to a class instead of a struct.

But this has got me thinking…
I have some other scripts where I started adding serializable classes to them as a means to store public variables without them taking up so much room in the inspector (since I can collapse them). I did this for scripts like my enemy behaviors, so those are on objects that do a lot of creating and destroying. Is that going to create a lot of unneeded garbage collection? Should I be looking into replacing those with structs?

6

I would just profile it before changing anything. By “a lot” of creating and destroying, I generally mean objects that are created and destroyed multiple times nearly every frame. The profile should tell you whether you’re A) generating a lot of garbage from a particular operation, and B) how often garbage collection is running, and how long it’s taking each time. So, you might not need to change anything.

7

For your particular issue, a struct is fine. KurtDecker wrote the fix: change the foreach for a for loop.

The issue is that a foreach makes gcd into a copy of each item in the list. If GoldCoinData were a class, gcd would be a copy of a reference, so would be fine. But now it’s a copy of the whole struct, since there’s no such thing as a reference to a struct. It’s the same problem with a list of ints:

foreach(x in IntList) sum+=x; // this is fine

foreach(x in IntList) x=0; // does nothing

for(int i=0;i<IntList.length;i++) IntList[i]=0; // this works

foreach is a shortcut – nice for some things, but a basic for-loop is the real thing. And Java, which C# is a copy of, purposely has no structs. So you’re working with 2 somewhat delicate shortcuts. It’s no surprise there’s a weird hard-to-explain interaction somewhere in there.

2 Likes
8

THIS ^ ^

DO NOT speculatively optimize. EVER! Don’t be silly about implementing stuff badly (to a reasonable extent), but do not optimize until the profiler tells you “this is the problem part.”

9

Now that I have my code working for storing everything, now I can properly find out that I screwed up all my bitwise operators when I tried to store my data as an int instead of an array of bools.
There’s just a special piece of irony that I while researching bitwise operators I somehow missed that ^ is one of them and not used for indicating powers operations.

10

This is a textbook case of optimizing without an actual need. Just use bools. Until you get in the tens of thousands of bools, it’s probably never gonna be a chokepoint. If they need to be compressed for a save game (for instance), then do some code that ONLY compresses them then.

11

I just leave this here as an interesting thing:
https://docs.microsoft.com/en-us/dotnet/api/system.collections.specialized.bitvector32?view=netstandard-2.1

There are two things I don’t agree with you my friend, this (bit storage isn’t optimization) and on your pervert attraction to JSON. :smile:

1 Like
12

Sorry Kurt, but I’ve been in love with the idea of storing an array of bools as a single int since before this century even started. Not gonna pass up a perfectly honest chance to use it in my game.

Besides, this wasn’t an issue with my math, this was an issue with there not being a shorthand for powers in the C# language, and I have to use the math.pow function instead… I learned something very useful from this, so net result is a win.

13

That’s an important operator–it’s how you win at nim! (Also shows up a lot in cryptography and hash functions.)

Not used much for bitflags, though.

I’m actually not sure I could name any programming languages where ^ is used to mean exponentiation. Some languages use double asterisk **

14

What I’m sure of: mostly Basic languages (C64 Basic, Visual Basic, etc), also MatLab (if we count it as a programming language), and Excel, which has scripting, but it is not a programming language.

15

I think you could name one. Do you know any languages popular for teaching and web development, as long as you can get past the dead parrot jokes? IT’S…

16

The thing is, there was a time when packing stuff into bits was an important skill. All of those books teaching (x>>2)&0x11 to pull out the 2nd pair of bits as a number weren’t for parties. An Apple-II had 48K, and that was with the expansion. Saving memory with custom bit manipulation was paramount. But today, doing the math – our 4 gigs of RAM is 100,000 times more, plus hard drive swap space – makes it seem silly.

17

I enjoy hitting a breakpoint and doing this:

6093756--662133--Screen Shot 2020-07-15 at 10.52.18 AM.png

In contrast, I do NOT enjoy using a hex calculator to figure out which bit inside of 0xfff4ff83 corresponds to “HaveSword”

18

Sorry, not getting it.

19

I think he was referring to Perl and the Parrot VM? (Guessing)
Or Python and the Monty Python and the dead parrot sketch? IDK.
However Perl and Python use the ** operator.

BTW, one more language comes to mind which is using the ^ as exponent operator: Lua.

20

Ah yes, I remembered wrong. I remembered Python as using an “old, stupid” way to exponentiate, made extra bad since Python isn’t an engineering language, which at least BASIC sort of was. Not sure how I translated that into 2^4, since 2**4 is a lot stupider, not even resembling regular math.

You’ve really never looked at Python docs? It can be excruciating as they quote sketches and use variables from movies. And I even liked most of them. It almost makes you miss the microsoft C# examples which are 3x as long as they need to be and only show the easiest case that you knew anyway.