Issues with list copying

ID:2284017
 
Aug 17 2017, 12:27 am
(See the best response by Ter13.)
Code:
    
obj/buyableClothing
    var/clothingBuying
    var/list/BuyableClothes = newlist(/obj/Armor/KasablancRobeMale,
                                      /obj/Armor/KasablancRobeFemale,
                                      /obj/Armor/KasablancPants,
                                      /obj/Armor/KasablancCloak,
                                      /obj/Armor/KreptkinPantsMale,
                                      /obj/Armor/KreptkinShirtFemale,
                                      /obj/Armor/KreptkinSkirt,
                                      /obj/Armor/KreptkinSuitMale,
                                      /obj/Armor/NutoriRobeFemale,
                                      /obj/Armor/NutoriRobeMale,
                                      /obj/Armor/PuraelicChest,
                                      /obj/Armor/PuraelicGauntlets,
                                      /obj/Armor/PuraelicPantsMale,
                                      /obj/Armor/PuraelicPantsFemale,
                                      /obj/Armor/ShaedralGoggles,
                                      /obj/Armor/ShaedralMask,
                                      /obj/Armor/ShaedralPants,
                                      /obj/Armor/ShaedralShirtFemale,
                                      /obj/Armor/ShaedralShirtMale)
    DblClick()
        switch(input("Are you sure you want to buy [src.name]?") in list ("Yes","Cancel"))
            if("Yes")
                if( (usr.capsAmount - src.price) >= 0)
                    var/n = ClothingList.Find(src)
                    usr.contents += BuyableClothes.Copy("[n]","[n]")
                    usr.Inventory()
                    usr.takeCaps(src.price)
                else
                    alert ("You can't afford that!")
            else if("Cancel")
                return



var/list/ClothingList = newlist(/obj/buyableClothing/KasablancRobeMale,
                                /obj/buyableClothing/KasablancRobeFemale,
                                /obj/buyableClothing/KasablancPants,
                                /obj/buyableClothing/KasablancCloak,
                                /obj/buyableClothing/KreptkinPantsMale,
                                /obj/buyableClothing/KreptkinShirtFemale,
                                /obj/buyableClothing/KreptkinSkirt,
                                /obj/buyableClothing/KreptkinSuitMale,
                                /obj/buyableClothing/NutoriRobeFemale,
                                /obj/buyableClothing/NutoriRobeMale,
                                /obj/buyableClothing/PuraelicChest,
                                /obj/buyableClothing/PuraelicGauntlets,
                                /obj/buyableClothing/PuraelicPantsMale,
                                /obj/buyableClothing/PuraelicPantsFemale,
                                /obj/buyableClothing/ShaedralGoggles,
                                /obj/buyableClothing/ShaedralMask,
                                /obj/buyableClothing/ShaedralPants,
                                /obj/buyableClothing/ShaedralShirtFemale,
                                /obj/buyableClothing/ShaedralShirtMale)




Problem description:
Sooo for some reason this results in the ENTIRE list of items being copied over, rather than just a single item. I've tested and 'n' is outputting as the right index, everything else is working fine (it takes away the right amount for ONE item), but it just seems to be that Copy that's misbehaving...
I have the items in two separate lists because one responds to the option of double clicking to buy, whereas the other has a separate verb list.
Aug 17 2017, 1:32 am (Edited on Aug 17 2017, 2:50 am)
There's a couple of problems with this.

Let's start with your specific question.

but it just seems to be that Copy that's misbehaving...

                    var/n = ClothingList.Find(src)
                    usr.contents += BuyableClothes.Copy("[n]","[n]")


list.Find(x) returns a number.

list.Copy() takes two numeric arguments.

"[x]" is a string.

You are feeding Copy() two strings instead of numbers, causing Start and End to be invalid. Copy(), instead of failing out is just acting as though you called it without any arguments, and copying the whole list.

If you did in fact want to copy a portion of the list (which you don't, by the way, I'll get to that in a second), you'd want to be doing this instead:

                   var/n = ClothingList.Find(src)
                    usr.contents += BuyableClothes.Copy(n,n+1)


Start is the position of the first item you want to copy.
End is the position immediately after the last item you want to copy.


I mentioned a couple problems, right?

So, your list is full of objects. You are attempting to make a copy of that item and give it to the user.

Problem #1) list.Copy() cannot achieve this. You are cloning a new list with references to the objects already in the list. It will appear to work just fine until someone selects the same item from the store and buys it. The last person to have bought it will suddenly lose the item, and the new player will get it. That's because you aren't actually cloning the item. You are only copying the reference to the existing object.

Problem #2) You don't want to be using a list of objects.

Problem #3) Your buyable clothes list is defined under the wrong object, and defined in such a way that you are creating one copy of every object in list/BuyableClothes for every object in list/ClothingList. This means that you are creating a whopping 361 /obj/Armor objects just by creating the 19 objects in the global ClothingList, for a grand total of 380 object instances, and 19 lists. This system should require no more than 38 object instances and one list.

I'll go over how to fix this in the next post. This is getting lengthy.
Aug 17 2017, 2:35 am (Edited on Aug 17 2017, 3:03 am)
Best response
 
mob/shopkeeper
    //set up the inventory list. This should only contain types or modified types, and price information. This is more of a table than a linear list.
    //this list will be destroyed once we create all of the shopitems for the store, making this only useful for setting the initial state of the store
    //you probably don't want to save shopkeepers, because this object isn't set up to handle saving.

    var                     /*  item type     price  */
        list/inventory = list(/obj/some/thing, 200,
                              /obj/some/odd,   500,
                              /obj/herp/derp,  750,
                              /obj/boop/doop,  1000)

    Stat() //Not sure if you are using stat panels. If not, we are pulling directly from the shopkeeper's contents.
        if(usr!=src)
            if(get_dist(usr,src)>1)
                usr.client.statobj = usr
                usr.Stat()
                return
            if(statpanel("Buy"))
                statpanel("Buy",contents)
        else
            ..() //can't rule out the user playing as a shopkeeper?

    proc
        Buy(mob/buyer,obj/shopItem/item) //called when you double-click on an item in the shopping statpanel
            if(buyer.capsAmount>=item.price) //make sure the user has enough caps
                if(alert(buyer,"Are you sure you want to buy [item.name]?","Buy [item.name]","Yes","No")=="Yes")
                    new item.item_type(buyer) //create the item in the buyer's contents
                    buyer.capsAmount -= item.price //deduct the caps from the buyer
            else
                alert(buyer,"You can't afford that.","Buy [item.name]","OK")

        BuildInventory() //loop through the shop inventory table to create shopItems from the information.
            var/ilen = inventory.len
            for(var/count in 1 to ilen step 2)
                new/obj/shopItem(src,inventory[count],inventory[count+1])
            inventory = null //null out inventory so that this doesn't get called again

    verb
        shop() //simple right-click verb to begin shopping
            set src in oview(1)
            if(inventory) //don't build the inventory until the first time the shopkeeper has been used.
                BuildInventory()
            usr.client.statobj = src //make this object responsible for building the user's statpanel list.


//shopItems basically are just visual representations of their regular old item counterparts, meant to be unique to each store.

obj/shopItem
    var
         item_type
         price
    //when we create one of these we need to pass in the shop that it belongs to, a type path, and the price at that shop.
    //the New() function will duplicate the visual appearance of the item that it is referencing, store that reference, and the price you passed.

    New(loc,obj/item_ref,cost)
         //store the path of the item currently being sold.
         item_type = item_ref
         //look to the type to work out its name, icon, and icon_state from the prototype
         name = initial(item_ref.name)
         icon = initial(item_ref.icon)
         icon_state = initial(item_ref.icon_state)
         //set up the cost information
         price = cost
         suffix = cost

    //when double-clicked, tell the shop that the user is attempting to purchase something.
    DblClick()
        var/mob/shopkeeper/shop = loc
        //this object can reference the store based on its location
        if(istype(shop))
            shop.Buy(usr,src)



We've constructed this system to be a bit more manageable.

Now it is only creating a single object per item in the shop's inventory, and isn't constantly hunting through the shop list looking for the right items. It hunts the list one time, the first time the shop is used, constructs the shop statpanel, and from there everything is simpler.

As a few added bonuses:

* Shops can set their own prices for things to add regional discounts that aid in the creation of local economies or faction discounts for areas only one faction can get to.

* Shops can use modified types, allowing the creation of unique items per-shop.

* 380 to 361 fewer objects, 18 to 19 fewer lists. Our total object count is 0 before the shop is opened, and one per item in the shop after the shop is opened. Our total list count is 1 before the shop is opened, and 0 after.

* No need for ugly searches. Everything is being done by direct references to objects and types, making the whole operation faster.
Aug 17 2017, 9:58 am
This looks pretty awesome, and thanks for the detailed help. Though, I tried using (n, n+1) and that didn't work either, but I get the theory behind it.

I'm not using Stat panels though. I'm outputting the list to a grid instead like so.

obj
    NPCs
        ClothingShopS
            icon = 'Npcs Basic.dmi'
            icon_state = "clothingillrique"
            density = 1
            DblClick()
                
                if(src in oview(3))
                    winset(usr, "game.shopping","is-visible=true")
                    winset(usr, "shopping", "left=shop")
                    
                    var/i=1
                    winset(usr,"shop.grid",{"cells="3x[contents.len+1]""})
                    usr<<output("<b>Item</b>","shop.grid:1,1")
                    usr<<output("<b>Stock</b>","shop.grid:2,1")
                    usr<<output("<b>Price</b>","shop.grid:3,1")
                    for(var/obj/buyableClothing/I in ClothingList)
                        usr<<output(I,"shop.grid:1,[++i]")
                        usr<<output(I.stock,"shop.grid:2,[i]")
                        usr<<output(I.price,"shop.grid:3,[i]")



I tried adapting the code you gave me like so (I'm not sure if this is right)

mob
    ClothingShopS
        icon = 'Npcs Basic.dmi'
        icon_state = "clothingillrique"
        var/list/ClothingList = list(/obj/buyableClothing/KasablancRobeMale,50,
                                /obj/buyableClothing/KasablancRobeFemale,50,
                                /obj/buyableClothing/KasablancPants,50,
                                /obj/buyableClothing/KasablancCloak,50,
                                /obj/buyableClothing/KreptkinPantsMale, 50,
                                /obj/buyableClothing/KreptkinShirtFemale, 50,
                                /obj/buyableClothing/KreptkinSkirt, 50,
                                /obj/buyableClothing/KreptkinSuitMale, 50,
                                /obj/buyableClothing/NutoriRobeFemale, 50,
                                /obj/buyableClothing/NutoriRobeMale, 50,
                                /obj/buyableClothing/PuraelicChest, 50,
                                /obj/buyableClothing/PuraelicGauntlets, 50,
                                /obj/buyableClothing/PuraelicPantsMale, 50,
                                /obj/buyableClothing/PuraelicPantsFemale, 50,
                                /obj/buyableClothing/ShaedralGoggles, 50,
                                /obj/buyableClothing/ShaedralMask, 50,
                                /obj/buyableClothing/ShaedralPants, 50,
                                /obj/buyableClothing/ShaedralShirtFemale, 50,
                                /obj/buyableClothing/ShaedralShirtMale, 50)
        density = 1
        DblClick()
            if(src in oview(3))
                winset(usr, "game.shopping","is-visible=true")
                winset(usr, "shopping", "left=shop")
                    
                var/i=1
                winset(usr,"shop.grid",{"cells="3x[contents.len+1]""})
                usr<<output("<b>Item</b>","shop.grid:1,1")
                usr<<output("<b>Stock</b>","shop.grid:2,1")
                usr<<output("<b>Price</b>","shop.grid:3,1")
                for(var/obj/buyableClothing/I in src.contents)
                    usr<<output(I,"shop.grid:1,[++i]")
                    usr<<output(I.stock,"shop.grid:2,[i]")
                    usr<<output(I.price,"shop.grid:3,[i]")
            
        proc
            Buy(mob/buyer, obj/shopItem/item)
                if(buyer.capsAmount>=item.price)
                    if(alert(buyer,"Are you sure you want to buy [item.name]?","Buy [item.name]","Yes","No")=="Yes")
                        new item.item_type(buyer)
                        buyer.capsAmount -= item.price
                else
                    alert(buyer,"You can't afford that.","Buy [item.name]","OK")
                    
            BuildInventory()
                var/ilen = ClothingList.len
                for(var/count in 1 to ilen step 2)
                    new/obj/shopItem(src,ClothingList[count],ClothingList[count+1])
                ClothingList = null
            
        verb
            shop()
                set src in oview(1)  
                if(ClothingList)
                    BuildInventory()

obj/shopItem
    var
        item_type
        price
    
    New(loc,obj/item_ref,cost)
        item_type = item_ref
        name = initial(item_ref.name)
        icon = initial(item_ref.icon)
        icon_state = inital(item_ref.icon_state)
        price = cost
        suffix = cost
        
    DblClick()
        var/mob/ClothingShopS/shop = loc
        if(istype(shop))
            shop.Buy(usr,src)


but it's complaining 'inital' is an undefined proc. Also, the reason I had a separate list for BuyableClothes is because they're of type obj/Armor, which has with it it's own verbs for wearing and dropping items. That's why I was trying to use Copy and Find (which according to the documentation Find was supposed to return the -index-, not the object, hence why I was trying to use Find to reference the index in the first list to relate to the index in the second list)
Aug 17 2017, 11:49 am
but it's complaining 'inital' is an undefined proc.

'initial' is what I meant to type on that line. Small typo on my part.

As for your modifications, it appears that BuyableClothing does a bit more than I thought it did.

The reason obj/shopItem exists is to eliminate the need for an extra object prototype per type of item that your current implementation has.

shopItem is essentially your BuyableClothing prototype, but done in such a way as to not require you to associate two different lists and do all the Find()/Copy() business.


I tried adapting the code you gave me like so (I'm not sure if this is right)

It's not. You really hacked together a lot of stuff that isn't meant to work together at all. The point was to get rid of the BuyableClothing type and all of its subtypes completely, and use a single generic object (/obj/shopItem) to serve as a visual placeholder meant to be displayed on the screen, statpanels, or in grids.

When /obj/shopItem is created, it copies the visual appearance of the item that it references in the store's inventory.

When /obj/shopItem is double-clicked, it attempts to make a purchase via the store.

The reason the code I wrote is set up the way it is, is to show how information should flow through this system. The shopkeeper should handle everything having to do with item stock, prices, and the actual purchase itself.

The way yours was set up required these hacky double-lists that was ultimately not doing what you intended them to do. (Not just because of Find()/Copy(), but because the data and structure of the system itself was wrong.)

I really recommend you read through the code I posted a few more times and thing about the way that it is structured. If there's stuff you don't understand, I'd be happy to explain.
Aug 17 2017, 4:14 pm
SO FINALLY figured it out!!
First of all, shop() was never being called, so I added it to the DblClick. Secondly, I completely missed the part where I.price existed and could be called, mistakingly thinking that every second thing in shopItem was its price, so I was trying to call price from I++.

It works, even with the NPC as an obj. Thankyou for your patience and help.