rm46
rm46

Reputation: 21

C# Updating a value in a list of objects is updating other values too

I was working on a simple turn based RPG game. I was working on a battle system. I had it working with 1 enemy, and wanted to update it to work on multiple enemies. I have it spawning multiple enemies, but I am having issues targeting them.

Basically, the enemies are stored in a list called actualEnemies. The enemies are generated at run time, and there can be between 1-3. A label is generated to display the enemy name, and the enemy is selected by double clicking on it.

The issue seems to be in the naming of the enemy's. They have a property called characterName to identify an enemy. The problem is, there can be 2 or more of the same type of character (for example, 3x Goblin). So in the code where the enemies are generated, I was wanting to add a 1 to the characterName if there already is one in the list. The idea is to give each enemy a unique name.

The problem is, when I try this, it changes the values of all of the enemies of the same name, and this is screwing with everything. Here is the relevant section of code:

if (EnemyExists(e)) {
    e.characterName += c.ToString();                            
}

c is simply a counter that is being used, to let me know if it is enemy 1, 2 or 3.

The EnemyExists function is as follows:

public bool EnemyExists(Enemy e) {
    bool exists = false;
    int eCount = 0;

    foreach (Enemy en in actualEnemies) {
        if (en.characterName == e.characterName) {
            eCount++;
        }
    }

    if (eCount > 1) {
        exists = true;
    }

    return exists;
}

Just to note, the enemies all start off with a default name. For this example I am working on, there are only 2 kinds of enemy. The game picks a random number between 1-3, to decide how many enemies there are. Each enemy will be one of the 2 pre defined enemies, picked at random again. This is why I just want to add a number to the end of a name that is already present.

I suspect it has something to do with how I am creating the list of enemies:

public void CreateActualEnemies() {
    int r;
    int potEnemyNum = potentialEnemies.Count;         
    int numEnemies;
    Random rand = new Random();
    numEnemies = rand.Next(1, 4 );

    for (int i = 0; i < numEnemies; i++) {
        r = rand.Next(0,potEnemyNum);
        Enemy ene = new Enemy();
        ene = potentialEnemies.ElementAt(r);
        actualEnemies.Add(ene);
    }

    DisplayEnemyDetails();
}

When I add to actualEnemies, am I just pointing to an address in potentialEnemies, so even though I am instantiating a new Enemy, each one, if it has the same name, points to the same place? Do I need to properly copy the object?

It is late, I am tired, but I feel like I am getting closer, any help would be great, thanks.

EDIT:

Enemy Class:

public class Enemy:Character {
    public int xpGiven { get; set; }
    public LootTable lootTable { get; set; }
    public int goldDropped { get; set; }
    public bool alive { get; set; }
    public NamedTimer timer { get; set; }

    public Enemy() {
        alive = true;
        xpGiven = 10;
        lootTable = new LootTable();
        goldDropped = 10;
        timer = new NamedTimer();            
    }
}

Upvotes: 0

Views: 137

Answers (2)

CthenB
CthenB

Reputation: 788

1: Property names should start with a capital letter in C#. I.e. public int xpGiven { get; set; } should be public int XpGiven { get; set; }. If it's a class variable (no get or set defined), you start with a lower case letter. To more easily identify class variables from local variables, programmers typically start them with a _, although this is not mandatory.

2: When doing ==, please be aware enemyA == enemya will return false. Use enemyA.Equals(enemya, StringComparison.OrdinalIgnoreCase) to ignore capital differences.

3:

public void CreateActualEnemies()
    {
        int r;
        int potEnemyNum = potentialEnemies.Count;         
        int numEnemies;
        Random rand = new Random();
        numEnemies = rand.Next(1, 4 );
        for (int i = 0; i < numEnemies; i++)
        {
            r = rand.Next(0,potEnemyNum);
            Enemy ene = new Enemy();
            ene = potentialEnemies.ElementAt(r);
            actualEnemies.Add(ene);
        }
        DisplayEnemyDetails();
    }

should be:

public void CreateActualEnemies()
    {
        Random rand = new Random();
        int numEnemies = rand.Next(1, 4 );

        for (int i = 0; i < numEnemies; i++)
        {
            Enemy ene = new Enemy();
            ene.characterName = "" + ene.GetType().Name + " " + i; // or something similar to give the ene a unique name.
            actualEnemies.Add(ene);
        }
        DisplayEnemyDetails();
    }

Upvotes: 0

Eric Wu
Eric Wu

Reputation: 917

Breaking down your question:

When I add to actualEnemies, am I just pointing to an address in potentialEnemies, so even though I am instantiating a new Enemy, each one, if it has the same name, points to the same place?

By doing

Enemy ene = new Enemy();

You are essentialy creating a new pointer in memory with a "blank" valued-reference. However, since you did

ene = potentialEnemies.ElementAt(r);

You overwrote it with the reference to the element in the list. So whatever changes you do the ene object will reflect the item in the list.

If you want to segregate said changes, try using a MemberwiseClone.

From the example given at MSDN:

public class Person 
{
    public int Age;
    public string Name;
    public IdInfo IdInfo;

    public Person ShallowCopy()
    {
       return (Person) this.MemberwiseClone();
    }

    public Person DeepCopy()
    {
       Person other = (Person) this.MemberwiseClone();
       other.IdInfo = new IdInfo(IdInfo.IdNumber);
       other.Name = String.Copy(Name);
       return other;
    }
}

Upvotes: 2

Related Questions