Class instance unwantedly "updates" all the other instances.

i have a base Item.cs with description,cost,sprite etc And *abstract Use()*., then sub-class Consumable.cs.
consumable.cs has few more things like health,mana (things to replenish) that it does so with It's Use().
Everything is working fine until i added "Uses" variable.
this is snippet of consumable.cs Use()

Code (CSharp):

1. if (Uses >= 1) {
2.  
3.             Uses--;
4.             PlayerManager.Instance.GiveHealth (this.Health);
5.             PlayerManager.Instance.GiveMana (this.Mana);
6.             MyGui.Instance.UpdateInventoryInfoTab ();
7.             PC.Instance.inventory.UpdateTooltip (slot) ;
8.             if (Uses == 0) {
9.                 slot.RemoveItem ();
10.                 PC.Instance.inventory.HideTooltip ();
11.             }
12.         }

Now suppose i have one Big Health Potion in my inventory It has 3 uses, and Shopkeeper has one ,3 uses..
i drink mine and his uses goes down too.
I Have No Idea Why Though...
Thanks J
p.s. just ask for more info if needed

 

Post how that Uses field is declared. Sounds like it is static?

 

not a static

 

Obviosly shopkeeper instance and player's one share same data somehow. This is possible either if Uses is static variable or they are the same asset or reference the same script. For example if your consumable is scriptable object and both potions instances from inventory and from store share same scriptable object reference, thing will be just like you said. You need to instantiate scriptable objects or store their state within mono behaviour.
Else, some other code may affect the Uses variable decreasing it when potion uses. With comples event system it's very esy to introduce bug like this.
Hope this helps...

 

well i store things in xml, i don't use scriptable objects. but it's not just shopkeeper if i have 2 big potions, and drink one 2 times, the other has 1 use left too.

this is ItemContainer.cs

Code (CSharp):

1.  
2. public class ItemContainer
3. {
4.  
5.     private List<Item> consumables = new List<Item>();
6.     public List<Item> Consumables {
7.         get{ return consumables; }
8.         set{ consumables = value; }
9.     }
10.  
11.     public ItemContainer ()
12.     {
13.      
14.     }
15.  
16. }

this is how i put items to it ( InventoryManager.cs)

Code (CSharp):

1. public void Awake()
2.     {
3.         Type[] itemTypes = { typeof(Equipment), typeof(Weapon), typeof(Consumable) };
4.         XmlSerializer serializer = new XmlSerializer (typeof(ItemContainer), itemTypes);
5.         TextReader textReader = new StreamReader (Application.streamingAssetsPath + "/Items.xml");
6.         itemContain = (ItemContainer)serializer.Deserialize(textReader);
7.         textReader.Close ();
8.  
9.     }

and this is how i give it to vendor

Code (CSharp):

1. private void GiveItem(string itemName)
2.     {
3.         GameObject tmp = Instantiate (InventoryManager.Instance.itemObject);
4.  
5.         tmp.AddComponent<ItemScript> ();
6.  
7.         ItemScript newItem = tmp.GetComponent<ItemScript> ();
8.  
9.         else if (InventoryManager.Instance.ItemContain.Consumables.Exists(x => x.ItemName == itemName))
10.         {
11.             newItem.Item = InventoryManager.Instance.ItemContain.Consumables.Find (x => x.ItemName == itemName);
12.         }
13.  
14.         if (newItem != null) {
15.             AddItem (newItem);
16.         }
17.         Destroy (tmp);
18.     }

What Are You Saying About Event System?

 

Sounds like you are somehow confusing data for the item blueprint with data for the item instance. You need some blob of data somewhere that describes all Big Potions, and that should have a variable that indicates the starting number of uses. Then you need another blob of data somewhere that defines one specific Big Potion that is sitting in your player's inventory, and that needs to have a separate variable that indicates the number of uses remaining.

 

Its very difficult to pinpoint every important detail, but i have script ItemScript.cs

Code (CSharp):

1. public class ItemScript : MonoBehaviour {
2.  
3.     public Sprite spriteNeutral;
4.     public Sprite spriteHighlight;
5.  
6.     private Item item;
7.  
8.     public Item Item {
9.         get {    return item;    }
10.         set {    item = value;  
11.             spriteHighlight = Resources.Load<Sprite> (value.SpriteHighlighted);
12.             spriteNeutral = Resources.Load<Sprite> (value.SpriteNeutral);
13.         }
14.     }
15.  
16.     public string GetTooltip(Inventory inv)
17.     {
18.         return item.GetTooltip (inv);
19.     }
20.  
21.     public void Use(Slot slot)
22.     {
23.         item.Use (slot,this);
24.     }
25.  
26.  

which get used like this for example if i drop item on the ground. I think it has to be different instance.

Code (CSharp):

1. GameObject tmpDrop = (GameObject)Instantiate (InventoryManager.Instance.dropItem, PlayerRef.transform.position - v, Quaternion.identity);
2.  
3.                     tmpDrop.AddComponent<ItemScript> ();
4.                     tmpDrop.GetComponent<ItemScript> ().Item = item.Item;

 

Sorry, I don't understand how any of that is a response to anything I said. I don't see anything in there that describes how you are tracking the number of uses an item has, or anything suggesting that you have created a distinction between a item blueprint and an item instance.

 

After line 11 in your GiveItem, presumably you want to remove that item from the Inventory manager as it has just been given out.

If you don't remove it (or mark it as spoken for), how do you prevent handing out the same item multiple times?

 

well i thought that having itemscript which gets copy of that item must be different script ,if i drop it on the ground and click on it,it has itemscript with variable Item which is base class for consumable. also item is an abstract class, if that's somehow important

 

isn't new item another instance if i delete it each time after handing it out?

 

Can you show the code for Item.cs and Consumable.cs? In particular the class (and

Uses

) definition parts (if the code needs snipping).

 

Is

Item

a struct or a class?

 

Item.cs is an abstract class with this function

Code (CSharp):

1. public abstract void Use (Slot slot, ItemScript item);

 

ITEM.CS

Code (CSharp):

1. public abstract class Item
2. {
3.     public ItemType ItemType { get; set; }
4.  
5.     public Quality Quality{ get; set; }
6.  
7.     public string SpriteNeutral{ get; set; }
8.  
9.     public string SpriteHighlighted{ get; set; }
10.  
11.     public int MaxSize{ get; set; }
12.  
13.     public string ItemName{ get; set; }
14.  
15.     public string Description { get; set; }
16.  
17.     public int CostGold {     get; set; }
18.  
19.     public float Weight {     get; set; }
20.  
21.     public  Item()
22.     {
23.  
24.     }
25.    
26.  
27.     public  Item(string itemName,string description,ItemType itemtype,Quality quality,string spriteNeutral,string spriteHighlighted,int maxSize,float weight)
28.     {
29.         this.ItemName = itemName;
30.         this.Description = description;
31.         this.ItemType = itemtype;
32.         this.Quality = quality;
33.         this.SpriteNeutral = spriteNeutral;
34.         this.SpriteHighlighted = spriteHighlighted;
35.         this.MaxSize = maxSize;
36.         this.Weight = weight;
37.     }
38.  
39. public abstract void Use (Slot slot, ItemScript item);
40.    
41.  

CONSUMABLE.CS

Code (CSharp):

1. public class Consumable : Item {
2.  
3.  
4.     public int Health { get; set; }
5.  
6.     public int Mana { get; set; }
7.  
8.     //public int Uses { get; set; }
9.  
10.     public Consumable()
11.     {
12.    
13.     }
14.  
15.     public Consumable(string itemName,string description,ItemType itemtype,Quality quality,string spriteNeutral,string SpriteHighlighted,int maxSize,int health,int mana/*,int uses*/,float weight)
16.         :base(itemName,description,itemtype,quality,spriteNeutral,SpriteHighlighted,maxSize,weight)
17.     {
18.         this.Health = health;
19.         this.Mana = mana;
20.         //this.Uses = uses;
21.         this.Weight = weight;
22.     }
23.  
24.     public override void Use(Slot slot,ItemScript item)
25.     {
26.  
27.         if (ItemName == "Antidote Potion")
28.         {
29.             PlayerManager.Instance.HealPoison();
30.             slot.RemoveItem ();
31.             PC.Instance.inventory.HideTooltip();
32.             PlayerManager.Instance.CalcWeight (true,Weight);
33.             return;
34.         }
35.  
36. //        if (this.Uses >= 1) {
37. //
38. //            this.Uses--;
39.         PlayerManager.Instance.GiveHealth (this.Health);
40.         PlayerManager.Instance.GiveMana (this.Mana);
41.         MyGui.Instance.UpdateInvHP_MP_tab ();
42.         slot.RemoveItem ();
43. //        }
44. //        if (this.Uses == 0)
45. //            slot.RemoveItem ();
46.     }
47.  

 

Then you are not making a copy of it in ItemScript. Classes are reference types, which means when you say something like

Code (CSharp):

1. Item myItem = someExistingItem;

You are not creating a new Item, you are just saving a reference to an existing Item. Any changes to someExistingItem will be reflected in myItem, and vice versa, because there's really only one Item. The variables are more like the Item's phone number; they let you talk to the Item, but they aren't the Item itself.

 

ohh,so if a base class is abstract , i can't make instances of its sub-class?
or just to redo the handing out part?

 

You can make instances of concrete classes just not abstract ones

However, the issue you have to resolve is who 'owns' which instances. I would suggest you have 2 main options: use object pooling & don't hand out in-use objects, or, create & hand out brand new objects for each request received.

Do, however, bear in mind what Antistone said above about making one object "=" another. They will both just reference the same instance.

 

No, this has nothing to do with abstract or with subclassing. You can make more instances of your class, but only with the "new" keyword. Just assigning one variable to another doesn't create a new instance of a class.

In this case, I would suggest you add a new method to Item that returns a copy of itself. Something along the lines of:

Code (CSharp):

1. public virtual Item Copy()
2. {
3.     Item result = new Item();
4.     result.ItemType = this.ItemType;
5.     result.Quality = this.Quality;
6.     // etc.
7.  
8.     return result;
9. }

Subclasses should override that method and extend it to include any extra copying that they need to do.

Then, whenever you intentionally want to create a copy of an item, you can call that function.

But just saying

Item a = b

doesn't create a copy; it only creates a reference.

 

yes and it's working fine, if i don't want to use field "uses". I'll try Your Suggestions And Keep You Posted

 

Item.cs(51,40): error CS0144: Cannot create an instance of the abstract class or interface `Item'

 

OK, so either make Item non-abstract, or make that function abstract in the Item class and just rely on subclasses to implement it.