- Home /
Deleting item from inventory system...
As the title says, I'm trying to create a method to delete items from my inventory system but it's not functioning as anticipated. Right now it's meant to delete a stack of a consumable item when I right click, and that does happen, except that it doesn't go lower than 1 and the item is not destroyed. The same with the other items that aren't consumable, they still have a stack size of 1 even though they aren't stackable but they aren't being destroyed on right click. Can anyone help me out please? Here's my script with the issue, specifically the RemoveItem method:
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using UnityEngine.UI;
public class Inventory : MonoBehaviour {
GameObject inventoryPanel;
GameObject slotPanel;
ItemDatabase database;
public GameObject inventorySlot;
public GameObject inventoryItem;
int slotAmount;
public List<Item> items = new List<Item>();
public List<GameObject> slots = new List<GameObject>();
void Start() {
database = GetComponent<ItemDatabase>();
slotAmount = 15;
inventoryPanel = GameObject.Find("InventoryPanel");
slotPanel = inventoryPanel.transform.FindChild("SlotPanel").gameObject;
for(int i = 0; i < slotAmount; i++) {
items.Add(new Item());
slots.Add(Instantiate(inventorySlot));
slots[i].GetComponent<Slot>().id = i;
slots[i].transform.SetParent(slotPanel.transform);
}
AddItem(0);
AddItem(0);
AddItem(1);
AddItem(0);
AddItem(2);
AddItem(2);
AddItem(2);
AddItem(3);
AddItem(3);
}
public void AddItem(int id) {
Item itemToAdd = database.FetchItemByID(id);
if(itemToAdd.Stackable && CheckIfItemIsInInventory(itemToAdd)) {
for(int i = 0; i < items.Count; i++) {
if(items[i].ID == id) {
ItemData data = slots[i].transform.GetChild(0).GetComponent<ItemData>();
data.amount++;
data.transform.GetChild(0).GetComponent<Text>().text = data.amount.ToString();
break;
}
}
} else {
for(int i = 0; i < items.Count; i++) {
if(items[i].ID == -1) {
items[i] = itemToAdd;
GameObject itemObj = Instantiate(inventoryItem);
itemObj.GetComponent<ItemData>().item = itemToAdd;
itemObj.GetComponent<ItemData>().slot = i;
itemObj.transform.SetParent(slots[i].transform);
itemObj.transform.position = Vector2.zero;
itemObj.GetComponent<Image>().sprite = itemToAdd.Sprite;
itemObj.name = itemToAdd.Title;
//next two lines before break handle stack index correction; remove if errors appear in the future
ItemData data = slots[i].transform.GetChild(0).GetComponent<ItemData>();
data.amount = 1;
break;
}
}
}
}
public void RemoveItem(int id) {
Item itemToRemove = database.FetchItemByID(id);
if(itemToRemove.Stackable && CheckIfItemIsInInventory(itemToRemove)) {
for(int j = 0; j < items.Count; j++) {
if(items[j].ID == id) {
ItemData data = slots[j].transform.GetChild(0).GetComponent<ItemData>();
data.amount--;
if(data.amount > 1) {
data.GetComponentInChildren<Text>().text = data.amount.ToString();
} else if(data.amount == 1) {
data.GetComponentInChildren<Text>().text = "";
} else if(data.amount < 1) {
itemToRemove.ID = -1;
}
}
}
}
}
bool CheckIfItemIsInInventory(Item item) {
for(int i = 0; i < items.Count; i++)
if(items[i].ID == item.ID)
return true;
return false;
}
}
Thanks in advance :)
Answer by Jamoy1993 · May 29, 2017 at 01:15 AM
Your remove function looks like it will not work as it is using the database objects as a reference. You want an actual reference to the object you picked up and put into the list itself.
What i would do is have the stack keep a reference to the actual item itself in the list. This is the way my inventory works. So when your script that has the IPointerDown Handler on it receives an OnClick call you can say thisSlot.item.Destroy(); or you could do thisSlot.item.RemoveAmmount(20) for instance. If it is to destroy the item then since you are using lists you can actually use the List.Remove(Item item). It will automatically find the item in the list and remove it.
so basically it will go something like:
1 Item Clicked 2 Get Slot Item Reference 3 Remove this item from list using List.Remove(Item item) 4 Delete Item from world 5 Destroy the Icon referencing this item 6 Update icons and Tidy Up.
You could also have the slot store the array position on it when you add an item which would be ItemsList.Count at that time (since count returns the actual number which you would have to -1 to get the position of the array.
Once you have deleted the item you will then have to remove the icon if the item referenced to it is null.
Take a look at this for info on the remove command: https://msdn.microsoft.com/en-us/library/cd666k3e(v=vs.110).aspx
Hey there, thanks so much for your reply! I've followed some of your guidelines and others I've found online and here's my new and updated RemoveItem method - only problem is, if I have, say, two iron swords in my inventory and I right click on one of them, regardless of the one I've right-clicked on, the first one in the inventory is always deleted first. Is there any way I can reference the actual item within the slot I'm right-clicking on? I might've coded in that functionality but it's been almost a year since I've touched this project, if you could help me with that, that would just be awesome.
public void RemoveItem(int id) {
Item itemToRemove = database.FetchItemByID(id);
if(itemToRemove.Stackable && CheckIfItemIsInInventory(itemToRemove)) {
for(int j = 0; j < items.Count; j++) {
if(items[j].ID == id) {
ItemData data = slots[j].transform.GetChild(0).GetComponent<ItemData>();
data.amount--;
data.transform.GetChild(0).GetComponent<Text>().text = data.amount.ToString();
if(data.amount == 0) {
Destroy(slots[j].transform.GetChild(0).gameObject);
items[j] = new Item();
break;
}
if(data.amount == 1) {
slots[j].transform.GetChild(0).transform.GetChild(0).GetComponent<Text>().text = "";
break;
}
break;
}
}
} else
for(int i = 0; i < items.Count; i++)
if(items[i].ID != -1 && items[i].ID == id) {
Destroy(slots[i].transform.GetChild(0).gameObject);
items[i] = new Item();
break;
}
}
Oh and another bug I've found is when I swap the locations of an item with another and I right click that item, it removes 1 from the stack of the item that was originally in that slot - maybe you can see something that I can't, if you could point that out for me, I'd really appreciate it :)
EDIT: I'm calling the remove item method through a right click from another script, here's the part relevant:
public void OnPointerDown(PointerEventData eventData) {
if(eventData.button == PointerEventData.InputButton.Right) {
inv.RemoveItem(item.ID);
}
}
I have had to rewrite this due to an error with the website. Anyways i have had a look at your remove item script and i came up with this:
//For this function i assume you want to delete the item that is in slot id...
public void RemoveItem(int id)
{
//We dont need to know if the item is stackable or not, we can remove any items from our inventory i assume?
for (int j = 0; j < items.Count; j++)
{
if(items[j].data == id)
{
ItemData data = items[j].data;
if (data.amount > 1)
{
data.GetComponentInChildren<Text>().text = data.amount.ToString();
data.ammount--;
}
else if (data.amount == 1)
{
data.GetComponentInChildren<Text>().text = "";
data.ammount--;
}
else if (data.amount < 1)
{
itemToRemove.ID = -1;
//Add function to delete the object from the world?
//items.Remove(items[j]);
//slots.Remove(items[j].slotScript);?????
}
}
}
}
As i hinted at before, i believed that you were using the items list as a reference of what items to delete. This is what is causing the problems i think, for instance you were telling the inventory to delete the first object of type x, x being what the item was in the database. However i believe this will fix it. There are comments throughout of changes i made.
Also i have had a look at your pointer down command. I assume its because even though you have swamped the items, you have not properly swapped the references. I wrote a simple piece of code that would jog your memory and get you started!
public void SwapItem(SlotScript a, SlotScriptB b)
{
SlotScript temp;
a = temp;
b = a;
b = temp;
//Update slots or whatever else you need to do here to get things showing right.
}
Thanks again dude :)
I put the idea into use and came up with these modular methods for handling the deletion of items, part of it was following a text tutorial I found online - the problem is, in order to delete an item, the tutorial said to call this:
inv.RemoveUniqueItem(item.transform.GetInstanceID(), item.ID);
where "inv" is a reference to the Inventory.cs script, and "item" is a reference to the Item class.
$$anonymous$$y problem is, nobody else seemed to have a problem with it except me, but you shouldn't be able to retrieve the transform of a class, right? That's where I'm getting an error :/ could you possibly help me with that?
Thanks in advance!!
Here are the updated remove methods (the one I'm using in this instance is RemoveUniqueItem):
private int RemoveAtPos(int pos, Item itemToRemove) {
if(pos != -1) {
if(items[pos].Stackable) {
ItemData data = slots[pos].transform.GetComponentInChildren<ItemData>();
data.amount--;
if(data.amount == 0) {
items[pos] = new Item();
Transform t = slots[pos].transform.GetChild(0);
Destroy(t.gameObject);
return 0;
} else {
if(data.amount == 1)
data.transform.GetComponentInChildren<Text>().text = "";
else
data.transform.GetComponentInChildren<Text>().text = data.amount.ToString();
return data.amount;
}
} else {
items[pos] = new Item();
Transform t = slots[pos].transform.GetChild(0);
Destroy(t.gameObject);
return 0;
}
}
return -1;
}
public int RemoveItem(int id) {
Item itemToRemove = database.FetchItemByID(id);
int pos = ItemCheck(itemToRemove);
return (RemoveAtPos(pos, itemToRemove));
}
public int RemoveUniqueItem(int uniqueId, int itemId) {
Item itemToRemove = database.FetchItemByID(itemId);
int pos = UniqueItemCheck(uniqueId);
return (RemoveAtPos(pos, itemToRemove));
}
int UniqueItemCheck(int id) {
GameObject invSlots = GameObject.Find("SlotPanel");
foreach(Transform child in invSlots.transform) {
try {
if(child.transform.GetChild(0).GetInstanceID() == id)
return child.GetComponent<Slot>().id;
} catch {
}
}
return -1;
}
Just a sidenote, I'm so so grateful for your help so far :) I hope this doesn't sound rude, but if you don't have the time to help me please let me know, so that I can post a new answer ins$$anonymous$$d of waiting for this one. Again, I'm so sorry if I sound kinda rude, I really don't mean it that way, you've been an amazing help so far :D
GameObject invSlots = GameObject.Find("SlotPanel");
foreach(Transform child in invSlots.transform) { }
Is invalid, because you say invSlots is a GameObject, not a list or array of gameobjects, so you cannot loop over it, as the transform of an object is not a list or array or enumeration either.
You will have to give each slot object a tag called Slot and have to use something like this:
//Get all slot gameobjects
IEnumerable<Transform> invSlots = GameObject.FindGameObjectsWithTag("Slot");
//use LINQ select to select the transform of all of them
foreach (Transform slot in invSlots.Select(f => f.transform)) { }
You could actually do something like this too:
SlotScript[] slots = FindObjectsOfType<SlotScript>().
foreach(SlotScript slot in slots)
{
//Then do
slot.transform;
//you could even push them into an array if needed in the Start() method!
}
@ShadyProductions This is an interesting way of doing it. Never used IEnumerable for this before!
Your answer
Follow this Question
Related Questions
GetItemByID error 0 Answers
Decorator pattern for inventory item class 0 Answers
How to add item to my inventory when I pick them up? 1 Answer