I have a Unity3D project, and I have an Inventory Class, Item Class, and CraftingSystem Class.
In my Inventory class, I have the inventory itself defined as a dictionary, where the keys are the item objects, and the values are the item quantities (ints).
I have a GetItem(string key) method in my inventory class, which returns the item if the input string matches what is returned from the item's getName() method (i.e., does the input match the item's name).
For whatever reason, it is successfully able to find the Item (lets say, Apple) in the inventory if I call, say, GetItem("Apple") once I pick up an apple, but whenever I check if it's null, it always is.
Inventory Class:
public class Inventory : MonoBehaviour
{
public Dictionary<Item, int> inventory = new Dictionary<Item, int>();
public TextMeshProUGUI inventoryText;
public static event Action InventoryUpdated;
// Update is called once per frame
void Update()
{
InventoryUpdated += UpdateInventoryDisplay;
UpdateInventoryDisplay();
}
private void UpdateInventoryDisplay()
{
inventoryText.text = "";
foreach (var item in inventory)
{
inventoryText.text += $"{item.Key.getName()}: {item.Value}\n";
}
}
/// <summary>
/// Method to add an item to the player's inventory.
/// Each unique item is stored as a key.
/// If an item doesn't already exist as a key, then add it to the inventory, and add it as a key.
/// Else, just increment the count of the item.
/// </summary>
public void Add(Item item){
if (inventory.ContainsKey(item)){
inventory[item] += 1;
}
else {
inventory.Add(item, 1);
}
// Trigger the InventoryUpdated event
InventoryUpdated?.Invoke();
}
/// <summary>
/// Method to use an item in the inventory dictionary.
/// Need to specify what item to use, and how many.
/// </summary>
public void Use(Item item, int amount){
if (inventory.ContainsKey(item)){
inventory[item] -= amount;
if (inventory[item] <= 0){
inventory.Remove(item);
}
}
InventoryUpdated?.Invoke();
}
// FIXME: Method successfully finds the key, but it doesn't seem to be returning the apple item? Since it's null when I
// Since it's null when I call the method.
public Item GetItem(string key)
{
foreach (var kvp in inventory)
{
Item item = kvp.Key;
if (item.getName().Equals(key)){
Debug.Log(item.getName() + " found in inventory");
return item;
}
}
Debug.Log(key + " not found");
return null;
}
}
Item Class:
public class Item : MonoBehaviour
{
public string itemName;
public GameObject itemPrefab;
public Collider itemCollider;
public Item(string name, GameObject prefab){
itemName = name;
itemPrefab = prefab;
}
// Update is called once per frame
void Update()
{
transform.Rotate(0, 0.25f, 0, Space.Self);
}
public override bool Equals(object obj)
{
if (obj is Item other)
{
return itemName.Equals(other.itemName);
}
return false;
}
public override int GetHashCode()
{
return itemName.GetHashCode();
}
public string getName(){
return itemName;
}
private void OnTriggerEnter(Collider other){
if (other.gameObject.tag == "Player"){
Inventory inventory = other.gameObject.GetComponent<Inventory>();
if (inventory != null){
Item item = new Item(itemName, itemPrefab);
inventory.Add(item);
Destroy(gameObject);
}
}
}
}
CraftingSystem Class:
public class CookingSystem : MonoBehaviour
{
[Header("References")]
public Inventory ingredients;
// Update is called once per frame
void Update()
{
// Apple Key is found, but it's still null.
if (ingredients.GetItem("Apple") == null){
Debug.Log("Apple is null");
}
}
}
As you can see in the code I've provided, I included various checks to see if the item in question (Apple) is successfully being found.
When I go to run the game, It outputs "Apple not found" (from the GetItem() method in the inventory class), and "Apple is null" (from the Crafting System Update method). Both logs output, indicating that apple is not found in the inventory, and is null
Once I pick up an apple, it stops outputting "Apple not found", but it still continuously prints "Apple is null", and for whatever reason, "Apple found in inventory" (from the GetItem() method) simultaneously. Stops logging that apple is not found, but still logs that apple is null
So as commented I don't exactly know the issue. My guess would be though that it is related to you creating instances of MonoBehaviour using new.
This is invalid as the console warns you. A component can not really exist without being attached to a GameObject. You basically create a c# API layer component instance without the according underlying c++ core engine object existing.
Further UnityEngine.Object has a custom == operator which in your case might return true for == null - without the object actually being null!
Why? Because it basically exactly checks whether the component exists and is alive in the underlying c++ engine - which doesn't know it because you only created the c# API layer part using new.
Excerpt of the source code for this custom operator
public static bool operator==(Object x, Object y) { return CompareBaseObjects(x, y); } static bool CompareBaseObjects(UnityEngine.Object lhs, UnityEngine.Object rhs) { bool lhsNull = ((object)lhs) == null; bool rhsNull = ((object)rhs) == null; if (rhsNull && lhsNull) return true; if (rhsNull) return !IsNativeObjectAlive(lhs); if (lhsNull) return !IsNativeObjectAlive(rhs); return lhs.m_InstanceID == rhs.m_InstanceID; } static bool IsNativeObjectAlive(UnityEngine.Object o) { if (o.GetCachedPtr() != IntPtr.Zero) return true; //Ressurection of assets is complicated. //For almost all cases, if you have a c# wrapper for an asset like a material, //if the material gets moved, or deleted, and later placed back, the persistentmanager //will ensure it will come back with the same instanceid. //in this case, we want the old c# wrapper to still "work". //we only support this behaviour in the editor, even though there //are some cases in the player where this could happen too. (when unloading things from assetbundles) //supporting this makes all operator== slow though, so we decided to not support it in the player. // //we have an exception for assets that "are" a c# object, like a MonoBehaviour in a prefab, and a ScriptableObject. //in this case, the asset "is" the c# object, and you cannot actually pretend //the old wrapper points to the new c# object. this is why we make an exception in the operator== //for this case. If we had a c# wrapper to a persistent monobehaviour, and that one gets //destroyed, and placed back with the same instanceID, we still will say that the old //c# object is null. if (o is MonoBehaviour || o is ScriptableObject) return false; return DoesObjectWithInstanceIDExist(o.GetInstanceID()); } [NativeMethod(Name = "UnityEngineObjectBindings::DoesObjectWithInstanceIDExist", IsFreeFunction = true, IsThreadSafe = true)] internal extern static bool DoesObjectWithInstanceIDExist(int instanceID);
This is what I would rather do instead:
Have the items rather as assets
[CreateAssetMenu]
public class Item : ScriptableObject
{
public GameObject itemPrefab;
}
This doesn't even need a name field as assets already have a name anyway! You might want to have a check for unique names if you want but that would include some further editor scripting which I will skip for now.
Then the component which provides such an item
public class CollectableItem : MonoBehaviour
{
public Item item;
private void Start ()
{
var itemObject = Instantiate(itemPrefab);
itemObject.transform.parent = transform;
}
void Update()
{
transform.Rotate(0, 45f * Time.deltaTime, 0);
}
private void OnTriggerEnter(Collider other)
{
if(!other.TryGetComponent<Inventory>(out var inventory)) return;
inventory.Add(item);
Destroy(gameObject);
}
}
Then for the recipes you just need two more helper classes:
[Serializable]
public class ItemAmountPair
{
public Item Item;
[Min(1)] public int Amount;
}
[CreateAssetMenu]
public class RecipeRequirements : ScriptableObject
{
public ItemAmountPair[] Ingredients;
}
Then the Inventory would be a lot simpler
public class Inventory : MonoBehaviour
{
private readonly Dictionary<Item, int> inventory = new();
public event Action InventoryChanged;
public void Add(Item item)
{
if(inventory.ContainsKey(item)
{
inventory[item] += 1;
}
else
{
inventory[item] = 1;
}
InventoryChanged?.Invoke();
}
public bool CanUse(RecipeRequirements requirements)
{
// check whether all ingredients available
foreach(var ingredient in requirements.Ingredients)
{
if(!inventory.TryGetValue(ingredient.Item, out var amount)) return false;
if(amount < ingredient.Amount) return false;
}
return true;
}
public void Use(RecipeRequirements requirements)
{
if(!CanUse(requirements))
{
throw new InvalidOperationException($"Not all items available required by {requirements}!");
}
foreach(var ingredient in requirements.Ingredients)
{
inventory[ingredient.Item] -= ingredient.Amount;
// Optional but sure if you want you can remove empty entries
if(inventory[ingredient.Item] <= 0) inventory.Remove(ingredient.Item);
}
InventoryChanged?.Invoke();
}
}
Finally the inventory display should rather be a separate component listening to InventoryChanged. Subscribing to your own event every frame is definitely wrong.