Changing item properties via code

Rairun

I have a "nest" container with 4 slots that can contain eggs. The item property #0 is an age integer. By running this code, I intend to make every egg currently in the nest older by 1:

         if (nest.InvCollection.GetInstanceAtIndex(0) != null)
            {                 
                nest.InvCollection.GetInstanceAtIndex(0).GetProperty(0).IntegerValue++;            
            }

            if (nest.InvCollection.GetInstanceAtIndex(1) != null)
            {
                nest.InvCollection.GetInstanceAtIndex(1).GetProperty(0).IntegerValue++;
            }

            if (nest.InvCollection.GetInstanceAtIndex(2) != null)
            {
                nest.InvCollection.GetInstanceAtIndex(2).GetProperty(0).IntegerValue++;
            }

            if (nest.InvCollection.GetInstanceAtIndex(3) != null)
            {
                nest.InvCollection.GetInstanceAtIndex(3).GetProperty(0).IntegerValue++;
            }

If I run this code when I have eggs in my inventory and other eggs in the nest container, only the eggs in the container will get older. However, if I select one egg from a stack in the inventory and transfer that one egg to the container, and then run this code, the entire stack in the inventory will also get older! I believe this happens even if I split the stack inside the inventory into supposedly separate instances - somehow they all inherit a connection to the egg that was placed in the container, and they get older at the same time the instance in the nest gets older.

Any idea why this is happening?

Rairun

And if it matters, this is the custom code I use to transfer eggs from the inventory to the nest (it's a little convoluted because I don't want eggs to be stackable in the nest, only in the inventory, so I'm disallowing transfers of more than 1).

            if (!InvInstance.IsValid(clickedInstance) && InvInstance.IsValid(selectedInstance))
            {

                InvCollection collection = (inventoryBox.title == "Container") ? inventoryBox.OverrideContainer.InvCollection : KickStarter.runtimeInventory.PlayerInvCollection;
                // Only allow one item to be placed

                if (selectedInstance.TransferCount > 1 && selectedInstance.ItemID == 10)
                {
                    int x = selectedInstance.TransferCount;
                    selectedInstance.TransferCount = 1;
                    collection.Insert(KickStarter.runtimeInventory.SelectedInstance, slot, OccupiedSlotBehaviour.SwapItems);
                    selectedInstance.TransferCount = x - 1;
                    return;
                }

                collection.Insert(KickStarter.runtimeInventory.SelectedInstance, slot, OccupiedSlotBehaviour.SwapItems);
                KickStarter.runtimeInventory.SetNull();
                return;
            }

Rairun

After some more testing, I've noticed that if I have a stack of 99 eggs (age 0) in the inventory, and I place one egg in each of the four slots in the nest (leaving 95 eggs stacked in the inventory), then running the script once will instantly increase the age integer of all those instances to 4!

But if I create a new instance like this, it won't be affected by the code above:

            InvInstance newEgg = new InvInstance(10);
            nest.InvCollection.Insert(newEgg, 0);

So the issue really isn't that the code is affecting all instances. It's that somehow AC is keeping track of the origin of separate instances as if they were still the same.

ChrisIceBox

This is down to how C# retains links to variables, but it's hard to tell where the fault lies. Does the issue occur if you temporarily do away with your custom transfer script and allow AC to move items by itself?

When transferring items, you will need to insert a new instance. There is a constructor to help with this when you want to transfer an existing item. Instead of:

collection.Insert(KickStarter.runtimeInventory.SelectedInstance, slot, OccupiedSlotBehaviour.SwapItems);

call:

collection.Insert (new InvInstance(KickStarter.runtimeInventory.SelectedInstance), slot, OccupiedSlotBehaviour.SwapItems);

It's also important to use InvInstance's IsValid function when null-checking - since an InvInstance can technically be empty but non-null. Replace:

if (nest.InvCollection.GetInstanceAtIndex(0) != null)

with:

if (InvInstance.IsValid (nest.InvCollection.GetInstanceAtIndex(0)))

Rairun

Ah, that makes sense! Thanks. The issue still occurs if you use the default menu behaviour. But thanks for the pointer, I will find a way around this with my custom script.

Rairun

Chris, the issue with the line of code you provided is that it essentially duplicates the selected item and places the duplicate in the target slot, while leaving the original untouched. It shouldn't be hard to add a couple more lines to delete the original to my custom script, but is this something you plan to account for in the next AC release, so that the insert function does the duplicating/deleting automatically?

I ask because if you are planning to change this, I might just leave my inventory script untouched for now and do the deletion/creation of a new instance only when updating an instance's property.

Rairun

Ahh bad news. Even if the new code didn't duplicate the item, it wouldn't work. I tried this to increase the property integer:

        if (InvInstance.IsValid(nest.InvCollection.GetInstanceAtIndex(0)))
        {
            InvInstance newInstance = new InvInstance(nest.InvCollection.GetInstanceAtIndex(0));
            nest.InvCollection.Delete(nest.InvCollection.GetInstanceAtIndex(0));
            newInstance.GetProperty(0).IntegerValue++;
            nest.InsertAt(newInstance, 0);

        }

And the original stack's property still changes.

Rairun

The following did the trick, but would be very convoluted if I had many properties/number of items to keep track of:

        if (InvInstance.IsValid(nest.InvCollection.GetInstanceAtIndex(0)))
        {

            int age = nest.InvCollection.GetInstanceAtIndex(0).GetProperty(0).IntegerValue + 1;
            InvInstance newInstance = new InvInstance(10, 1);
            newInstance.GetProperty(0).IntegerValue = age;
            nest.InvCollection.Delete(nest.InvCollection.GetInstanceAtIndex(0));
            nest.InsertAt(newInstance, 0);
        }

ChrisIceBox

Sorry for not being clear.

You don't need to delete/re-insert a copy of an item instance when updating its properties. You only need to create a new copy when adding transferring an item from one collection to another.

Once an item has been transferred by duplication from your Inventory to the Container, you can use your original code with the IsValid check as before.

Here's a function that you can use to increase any integer property from an item collection:

void IncreaseProperty (InvCollection invCollection, int index, int propertyID, int increaseAmount = 1)
{
    InvInstance invInstance = invCollection.GetInstanceAtIndex (index);

    if (InvInstance.IsValid (invInstance))
    {
        InvVar property = invInstance.GetProperty (propertyID);
        int newValue = property.IntegerValue + increaseAmount;
        property.IntegerValue = increaseAmount;
    }
}

The code in your first snippet could then be reduced to:

for (int i = 0; i <= 3; i++)
{
    IncreaseProperty (nest.InvCollection, i, 0);
}

Rairun

Chris, apologies for being unclear too.

I understood what you were trying to do with your original solution. Correct me if I am wrong: by using new InvInstance(KickStarter.runtimeInventory.SelectedInstance) instead of KickStarter.runtimeInventory.SelectedInstance, you were hoping that the creation of an entire new InvInstance from the original would get around the way C# retains links to variables. I'm saying that I've tested this and this is not true. The links are still retained when you create a new InvInstance based on an existing one.

Let me recap how I got here:

I initially believed your solution would work. It made sense! So I went through my entire custom inventory/container script and replaced the snippets above. All in all, there were about 10 instances of this, accounting for splitting item stacks within the same inventory, moving items from one inventory slot to another, as well as transferring items between several types of container that need to display slightly different behaviours.

Having done that, I ran the game and quickly realised that a simple search/replace job like this wouldn't do: it creates a new instance in a new slot without deleting the original, essentially giving the player the ability to have infinite items. "No problem", I thought, "I just have to add a few extra lines of code to delete them myself".

But there were 10 cases I had to modify, and I got lazy. I figured I would first test if the default menu behaviour had the same issues with the variables remaining linked when stacks were split into a different inventory/container slot. When I ran this test, I found that yes, the issue is there for the default menu behaviour too. At this point, I still (wrongly) believed that creating a new InvInstance based on an old one severed the variable links.

Since the default menu behaviour displayed the same problem, I thought there was a high chance you'd issue a fix in the next AC release, so that the insert function severed the variable connection by doing all the duplicating/deleting under the hood. So I thought, "At this point in my game, aging the eggs in the nest is the only time I modify an item property. As a temporary workaround while I wait for the official fix, I might as well leave my inventory/container script alone and just do the duplicating when I actually need to update the property."

So I did that, but I quickly realised that creating a new InvInstance from an old one didn't actually solve the issue at all. If you move one item from a stack to a different slot, create an entire new InvInstance from it (new InvInstance(oldInstance)), delete the oldInstance, and insert the new InvInstance into the slot where oldInstance was, then modifying a property in the new InvInstance will also modify the property in the item stack oldInstance (which no longer exists) originally came from.

This was the only way I found to get around this:

• Declare an int and set it to the oldInstance property's integer value.
• DELETE oldInstance
• Create a new InvInstance with InvInstance newInstance = new InvInstance(10, 1) (this is the only way, no reference to an existing instance allowed)
• Modify the desired newInstance property so that it equals the int where we recorded the relevant value

This works well enough as a workaround in my current situation, but it would be an even more convoluted process if I were dealing with a stack of more than one item (I'd need to save that to a separate int too), or if I had more properties that could be modified.

ChrisIceBox

Can you share steps or PM me an example project to recreate the issue with? There're a lot of code snippets involved, but I need a more concrete example to work with.

If you're encountering an issue with AC's default InventoryBox, let's please start with that.

ChrisIceBox

Thanks for the files.

The property-linking issue can be fixed by opening up AC's InvInstance script and replacing line 63:

invVars = invInstance.invVars;

with:

invVars = new List<InvVar> ();
if (invInstance.invVars != null)
{
    for (int i = 0; i < invInstance.invVars.Count; i++)
    {
        invVars.Add (new InvVar (invInstance.invVars[i]));
    }
}

My point above about creating a new InvInstance class when transferring items still stands, but now that process should work correctly.

Rairun

Thanks, Chris! I'll do some more testing, but so far something like this works:

    void InsertByDuplication(InvCollection invCollection, int index, InvInstance invInstance)
    {

        invCollection.Insert(invInstance, index, OccupiedSlotBehaviour.SwapItems);        
        InvInstance instanceDupe = new InvInstance(invCollection.GetInstanceAtIndex(index));
        invCollection.Delete(invCollection.GetInstanceAtIndex(index));
        invCollection.Insert(instanceDupe, index);

    }