Issue with value being changed in different places at the same time

Unity Engine
1

Hello !

I’m currently working on a personnal project and have an issue i can’t seem to find a solution to. I’ve looked on the forum and can’t find a similar problem.

(I’ll link the scripts below)
In my project i generate 16 rooms(custom classes) that have different values(id name description etc…)
and they all have one single enemy of type Hostile(another custom class).
When i generate the rooms, i create a random room from a json, and then adds to it the values that i want and a random hostile(from an hostiles json too), then i send this room to my manager which holds all the rooms(a rooms[ ] array), and i do this 16 times for all the rooms;

My issue is this one:
Later on, if i am in a room with my character, and attacks the enemy of that room, all the enemies that have the same name are affected.
ex: 10 rooms have a ghost enemy, 6 have a monster enemy. I attack a ghost in my room, all the ghosts in the 6 rooms which have ghosts take damage.
I hope i’m clear on my issue :/.

It’s like all the enemies of the same name are connected, even if i specifically created a new Hostile for each room.

Here’s the code related to that issue:

The Room class

using System;
using System.Collections;
using System.Collections.Generic;
using UnityEngine;

[Serializable]
public class Room
{
    public int id;
    public int objectLocation;
    public string name;
    public string description;
    public string iconPath;
    public int roomLevel;
    public Hostile enemy;
    public Item[] enemyLoot;
    public bool hostileRoom;
    public Item[] loot;
}

The Hostile class

using System;
using System.Collections;
using System.Collections.Generic;
using UnityEngine;

[Serializable]
public class Hostile
{
    public int id;
    public string name;
    public string description;
    public int health;
    public int attack;
    public int defense;
    public int prob;
    public int[] specialLootID;
}

The part when i generate the rooms

for(int i = 0; i < 16; ++i){
            encounterManager.GenerateRoom(i);
        }

The actual function generating the rooms

public void GenerateRoom(int i){
        Room room = new Room();
        room = mapManager.GetRandomRoom();//Returns a random room
        if(room.hostileRoom){
            room.enemy = GetHostile(room.roomLevel);//Returns a random hostile
            room.enemyLoot = GetEnemyLoot(room.enemy);
        }
        room.loot = GetLoot();
        Manager.instance.rooms[i] = room;
    }

The hostile creation ( GetHostile() )

public Hostile GetHostile(int roomLevel){
        Hostile hostile = new Hostile();
        hostile = hostiles[Random.Range(0,hostiles.Length)];
        while(Random.Range(0,100) > hostile.prob){
            hostile = hostiles[Random.Range(0,hostiles.Length)];
        }
        return hostile;
    }

And the moment where i actually change the value

public void Weapon1Hit(){
        int damage =        Random.Range(Manager.instance.character.weapon1.powerMin,Manager.instance.character.weapon1.powerMax);
        Manager.instance.rooms[Manager.instance.currentRoomID].enemy.health -= damage;
        mainViewUI.WriteInLog("You hit " + Manager.instance.rooms[Manager.instance.currentRoomID].enemy.name + " for "
        + damage + " damage with " + Manager.instance.character.weapon1.name);
        //Attack();
    }

Don’t hesitate if you want more information or other pieces of code where the problem could be coming from.
I use a singleton, that’s why i reference like : Manager.instance. , so i can keep the data between scenes.(for that issue i never change scenes so the problem can’t come from this).

Thanks
Guilhaume

2

Could you post your GetHostile method code? It probably returns same enemy instance per enemy type.

3

I just edited for the GetHostile method :wink:

4

if ‘hostiles’ is an array or list of ‘Hostile’, then your method appears to be picking one from said array (/list) and returning it; this means each room with the same type, is actually referencing the same one.
Perhaps you were intending to have the kind/type of hostile be random, but the instances (should be) unique :slight_smile:

5

Your GetHostile method should return a new instance of Hostile with every call, something like

// ...
switch (hostileType)
{
    case HostileType.Monster:
        return new MonsterHostile();
    case HostileType.Ghost:
        return new GhostHostile();
}
// ...
6

Hmm i’m not sure to understand this correctly.
I indeed want to have random type of hostiles, but unique instances, as in each room there is only one enemy(Hostile).

but isn’t it what i’m doing when i declare this :

Hostile hostile = new Hostile();

I declare a new hostile of type Hostile,( a new instance then right ?) and assign the values of a random hostile from my hostiles array, then assigns that hostile to my current room.

7

Not exactly, because yes that 1 line does make a new instance, but when you assign from your array / list, you are then assigning the reference to whichever object that is. It’s not copied. Something like @Fido789 's answer would be what you want.

Then, you can pass the type of hostile (enum possibly) from a random list to the hostile constructor , or use different methods to create different types… whichever suits you.
Then, that method can create a brand new one based on your design. :slight_smile:

8

This line in you code is meaningless, because right on the next line you are overwriting it with a “shared instance” of hostile.

9

Or you can also change it like this:

public Hostile GetHostile(int roomLevel){
    Hostile hostile = new Hostile();
    var hostileTemplate = hostiles[Random.Range(0,hostiles.Length)];
    hostile.name = hostileTemplate.name;
    // ....
    return hostile;
}
10

Hmm ok i understand, thank you i get the idea now.
Since i don’t really want to add each hostile value one by one, can i do something like that ? :

public Hostile GetHostile(int roomLevel){
    Hostile hostile = new Hostile();
    var hostileTemplate = hostiles[Random.Range(0,hostiles.Length)];
    hostile = hostileTemplate;
    // ....
    return hostile;
}
11

No, you can not. You have to somehow clone your hostile and it means to copy all the properties.

12

Ok,thank you, i tried this :

public Hostile GetHostile(int roomLevel){
        Hostile hostile = new Hostile();
        var hostileTemplate = hostiles[Random.Range(0,hostiles.Length)];
        hostile.name = hostileTemplate.name;
        hostile.id = hostileTemplate.id;
        hostile.description = hostileTemplate.description;
        hostile.health = hostileTemplate.health;
        hostile.attack = hostileTemplate.attack;
        hostile.defense = hostileTemplate.defense;
        hostile.prob = hostileTemplate.prob;
        hostile.specialLootID = hostileTemplate.specialLootID;
        return hostile;
    }

and it is stills affecting all instances :confused:

13

Don’t you have same problem with GetRandomRoom method returning shared instance?

14

I do indeed… :confused:
Oh, so i should try to fix the room too and then see if it works, since the rooms are “higher” in the hierarchy so maybe somehow it affects this too ?
Got to admit i have a little trouble projecting me with this issue… I’m learning code by my own for a few weeks now some maybe obvious things aren’t that obvious to me :p.

This is my GetRandomRoom():

public Room GetRandomRoom(){
        Room room = rooms[Random.Range(2,rooms.Length)];
        return room;
    }
15

And my new GetRandomRoom():

public Room GetRandomRoom(){
        Room room = new Room();
        var roomTemplate = rooms[Random.Range(2,rooms.Length)];
        room.id = roomTemplate.id;
        room.objectLocation = roomTemplate.objectLocation;
        room.name = roomTemplate.name;
        room.description = roomTemplate.description;
        room.iconPath = roomTemplate.iconPath;
        room.enemy = roomTemplate.enemy;
        room.enemyLoot = roomTemplate.enemyLoot;
        room.hostileRoom = roomTemplate.hostileRoom;
        room.loot = roomTemplate.loot;
        return room;
    }

And it seems to work !
I’m going to make a few tests to see if it really works then i’ll get back here and tell you :wink:

16

Ok it seems to work fine now !
So the point is that if i do :
Hostile hostile = hostiles(Random.Range(0,16))
I always thought this would take the data from the hostiles “database” and put a new instance in the hostile… i didn’t know this would get me a reference to it… this changes a lot now, i’m gonna recheck all my code to fit this :wink:

So i have to manually assign the values of the class but can’t directly pass the class as one.

Thanks you very much methos5k and fido789 :wink:

17

Great!

Btw, to make your code look a bit cleaner you can later make a Clone methods for your Room and Hostile classes and move your cloning code there:

public class Hostile
{
// ...

    public Hostile Clone()
    {
        return new Hostile {
            name = this.name
            // ...
        };
    }

// ...
}
1 Like
18

Cool, you got the idea :slight_smile:

19

Hmm, i’m not sure to understand how the clone works, Do i need to put this in my Hostile Class ? like :

using System;
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
[Serializable]
public class Hostile
{
    public int id;
    public string name;
    public string description;
    public int health;
    public int attack;
    public int defense;
    public int prob;
    public int[] specialLootID;

    public hostile Clone()
     {
          return new Hostile {
               name = this.name
               description = this.description
               //
          };
      }
}

And so i would call it like :

public Hostile GetRandomHostile(){
        Hostile hostile = new Hostile();
        hostile = hostileClone();
        return hostile;
    }

It’s probably (surely) not that syntax but i really have no clue about this, it’s like getters and setters, haven’t checked that yet :p.

20

Or more something like :

public Hostile GetRandomHostile(){
        Hostile hostile = new Hostile();
        hostile = CloneHostile(hostile);
        return hostile;
    }

public void CloneHostile(Hostile hostile){
     hostile.name = ?
     //...
     return hostile
}