One of my variables is getting overwritten and I can't figure out why.

Unity Engine
,
1

Hey, I’m reasonably new to Unity and C# so this is probably just a novice mistake but I’m hoping someone can help me out.

I am trying to create an array/list (still not clear on C# terminology, I think it’s an array though) populated randomly with 0s and 1s. This is done in GenTrialList(). I then want “copy” this array (called trialList) into orientationArray, and then for each element in the array I want to change a percentage of them from 1 to 0 or vice versa - currently I have this at 80%.

Most of the code works fine, however when I run ReliabilityModifier() in Awake(), it changes both trialList and orientationArray, and I can’t figure out why it is overwritting trialList. I’ve attached the code below, as well as some print functions in Start() that show it is not working as I would like.

public static class OutputData
    {
        public static int[] trialList;
        public static int[] orientationArray;
    }

public static float perc = 80;
public int numTrials = 10;

public void Awake()
    {
        OutputData.trialList = GenTrialList(numTrials); // Generates the trial list
        print("trial list original:" + string.Join(",", OutputData.trialList));
        OutputData.orientationArray = OutputData.trialList;
        OutputData.orientationArray = ReliabilityModifier(OutputData.orientationArray, perc);
    }

    public int[] GenTrialList(int numTrials)
    {

        int[] trialList = new int[numTrials]; //
        System.Random r = new System.Random();
                                            
        for (int i = 0; i < trialList.Length; i++) //(start; end; step size)
        {
            trialList[i] = r.Next(0, 2);
        }
        return trialList;
    }

public int[] ReliabilityModifier(int[] array, float perc)
    {
        double modi = (perc / 100) * array.Length;
        int[] percArray = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 0 };
        Shuffle(percArray);
        for (int i = 0; i < percArray.Length; i++)
        {
            if (percArray[i] >= modi)
            {
                if (array[i] == 1)
                {
                    array[i] = 0;
                }
                else if (array[i] == 0)
                {
                    array[i] = 1;
                }
            }
        }
        return array;
    }

    void Start()
    {
        print("trial list:" + string.Join(",", OutputData.trialList));
        print("orientation array: " + string.Join(",", OutputData.orientationArray));
    }

I have tried messing around with it a bit, like just having the following instead of it being split over two lines:

OutputData.orientationArray = ReliabilityModifier(OutputData.trialList, perc);

I could guess that it has something to do with the return statement in ReliabilityModifier, or the way I am populating orientationArray in Awake(), but I’m not experienced enough to be able to figure out what to change and thought I’d ask here. Thanks

edit: I also realize that there are probably a lot of examples of poor practice within the code I provided, I’d also appreciate any advice on how I can write this code better in general.

2

Arrays are a reference type in c#, so if you set someArrayVariable = someOtherArrayVariable, you are not creating a copy of the array, you are creating two ways of accessing a single array.

You can use Clone() to create a copy.

3

In general, having statically-accessible read/write arrays is very poor form. Don’t do that.
Use singletons to achieve that behavior in a manner that lets you provide some logic to it later.

i.e.

public class CommonData {

  static private CommonData _instance;
  static public CommonData Instance => _instance = _instance?? new CommonData();

  static private int[] trialList;
  static private int[] orientationArray;

  // you can now add access logic to this
  // usage: CommonData.Instance.GetTrialList(); etc.

}

But in your case you don’t really need publicly accessible floating data, as everything seems to be used privately.

Can you provide me an example of input and output? I can show you how to make this from scratch, using a healthier form factor.

4

Ah yep, makes sense. Thanks so much, I will give that a try.

Could you explain why having statically accessible arrays is poor form? Usually I only use static variables when I need that variable (or array, etc) to be accessible in another script - although typically I just need to be able to read the variable as it is updated in another script and don’t need to write to it.

Either way I’ll have a better look into singletons - from what I’ve seen so far it looks like they might also work with how I’m using static arrays.

Yes I can do that.

trialList should be something like int[ ] { 0, 0, 1, 0, 1 }
So the input to orientationArray should be the same as trialList, whereas the output should change 20% of the elements to either a 0 or 1, whatever it was not originally. For an array with a length of 5 like in the example above, the output could look like:
int[ ] orientationArray = { 0, 1, 1, 0, 1}
where the 2nd element has changed, but the rest is the same.

It might be worth noting that I need to be able to save a copy of both trialList and orientationArray to a .csv file (or a file type to the same effect), and to have other scripts be able to access the value of trialList and orientationArray by an indexing variable that is incremented under certain conditions - I already have this set up but maybe this would effect the way you would do this.

Thanks, really appreciate the help.

5

I think there might be some confusion between static and public. “Public” means you can access it from outside the class. “Static” means that something is associated with the class in the abstract rather than with a particular copy of the class.

It is a common rule-of-thumb that you shouldn’t make something more public than necessary. The core idea is that if you intend to never do something, you want to inform the computer that you’re never going to do it, so then if you ever do it by accident the computer can point out your mistake for you. (This is pretty minor for a solo programmer on a small project, but it becomes more important when lots of programmers collaborate on a big project.)

Wanting to access something from outside of the class is a bad reason to make it static. If that’s the only reason, you’re likely to run into problems if you ever want to have more than one copy of that class.

But if it’s appropriate to put something into a singleton, it’s probably fine to make it static, too. In theory, singletons can be done in such a way that makes it easier to change later if you ever decide you need more than one…but most examples you’ll find of people using singletons in Unity don’t actually do that. And there’s a real limit to how much work you ought to do now to help with problems that you might or might not have at some point in the future–especially when you’re just learning.

6

In general, in OO, anything that is publicly accessible but not properly organized into objects, is a poor form. This is because it produces system workarounds, and thus anything can read/write data, even things that should have no access to it. This is a major pain for your future self and any of your potential colleagues. Chasing which part of the code changed which static field and when, is ludicrous, especially if you had no serious need for this.

Avoiding objects in OO is like going to an Italian restaurant and ordering Greek gyros or Hungarian goulash. And if the fields are not just fields, but entire arrays, that is a statement of how deliberately you work around something, ending up with a production code of dubious quality.

Other non-philosophical reasons why static fields are bad and singletons are better:

  • Nobody “owns” the (access to) data
  • You can’t expand the utility of it because you have no natural bottlenecks
  • Obviously no constructor
  • You can’t add accessors at a later point
  • You can’t validate writes, and you cannot protect data from other parts of code (i.e. in a multithreading environment)
  • You can’t inherit a static class (granted, you cannot do this with a singleton I wrote as well, but there are ways to make this work)
  • You can’t implement interfaces
  • You can’t implement disposing or cloning
  • You can’t have generic static classes

It’s just something non-beginner people don’t do. And when something is to be avoided like a plague, it is said to be in ‘poor form’. That’s a more severe form of ‘code smell’ – a code smell just looks problematic, and might turn out problematic due to “clever design” or a misconception – a poor form, however, is something that will surely bring on a headache at some point and is likely avoidable, i.e. playing with fire, or a language feature that is known to backfire in the real world situations.

Singleton is similarly bad, don’t get me wrong, but at least you have prepared your code for future expansions and have declared your intentions with it much more clearly. It is thus likely you’ll be more responsible about its usage as well.

Yes it’s an array. Arrays are the simplest possible collections of similar data. Lists are “smarter” and more pliable, but made out of arrays anyway. There are also Arraylists, a legacy variant of the List, but we don’t use them nowadays, so just forget about them and save yourself from confusion. There are also non-generic top-level collections included only for backward compatibility.

int[] a = genTrialList();
show(a); // 0, 0, 1, 0, 1
a = reorient(a, .2f); // modulation: 0, 0, 0, 1, 0
show(a); // 0, 0, 1, 1, 1

// t = threshold
int[] reorient(int[] a, float t) {
  int c = a.Length;
  int[] m = makeModulation(t, c);
  show(m);
  for(int i = 0; i < c; i++) {
    if(m[i] > 0) a[i] = 1 - a[i]; // change as instructed by modulation
  }
  return a;
}

// modulation is guaranteed to build an array that
// has a ratio of 0s and 1s according to t, and with size of count
// normally I'd make this function repurpose an already
// allocated array (if there was one) stored as private static
int[] makeModulation(float t, int count) {
  var t = Mathf.Clamp01(t); // make sure it's inside (0 .. 1)
  int s = Mathf.FloorToInt(t * count);
  int[] r = new int[count];
  while(s > 0) { // we know it'll finish, but we don't know when, not a pinnacle in efficiency
    int i = Math.FloorToInt(Random.value * count); // I could've used Random.Range here
    if(r[i] == 0) {
      r[i] = 1;
      s--;
    }
  }
  return r;
}

// displays an array for debugging
void show(int[] a) {
  var s = string.Empty;
  for(int i = 0; i < a.Length; i++)
    s = $"{s}{a[i].ToString()}{((i < a.Length - 1)? ", " : string.Empty)}";
  Debug.Log(s);
}

Now if you wish to organize this well and have these arrays accessible from the outside, that’s a different topic altogether. Imagine that code above was a class in itself. Now define the responsibilities of this class, let’s say it should do both genTrialList() and reorienting and expose the results.

public class Reorienting {

  int[] _array;
  public int[] Result => (int[])_array.Clone();

  static int[] _mod; // note that this is private on purpose

  public Reorienting(int size, int trials) {
    _array = genTrialList(trials); // implement this on your own
    // edit: oops I got this wrong in the first version
    // but take this as an example anyway, there are problems with this anyway
  }

  public void Process(float threshold) {
    reorient(_array, threshold);
  }

  void reorient(int[] a, float t) {
    buildModulatingArray(t, a.Length);
    for(int i = 0; i < a.Length; i++) if(_mod[i] > 0) a[i] = 1 - a[i];
  }

  void buildModulatingArray(float t, int count) {
    if(_mod == null || _mod.Length != count) {
      _mod = new int[count];
      int s = Mathf.FloorToInt(Mathf.Clamp01(t) * count);
      while(s > 0) {
        int i = Random.Range(0, count);
        if(_mod[i] == 0) {
          _mod[i] = 1;
          s--;
        }
      }
    }
  }

}

you now use this from someplace else

var processor = new Reorienting(5, trials: 6);
processor.Process(0.2f);
Debug.Log(processor.Result);

etc.

7

But then I forgot to include that I’m now rebuilding that modulation (so this code above has a critical bug)

  void rebuildModulatingArray(float t, int count) {
    if(_mod == null || _mod.Length != count) {
      _mod = new int[count];
      int s = Mathf.FloorToInt(Mathf.Clamp01(t) * count);
      while(s > 0) {
        int i = Random.Range(0, count);
        if(_mod[i] == 0) {
          _mod[i] = 1;
          s--;
        }
      }
    }
  }

  public void FlushMod() => _mod = null;

Now you can flush the static modulating array on demand, from public interface, because it’ll remain stored.
This is all just an example, it’s not a step by step guide how you should do it.

8

Thanks for that answer, makes a lot of sense. I am a solo programmer working on a small project which is maybe why my solutions to a lot of the problems I’ve come up against so far has just been to take whatever I got working first and move on to something else, but I had assumed that even for larger projects it is still standard that scripts need to be able to interact with one another, reading and potentially writing to variables.

So I guess my follow-up question would be, if this is true, and using static variables isn’t the best way to have something accessible to read, and potentially be written to by another script, what is the best way to do this?

9

Thanks so much for the detailed answer. I’m not familiar with a lot of the functions you used so I’ll have to go through it when I have a chance but hopefully it all makes sense.

10

Of course scripts have to interact on large projects; if anything, they interact even more. But that’s not the problem that “static” is designed to solve. You normally interact with another script through an instance variable, which indicates which copy of the script you want to interact with.

Suppose you’ve got a game with 100 objects in it, and you want to move one of those objects to a different location. You can’t just say “Transform.position = something” because that doesn’t tell the computer which of the 100 different objects you are trying to move. You need a variable of type Transform that refers to some specific Transform, and then you move that one.

If you turned “position” into a static variable, then now all 100 of the objects have to be at the same position as each other, because you now have only one “position” variable that is shared by those 100 objects, instead of a separate variable for each one.

11

@Latchh
Basically overusing static is similar to overusing goto. Such usage defeats object orientedness in C# and turns all code into a maintenance-hell. You lose any encapsulating behavior, and you lose any benefits from using coupling/decoupling designs, in effect ending up with a centralized telephone system that needs absolute addressing to resolve every piece of communication in an ever-escalating body of code. Abusing static is practically some kind of inverse of goto, in that you’re passively producing a spaghetti effect without even having to mess with the flow of execution.

About my code, it could’ve been better, it just turned out messy because I forgot about some aspect of its usage, and messed up when fixing it, but I still think it’s a good example of how to solve this problem in a strictly OO manner. The only reason why I use a private static there is because I don’t want to allocate memory if not absolutely necessary. But it is not required for cross-communication.

12

Ok, I think I get it now, thanks a lot for that explanation from both of you. I’ll definitely look into this more in my own time, at the very least you’ve convinced me that I should be looking for alternatives to static (or not even alternatives, just whatever is “correct” as opposed to incorrect usages of static), and should probably better understand their intended usage while I’m at it.