Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

Inventory System - Picking up an item from scene and adding it to inventory

Discussion in 'Scripting' started by Fellshadow, Mar 4, 2014.

  1. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    I'm currently working on an inventory system in which you can pick up an item from a scene and add it to your inventory, as well as drop it back into the scene. There are also some cases where you can interact with a completely unrelated object and have an item added to your inventory (ie, interact with carrot plant and have a carrot added to your inventory).

    At the moment, I'm having issues with removing the item from the scene, after it has been added to the inventory. If I destroy the instance from the scene, it destroys the one from the inventory as well. I assume this is because I'm just adding a reference to the inventory, not a separate entity. Preferably, though, I just want the inventory to hold the item data, rather than a GameObject.
    I was also thinking of adding a pickup function to the Item class, but I'm not sure how to reference itself in the function. (ie. return mydata)

    Here are my current scripts:
    Item Class
    Code (csharp):
    1. using UnityEngine;
    2. using System.Collections;
    3.  
    4. public class Item : MonoBehaviour {
    5.     public string myName;
    6.     public string description;
    7.     public int price;
    8.     public bool consumable;
    9.     public ConsumeType consumeType;
    10.     public int restoreAmount;
    11.     public Rarity rarity;
    12.     public Texture2D icon;
    13.     public GameObject objectPrefab;
    14.  
    15.     public void ItemPickup(){
    16.         Debug.Log("Called function");
    17.     }
    18. }
    19.  
    20. public enum ConsumeType{
    21.     None,
    22.     HPRestore,
    23.     StamRestore
    24. }
    25. public enum Rarity{
    26.     Common,
    27.     Uncommon,
    28.     Rare,
    29.     Legendary
    30. }
    and the player inventory code (TestObject just has an item in the scene dragged into it to test adding from the scene, this will be removed once I get everything working)
    Code (csharp):
    1. using UnityEngine;
    2. using System.Collections;
    3. using System.Collections.Generic;
    4.  
    5. public class scr_PlayerInventory : MonoBehaviour {
    6.     public GameObject TestObject;
    7.     public List<Item> itemInv = new List<Item>();
    8.  
    9.     void Update(){
    10.         if(Input.GetKeyDown("space")){
    11.             Debug.Log("Space Key was pressed");
    12.             itemInv.Add(TestObject.GetComponent<Item>());
    13.         }
    14.    }
    15. }
    And here's what one of my items looks like:
    $exampleitem.png

    How would I go about adding this item to my inventory and removing it from the scene?
     
  2. AlwaysSunny

    AlwaysSunny

    Joined:
    Sep 15, 2011
    Posts:
    260
    Like most things, there's no one right way to go about this. What seems like the simplest, safest way involves creating a generic SceneItem monobehaviour with an ItemData field, while moving the meaningful data about your items into this new ItemData class. You could create these as prefabs using otherwise-empty gameobjects if you want to populate ItemData objects in the Unity inspector. This way, you firmly separate instances from originals, and can cleanly manage in-scene items, player inventories, and containers, and only ever work with ItemData objects.

    So a SceneItem gets an ItemData field, and changes its renderer material to correspond to the sprite indicated within ItemData. When the player picks it up, the player can just get a clone of the ItemData object in their Inventory, which could be a Dictionary structure which takes <ItemData, Quantity> for instance. Then the Inventory doesn't care that the SceneItem is destroyed. Vice versa to drop from inventory - a new SceneItem is created at the player position, and passed a copy of the ItemData object the player dropped. This method will also allow you to serialize your inventory for saving and loading without worrying about dead references to instances.
     
    eses likes this.
  3. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    Wow, you're in all my threads!

    You totally lost me with this explanation though; I'm still relatively inexperienced with Unity.
    What is a SceneItem and an ItemData field?
     
  4. AlwaysSunny

    AlwaysSunny

    Joined:
    Sep 15, 2011
    Posts:
    260
    Hey again, I'll see if I can improve on my previous response. To answer your immediate question, SceneItem and ItemData are just made-up names I gave the classes I was suggesting that you build.

    When a game involves an inventory system - or that seems to be the way many people encounter this concept - you really ought to have a way to work with your different items that will "just work" no matter what you're doing with that item. To do this, it's often wise to put the unique properties of each item into a type of object that doesn't care how it's being used. You give this class every variable that you'd ever want to know about an item - its cash value, weight, level, the sprite texture that represents it - all that goes in an ItemData class. That way, scripts that need to know things about items can have an ItemData variable, and read from (and write to) it as needed.

    So when you need an item in the scene to just sit around waiting to be picked up, you can use a highly generic prefab for pick-up-able items - which has an ItemData variable exposed - and drop in whatever ItemData object you need. I was calling this a SceneItem, but that's arbitrary. This re-usable SceneItem class would have a method which examines its ItemData variable, making whatever changes are necessary to properly represent that item. For instance:

    Code (csharp):
    1. public class SceneItem : Monobehaviour {
    2.      public ItemData myData;
    3.      void Initialize( ItemData newData ) {
    4.         myData = newData;
    5.         renderer.material.SetTexture("_MainTex", myData.sprite);        
    6.      }
    7. }
    A SceneItem should also feature a method for getting picked up and for properly destroying itself. This should compliment some method you create for any scripts that can pick up SceneItems. That complimentary method would take an ItemData object as an argument, and add that object to a collection of ItemData objects. That collection of ItemData objects represents the picker-upper's inventory!

    To carry the concept a little further, anything that can "take in" new items could implement this complimentary method - a slain monster has random items added to his corpse upon dying, a newly generated loot chest has random items added upon creation. Let's call this new monobehaviour an Inventory. Corpses and chests (and players!) have Inventory components, with methods like AddItem() which should add to and LookInside() which should return the collection of ItemData objects held by their respective scripts. Whatever GUI script you write to allow the player to view their inventory can use that LookInside() method to inspect the collection!

    Two important things to note -
    First, this approach changes how you hand-craft your items in the inspector. Instead of creating items the most obvious way, you're making them in a more abstract, more useful form. The most straightforward way to do this is probably to make your ItemData script a monobehaviour, and create a prefab for each different kind of item, populating the fields by hand for each item. This doesn't make a whole lot of intuitive sense - an ItemData object isn't really a behavior - but it allows you to easily modify each item using familiar Unity techniques and follows the "make everything a prefab" mindset Unity is built to support.

    Second, when you're passing around ItemData objects between scripts, you sometimes need to clone them as part of the passing process! Otherwise you're just passing around references which can and will be destroyed, and nothing's going to work right. The easiest way to do this is probably to code a public Clone() method for your ItemData script, which returns a newly created copy of the instance you called it on. For example:

    Code (csharp):
    1.  
    2. public ItemData Clone() {
    3.     ItemData clone = new ItemData();
    4.     clone.value = this.value;
    5.     clone.enchantments = this.enchantments;  // and so on for every variable
    6.     return clone;
    7. }
    8.  
    Then, for instance, your SceneItem script's OnPickedUp method could work like this:

    Code (csharp):
    1. void OnPickedUp( GameObject grabber ) {
    2.     Inventory inventory = grabber.GetComponent<Inventory>() as Inventory;
    3.     if (inventory) {
    4.         inventory.AddItem( myData.Clone() );  // passing a clone ensures we can destroy myData safely
    5.     }
    6.     GameObject.Destroy( gameObject );
    7. }


    If you feel like you really don't want or need to make your system this complicated right now, we can talk about a simpler alternative. Just let me know. Cheers,
     
    Last edited: Mar 5, 2014
    matzomat likes this.
  5. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    Thanks a lot for the detailed reply!

    From what I understand of what you wrote, I was actually close to the right path. In my case, the Item class I made would be the ItemData class you mentioned.

    Let me check if I understood the rest of what you were saying:
    -To create a new item for my game, I would create an empty object and attach the ItemData script to it, filling in the variables. save it as a prefab.
    -The SceneItem script would be attached to an arbitrary prefab with a sprite renderer, but leave all variables blank
    -When dropping an item, I would create a new SceneItem and pass the ItemData into it
    -When picking up an item, I would create a new ItemData in the inventory, and pass the ItemData from the SceneItem into it

    Does that sound right?
    If so, I have a few more questions:

    -In the example SceneItem class you wrote, the initialize takes in an ItemData. How would I pass the ItemData when instantiating it? Would I just manually call the function, or is that a Unity defined function that is automatically called when the GameObject is created?

    -What if I wanted an item on the ground by default? It's not dropped by anything, it's just there from the very start of the game? Would I attach both the SceneItem and ItemData script? That doesn't seem like it would work properly

    -If I have something like a plant, that is grown and gives out a specific vegetable when fully grown, it's not really a "container". Would this still work if I just had an ItemData slot on the plant and put in the item prefab? I suppose it would work similar to the pickup item, I just would skip the SceneItem part, and directly clone it from the plant to the players inventory?

    -Last question; Currently I had it so that each item prefab had it's own collision bounds. One sphere collider set as trigger for detecting if it comes into pickup range, and one of any other collider to provide some physics collision. It helps make the item land on the ground more realistically; since I'm using sprites, there are times where the collision box would be too large for the item, making it look like it floats when on the ground. Previously, I could adjust each item's collision individually, but won't I lose the ability to do this if I make a generic SceneItem that any ItemData can go in to?

    Once again, thanks a lot for all the help and advice. You've single-handedly saved me from a lot of frustration.
     
  6. AlwaysSunny

    AlwaysSunny

    Joined:
    Sep 15, 2011
    Posts:
    260
    "Let me check if I understood the rest of what you were saying" <- You've got all that just right!

    In the example SceneItem class you wrote, the initialize takes in an ItemData. How would I pass the ItemData when instantiating it? Would I just manually call the function, or is that a Unity defined function that is automatically called when the GameObject is created?


    I tend to favor naming my initialization functions Initialize() but that is just a preference, and not a built-in Unity function. Pretty much every time I instantiate a game object that needs some data from the class or method that created it, I write an Initialize() function on whatever scripts require that data, and call it the very next line:

    Code (csharp):
    1.  
    2. someVar = whatever;
    3. GameObject go = GameObject.Instantiate( prefab, pos, rot ) as GameObject;
    4. MyScript myScript = go.GetComponent<MyScript>() as MyScript;
    5. myScript.Initialize( someVar );
    What if I wanted an item on the ground by default? It's not dropped by anything, it's just there from the very start of the game? Would I attach both the SceneItem and ItemData script? That doesn't seem like it would work properly

    Right you are! You'd need to code the Start() method of a SceneItem to perform the same function as Initialize() - but without changing the myData variable. Instead you'd want to use whatever myData you've set in the Inspector. The cleverest way might simply be to call Initialize(myData); from Start();

    If I have something like a plant, that is grown and gives out a specific vegetable when fully grown, it's not really a "container". Would this still work if I just had an ItemData slot on the plant and put in the item prefab? I suppose it would work similar to the pickup item, I just would skip the SceneItem part, and directly clone it from the plant to the players inventory?

    This is a little tricky, I see. I'd say this is enough of a special case to warrant its own class, and you'd therefor skip the SceneItem stage altogether, yeah. If you were determined to be a stickler for code re-use, you might consider making the Inventory class more robust, so your Plant script could let the player LookInside() or TakeAll() but not AddItem(); Also there's nothing special about an ItemData object that says it has to be used a certain way, so doing what you've suggested is fine! :)

    Last question; Currently I had it so that each item prefab had it's own collision bounds. One sphere collider set as trigger for detecting if it comes into pickup range, and one of any other collider to provide some physics collision. It helps make the item land on the ground more realistically; since I'm using sprites, there are times where the collision box would be too large for the item, making it look like it floats when on the ground. Previously, I could adjust each item's collision individually, but won't I lose the ability to do this if I make a generic SceneItem that any ItemData can go in to?


    Hmm. Another tricky thing. To follow the principles of the idea, you'd want to add variables to ItemData that allow you to build whatever kind of collider is appropriate for that item during SceneItem.Initialize(); The merits of this approach can stretch pretty thin when you need ItemData or WhateverData to hold a zillion different things... The answer here depends on your needs and personal preference.

    Sometimes, when I have an "almighty re-usable prefab for stuff" like we're trying to do with SceneItem, I'll build what's known as a factory class, which is basically a static class or singleton with lots of convenience methods for getting a SomeObject built correctly. I think of these as a way of putting common tasks somewhere out of the way so I don't have to deal with them every time I want to build a SomeObject. This doesn't necessarily help with the task at hand - rather it's a way to turn the 3+ lines of my code snippet above into one line - but it might give you some ideas for simplifying complex and repetitive needs.

    If your heart is set on having different colliders (or sizes of the same kind of collider) per item, your best bet might be sticking that info in ItemData and letting SceneObject.Initialize() change or add a collider to match the requested type and size.

    You're very welcome for the writing - I like answering good questions and testing my own knowledge by having to explain it to others. Feel free to respond and to send me a message if you're ever stuck on something. :D
     
    matzomat likes this.
  7. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    Awesome, thanks a lot!
    I'm messing around with it right now, and everything seems to be working fine!

    I'll probably take you up on that offer at some point, there's still so much for me to learn!
     
  8. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    It started breaking when I tried to pass an item into the player's inventory. It gave me an error saying that I can't use new ItemData(); because ItemData derives from Monobehaviour and that can't be created. It told me to change it to use ScriptableObject instead, but now that's giving me plenty of trouble as well...

    EDIT: I think I've got it working now. I have it set as a ScriptableObject, and it's able to add them to the inventory array with no problems. No idea how to actually destroy them, but I'm sure I'll figure it out....
     
    Last edited: Mar 7, 2014
  9. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    Sooo ScriptableObjects aren't working out as well as I thought. Apparently, a script that derives from ScriptableObject can't be added to a gameobject, which means that I can't really make new items in the inspector.

    Funny thing is, if I make the script derive from Monobehaviour, attach the script to the object, then switch it back to ScriptableObject it works fine! What's with that!? Just let me attach it then!
    Any one have any insight on how to make item prefabs in the editor using ScriptableObject?
     
  10. Giometric

    Giometric

    Joined:
    Dec 20, 2011
    Posts:
    170
    For whatever reason (maybe to avoid confusion with creating a ScriptableObject vs creating a MonoBehaviour class), Unity has no way built into its interface to let you create an instance of a ScriptableObject class you've made. The support for it is all script-based.

    Check this page out: http://wiki.unity3d.com/index.php?title=CreateScriptableObjectAsset

    The editor script shown in there adds some utilities so that, in your own editor script, you can easily add a menu item to create a ScriptableObject of any type you want. It even includes an example of what the script you would need to write would look like.
     
  11. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    That's a great link, thanks a lot! It's so weird that I can just switch it from Monobehaviour after its been added to the object and it works fine though.

    Also, I discovered that if I just don't make my itemclass inherit from anything at all, then make it serializable, I'm able to add things to the inventory using new ItemData(). This would require a third script to handle making my items though, as the script still isn't able to be attached to objects directly.

    Which do you think the better route would be? All I want to be able to do is make new items in the inspector, and be able to add them to the inventory and easily save/load the inventory data.

    EDIT: I've also found that I can just have a list of non instantiated Gameobjects for my inventory. Is there any disadvantage to doing it this way? Maybe I wouldn't be able to control how many are in the stack like this?

    EDIT 2: Alternatively I can just duplicate a prefab with the ScriptableObject ItemData script attached to it and change the info, thus creating new items... If I do this, and the ItemData has a value called "Stack" and I change it on the ItemData instance in the player's inventory, when I save (serialize I guess?) the inventory data, the next time I load it, will it have the default values of that prefab, or does it keep the data that was in the inventory?

    EDIT 3: I realized that using any route other than the GameObject route makes things difficult for updating. Since all other ways just copy over the information from the object, it's possible that you can patch the game later to adjust values, but if the player had that item in his inventory, it won't be updated, since it just holds the values of when it was originally passed. Using non-instantiated GameObjects solves this problem, but introduces the problem of being unable to have a "stack" value, since if you change it on the non-instantiated object, it changes it on the prefab. I suppose I could just instantiate all the objects in the inventory, then the stack value would be per instance, but then I've got these objects floating around with the character...

    Ugh, this is pretty difficult to figure out. Some input/discussion would be much appreciated.
     
    Last edited: Mar 15, 2014
  12. AlwaysSunny

    AlwaysSunny

    Joined:
    Sep 15, 2011
    Posts:
    260
    Hey again,

    I'm afraid I made some careless mistakes in previous posts which I'll try to amend. The original point of creating an ItemData class was to trade these major headaches for some minor ones.

    The way to go forward might come down to preference, but first a word about the limitation: My suggested approach assumes your items are built in the inspector, and that no value on them ever changes after you've set them up at design time. In other words, I'm assuming there's not, for instance, an integrity variable that decreases with use and causes items to break. You can go down that road if necessary, but doing it wisely requires a bit more effort. (Instead of operating on ItemData, that instance-specific stuff would go on something like ItemInstance, which looks at an ItemData object to set up starting values). In my suggestion, ItemData objects were meant to act as presets - not as individual instances of intuitive "items". That was kinda the whole point.

    The key point here is: ItemData objects should inherit from nothing, and get a [Serializable] attribute. My mistake was telling you to make them monobehaviors. What I should've said was: Create a monobehavior ItemTemplate which exposes an ItemData variable, set that up and save it as a prefab, and its only function is exposing the ItemData object - to you at design time, and to anything that wants it at runtime.

    You'd need to amend any script that expects an ItemData object assigned in the inspector to take an ItemTemplate instead. However, things like your player's inventory can still hold (and easily save!) ItemData objects:

    Code (csharp):
    1. void OnPickedUpItem(SceneItem sceneItem) {
    2.     inventory.AddItem( sceneItem.itemTemplate.itemData, 1 );
    3. }  
    Again, ItemTemplate's only reason for existing is to expose an ItemData object to whatever needs it, like itemTemplate.itemData.itemName. Using something like ItemTemplate is the quickest workaround to the inability to create non-monobehaviors as prefabs.

    Code (csharp):
    1. public sealed class ItemTemplate : Monobehaviour {
    2.     public ItemData itemData;
    3. }
    If your needs grow, you might later consider making a script ItemFactory, which grabs (or gets assigned by you at design time) all your ItemTemplate prefabs. Then you could request searches that would return random items of a specific type, as well as give yourself string-based search access to your whole library of items at runtime.
    ItemData newItem = ItemFactory.Random( ItemType.Sword );
    ItemData newItem = ItemFactory.Named("WoodShortSword");


    An alternative to the ItemTemplate method would be to create an ItemFactory class which exposes a list of ItemData objects, and you create and edit them within that list. With a custom inspector to help, this approach can be quite nice, but it requires everything that wants an ItemData object to request it via searching. So for instance on SceneItem, your ItemTemplate goes back to being ItemData but can be hidden in inspector, and instead of drag-n-drop in inspector, you link it to the ItemData you want by telling SceneItem what string name to search for on Start(); It comes down to which approach creates less work for you.
     
    Last edited: Mar 15, 2014
  13. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    Thanks a lot for the info! That's actually something I ended up figuring out, it's good to see I was on the right track though!

    Can't your method still be used to hold individual values for each item? My items don't need any specific variable except for stack, but my weapons/tools have variables that need to be tracked, like stack, fill amount, gem slots, etc.
    When you add an item to the inventory, for example, you're not actually putting an instance of that item in there. You're essentially just copying the data from the item prefab into the inventory. Doesn't this mean you could just change the stack/durability/whatever value in the inventory? Or is there something I'm overlooking? Or is there just a better way to do this when you want to do things like that?

    I also had the thought that this isn't very update friendly. For example, if I have an item that sells for 50g, and the player picks it up and saves the game. Later on, I may feel that that item sells for too much, so I would update the game and lower the value. This wouldn't be reflected on the item in the player's inventory, however, since it only has the data that was originally copied over; it's not "linked" to its original in any way.
    So, I had the idea of adding a public GameObject variable to my ItemData class, and when I make the item in the inspector, I would just put itself in that slot. Then, when the player loads his game, it would go through the inventory and call an "updatedata" function on every item. This function would get the default values from the prefab provided and overwrite specific ones (sellValue, name, etc) while ignoring ones that I don't want overwritten (stack). What do you think? Is there a better way to go about this?
     
    eses likes this.
  14. AlwaysSunny

    AlwaysSunny

    Joined:
    Sep 15, 2011
    Posts:
    260
    The automatic update idea sounds really clever, but I don't have any familiarity with updating Unity games - my assumption was that the user had to re-download the whole game each time, unless you did all the hard work of creating some kind of custom updater and asset pipeline. But you're right, if they keep their savegame and you treat ItemData objects as read/write instances, the problem you describe would occur. (Might wanna make a separate thread for this updating question, it may be much more complex than I realize)

    Now I see you do want to have items whose instances matter - to track what gems are slotted on a specific item for instance. I guess you don't have to have a separate class like ItemInstance - you can simply operate on the specific ItemData object. My thought was that they should act as readonly objects once created in the inspector, but especially with that self-referencing trick you described, you could abandon that notion and let them behave like instances too. Seems a wee bit confusing, but you'd just have to be careful to always know which you're working on - instance or preset.

    Good on ya for putting in this much effort - you're gonna have a solid foundation to build from if you're careful. :)
     
    Last edited: Mar 16, 2014
  15. jister

    jister

    Joined:
    Oct 9, 2009
    Posts:
    1,749
    here's an example project for an inventory system using scriptableObject.
    check the last post of this thread for a link

    the idea is similar to what always sunny suggest only the other way around if that makes sense ;-)

    you derive your Item base class from scriptable object with a field for a GameObject thisObject; or something...

    thisObject holds the prefab to represent the Item in game. You can easily track the scriptableObject instance in your code (since ScriptableObject is a code package)

    you can build the creation of your Object into void OnEnable(){ //Instantiate prefab } which runs when you instantiate an instance with ScriptableObject.CreateInstance<Item>();
     
    Last edited: Mar 16, 2014
  16. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    Thanks for the feedback! Updating SHOULD work similar to SVN, if you've used that. It keeps track of what has been changed in the game, and only updates those files. I've never created a game that needs to be updated before, so I haven't actually done it, but that is my impression of how it should work.

    Thanks for the info, Jister! I was actually having problems destroying ScriptableObjects as well. I tried calling Destroy, DestroyObject, DestroyImmediate on the first ScriptableObject in my array, but it was never destroyed.
     
  17. jister

    jister

    Joined:
    Oct 9, 2009
    Posts:
    1,749
    if you downloaded the project, you can see in the player class the DestroyItem(Item item){} and InstantiateItem(Item item){} functions are added to the pick up and drop events. together with some other like AddToInventory and RemoveFromInventory which simply handle Adding or Removing from a List.
    there's also 2 editor extensions, one for creating items and one for characters.
    let me know if you can use it or have any questions about the system.
     
  18. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    I did a quick look through the project and couldn't seem to find any place that you are actually destroying the items. When you remove it from the inventory, it seems like you just remove it from the list. Doesn't this cause a memory leak? I was under the impression that ScriptableObjects needed to be destroyed like any other GameObject.


    Also, I've realized for my equipment, I want all of it to be in one list (ie, both tools and weapons). Apparently, if I just make both tools and weapons derive from the same base class, I can put them both in a base class type array. However, this won't let me access variables from the other classes (So I can access variables from the base class, but not from the tool/weapon child classes). I have to essentially cast the object in the list as the type it is (cast it as tool or weapon) before I can access it's variables.
    Is this really any more effective than just having a "toolType" variable on one class and using an if statement to figure out what to do with it? I read about polymorphism, but I'm not sure it would help in this case, or maybe I just don't know the correct usage?
     
    Last edited: Mar 17, 2014
  19. jister

    jister

    Joined:
    Oct 9, 2009
    Posts:
    1,749
    well it's still a wip testing project, i have kind of the same but that all is derived from monobehaviour...
    anyway you can find the destroy as i said in the player class (which doesn't make all that much sense... I'm moving it to the Item class itself later i think)
    also for now dropping an Item still has things to be added, like putting the item back in the right layer and stuff...
    I'll keep working on this when ever i have time, so if you wish you can copy the project and follow along.
    anyway it's here;
    Code (csharp):
    1. using UnityEngine;
    2. using System.Collections;
    3.  
    4. public class Player : MonoBehaviour
    5. {
    6.     public PlayerCharacter player;
    7.     public int radius = 2;
    8.     private bool interaction;
    9.     private bool disInventory;
    10.     public Collider[] objects;
    11.     public Item item;
    12.  
    13.     void OnEnable()
    14.     {
    15.         EventManager.PickUpItem += DestroyItem;
    16.         EventManager.DropItem += InstantiateItem;
    17.     }
    18.     void OnDisable()
    19.     {
    20.         EventManager.PickUpItem -= DestroyItem;
    21.         EventManager.DropItem -= InstantiateItem;
    22.     }
    23.     void Update ()
    24.     {
    25.         objects = Physics.OverlapSphere(transform.position, radius, 1<<8);
    26.         if(objects.Length>0)
    27.             item = objects[0].GetComponent<Instance>().reference;
    28.         else
    29.             item=null;
    30.         if(item!=null  !item.IsInventory)
    31.         {
    32.             if(Input.GetKeyDown(KeyCode.E))
    33.             {  
    34.                 EventManager.PickedUpItem(item);
    35.             }
    36.         }
    37.         if(Input.GetKeyDown(KeyCode.I))
    38.         {
    39.             if(!disInventory)
    40.                 disInventory=true;
    41.             else if(disInventory)
    42.                 disInventory=false;
    43.         }
    44.     }
    45.     void OnGUI ()
    46.     {
    47.         if(item!=null  !item.IsInventory)
    48.             EventManager.OnDrawInteraction(item);
    49.         if(disInventory)
    50.             EventManager.OnDrawInventory(item);
    51.     }
    52.  
    53.     void DestroyItem(Item item)
    54.     {
    55.         Destroy(objects[0].gameObject);
    56.         item=null;
    57.     }
    58.     void InstantiateItem(Item item)
    59.     {
    60.         GameObject go =(GameObject) Instantiate (item.ItemObj, transform.position, transform.rotation);
    61.         go.AddComponent<Instance>().reference = item;
    62.         item=null;
    63.     }
    64. }
    65.  
    as for polyMorphism:

    it's easy to go from child to base (base)child, but from base to child it a bit different... i use a small forloop that copies all variables and values they have like:
    Code (csharp):
    1. Item i = new Component() as Item;
    2.         BindingFlags flags = BindingFlags.Public | BindingFlags.Instance;
    3.         foreach (PropertyInfo f in item.GetType().GetProperties(flags))
    4.         {
    5.             Debug.Log(f.PropertyType.Name);
    6.             if(f.PropertyType == typeof(Transform))
    7.                 break;
    8.             f.SetValue(i, f.GetValue(item, null), null);
    9.         }
    10.         Inventory.itemList.Add(i);
     
  20. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    This is really frustrating me. The engine can't decide whether or not this code throws an error.
    I create my array like so:
    Code (csharp):
    1.  
    2. void Start(){
    3.     itemInv = new ItemData[maxItemSize];
    4.     toolInv = new ToolBaseData[maxToolSize];
    5. }
    6.  
    and sometimes when I try to access it like so:
    Code (csharp):
    1.  
    2. if(invScript.toolInv[i].myName == null){
    3.     //unrelated code here
    4. }
    5. else{
    6.     //Other code here
    7. }
    8.  
    It gives me the error: "NullReferenceException: Object reference not set to an instance of an object"

    So I go and change the code to:
    Code (csharp):
    1.  
    2. if(invScript.toolInv[i] == null){
    3.     //unrelated code here
    4. }
    5. else{
    6.     //Other code here
    7. }
    8.  
    And it works!
    .....For a time. Then it starts acting as if the slots are not EVER null, and it keeps going to the else statement.
    If I add .myName back in then it works without an error again!

    And it just keeps switching back and forth like that, even though I'm not even touching any code related to this. What is going on!?
     
  21. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    Issue in the previous post is still occuring.
    Can anyone figure out what is going on? I can't figure out why it keeps changing.
     
  22. AlwaysSunny

    AlwaysSunny

    Joined:
    Sep 15, 2011
    Posts:
    260
    I spy nothing wrong with the code in the last post. The issue must be somewhere else. By the way, it's bad bad bad to not check for a null object before trying to access a property of that object, assuming it can ever be null.
     
  23. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    Yeah, that's why my original if statement was checking if it was null.
    It never returned null, so then I switched it to checking a property. Then it decided to start being null again.

    After experimenting for a while, I've found that if I exit Unity and start it up again, it will return to being null. If I hook up a button to set the space to null, like so:
    Code (csharp):
    1. itemInv[0] = null;
    It won't return null. I have to restart Unity, then it will return null again. It really doesn't make any sense.
     
  24. AlwaysSunny

    AlwaysSunny

    Joined:
    Sep 15, 2011
    Posts:
    260
    No chance you've got unexpected calls to things that write to or assign array members? Something silly in the editor like two instances of a script that should only exist once? Are these values being populated by a serialization service on startup? Just spouting ideas. Try to narrow it down some if there's too much code to post it all.
     
  25. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    Well, I'll show/explain my related code, and maybe you can spot something.

    My ItemData class. Stores all data related to the item.
    Code (csharp):
    1. using UnityEngine;
    2. using System.Collections;
    3.  
    4. [System.Serializable]
    5. public class ItemData {
    6.     public string myName;
    7.     public string description;
    8.     public int stack = 1;
    9.     public int maxStack = 1;
    10.     public int price;
    11.     public bool consumable;
    12.     public ConsumeType consumeType;
    13.     public int restoreAmount;
    14.     public Rarity rarity;
    15.     public Sprite icon;
    16.     public Sprite sprite;
    17.     public GameObject originalPrefab;
    18.  
    19.     public void CopyToInventory(scr_PlayerInventory invScript, int slot){
    20.  
    21.         invScript.itemInv[slot] = this; //new ItemData();
    22.         invScript.interactObject.GetComponent<scr_InteractTrigger>().interactList.RemoveAt(0);
    23.     }
    24.     public void CopyToSceneItem(GameObject sceneObject){
    25.         sceneObject.GetComponent<scr_SceneItem>().myData = this;
    26.     }
    27. }
    28.  
    29. public enum ConsumeType{
    30.     None,
    31.     HPRestore,
    32.     StamRestore
    33. }
    34. public enum Rarity{
    35.     Junk,
    36.     Common,
    37.     Uncommon,
    38.     Rare,
    39.     Legendary
    40. }
    My item holder script. Exposes the data on a prefab to allow for editting/creating items in editor.
    Code (csharp):
    1. using UnityEngine;
    2. using System.Collections;
    3.  
    4. public class scr_Item : MonoBehaviour {
    5.     public ItemData itemInfo;  
    6. }
    7.  
    My Scene Item script. This is the object that sits in the scene that can be interacted with. startupItem is only when the scene item exists by default (so I place the sceneItem in the scene, then drag one of the item prefabs on to it)
    Code (csharp):
    1. using UnityEngine;
    2. using System.Collections;
    3.  
    4. public class scr_SceneItem : MonoBehaviour {
    5.  
    6.     public ItemData myData;
    7.     public GameObject startupItem;
    8.     // Use this for initialization
    9.     void Start () {
    10.         if(startupItem != null){
    11.             Initialize(this.gameObject);
    12.         }
    13.     }
    14.  
    15.     void Initialize(GameObject sceneItem){
    16.         Debug.Log("Calling initialize");
    17.         startupItem.GetComponent<scr_Item>().itemInfo.CopyToSceneItem(sceneItem);
    18.         GetComponent<SpriteRenderer>().sprite = myData.sprite;
    19.     }
    20.  
    21.     public void Interact(GameObject activator){
    22.         //Adds an item to the player's inventory
    23.         scr_PlayerInventory invScript = activator.GetComponent<scr_PlayerInventory>();
    24.  
    25.         int slot = -1;
    26.         //Is the item stackable?
    27.         if(myData.maxStack > 1){
    28.             //If so search to see if this item is added already
    29.             for(int i = 0; i < invScript.itemInvSize; i++){
    30.                 //Check if this slot is empty
    31.                 if(invScript.itemInv[i] != null){
    32.                     //If slot is not empty, check if it has the same name, and make sure that it doesn't already have the max amount of stacks
    33.                     if(invScript.itemInv[i].myName == myData.myName  invScript.itemInv[i].stack < invScript.itemInv[i].maxStack){
    34.                         //Now check if adding this stack of items will put it past it's max
    35.                         if(invScript.itemInv[i].stack + myData.stack < invScript.itemInv[i].maxStack){
    36.                             slot = i;
    37.                             myData.CopyToInventory(invScript, slot);
    38.                             Destroy (this.gameObject);
    39.                             break;
    40.                         }
    41.                         //If it can't fit, throw a debug message for now
    42.                         else{
    43.                             Debug.Log ("Stack not at max, but item that is trying to be added has more than max stack");
    44.                         }
    45.                     }
    46.                 }
    47.             }
    48.         }
    49.  
    50.         if(slot == -1){
    51.             //Find first empty slot
    52.             for(int i = 0; i < invScript.itemInvSize; i++){
    53.                 if(invScript.itemInv[i] == null){
    54.                     slot = i;
    55.                     break;
    56.                 }
    57.             }
    58.             //If an empty slot was found, copy the data into the empty slot
    59.             if(slot != -1){
    60.                 myData.CopyToInventory(invScript, slot);
    61.                 Destroy(this.gameObject);
    62.  
    63.             }
    64.             //No slot was found, so inventory is full
    65.             else{
    66.                 Debug.Log("Inventory Full!");
    67.             }
    68.         }
    69.     }
    70.  
    71.     void OnDrawGizmos(){
    72.         Gizmos.DrawIcon(transform.position, "ItemIcon.png", true);
    73.     }
    74. }
    Now, it only starts not becoming null AFTER I use this test script a few times:
    Code (csharp):
    1.  
    2. if(Input.GetButtonUp("Dodge")){
    3.     itemInv[0] = null;
    4. }
    I also tried doing itemInv[0] = new ItemData(); instead and the same issue occurs. Restarting Unity makes everything work properly again with NO changes to my code.
     
    Last edited: Mar 18, 2014
  26. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    I've even tried adding this to the start function where my arrays are defined:
    Code (csharp):
    1.  
    2. for(int i = 0; i < itemInv.Length; i++){
    3.     itemInv[i] = null;
    4. }
    and it will still have the same issue until I restart Unity. Which doesn't make sense, as I'm overwriting all values in the array as null, but when I check later, they aren't null anymore...

    It almost seems like the data in the array is being saved, and is not allowing itself to be overwritten or something. Even though when I check the data in the editor or through a debug.log, it just has the default blank template from my ItemData class...

    EDIT: I don't understand. All I did was add a debug button that would loop through the entire array, and check if it's null or not, and now I can't get the issue to happen anymore... I didn't change anything else! This doesn't make any sense!
    Code (csharp):
    1.  
    2. if(Input.GetButtonUp("Fire2")){
    3.     for(int i = 0; i < itemInv.Length; i++){
    4.         if(itemInv[i] == null){
    5.             Debug.Log ("Slot " + i + " is null");
    6.         }
    7.         else{
    8.             Debug.Log("Slot " + i + " is NOT null");
    9.         }
    10.     }
    11. }
    12.  
     
    Last edited: Mar 18, 2014
  27. jister

    jister

    Joined:
    Oct 9, 2009
    Posts:
    1,749
    try letting your data class inherit from ScriptableObject.
     
    Last edited: Mar 18, 2014
  28. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    Neither. My ItemClass doesn't derive from anything.
    What does SerializeField do? Also, none of my item variables are private.

    I didn't want to use a list because then the player can't really have blank inventory slots. For example, with an array, I can have an item in slot 0, no item in slot 1, and an item in slot 2, and then all the other slots after that empty as well. I don't think that's possible with a list?

    EDIT: You changed what you said >.<
    If I make it derive from ScriptableObject, I'd have to go and reprogram the entire item system.

    Also, I got the issue to occur again after removing the debug code, then I readded the code and tested it. Apparently when this issue occurs, the array becomes unoverwriteable. Even if I set it to = null, when I check it right afterwards, it says its NOT null. What is happening?
     
    Last edited: Mar 18, 2014
  29. AlwaysSunny

    AlwaysSunny

    Joined:
    Sep 15, 2011
    Posts:
    260
    You can get more robust behavior from lists, and possibly avoid mucking around with indexers. Given the task, a list seems more appropriate. You can still use indexers with a list the same way you'd use them with an array e.g. somelist[0] accesses the zeroth element. And you can have empty "slots" if you set the list element to null, your code just has to accommodate null entries appropriately.

    I didn't see a problem leaping out, but I didn't proof it super thoroughly or fully grasp every nuance. It's possible there's a careless mistake in how you're accessing elements, and also possible that your copy method needs to actually copy the object, notassign itself. Even though they're not gameobjects/monobehaviors, they're still objects that need to be thought of as instances, and instances can be destroyed so things that point to them become null.
     
  30. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    Yeah, originally I used to copy over each variable, but it became really annoying to have to change it every time I added/removed/changed a variable, so I tried this and it worked (I checked to make sure it is copying and not referencing by deleting the object it copied over from, and it still kept all it's data without any issues)

    Although the problem is that it is NOT null though. I want empty slots to be null, lol.

    Maybe I'll have to try a list, though.
     
  31. jister

    jister

    Joined:
    Oct 9, 2009
    Posts:
    1,749
    yes sorry bout that i didn't read the 2nd page when writing previous comment ;-)
    i would go for changing the ItemData class to inherit from ScriptableObject, since it inherits from nothing now i can't be that hard to reprogram. mostly just change
    ItemData itemData = new ItemData();
    to
    ItemData itemData = ScriptableObject.CreateInstance<ItemData>();
    Inheriting from nothing is good to access some functionality in a script otherwise not needed, while ScriptableObject serves as a Data container for classes that don't really need to be on gameObject just store some data. which seems exactly what you need.
    My guess about the losing of variable values is they don't get restored after deserialization, which means they didn't get serialized right in the first place.
    [SerializeField] makes sure the field gets serialized... have a look at:
    http://docs.unity3d.com/Documentation/ScriptReference/SerializeField.html
    maybe also:
    http://forum.unity3d.com/threads/155352-Serialization-Best-Practices-Megapost
     
    Last edited: Mar 19, 2014
  32. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    Thanks for the response.

    Switching over to ScriptableObject is more than just changing that one line. Since ScriptableObject scripts can't be applied to prefabs, I'd have to go remake all my items. On top of that, as I mentioned before when I tried using a ScriptableObject, I was for some reason unable to destroy it, and I worry about memory leaks.

    Also, it's not really that I'm losing variable values. It's that my lists/arrays are sometimes not returning that they are null, even though I've just started the game and just initialized them. But, if I look at them in the inspector, they look like null lists/arrays (no data in them, just empty values).

    EDIT: Issue seems to occur with Lists as well.

    EDIT: Creating a separate thread for this, because something strange is going on here
     
    Last edited: Mar 19, 2014
  33. jister

    jister

    Joined:
    Oct 9, 2009
    Posts:
    1,749
    but scripts without inheritance also can't be applied to prefabs? only MonoBehaviour scripts can. we are talking about you ItemData class right?
    I'll gladly take a look at your project if you want...
     
  34. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    That's because I have an item holder script that inherits from monobehavior.
    It's in the code I listed above. Can't do that with ScriptableObjects, however.
     
  35. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    So I ended up switching over to ScriptableObject. However, now I'm having another issue in that I can't adjust the stack / individual values on a ScriptableObject in the inventory since it's just a reference to the asset.

    Lightstriker mentioned in another post:


    In which I responded:
    Looking for some more discussion and advice, please.
     
  36. LightStriker

    LightStriker

    Joined:
    Aug 3, 2013
    Posts:
    2,717
    Creating a new instance of ItemData - if it derive from ScriptableObject - won't work because you will not be able to save it anywhere at runtime. Asset serialization is an editor-only feature.

    I think runtime serialization is one of the biggest flaws of Unity - and it has MANY. Any time you want to save anything else than the stupid PlayerPrefs, you must bring on board a different serialization system. (JSon, XML, Proto, Binary)
     
  37. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    Well, I don't really need to save the asset at all since I already have an asset for the item (this new instance is just a clone afterall), I just want to save the data (the value of the variables) inside the new instance. Then I could just create a new instance when I want to load, and fill it with the old data. I also don't want to use PlayerPrefs, as that does not save to a file, and I want there to be an actual save file.
    Is that not possible?
     
    Last edited: Mar 20, 2014
  38. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    So, it would look something like this (pseudocode):

    Code (csharp):
    1.  
    2. public void SaveInventory(){
    3.     for(int i = 0; i < itemInventory.length; i++){
    4.         file = OpenFile(characterName + "_saveFile");
    5.         //Write the original asset that this item is a clone of
    6.         file.Write("Item[" + i + "]asset", itemInventory[i].originalAsset);
    7.         //Write the individual values from this instance that you don't want overwritten
    8.         file.Write("Item[" + i + "]stack", itemInventory[i].stack);
    9.     }
    10.  
    11. }
    12.  
    Then you would load the opposite way:

    Code (csharp):
    1.  
    2. public void LoadInventory(){
    3.     for(int i = 0; i < itemInventory.length; i++){
    4.         file = OpenFile(characterName + "_saveFile");
    5.         originalAsset = file.Read("Item[" + i + "]asset", itemInventory[i].originalAsset);
    6.         itemInventory[i].name = originalAsset.name;
    7.         //Repeat this process for every variable that you want loaded from the original asset
    8.        
    9.         //load instance variables
    10.         itemInventory[i].stack = file.Read("Item[" + i + "]stack", itemInventory[i].stack);
    11.     }
    12. }
    13.  
    Obviously, you'd have to do a null check first, and write/read data accordingly. Also, the loading and saving of each individual item could actually be a function inside the item class, then it could just be called through the originalAsset variable.

    This should somehow be possible, shouldn't it? It's really odd that I'd have to make a whole new array just to track individual values and then save that array...
     
  39. LightStriker

    LightStriker

    Joined:
    Aug 3, 2013
    Posts:
    2,717
    Yup. You'll have to write in a file manually.

    You should look into the System.IO namespace.
     
  40. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    Thanks for the response. I've actually been working on XML Serialization (I don't like how it's in a readable file, but I don't know how else to serialize).

    I've actually just encountered an error trying to serialize my ScriptableObject.

    Here is my SaveManager code:
    Code (csharp):
    1.  
    2. using UnityEngine;
    3. using System.Collections;
    4. using System.Collections.Generic;
    5. using System.Xml;
    6. using System.Xml.Serialization;
    7. using System.IO;
    8.  
    9. [XmlRoot("GameData")]
    10. public class scr_SaveManager {
    11.     [XmlArray("itemInv"),XmlArrayItem("item")]
    12.     public List<ItemData> itemInv = new List<ItemData>();
    13.    
    14.     public void Save(string path){
    15.         var serializer = new XmlSerializer(typeof(scr_SaveManager));
    16.        
    17.         using(var stream = new FileStream(path, FileMode.Create)){
    18.             serializer.Serialize(stream, this);
    19.         }
    20.     }
    21.    
    22.     public static scr_SaveManager Load(string path){
    23.         var serializer = new XmlSerializer(typeof(scr_SaveManager));
    24.        
    25.         using(var stream = new FileStream(path, FileMode.Open)){
    26.             return serializer.Deserialize(stream) as scr_SaveManager;
    27.         }  
    28.     }
    29. }
    30.  

    And my ItemData code:
    Code (csharp):
    1.  
    2. using UnityEngine;
    3. using System.Collections;
    4. using UnityEditor;
    5. using System.Xml;
    6. using System.Xml.Serialization;
    7.  
    8. public class ItemData : ScriptableObject{
    9.     [XmlIgnore]
    10.     public string myName;
    11.     [XmlIgnore]
    12.     public string description;
    13.     [XmlAttribute("stack")]
    14.     public int stack = 1;
    15.     [XmlIgnore]
    16.     public int maxStack = 1;
    17.     [XmlIgnore]
    18.     public int price;
    19.     [XmlIgnore]
    20.     public bool consumable;
    21.     [XmlIgnore]
    22.     public ConsumeType consumeType;
    23.     [XmlIgnore]
    24.     public int restoreAmount;
    25.     [XmlIgnore]
    26.     public Rarity rarity;
    27.     [XmlIgnore]
    28.     public Sprite icon;
    29.  
    30.     [XmlAttribute("tmp")]
    31.     public ItemData originalData;
    32.  
    33.     public void CopyToInventory(scr_PlayerInventory invScript, int slot){
    34.         Debug.Log("Copying to inventory slot " + slot);
    35.  
    36.         invScript.itemInv[slot] = this;
    37.  
    38.         //invScript.itemInv[slot] = ScriptableObject.CreateInstance<ItemData>();//new ItemData();
    39.         //invScript.itemInv[slot].myName = this.myName;
    40.         //invScript.itemInv[slot].name = this.name + "(instance)";
    41.         invScript.interactObject.GetComponent<scr_InteractTrigger>().interactList.RemoveAt(0);
    42.     }
    43.     public void CopyToSceneItem(GameObject sceneObject){
    44.         scr_SceneItem sceneData = sceneObject.GetComponent<scr_SceneItem>();
    45.  
    46.         sceneData.myData = this;
    47.     }
    48. }
    49.  
    50. public enum ConsumeType{
    51.     None,
    52.     HPRestore,
    53.     StamRestore
    54. }
    55. public enum Rarity{
    56.     Junk,
    57.     Common,
    58.     Uncommon,
    59.     Rare,
    60.     Legendary
    61. }
    62.  
    The error it is giving me is as follows:
    Code (csharp):
    1.  
    2. Value is not a convertible object: System.String to ItemData
    3.  
    and points to the return line of the load function.

    I've been googling a bit but still having trouble, do you know what's going on? I still don't really understand serialization.

    Also, my plan was to have the SaveManager have a variable for every variable in the entire game that I want to save (so, so far its itemInv and toolInv), is that a good idea? Or should I have each class have it's own save class?
    ie. Inventory has it's own save class, Monsters have their own save class, scenes have their own, etc, etc.
     
  41. Fellshadow

    Fellshadow

    Joined:
    Dec 2, 2012
    Posts:
    169
    I still have been unable to figure this out (google doesn't return very relevant results).

    Anybody know what is going on?

    EDIT: Does BinaryFormatter support lists/writing to file/etc? If it does, it might be better for me to use that instead, as I don't want my save files readable anyways.
     
    Last edited: Mar 21, 2014
  42. geoxgeo

    geoxgeo

    Joined:
    Jul 28, 2017
    Posts:
    1
    Hi Fellshadow can i get your full inventory project please?
    i'm badly need this i don't have enough time to do like this
    can you upload it for me?
    thanks in advance i'm hopefully you are still active.