peter-kish / gloot

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

Max Stack Size enhancement #235

Closed TheYellowArchitect closed 2 months ago

TheYellowArchitect commented 2 months ago

I have an item of max_stack_size = 4 If it has 3 stacks, and I move the same item type which has 2 stacks (so 5 stacks in total), it shouldn't cancel the transaction and have 3 and 2, but it should be 4 and 1.

This is practical, for when you want to "refill" your stacks from a drop you found on a loot table etc. I cannot think of a practical scenario where you want to straight up cancel the user's input as is currently done.


To recreate the example, see the scene inventory_grid_stacked_ex_transfer.tscn and add the property max_stack_size: 3 to the armor. Now if you move the 2 existing stacks to the other 2 stacks, both remain 2 and 2 stacks, instead of 3 and 1.

So for example, if I have a max stack 7 for our example item type, and I move 6 of itemstack1 and 10 of itemstack2, it should go 7 and 9, instead of 6 and 10.

peter-kish commented 2 months ago

CtrlInventoryGrid and CtrlInventoryGridEx don't try to split/merge item stacks when dropping items from one inventory to the other, which I admit is not ideal. Maybe an additional option can be added to control how item transfers are done. Or at least the default behavior should be changed so that it does the equivalent of InventoryGridStacked.transfer_autosplitmerge.

peter-kish commented 2 months ago

Or at least the default behavior should be changed so that it does the equivalent of InventoryGridStacked.transfer_autosplitmerge.

Should be implemented in 55816481ce886dce23aba727e0ce9154399ff5ac

TheYellowArchitect commented 2 months ago

Or at least the default behavior should be changed so that it does the equivalent of InventoryGridStacked.transfer_autosplitmerge.

Should be implemented in 5581648

I put that commit on my local copy, and tested the demo with this protoset:

[
    {
        "height": 1,
        "id": "item_1x1",
        "image": "res://images/item_book_blue.png",
        "width": 1,
       "max_stack_size": 3
    },
    {
        "height": 2,
        "id": "item_2x2",
        "image": "res://images/item_armour_silver.png",
        "width": 2,
"max_stack_size": 3
    }
]

The use-case is having "item1x1" with 2 stacks, and another "item1x1" with 2 stacks, and moving one onto the other. The one moved onto, should get 3 stacks, and the other 1. What happens currently is the same as what used to happen, that is they are 2 and 2.

To confirm it works, try the above use-case on your side, in case i missed some past commit

peter-kish commented 2 months ago

That was some sloppy testing from my side 😬 But I think I fixed it in the most recent commits on master. Let me know if I still missed something.

TheYellowArchitect commented 2 months ago

You didn't miss anything, it works perfectly!