[System] [GUI] Easy Item Stack 'n Split v3.0.0

[Ricola3D]

Forgot to say that to install you should also do the following:

In menu "Advanced" > "Game Interface": set "Text - Message - 'Inventory is full'" (Inventoryfull) to 1 space character " ". Since this system will fire it for you instead.

 

[Wrda]

In UnitStackItem you have:

    function UnitStackItem takes unit u, item item1 returns boolean
        local integer item1Charges = GetItemCharges( item1 )
        local integer inventorySize
        local integer item1Level
        local integer item1TypeId
        local item item2
        local integer item2Charges
        local integer item2Level
        local integer slot
        local real posX
        local real posY
        local real unitAngle


        // Make sure the item has charges
        if item1Charges <= 0 then
            // If not we just give it to the unit
            call DisableTrigger( gg_trg_EasyItemStacknSplit )
            call UnitAddItem( u, item1 )
            call EnableTrigger( gg_trg_EasyItemStacknSplit )
        else
            // Item has charges
            set inventorySize = UnitInventorySize( u )
            set item1Level = GetItemLevel( item1 )
            set item1TypeId = GetItemTypeId( item1 )
(...)

Just return false earlier and remove the else part.

        // Make sure the item has charges
        if item1Charges <= 0 then
            // If not we just give it to the unit
            call DisableTrigger( gg_trg_EasyItemStacknSplit )
            call UnitAddItem( u, item1 )
            call EnableTrigger( gg_trg_EasyItemStacknSplit )
            return false
        endif
        // Item has charges
        set inventorySize = UnitInventorySize( u )
        set item1Level = GetItemLevel( item1 )
        set item1TypeId = GetItemTypeId( item1 )
(...)

Also it's needed to null item2 before the last two returns.
Sound variable also must be nulled, in the text macro.

if deltax * deltax + deltay * deltay <= 22500 or orderId != 851986 then

Where does the number 22500 comes from? Curious.
I don't understand why you're using a text macro while the exact same thing can be done with a (private) function.
You should cache GetTriggerUnit and the other function calls since they'll be called several times within the loop in CancelController function

ActionController: There's not a single time that GetManipulatedItem returns null on such an event, unless it is manually removed (I haven't tested if one makes a trigger with the same event and remove the item makes a function ActionController GetManipulatedItem return null).

                if ( BlzGetItemBooleanField(item1, ITEM_BF_USE_AUTOMATICALLY_WHEN_ACQUIRED) == true and GetItemCharges(item1) == 1 ) then
                    // Nothing to add in inventory, all charges will be used on acquisition
                else
                    // The item is being acquired so we stack it
                    if UnitStackItem(u, item1) then
                        // Couldnot stack all charges in inventory, some remain on the floor. Play the "Item Drop" sound
                        set p = GetOwningPlayer( u )
                        //! runtextmacro EasyItemStacknSplit_PLAYITEMSOUND( "HeroDropItem1.wav" )
                    endif
                endif

Invert it with "not", removing the else part.

 if UnitInventoryFull(u) then
                            // The item is being targeted with a full inventory so we add it to the timer queue
                            set item1Charges = GetItemCharges( item1 )
                            set index = 0
                            if ( item1Charges > 0 ) then
                                // Item with charges, look up if there is an item available to receive those charges in unit inventory
                                set item1Level = GetItemLevel( item1 )
                                set item1TypeId = GetItemTypeId( item1 )
                                set index = 0
                                loop
                                    set item2 = UnitItemInSlot( u, index )
                                    set item2Level = GetItemLevel( item2 )
                                    if item2 != item1 and GetItemTypeId(item2) == item1TypeId and (not USE_ITEM_LEVEL or item2Level == 0 or GetItemCharges(item2) < item2Level) then
                                        // Found item with same type
                                        set index = inventorySize + 1
                                    else
                                        set index = index + 1
                                    endif
                                    exitwhen index >= inventorySize
                                endloop

"set index = 0" is being unnecessarily duplicated.

                            else
                                // Full inventory error
                                call IssueImmediateOrder( u, "stop" )
                                set p = GetOwningPlayer( u )
                                // Play error sound
                                if ERROR_SOUND != null and ERROR_SOUND != "" then
                                    set str = ERROR_SOUND
                                    set snd = CreateSound( str, false, false, false, 12700, 12700, "" )
                                    call SetSoundVolume( snd, 127 )
                                    if GetLocalPlayer() != p then
                                        call StartSound( snd )
                                    endif
                                    call KillSoundWhenDone( snd )
                                    call ClearTextMessages()
                                    call DisplayTimedTextToPlayer(GetOwningPlayer(udg_EasyItem_unit),0.50,-1.00,2.00,"|cffffcc00"+ERROR_MESSAGE+"|r")
                                endif
                            endif

I wouldn't clear text messages as that can be quite bad. If you're concerned with the messages stacking, there's a library somewhere around for specifically error messages. Currently the error message didn't work, nor the sound.
Looping to find the index of the unit with the items and other loop to match the index with item type start to become costly if accumulated, a hashtable would be a safer option.
ActionController function is very long, too long that it seems like some parts of the logic could be split into smaller pieces.
The GUI version seems very messy, everything is in one place, with lots of "magic numbers", even with the developer info trigger comment it's pretty hard to figure out what's happening. So the logic should be split into sections.

All in all, it's another iteration of item stacking-splitting mechanic, with a few unique quirks, such as unstacking the item can also stack the item on the next inventory slot if done within a time limit. However, since some patches there's a native way of stacking by enabling it in the gameplay constants (no unstacking mechanism however), so it renders the stacking part of this system redundant. Baradé's system handles the unstacking quite well, but only vJASS and doesn't include GUI.
I still wonder if even after the changes there is equal footing between this system and Baradé's, as in, has other pros than the latter. One surely can be that it supports GUI.

Needs fix.

 

[almawt]

glad to see the work is continued!

 

[Wrda]

Moved to Awaiting update, as it should be.