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

opening pylon container buggy until another container is opened #100

Closed jpw1991 closed 1 year ago

jpw1991 commented 1 year ago

Description

Opening pylon container buggy until another container is opened (Both Refueler and Neckro pylons affected).

Suspect

RPC_RequestOpen isn't being called

Steps to Reproduce

Workaround

image

More info

image


Update 20/05/2023

The error no longer appears, but the effect is still there of the inventory displaying nothing:

image

Opening any other container, then returning to open the pylon again, shows the correct contents:

image

Prefab has same setup as other vanilla containers:

Neckro Pylon Wooden Chest
image image
WalterWillis commented 1 year ago

Perhaps the variable m_inventory in Valheim's InventoryGrid for the pylons is not initialized?

WalterWillis commented 1 year ago

I think the error is hitting this code in InventoryGrid: Element element2 = (flag ? GetElement(m_selected.x, m_selected.y, width) : GetHoveredElement());

The function get Item looks like this: public Element GetElement(int x, int y, int width) { int index = y * width + x; return m_elements[index]; }

This fits the first message there saying the parameter name is index. I assume this means for the internal function that's getting the index of m_elements.
jpw1991 commented 1 year ago

Thanks. Next time I come back to this, I'll investigate with your comments in mind. Feel free to test and do anything you like in the meantime.

jpw1991 commented 1 year ago

So here's a weird thing. Based off your hint, I decided to put a patch inside that method to see what it's doing:

namespace ChebsNecromancy.Patches
{
    [HarmonyPatch(typeof(InventoryGrid), "GetElement")]
    class InventoryGridPatch
    {
        [HarmonyPrefix]
        //  Element GetElement(int x, int y, int width)
        static void Prefix(int x, int y, int width)
        {
            Jotunn.Logger.LogInfo($"GetElement: {x}, {y}, {width}");
        }
    }
}

And this is the resulting log for a bugged container:

Log ```ini [Info : Unity Log] 05/20/2023 09:27:11: Player 3593217780 wants to open ChebGonaz_NeckroGathererPylon(Clone) im: 3593217780 [Info : Unity Log] 05/20/2023 09:27:11: Setting selected recipe 0 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 4, 3, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 2, 3, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 0, 0, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 5, 0, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 1, 0, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 6, 3, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 3, 0, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 4, 0, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 7, 3, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 6, 0, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 0, 2, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 3, 2, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 1, 3, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 2, 0, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 6, 2, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 7, 2, 8 [Info : Unity Log] 05/20/2023 09:27:11: UI Group status changed root = True [Info : Unity Log] 05/20/2023 09:27:11: UI Group status changed Player = True [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 4, 3, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 2, 3, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 0, 0, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 5, 0, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 1, 0, 8 ... repeat for several kilometers ... [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 1, 3, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 2, 0, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 6, 2, 8 [Info :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 7, 2, 8 [Info : Unity Log] 05/20/2023 09:27:17: UI Group status changed MenuRoot = True ```

LogOutput.log

WalterWillis commented 1 year ago

Maybe also print out the index and array size, if it's not null.

jpw1991 commented 1 year ago

I'm not getting the error in the screenshot anymore either. It just shows an inventory panel with no slots now. Maybe subsequent valheim patches since then changed it.

None of the logged indices seem out of bounds.

namespace ChebsNecromancy.Patches
{
    [HarmonyPatch(typeof(InventoryGrid), "GetElement")]
    class InventoryGridPatch
    {
        [HarmonyPrefix]
        //  Element GetElement(int x, int y, int width)
        static void Prefix(int x, int y, int width, InventoryGrid __instance)
        {
            Jotunn.Logger.LogInfo($"GetElement: {x}, {y}, {width}, " +
                                  $"index={y * width + x}, " +
                                  $"m_elements.Count={__instance.m_elements.Count}");
        }
    }
}
[Info   : Unity Log] 05/20/2023 09:48:35: Player 1697063562 wants to open ChebGonaz_NeckroGathererPylon(Clone)   im: 1697063562

[Info   : Unity Log] 05/20/2023 09:48:35: Setting selected recipe 0

[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 4, 3, 8, index=28, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 2, 3, 8, index=26, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 0, 0, 8, index=0, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 5, 0, 8, index=5, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 1, 0, 8, index=1, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 6, 3, 8, index=30, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 3, 0, 8, index=3, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 4, 0, 8, index=4, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 7, 3, 8, index=31, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 6, 0, 8, index=6, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 0, 2, 8, index=16, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 3, 2, 8, index=19, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 1, 3, 8, index=25, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 2, 0, 8, index=2, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 6, 2, 8, index=22, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 7, 2, 8, index=23, m_elements.Count=32
[Info   : Unity Log] 05/20/2023 09:48:35: UI Group status changed root = True

[Info   : Unity Log] 05/20/2023 09:48:35: UI Group status changed Player = True

[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 4, 3, 8, index=28, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 2, 3, 8, index=26, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGridPatch] GetElement: 0, 0, 8, index=0, m_elements.Count=32

...

LogOutput.log

WalterWillis commented 1 year ago

That's odd. Is it possible the patch has fixed the error, but not the bug?

For the gui itself, maybe another open source project has the answers with their implementation. Might be something extra that's needed.

jpw1991 commented 1 year ago

That's odd. Is it possible the patch has fixed the error, but not the bug?

No - even without patching, the error is gone. For me, anyway.

I tested this forever, like hours, but eventually realized that it's as simple as this: The unity values, for whatever reason, aren't there in the moment that the container is being opened. So I just have to set the inventory's height & width in the Pylon's awake and it's fixed. Really weird & annoying but whatever. I'm happy to have found a solution.

Thanks again for your help & ideas Walter

jpw1991 commented 1 year ago

Scratch that - it's not fixed anymore! But it was. What on earth...

jpw1991 commented 1 year ago

Ok so I randomly started getting the issue again - I changed nothing - so I added the patch back to the method and here's the log:

[Info   :ChebsNecromancy.Patches.InventoryGuiPatches+InventoryGridPatch] GetElement: 6, 2, 8, index=22, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGuiPatches+InventoryGridPatch] GetElement: 0, 3, 8, index=24, m_elements.Count=32
[Info   :ChebsNecromancy.Patches.InventoryGuiPatches+InventoryGridPatch] GetElement: 0, 3, 4, index=12, m_elements.Count=0
[Error  : Unity Log] ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection.
Parameter name: index
Stack trace:
System.ThrowHelper.ThrowArgumentOutOfRangeException (System.ExceptionArgument argument, System.ExceptionResource resource) (at <695d1cc93cca45069c528c15c9fdd749>:0)
System.ThrowHelper.ThrowArgumentOutOfRangeException () (at <695d1cc93cca45069c528c15c9fdd749>:0)
(wrapper dynamic-method) InventoryGrid.DMD<InventoryGrid::GetElement>(InventoryGrid,int,int,int)
InventoryGrid.UpdateGui (Player player, ItemDrop+ItemData dragItem) (at <348c5fd88bf74f899d9b74d0c2228030>:0)
InventoryGrid.UpdateInventory (Inventory inventory, Player player, ItemDrop+ItemData dragItem) (at <348c5fd88bf74f899d9b74d0c2228030>:0)
InventoryGui.UpdateContainer (Player player) (at <348c5fd88bf74f899d9b74d0c2228030>:0)
(wrapper dynamic-method) InventoryGui.DMD<InventoryGui::Update>(InventoryGui)
jpw1991 commented 1 year ago

I was able to "fix" this finally by:

  1. Delete the Container script from the prefab
  2. Add it via code instead & set the name
  3. Do not attempt to resize the container/inventory -> instantly results in this error
            // originally the Container was set on the prefab in unity and set up properly, but it will cause the
            // problem here:  https://github.com/jpw1991/chebs-necromancy/issues/100
            // So we add it here like this instead.
            // Pros: No bug
            // Cons: Cannot set custom width/height
            _container = gameObject.AddComponent<Container>();
            _container.m_name = "$chebgonaz_neckrogathererpylon_name";
            // _container.m_width = ContainerWidth.Value;
            // _container.m_height = ContainerHeight.Value;

            var inv = _container.GetInventory();
            inv.m_name = Localization.instance.Localize(_container.m_name);
            // trying to set width causes error here: https://github.com/jpw1991/chebs-necromancy/issues/100
            // inv.m_width = ContainerWidth.Value;
            // inv.m_height = ContainerHeight.Value;

Resizing problem now tracked in issue #214

WalterWillis commented 1 year ago

Progress! I wonder what's going on under the hood that's causing that.

jpw1991 commented 1 year ago

Me too. Maybe I'll figure it out someday or someone else will figure it out for me (hopefully the latter hahaha).