koobas.hobune.stream
Checking for every item in crafting system?
I started coding a crafting system, and it's working, except for one small problem. It doesn't check for every material, just one. I tried different ways of fixing it (foreach loop, for loop, if statement in both of those, etc.), and I'm stumped. Anybody know what I'm doing wrong?
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using UnityEngine.UI;
public class CraftButton : MonoBehaviour {
public CraftingRecipe recipe;
public Sprite icon;
public Button craftButton;
Inventory inventory;
public bool canCraft;
void Start ()
{
inventory = Inventory.instance;
}
void Update ()
{
CheckForMaterials();
if(canCraft == true)
{
craftButton.interactable = true;
} else
{
craftButton.interactable = false;
}
}
public void Craft()
{
if(canCraft == true)
{
inventory.Add(recipe.result);
RemoveMaterials();
}
else
{
Debug.Log("materials not found");
}
}
public void CheckForMaterials()
{
for (int i = 0; i < inventory.items.Count; i++)
{
if (inventory.items[i] == recipe.Materials[i])
{
canCraft = true;
Debug.Log("crafting materials found");
break;
}
}
}
public void RemoveMaterials()
{
for (int i = 0; i < recipe.Materials.Count; i++)
{
inventory.Remove(recipe.Materials[i]);
}
}
}
I think the problem lies in this conditional:
if (inventory.items[i] == recipe.Materials[i])
What you're saying here is that there's a 1:1 relationship between the items in the player inventory and those on the recipe, which is highly unlikely. If the player's inventory has a different order (which will almost always be the case), this conditional fails consistently. You should use Contains, given that inventory is a List. You'll loop over the recipe's items instead of the entire inventory as well, which should be more efficient.
if (inventory.items.Contains(recipe.Materials[i]))
In the end you still have 2 nested loops, one iterating over the required materials and one iterating over the items in the inventory. "Contains" internally has to iterate through the whole list as well. However contains only works with the "exact same instance" while when doing a custom check you could be more precise how to match materials. Of course the condition he had originally just looks for equallity.
We don't know how a material is implemented so it's pointless to go too much into detail here without more information.
Given the indication by the OP that the search is successful for exactly one crafting material, equality resolution apparently does work. I indicated why the OPs code fails for cases beyond the first in my discussion.
