Baradé's Item Unstack System 1.3

[SmitingDevil]

Since this system is for the latest patch, you could use an library initializer to eliminate the need to initialize the system on the user end. The system will then be automatic without the user ever touching the code.

AFAIK, setting name and tooltip does not work on the latest patch. Extended tooltip and description are changeable in game, however. Though, if you were to use the new skin system, you could change all item properties during runtime. However, it is not dynamic as in you would need to create new items in the object editor and changing one property on demand is next to impossible, or at the very least, not feasible with the skin system.

I would not recommend placing code in the map header. There is this strange bug, where the code in the map header is replaced with code from another trigger.

 

[Barade]

I wanted to avoid using vJass and keep it minimalistic, so you can save without JassHelper enabled which is a bit faster.

Hmm I never had that bug. I have always used the map script like in the classic Blizzard maps but I can move it into a trigger.

I can disable some of the set function calls like tooltip and description. I don't know how the skin system works?
I can add an item with dynmically changed properties which stacks as proof of concept.

The only concern I had was the TriggerSleepAction. The item could theoretically have been sold/removed during this sleep. I hope the widget life check is sufficient but I might have to detect it in a different way.
I could also start a timer with a timeout of 0 seconds and attach the necessary data using a hashtable but that would increase the code.

edit:
I can create a vJass and GUI version like for the refered Easy Item Stack 'n Split system.

 

[SmitingDevil]

If that is what you are going for, you could use the InitTrig function of an converted GUI trigger. These are ran automatically.

You don't need that second loop to search for a free item slot. Just add the item to the unit via UnitAddItemById. The item added will automatically drop to the ground if the unit inventory is full.

The TriggerSleepAction is unnecessary. The system functions well without it.

 

[Barade]

Thx for the hints. I will adapt everything later.

The sleep is there to wait until the order has been completed. I had some bad experience when the item wasn't moved to the slot but I can try it differently.

edit:
Yes it only works with a sleep. It seems that the native stacking does take the custom descriptions and settings of the item already in the inventory so applying the item properties on unstacking is kind of useless except for stacked items which are given to heroes with triggers and set to invulnerable etc.

edit2:
The second loop is necessary to prevent Warcraft's native stacking. If I just add the item it will stack again!

edit3:
Release 1.1:

• Split into multiple functions.
• Do not apply item name, tooltip and icon path on unstacking since it is broken.
• Prefer the nearest empty slot on unstacking.
• Add a line break to the unstacking hint.
• Add some items with custom names and descriptions to test stacking/unstacking them.

Now the unstacking does not use the first free slots counting from 1 to 6 but the first free one next to the current slot preferring the slots after the current one.
The only concern I have with this system is that the item could be removed via triggers during the sleep and then the variable will have some invalid reference which theoretically could be used by another item in the same slot which would lead to some weird unstacking of another replaced item. The chances of this happening would be almost 0.

Since Warcraft's native stacking does always keep the properties of the item which receives additional stacks, keeping the properties on unstacking is only helpful if the stacked item has originally been created with some triggers and is unique in the whole map with these properties.

edit4:

Release 1.2 2022-04-13:

• Use UnitInventorySize instead of bj_MAX_INVENTORY to support different inventory sizes.
• Place Footmen with unit inventories to check different inventory sizes.

 

[Wrda]

I wanted to avoid using vJass and keep it minimalistic, so you can save without JassHelper enabled which is a bit faster.

This doesn't really make much sense, because since this is made for the latest patches, you just need one VJASS code or system or whatever for it to be required to run JassHelper. It doesn't really bother anyone for +3 seconds of saving, it's not like you're constantly trying to save the map every minute. You shouldn't think this in a minimalistic way, but rather a pratical way, with the things that are required.

function ItemUnstackApplyItemProperties takes item sourceItem, item targetItem returns nothing

This looks either too long or borderline acceptable length of a function, a better length would be
"ItemUnstackApplyProps".

You could shorten this

function ItemUnstackGetItemSlot takes unit hero, item whichItem returns integer
    local integer sourceSlot = -1
    local integer i = 0
    loop
        if (UnitItemInSlot(hero, i) == whichItem) then
            set sourceSlot = i
        endif
        set i = i + 1
        exitwhen (sourceSlot != -1 or i >= UnitInventorySize(hero))
    endloop
    return sourceSlot
endfunction

To this:

function ItemUnstackGetItemSlot takes unit hero, item whichItem returns integer
    local integer sourceSlot = -1
    local integer i = 0
    loop
        if (UnitItemInSlot(hero, i) == whichItem) then
            set sourceSlot = i
            exitwhen true
        endif
        set i = i + 1
        exitwhen i >= UnitInventorySize(hero))
    endloop
    return sourceSlot
endfunction

ItemUnstackAddItemToNearestFreeSlot function should also follow this logic

function ItemUnstackAddItemToNearestFreeSlot takes unit hero, integer itemTypeId, integer sourceSlot returns item
function ItemUnstackAddItemToNearestFreeSlotOrGround takes unit hero, integer itemTypeId, integer sourceSlot returns item
function ItemUnstackAddItemToNearestFreeSlotOrGroundUnstack takes unit hero, integer itemTypeId, integer sourceSlot, item sourceItem returns nothing

DUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUDE, are we playing who reads the whold functions fast? This is too long. I'm sorry but "wtf is this" was my first reaction, lol.

You could merge ItemUnstackAddItemToNearestFreeSlotOrGround and ItemUnstackAddItemToNearestFreeSlotOrGroundUnstack.

function ItemUnstackAddItemToNearestFreeSlotOrGroundUnstack takes unit hero, integer itemTypeId, integer sourceSlot, item sourceItem returns nothing //change function name for god's sake
    // search for a free slot to unstack the item
    local item unstackedItem = ItemUnstackAddItemToNearestFreeSlot(hero, itemTypeId, sourceSlot) //change this too please
    // create the item for the hero with one slot if all slots are used
    if (unstackedItem == null) then
        set unstackedItem = CreateItem(itemTypeId, GetUnitX(hero), GetUnitY(hero))
    endif
    call SetItemCharges(unstackedItem, 1)
    call ItemUnstackApplyItemProperties(sourceItem, unstackedItem)
    set unstackedItem = null
endfunction

Why name these functions TriggerConditionItemUnstack and TriggerActionsItemUnstack, when all the other functions start with "ItemUnstack"? InitItemStackSystem follows the convention of an init function, so it is an exception. This isn't a big deal, but it shows the inconsistencies. ItemUnstackCondition(s), ItemUnstack_Condition(s), Trig_ItemUnstackCondition(s), TrigItemUnstackCondition(s), Trig_ItemUnstack_Condition(s) are more suitable names.

The only concern I had was the TriggerSleepAction. The item could theoretically have been sold/removed during this sleep. I hope the widget life check is sufficient but I might have to detect it in a different way.

But isn't the item null if it has already been sold/removed?

I can create a vJass and GUI version like for the refered Easy Item Stack 'n Split system.

GUI version makes sense, vJass not, since you should integrate it here already to make things simpler, ahem,

keep it minimalistic

yes, making function names minimalistic

Aside from all the ranting, this resource proves to be certainly useful with no doubt.

 

[Barade]

Wow this is quite a rant about minor things like function names of functions you do not even call since it is no API.

The local item variable does not become null automatically I think. It might have a reference to a removed item. I would have to hook the RemoveItem call I think and the null my variable. If the reference is not null but not the one from any item in the inventory it is no issue.

I can adapt all the function names if you have an issue with them.
You do not even call them from anywhere. I can start them all with ItemUnstack.

I learned to define all conditions inside one exitwhen instead of use multiple ones (it's like multiple breaks instead of defining one single loop condition).
Hence, changing ItemUnstackGetItemSlot would save one single operation but lead to one more line of code and two exitwhens.

I think these are really minor things.

About vJass: Not all maps use vJass code and JassHelper can be quite annoying. Generating a map script with it takes some time.
If your map does not use vJass why should you use vJass for this one simple system? It doesn't even have global variables.
I myself have one big project which uses plenty of vJass but other maps which only use GUI triggers with some JASS functions.
I would totally disable the JassHelper/vJass if I did not define some free global variables.

edit:
I have refactored the code a bit with all functions starting with ItemUnstack and two functions became one now.
I think that should be sufficient. You do not have to call anything to use this system. Just copy the trigger from the example map or call the Init function yourself.
vJass is really not required with the name prefixes.