peter-kish / gloot

A universal inventory system for the Godot game engine.
MIT License
561 stars 20 forks source link

Dragged item release on null crash #196

Closed TreaBeardGaming closed 4 months ago

TreaBeardGaming commented 4 months ago

If you drag an item into a null space that is not considered an inventory slot the drag preview reverts back to the slot you dragged from, but if you attempt to click on that item the engine or game client will crash without any errors.

Narrowed this down to the drop event, and if we don't set anything at all the item will stay dragged at the mouse cursor and you can never place it down. BUT on plus side it doesn't crash now haha

Need to instead make the slot you are dragging from not usable till after the drag process has successfully completed. Either by an item swap or drag/clicking a valid area.

I partially fixed it by getting the tree, root, and finding the parent of the Inventory Grid node and setting that as the zone when drop is empty. This allowed me to still click back into the valid inventory and drop the item correctly.

*Still an issue though if drag from slot into empty space, then click back on same slot it came from and the drag it again there will be a crash....

TreaBeardGaming commented 4 months ago

*Still an issue though if drag from slot into empty space, then click back on same slot it came from and the drag it again there will be a crash....

This might be due to how my nodes are setup, since I'm searching for the CtrlInventoryGridBasic node it works fine for my main inventory. But Drag/Drop/repeat on Slots is still causing issue, this is probably fixable with some more code.

However it should be adjusted in original source so everyone doesn't have to make changes based on how their nodes are laid out.... Could just be a me thing... >.>

TreaBeardGaming commented 4 months ago

updated to Godot 4.2.2 and now I'm getting an infinite recursion error instead of an empty crash. Stack overflow (stack size: 1024). Check for infinite recursion in your script.

Kristo-Art commented 4 months ago

Im using 4.3 and i dont have that issue, did u try reproduce this in example template?

TreaBeardGaming commented 4 months ago

Tested in the example files and not able to replicate the original issue... sigh

peter-kish commented 4 months ago

This sounds somewhat similar to this other issue, which should have been fixed in the dev_v2.4.8 branch, though crashing without any error messages does sound strange.

updated to Godot 4.2.2 and now I'm getting an infinite recursion error instead of an empty crash. Stack overflow (stack size: 1024). Check for infinite recursion in your script.

Can you show us the call stack (or at least part of it)? It could give us some info on where to start investigating.

TreaBeardGaming commented 4 months ago

Can you show us the call stack (or at least part of it)? It could give us some info on where to start investigating.

It had to do with how I was adding nodes when items were created. Basically I wanted to add a rarity color for all items, but didn't realize how many times the scripts get called and it was somehow creating hundreds of child nodes when it should not have been. Still have no idea where the root cause of the Overflow was coming from, and I did try running with the console enabled but it didn't give anything else useful.

Resolution: Removed all my previous code modification, and did it all over... Now it just creates the one child node per item as intended to add a rarity TextureRect behind it, but it only does this for the Inventory and not Slots for some reason.

image image

peter-kish commented 4 months ago

Glad you managed to solve it, the plugin doesn't really support something like this at the moment. But I am planning to make item layouts more customizable in the next major release (v3.0, or maybe even sooner), probably by using a property that will point to any scene resource which will be instantiated to represent the item. That should basically unlock a bunch of item customizations such as backgrounds, animated items, particle effects for items etc.

TreaBeardGaming commented 4 months ago

Glad you managed to solve it, the plugin doesn't really support something like this at the moment. But I am planning to make item layouts more customizable in the next major release (v3.0, or maybe even sooner), probably by using a property that will point to any scene resource which will be instantiated to represent the item. That should basically unlock a bunch of item customizations such as backgrounds, animated items, particle effects for items etc.

that would be awesome to see some new modules in the future, great work btw, still learning godot so I make mistakes often hehe. At moment in ready() I check if item instance is valid, and if so I make my rect childs inside it if they don't already exist ofcourse. Learned that mistake the hard way, but I'm sure there is plenty more grow ahead.

Even have a tooltip scene instanced that gets loaded with the item properties and shows on mouse over... its kinda buggy but as the properties are empty sometimes... idk why

TreaBeardGaming commented 4 months ago

I see now the nodes are being removed in one area but not the other. Moving an item around InventoryGridStacked the Controls are deleted and remade. Moving an item around ItemSlot does not delete and remake the controls, the controls exist when scene starts and stay put.

So when I originally made the TextureRects for the rarity I added them to the CtrlInventoryGridBasic controls for that item. This worked fine for items in the InventoryGrid since they were being deleted and remade constantly. And I assumed the ItemSlots did the samething with their controls, but the CtrlItemSlotEx does not do that.

The AxeContainer in my screenshot does not have any item in its slot, but it still creates all the nodes. Removing the Slot Styles and Default Item Icon doesn't affect this as it still creates the child controls without any item in the slot.

This at least adds to why outside of my incompetence, that creating children in the CtrlItemSlotEx caused an overflow since they weren't being deleted like the gridinventory does....

TreaBeardGaming commented 4 months ago

Local vs Remote view of the slots, only Helmet and Amulet have items in them image image

peter-kish commented 4 months ago

Yeah I'd say it difficult to implement something like this on top of a plugin that is not designed to support it ☹️ The best advice I can give here is to perhaps look into addons/gloot/ui/ctrl_inventory_item_rect.gd. You'll see that it creates two childre, a TextureRect and a Label, that represent the item (both in an CtrlInventoryGrid and CtrlItemSlot). Maybe you can modify it so that it also contains a stylebox background. This of course is no ideal solution because it would have to be synced with every new version of the plugin, but it might do the job until I implement better item customization.

TreaBeardGaming commented 4 months ago

The best advice I can give here is to perhaps look into [addons/gloot/ui/ctrl_inventory_item_rect.gd]

Yeah I did that heavily ;) my [addons/gloot/ui/ctrl_inventory_item_rect.gd] is now 521 lines, and modified ctrl_item_slot.gd

Created PanelContainer with a override theme color that has 0 opacity normaly and changes based on the item_proto rarity color as I also modified that too.

TreaBeardGaming commented 4 months ago

This is what it ended up looking like, still need to tweek the label sizes, but not too shabby. Thanks for all the work you have put into GLoot! <3 image

peter-kish commented 4 months ago

Wow, that looks awesome 🤩