jpw1991 / chebs-necromancy

Cheb's Necromancy adds Necromancy to Valheim via craftable wands and structures. Minions will follow you, guard your base, and perform menial tasks.
The Unlicense
10 stars 4 forks source link

Treasure Pylon Shuffling Bug #187

Closed WalterWillis closed 1 year ago

WalterWillis commented 1 year ago

Describe the bug

The treasure pylon shuffles items, rather than aggregating items. It looks like the code only checks if an item exist in another container, rather than what the comment indicates.

To Reproduce (if applicable)

Have a few chests with random items scattered throughout. Ensure the chests do not have the same items.

Steps to reproduce the bug. For example:

  1. Have chests
  2. Have Unique items in chests
  3. Observe all items (depending on amount of unique items) fill the chest from chests surrounding.

Expected behavior (if applicable)

The chest that is being filled should only be filled with items that are already contained within it.

Screenshots (if applicable)

image

Desktop

If you can, please complete the following information:

Possible solution:

To modify the code to match the description in the comment, you can change the nested loops within the LookForPieces coroutine. The updated code snippet below will remove an item from another container and place it in the current container only if both containers have the same item:

Replace the following nested loops:

for (int j = 0; j < nearbyContainers.Count; j++)
{
    if (j == i) continue; // skip over self

    var jInventory = nearbyContainers[j].GetInventory();
    var jItems = jInventory.GetAllItems();

    for (int k = jItems.Count - 1; k > -1; k--)
    {
        var jItem = jItems[k];
        var itemsMoved = 0;
        if (currentContainerInventory.CanAddItem(jItem))
        {
            var currentItemCount = currentContainerInventory.CountItems(jItem.m_shared.m_name);
            currentContainerInventory.AddItem(jItem);
            itemsMoved = currentContainerInventory.CountItems(jItem.m_shared.m_name) -
                         currentItemCount;
        }

        if (itemsMoved > 0)
        {
            //movedLog.Add($"{itemsMoved} {jItem.m_shared.m_name}");
            jInventory.RemoveItem(jItem, itemsMoved);
        }
    }
}

With this updated code:

for (int j = 0; j < nearbyContainers.Count; j++)
{
    if (j == i) continue; // skip over self

    var jInventory = nearbyContainers[j].GetInventory();
    var jItems = jInventory.GetAllItems();

    for (int k = jItems.Count - 1; k > -1; k--)
    {
        var jItem = jItems[k];

        if (currentContainerInventory.ContainsItem(jItem))
        {
            var itemsMoved = 0;
            if (currentContainerInventory.CanAddItem(jItem))
            {
                var currentItemCount = currentContainerInventory.CountItems(jItem.m_shared.m_name);
                currentContainerInventory.AddItem(jItem);
                itemsMoved = currentContainerInventory.CountItems(jItem.m_shared.m_name) -
                             currentItemCount;
            }

            if (itemsMoved > 0)
            {
                //movedLog.Add($"{itemsMoved} {jItem.m_shared.m_name}");
                jInventory.RemoveItem(jItem, itemsMoved);
            }
        }
    }
}

The main change is the addition of this conditional statement:

if (currentContainerInventory.ContainsItem(jItem))

This ensures that the code only moves items between containers if the current container already has the same item. This modification makes the code behave as described in the original comment. However, it also assumes that there is a ContainsItems function. I have only just started playing Valheim recently and I have not made my own mod. I am not familiar with the APIs that are used.

WalterWillis commented 1 year ago

Alternatively, perhaps this conditional statement would work based on the above code:

if (currentContainerInventory.CountItems(jItem.m_shared.m_name) > 0)

instead of this line:

if (currentContainerInventory.ContainsItem(jItem))

jpw1991 commented 1 year ago

Thanks @WalterWillis , awesome contribution. I will test this soon.

Edit: if you want my help setting the project up locally, let me know. Then you can really get stuck in changing whatever you want to.

WalterWillis commented 1 year ago

I hope it helps!

That sounds like it could be fun. I'm down. I had the repo cloning, but it looks like it failed. I'll try again.

jpw1991 commented 1 year ago

Ok, do you have discord? Feel free to add me if you get stuck.