ID:1264717
 
(See the best response by Stephen001.)
Code:
var/list/Inventory = list()

world/New()
..()
var/item/Potion/P = new /item/Potion
var/item/Sword/S = new /item/Sword
Inventory += S
Inventory += P
Inventory += P

item
var/stackable = 0
Potion
stackable = 1
Sword

proc/OutputInventory()
var/list/items = list()
var/list/stackeditems = list()
for(var/item/O in Inventory)
if(stackeditems.Find(O))
stackeditems[O] += 1
continue
if(I.stackable)
stackeditems += O
stackeditems[O] = 1
else
items += O
for(var/i in items)
world << i
for(var/i in stackeditems)
var/i2 = stackeditems[i]
world << stackeditems[i2]


Problem description:I have an on-screen inventory system, and i programmed it to stack items and output the number of items of that type you have in your inventory. Almost exactly like above(I just removed a lot of the extra stuff I do, and without the on-screen part and such. This is the procedure). The problem is, anything I create after run-time, that is suppose to stack, Like "potion" for example, counts itself as a completely new item. Even if i make multiple copies of it. While on the other hand, the ones generated at New(), stack and work perfectly fine.

I have no idea at all as to why this is happening, Any help would be nice. Thanks.


D-Cire wrote:
Code:
>       if(stackeditems.Find(O))


Think about what this line is actually searching for: an individual instance of an item, not a type! If you want to search a list for a specific type, you'll have to loop through it and manually check each item's type variable against the new item's.*

The reason the items created in world/New() work is because you're only creating one /item/Potion instance, and adding that same instance to the list twice.

* you can't use locate() here, see if you can figure out why ;)
That helps, but now i'm getting an out of bounds index error for something completely unrelated. Nor can I come to a conclusion as to why i'm getting this error. Instead of posting the wall of code, which im sure is extremely inefficent; Would it be easier for me to just message you through the pager?
Well, posting here allows help from other people also, and documents the issue in the forums so other people can search it, so I'd recommend you keep posting here if convenient for you.

Can you show us the area the error is in, and point to the line it's complaining about?
I have no problem with that, I guess i'll just post the procedure in which it is relating to, and give the error. It' just a lot of inefficent, messy, code due to me not programming for a good 9 months, and this being me brushing up.

        ShowInventory
layer = 31
screen_loc = "2,13"
icon_state = "ShowInventory"
Click()
for(var/obj/A in usr.client.StackBackz)
usr.client.screen -= A
usr.client.StackBackz -= A
for(var/obj/A in usr.client.Inventory_HUD)
usr.client.screen += A
var/x = 1
var/y = 1
var/items = 0
var/i = 1
var/i2 = 1
var/list/isequipped = list()
var/list/stackeditems = list()
var/list/itemstackable = list()
Inven_Loop:
for(var/items/O in usr.Inventory)
var/items/O2 = O
for(var/items/I in stackeditems)
if(istype(O2,I))
stackeditems[O2] = stackeditems[O2] + 1
continue Inven_Loop:
O2.screen_loc = "CENTER + [(((4*x) - 8)-2)],CENTER + [(((4*y) - 8)-2)]"
if(istype(O2,/items/Weapons))
O2.screen_loc = "CENTER + [(((4*x) - 8)-2)]:-16,CENTER + [(((4*y) - 8)-2)]"
O2.layer = 32
usr.client.screen += O2
usr.client.Inven_HUD += O2
if(O2.equiped) isequipped += 1
else isequipped += 0
if(O2.stackable)
var/obj/Inventory/MapTextStack/MTS = new /obj/Inventory/MapTextStack
usr.client.StackBackz += MTS
usr.client.screen += MTS
MTS.screen_loc = "CENTER + [(((4*x) - 8)-1)]:16,CENTER + [(((4*y) - 8)-1)]:16"
stackeditems += O2
stackeditems[O2] = 1
itemstackable += 1
else
itemstackable += 0
x += 1
items += 1
if(x > 4)
x = 1
y += 1
for(var/obj/Inventory/StackBack/SB in usr.client.StackBacks)
var/obj/Inventory/StackBack/SB2 = SB
if(i2 <= items)
if(itemstackable[i2] == 1)
var/obj/Inventory/MapTextStack/MTS = usr.client.StackBackz[i2] //Line that generates the issue.
var/I = stackeditems[i2]
if(stackeditems[I] >= 1)
SB2.icon_state = "StackBack"
MTS.maptext = "<font color = #FFFF00>[stackeditems[I]]x"
else
SB2.icon_state = "blank"
SB2.maptext = ""
i2 += 1
for(var/obj/Inventory/EquippedMapText/EMT in usr.client.EquippedText)
var/obj/Inventory/EquippedMapText/EMT2 = EMT
if(i <= items)
if(isequipped[i] == 1)
EMT2.icon_state = "Equipped"
else
EMT2.icon_state = "Use"
else
EMT2.icon_state = "blank"
i += 1
usr.Inven_Open = 1
..()


The Error is on the line : var/obj/Inventory/MapTextStack/MTS = usr.client.StackBackz[i2], It's an index out of bounds error during runtime.

This is most likely something im missing, but this code works fine up until I add a new instance of a stackable item to the usr's inventory.

I apologize for complete lack of knowledge or know how in proper code orginization and such. Some of this is just sloppy habits.
Bump
If its giving an out of bounds error at that line than it means that at the line for stackbacks[i2] when the index is being pulled for stackbacks, byond doesn't know what it is because the array is not that large. So in this case it looks like you took a normal list called stackbacks and are trying to override whatever is in the associtive lst of stackbacks[i2] so because of the if statement i2 will equal one, so stackbacks[1] is not associated with anything because from what I see stackbacks was not normally an associative list in prior use of it. So in this case we could not change stackbacks to an associtive list later on, to my understanding from some previous posts.


Atleast this is my thought process.
I've updated it to fix the issue - i can't tell why i didnt notice that, but I'm now presented with another. For some reason the new instance of the item, still doesn't stack with the former item, like in the original post. It just generates a new hidden item until an item in the inventory is deleted, it will then show the new "Added potion" and if you use that, the game bugs out and the inventory freazes.

                for(var/obj/A in usr.client.StackBackz)
usr.client.screen -= A
usr.client.StackBackz = list()
for(var/obj/A in usr.client.Inventory_HUD)
usr.client.screen += A
var/x = 1
var/y = 1
var/items = 0
var/i = 1
var/i2 = 1
var/list/isequipped = list()
var/list/stackeditems = list()
var/list/itemstackable = list()
Inven_Loop:
for(var/items/O in usr.Inventory)
var/items/O2 = O
for(var/items/I in stackeditems)
if(istype(O2,I))
stackeditems[O2] = stackeditems[O2] + 1
continue Inven_Loop:
O2.screen_loc = "CENTER + [(((4*x) - 8)-2)],CENTER + [(((4*y) - 8)-2)]"
if(istype(O2,/items/Weapons))
O2.screen_loc = "CENTER + [(((4*x) - 8)-2)]:-16,CENTER + [(((4*y) - 8)-2)]"
O2.layer = 32
usr.client.screen += O2
usr.client.Inven_HUD += O2
if(O2.equiped) isequipped += 1
else isequipped += 0
if(O2.stackable)
var/obj/Inventory/MapTextStack/MTS = new /obj/Inventory/MapTextStack
usr.client.StackBackz += MTS
usr.client.screen += MTS
MTS.screen_loc = "CENTER + [(((4*x) - 8)-1)]:16,CENTER + [(((4*y) - 8)-1)]:16"
stackeditems += O2
stackeditems[O2] = 1
itemstackable += 1
else
itemstackable += 0
usr.client.StackBackz += null
stackeditems += null
x += 1
items += 1
if(x > 4)
x = 1
y += 1
for(var/obj/Inventory/StackBack/SB in usr.client.StackBacks)
var/obj/Inventory/StackBack/SB2 = SB
if(i2 <= items)
if(itemstackable[i2] == 1)
if(usr.client.StackBackz[i2] == null) continue
if(stackeditems[i2] == null) continue
var/obj/Inventory/MapTextStack/MTS = usr.client.StackBackz[i2]
var/I = stackeditems[i2]
if(stackeditems[I] >= 1)
SB2.icon_state = "StackBack"
MTS.maptext = "<font color = #FFFF00>[stackeditems[I]]x"
else
SB2.icon_state = "blank"
SB2.maptext = ""
i2 += 1
for(var/obj/Inventory/EquippedMapText/EMT in usr.client.EquippedText)
var/obj/Inventory/EquippedMapText/EMT2 = EMT
if(i <= items)
if(isequipped[i] == 1)
//EMT2.maptext = "<font size = 1>Equipped"
EMT2.icon_state = "Equipped"
else
//EMT2.maptext = "<font size = 0>Use"
EMT2.icon_state = "Use"
else
//EMT2.maptext = ""
EMT2.icon_state = "blank"
i += 1
usr.Inven_Open = 1
..()


I'm starting to get fusterated with this because it worked fine until i decided to try and add stackable items. I might just scrap all the progress and forget about it.
Don't do that, keep working ata it I will help when I get home.
Could you try adding some debugging code in the for loop when a new item is added. Such as:

world << "[stackeditem+1]"


So that way we can make sure it is adding it to the players content and not the items content. When I was doing my stacking system, if it was added to the user content then it still showed the extra item, but it did not stack. I am thinking that these are two seperate things...

MTS.maptext = "<font color = #FFFF00>[stackeditems[I]]x"


I think this line is why you can not see the item because your not doing anything to increment the number next to the potion. Such as you know when you see a stacked item it says for example:

Potion x 2

well with what code you have it would only be:

Potion x

or

[Whatever the list Potion associates I with] x

So the useing and it not displaying properly may be two different things.
I'll try the debugging code, However your second assumption is wrong. During the loop, when i add the item to the stacked items list i set it to a value starting at one. When the type is varified as the same for the next item of the same kind, it adds one to that value. Stackeditems[I] refers to that value.
Well, I added

for(var/I in stackeditems)
world << I


Before the stackback loop, and it outputs that all the items are located in the inventory, but it has multiple copies of the stackable items(Excluding the original instances, there are only one copy of those no matter how many i add at runtime).
Can you explain the purpose of a few of the client variables to me?

usr.client.StackBackz
usr.client.Inventory_HUD
usr.client.Inven_HUD
usr.client.EquippedText
usr.client.StackBackz contains objects generated during the loop to use as an overlay to the "StackBacks". the items generated are just maptext saying how many of a said item there are.
usr.client.Inventory_HUD holds pretty much every object in the inventory.
usr.client.Inven_HUD holds your inventory's objects once they are created in the first for() loop.
usr.clent.EquippedText contains objects generated as overlays which lay over the slots to show if an item is equipped, or can be used.

I managed to fix the maptext not updating to the correct amount of the item being in the inventory. The issue was not setting the existing object in stackeditems to an upper value, and instead creating a new instance.
stackeditems[O2] = stackeditem[O2] + 1


Should have been:

stackeditems[I] = stackeditems[I] + 1


and i've already updated it to -

stackeditems[I] += 1


The new issue is after using all the stackable items(Aside from the pre-generated ones at runtime, the maptext overlay will disappear, but stackbacks overlay and item overlay wouldn't disappear. I realized the item wouldn't disappear because of usr.client.Inven_HUD, so i made a loop to remove those and that part works fine. I did the same to loop and set the stackbacks overlay to "blank". So Item's numbers, and overlays are created and deleted correctly now.

The only issue i see now is when the amoutn of items(Such as the two stackable potions generated at runtime) are used, and you have three, the potions skip over to the spot where the third was added. All info is correct, it just changes slots. Which i'd prefer to keep in the original slot until the full stack is deleted. This is most likely because the new "Potion" is located in the 7th spot of the inventory array and the original would be in the first. Since the items are added one by one, it would change the slot on the screen update.

Would it be more beneficial to scrap this entire system and just make items stack by setting the item's inventory value to a number, then once that number is depleted deleting that item? Basically forgetting the entire "stackeditems" and such? Or would what I posted above have an easier fix?
It does seem quite counter-intuitive at the moment. Can all item types stack, or can only some?
Only some, thats why the first loop checks
if(O2.stackable)
Best response
I suppose you could introduce a datum to handle the stacking, which basically lets you handle all of this business in one object.

ItemStack
var/count = 0
var/items/object = null

New(var/items/I)
ASSERT(I.stackable)
src.object = I
src.count = 1

proc
add(var/items/I)
if (I.type == src.object.type)
src.count++
else
CRASH("[Error] Tried to add [I.type] to a [src.object.type] stack")

get_descriptive_count()
return "<font color = #FFFF00>[count]x"


You then build an associative list of these, where the list is like:

var/list/L = new()
var/items/test/A = new()
var/items/test/B = new()
L[A.type] = new/ItemStack(A)
var/ItemStack/stack = L[B.type] // If there's no stack, this is null.
if (isnull(stack))
L[B.type] = new/ItemStack(B)
else
stack.add(B)


This datum cannot by itself be displayed in your HUD, but it could be given procedures to make it produce stuff your HUD needs to display, like the map-text I've given as an example.
I ended up going with the method I posted above, which was in my option, such an easier idea and allowed me to simplify the code to extreme lengths. Thanks for everyones help.
Glad I could help.
Did you fix the overlay error?
Page: 1 2