Question Is this how I'd make items?

p.s: Never used the unity forums so sorry if this is the incorrect section

I'm trying to make items in my game and I wanted to know if the way I've done things are fine or if I should change anything. Currently, I have 3 scripts PickUpSystem, Item, and BaseItem. PickUpSystem is a MonoBehaviour that basically handles picking up the item, dropping it, and switching items you're holding, and here's the code for everything else.

Code (CSharp):

1.     public class Item : MonoBehaviour
2.     {
3.         public BaseItem item; //Scriptable Object
4.         public Rigidbody rigidBody;
5.         Outline outline;
6.  
7.         void Awake()
8.         {
9.             // Adds the outline script which outlines the item when the camera is looking at it
10.             outline = gameObject.AddComponent<Outline>();
11.             rigidBody = gameObject.AddComponent<Rigidbody>();
12.         }
13.  
14.         public void UseItem()
15.         {
16.             if (item != null)
17.             {
18.                 item.Use();
19.             }
20.         }
21.     }

Code (CSharp):

1.   [CreateAssetMenu(fileName = "New Item", menuName = "Item")]
2.     public class BaseItem : ScriptableObject
3.     {
4.         public string itemName;
5.         public string itemDescription;
6.      
7.         public virtual void Use()
8.         {
9.             Debug.Log("Using Item");
10.         }
11.     }

This method is in PickUpSystem and gets called when the left mouse button is pressed

Code (CSharp):

1.         void UseCurrentItem()
2.         {
3.             if(currentItem != null)
4.             {
5.                 currentItem.UseItem();
6.             }
7.         }

Overall I don't think I did anything really wrong but would still like to make sure I'm going in the right direction.

 

So my recommendation. Create a basic abstract Item class with a few Key methods like UseItem.
Like that you can always call this Method even if you dont know what Item it is, as long as it is an Item.
Could be a HealthPotion giving yourself some HP or an InvisiblePotion that makes you Invisible. Like that you can always just call UseMe on your item and it will performe the correct code.

I also would put logic that isnt quite necessary into a static class that handles these Methods for you. Example PickUpItem. Since you will sooner or later have several Items (maybe 1k) that all hold the Method PickUpItem which always performs the same code.

Also think about needing MonoBehaviour, most of the time an Item does nothing special for you except some Stat changes.

Maybe something like that:

Code (CSharp):

1.  
2. public abstract class Item : ScriptableObject
3. {
4.     public string ItemName;
5.     public string ItemDescription;
6.  
7.     public abstract void UseItem(Character character);
8. }
9.  
10. public class HealPotion : Item
11. {
12.     private int Amount;
13.    
14.     public override void UseItem(Character character)
15.     {
16.         character.Health += Amount;
17.     }
18. }
19.  
20. public class Character : MonoBehaviour
21. {
22.     public List<Item> Inventory = new List<Item>();
23.     public int Health = 10;
24.  
25.     public Item HoldingItem;
26.    
27.    
28.     // These Methods should be in an controller Class, for readability. Or with several Characters in another static class
29.     private void PickUpItem()
30.     {
31.         //Raycast logic for detecting if there is something to pick up
32.         //...
33.  
34.         //This is actually the Item you detected and not a (new HealPotion)
35.         var detectedItem = new HealPotion();
36.  
37.         HandleItem.PutItemInInventory(ref Inventory, detectedItem);
38.     }
39.  
40.     private void SelectHoldingItem(Item item) => HoldingItem = item;
41.     private void UseItem() => HandleItem.UseItem(this, HoldingItem);
42. }
43.  
44. public static class HandleItem
45. {
46.     // Could, depending on how many Character you hold, just be in your character
47.     public static void PutItemInInventory(ref List<Item> myInventory, Item item)
48.     {
49.         myInventory.Add(item);
50.     }
51.  
52.     // Same goes for that Method
53.     public static void UseItem(Character character, Item item) => item.UseItem(character);
54. }
55.  

Remeber, everything depends on how you make your Game and what performance do you need. Do you have several Characters that can use different Items, i would recommend a static Class that Handles that. If you got just one Character that uses Items, you wont need it, the Character can perfom that for himself.

Do you need performance or is readability more important for you.
Making your project as Modular as possible is always a great goal, since that makes future development much easier.

 

Questions.

1. Does it work like you want? Check!

2. Do you understand all parts of it? Check!

Ship it.

That's the best solution.

Anything more is wasted effort. Move onto the next thing.

You will know when you need something more complex when one of the questions above answers "NO"

 

Haha I guess you're right if everything works I should just move on and come back to it whenever I realize I screwed up. I guess I just go too hard on trying to make sure everything is good