[vJASS] SmoothItemPickup

[Bannar]

Small snippet for cheating item pickup despite unit inventory being full.
This is done by periodic timer with distance checking that accounts for unit's collision size.
Generates custom event for triggering item pickup.

Allows for specifying arbitrary conditions via SmoothItemPickupPredicateModule for deciding whether to account for given unit in smooth item pickup process.

Example of usage: ItemRecipe, StackNSplit.
Makes use of ExtensionMethods, small code snippet.

/*****************************************************************************
*
*    SmoothItemPickup v1.0.2.5
*       by Bannar
*
*    Allows for item pickup despite unit inventory being full.
*
*    Special thanks for Jampion.
*
******************************************************************************
*
*    Requirements:
*
*       RegisterPlayerUnitEvent by Bannar
*          hiveworkshop.com/threads/snippet-registerevent-pack.250266/
*
*       ListT by Bannar
*          hiveworkshop.com/threads/containers-list-t.249011/
*
******************************************************************************
*
*    Event API:
*
*       integer EVENT_ITEM_SMOOTH_PICKUP
*
*       Use RegisterNativeEvent or RegisterIndexNativeEvent for event registration.
*       GetNativeEventTrigger and GetIndexNativeEventTrigger provide access to trigger handles.
*
*
*       function GetSmoothItemPickupUnit takes nothing returns unit
*          Returns unit attempting to pickup event item.
*
*       function GetSmoothItemPickupItem takes nothing returns item
*          Returns item that is being picked up.
*
******************************************************************************
*
*    Interface SmoothItemPickupPredicate:
*
*       static method canPickup takes unit whichUnit, item whichItem returns boolean
*          Determinates whether unit can pickup specified item.
*
*       module SmoothPickupPredicateModule
*          Declares body for new predicate type.
*
*
*    Predicate implementation example:
*
*        | struct MyPredicate extends array
*        |     static method canPickup takes unit whichUnit, item whichItem returns boolean
*        |         return true
*        |     endmethod
*        |
*        |     implement SmoothPickupPredicateModule
*        | endstruct
*
******************************************************************************
*
*    Constants:
*
*       constant real PICK_UP_RANGE = 150
*
*    Functions:
*
*       function AddSmoothItemPickupCondition takes SmoothItemPickupPredicate predicate returns nothing
*          Adds new condition for item to be picked up smoothly.
*          Conditions are aggregated in 'OR' fashion.
*
*       function RemoveSmoothItemPickupCondition takes SmoothItemPickupPredicate predicate returns nothing
*          Removes specified condition from predicate list.
*
*****************************************************************************/
library SmoothItemPickup requires /*
                         */ RegisterPlayerUnitEvent /*
                         */ ListT /*
                         */ ExtensionMethods

globals
    private constant real PICK_UP_RANGE = 150
endglobals

globals
    integer EVENT_ITEM_SMOOTH_PICKUP
endglobals

native UnitAlive takes unit whichUnit returns boolean

globals
    private IntegerList conditions = 0
    private IntegerList ongoing = 0
    private Table table = 0
    private timer looper = CreateTimer()
    private unit eventUnit = null
    private item eventItem = null

    private trigger array triggers
    private unit argUnit = null
    private item argItem = null
endglobals

struct SmoothItemPickupPredicate extends array
    implement Alloc

    static method canPickup takes unit whichUnit, item whichItem returns boolean
        return false
    endmethod

    static method create takes nothing returns thistype
        local thistype this = allocate()
        set triggers[this] = CreateTrigger()
        return this
    endmethod

    method destroy takes nothing returns nothing
        call DestroyTrigger(triggers[this])
        set triggers[this] = null
        call deallocate()
    endmethod
endstruct

module SmoothPickupPredicateModule
    private delegate SmoothItemPickupPredicate predicate

    private static method onInvoke takes nothing returns boolean
        return thistype.canPickup(argUnit, argItem)
    endmethod

    static method create takes nothing returns thistype
        local thistype this = SmoothItemPickupPredicate.create()
        set predicate = this
        call TriggerAddCondition(triggers[this], Condition(function thistype.onInvoke))
        return this
    endmethod

    method destroy takes nothing returns nothing
        call predicate.destroy()
    endmethod
endmodule

function GetSmoothItemPickupUnit takes nothing returns unit
    return eventUnit
endfunction

function GetSmoothItemPickupItem takes nothing returns item
    return eventItem
endfunction

function AddSmoothItemPickupCondition takes SmoothItemPickupPredicate predicate returns nothing
    if predicate != 0 then
        call conditions.push(predicate)
    endif
endfunction

function RemoveSmoothItemPickupCondition takes SmoothItemPickupPredicate predicate returns nothing
    if predicate != 0 then
        call conditions.erase(conditions.find(predicate))
    endif
endfunction

private struct PeriodicData extends array
    unit picker
    item itm
    real range

    implement Alloc

    static method create takes unit u, real range returns thistype
        local thistype this = allocate()
        set this.picker = u
        set this.range = range

        call ongoing.push(this)
        set table[GetHandleId(u)] = this
        return this
    endmethod

    method destroy takes nothing returns nothing
        call table.remove(GetHandleId(picker))
        set picker = null
        set itm = null

        call ongoing.erase(ongoing.find(this))
        if ongoing.empty() then
            call PauseTimer(looper)
        endif

        call deallocate()
    endmethod

    static method get takes integer index returns thistype
        if table.has(index) then
            return table[index]
        endif
        return 0
    endmethod
endstruct

private function FireEvent takes unit u, item itm returns nothing
    local unit prevUnit = eventUnit
    local item prevItem = eventItem
    local integer playerId = GetPlayerId(GetOwningPlayer(u))

    set eventUnit = u
    set eventItem = itm

    call TriggerEvaluate(GetNativeEventTrigger(EVENT_ITEM_SMOOTH_PICKUP))
    if IsNativeEventRegistered(playerId, EVENT_ITEM_SMOOTH_PICKUP) then
        call TriggerEvaluate(GetIndexNativeEventTrigger(playerId, EVENT_ITEM_SMOOTH_PICKUP))
    endif

    set eventUnit = prevUnit
    set eventItem = prevItem
    set prevUnit = null
    set prevItem = null
endfunction

private function Test takes unit u, item itm, real range returns boolean
    local real dx
    local real dy

    if UnitHasItem(u, itm) then
        return true
    endif

    set dx = GetItemX(itm) - GetUnitX(u)
    set dy = GetItemY(itm) - GetUnitY(u)
    // Assumes range is multipled to avoid invoking SquareRoot
    return (dx * dx + dy * dy) <= range
endfunction

private function OnCallback takes nothing returns nothing
    local IntegerListItem iter = ongoing.first
    local PeriodicData data

    loop
        exitwhen iter == 0
        set data = iter.data
        if not UnitAlive(data.picker) or GetUnitCurrentOrder(data.picker) != 851986 /*
        */ or not IsItemPickupable(data.itm) then // order move
            call data.destroy()
        else
            if Test(data.picker, data.itm, data.range) then
                call FireEvent(data.picker, data.itm)
                call data.destroy()
            endif
        endif
        set iter = iter.next
    endloop
endfunction

private function OnNullTimer takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local integer id = GetHandleId(t)
    local unit u = table.unit[id]
    local item itm = table.item[-id]

    if UnitAlive(u) and IsItemPickupable(itm) then
        call FireEvent(u, itm)
    endif

    call table.unit.remove(id)
    call table.item.remove(-id)
    call DestroyTimer(t)
    set t = null
    set u = null
    set itm = null
endfunction

private function OnTargetOrder takes nothing returns nothing
    local PeriodicData data
    local boolean proceed = false
    local IntegerListItem iter
    local SmoothItemPickupPredicate condition
    local real collision
    local real range
    local timer tmr
    local real angle
    local real x
    local real y
    local trigger t
    set argUnit = GetTriggerUnit()
    set argItem = GetOrderTargetItem()

    if not IsUnitInventoryFull(argUnit) or GetIssuedOrderId() != 851971 then // order smart
        return
    elseif argItem == null or IsItemPowerup(argItem) then
        return
    endif

    set iter = conditions.first
    loop
        exitwhen iter == 0
        set condition = iter.data
        if TriggerEvaluate(triggers[condition]) then
            set proceed = true
            exitwhen true
        endif
        set iter = iter.next
    endloop
    if not proceed then
        return
    endif

    set collision = BlzGetUnitCollisionSize(argUnit)
    set range = (PICK_UP_RANGE + collision) * (PICK_UP_RANGE + collision)

    if Test(argUnit, argItem, range) then
        // Ensures order is finished before item is picked up.
        // Fixes the issue with unit moving towards the item location, rather than stopping
        set tmr = CreateTimer()
        set table.unit[GetHandleId(tmr)] = argUnit
        set table.item[-GetHandleId(tmr)] = argItem
        call TimerStart(tmr, 0.0, false, function OnNullTimer)
        set tmr = null
    else // if unit is not nearby target item, issue artificial move order
        set data = PeriodicData.get(GetHandleId(argUnit))
        if data == 0 then
            if ongoing.empty() then
                call TimerStart(looper, 0.031250000, true, function OnCallback)
            endif
            set data = PeriodicData.create(argUnit, range)
        endif
        set data.itm = argItem

        set angle = bj_RADTODEG * Atan2(GetUnitY(argUnit) - GetItemY(argItem), GetUnitX(argUnit) - GetItemX(argItem))
        set x = GetItemX(argItem) + PICK_UP_RANGE * Cos(angle * bj_DEGTORAD)
        set y = GetItemY(argItem) + PICK_UP_RANGE * Sin(angle * bj_DEGTORAD)
        set t = GetAnyPlayerUnitEventTrigger(EVENT_PLAYER_UNIT_ISSUED_POINT_ORDER)
        call DisableTrigger(t)
        call IssuePointOrderById(argUnit, 851986, x, y) // order move
        call EnableTrigger(t)
    endif
endfunction

private module SmoothItemPickupInit
    private static method onInit takes nothing returns nothing
        set EVENT_ITEM_SMOOTH_PICKUP = CreateNativeEvent()
        set conditions = IntegerList.create()
        set ongoing = IntegerList.create()
        set table = Table.create()
        call RegisterAnyPlayerUnitEvent(EVENT_PLAYER_UNIT_ISSUED_TARGET_ORDER, function OnTargetOrder)
    endmethod
endmodule

private struct SmoothItemPickup extends array
    implement SmoothItemPickupInit
endstruct

endlibrary

 

[Jampion]

Looks good. In a normal scenario it feels exactly like the unit would try to regularly pick up the item.
I will call it default behaviour, if the unit behaves with the system like it would with a free inventory.
Following default behaviour is important in this kind of system, so the player won't even realize there is a system running in the background and thinks this is a part of the game.


There are a few scenarios in which, the system does not follow default behaviour.


1. Target item killed.

The unit will still move towards the item (default behaviour), but the system fires it's event.
​

2. Target item moved

With default behaviour, the unit will correctly move towards the new location. In your system the unit will move towards the old location, stop there and fail to pick up the item.
​

3. Target item removed (same as item being sold)

Default behaviour for the unit is to stop. With the system, the unit will keep moving, but fire an event.
Small difference. I don't even know it is important to follow default behaviour here, because in default behaviour killing and removing item is different, but a player won't see the difference between an item being killed or removed.
​

4. Target item hidden

With default behaviour the unit will still pick up the item. Same with the system. I feel it should not be like this and I would consider it a bug in the game. I don't know if there exists a system that fixes this. If there is no such system, it seems to be an issue no cares about and it would not be necessary to fix it in your system.
​

1. needs to be fixed.
2. would be good to follow default behaviour.
3. and 4. are very rare scenarios and I personally don't like the default behaviour. I think killed, removed and hidden should all be the same, as they are the same for a player: item is gone

If the item is picked up, the system follows default behaviour and the unit moves towards the item position.

Additional notes:
You should add in the documentation, that conditions follow the or-logic. If one is true, the system will run.

 

[Bannar]

Thank you for your feedback, however, you missed a crucial point:
- order queue

We devs, have no power of it, we cannot alter order queue in any manner, thus there is a choice to make. Either pause/stop the unit and break order queue or leave it be.
I chose the second option, which is a valid choice for many action-based maps like NotD: Aftermath and Island Troll Tribes (which this system and its Wurst brother are used in).

I've iterated over dozens of scenarios, including ones you listed above. Breaking order queue did more harm than unintentional move.

Just to clarify:
There is a data.item == null check in periodic loop. Removed items will cause this reference to point to null, thus data instance will be removed and everything will be cleaned up as expected.

 

[Jampion]

- order queue

good point

I think it's fine, if the unit keeps moving towards the item's original position in 2. After all this is what happens also if the item is picked up by another units. Leaving the order queue is more important than following default behaviour here, I agree.

Just saw, that I forgot a word in my review. Sorry for that.

Default behaviour for the unit is to stop. With the system, the unit will keep moving, but fire an event.

What I meant is: Default behaviour for the unit is to stop. With the system, the unit will keep moving, but not fire an event.

Removed items will cause this reference to point to null

Unfortunately this is not true. The reference points to a non-existing item, it is not null.

Spoiler: example

scope MyScope initializer onInit
    private function onInit takes nothing returns nothing
        local item itm = CreateItem('phlt', 0, 0)
        call RemoveItem(itm)
        if(itm == null) then
            call BJDebugMsg("null")
        else
            call BJDebugMsg("not null")
        endif
       
        if(GetItemTypeId(itm) == 0) then
            call BJDebugMsg("destroyed")
        endif
    endfunction
endscope

The unit will keep moving towards the location and the system will stop when the unit reaches the position, because the order is no longer move.

However there is a small problem. The item is removed, but the system keeps running. Specifically it will keep calling Test. The item is removed and therefore has no position, so the position will default to 0,0. This means if the unit moves close to 0,0 the event (with a non-existing item) will fire in this situation.
If you use GetItemTypeId(data.item) == 0 instead of comparing it to null, you should get your intended result.

 

[IcemanBo]

To keep periodic validation I used in library FullInventory here:

    // check if item is still available to be picked up
    private static method itemValidate takes item it returns boolean
        return GetItemTypeId(it) != 0 and IsItemVisible(it) and not IsItemOwned(it)
    endmethod

^maybe a general condition can makes sense. I also like custom defined, though.

I also had trouble with, I called them queue-orders, that were fired but did not cancel the move order, which lead to some not 100% smooth solution (same library ):

    /*
        onCancel will cancel the periodic check for any new no-target, or point-orders the unit gets
        because then, the unit obviously doesn't wanna pick up the item anymore.
   
        problem:
        The problem is that the unit has currently a "move" order onto a point,
        instead of a default itemTarget-order.
        So, when a Queue-Order like Avatar finished, then it will automaticaly order this normal move-point order again.
        The problem is, by nature we don't trust move orders, and usually cancel the periodic check.
        So the trick is, after we ordered a Queue-Order, we allow the same unit to make ONE move order,
        and if the move order is exactly the same as the old, means same x/y and onto a point, then we allow it.
        We also make a item validation check, to check if item visible at that very position, and pickable.
        These together hopefully ensure not to fail.
   
    */

private static method onCancel takes nothing returns nothing
        local unit u = GetTriggerUnit()
        local integer orderId = GetIssuedOrderId()
        local thistype this
        if IsUnitInGroup(u, GROUP) and cancel_enabled_p and OrderType[orderId] != OrderType.INSTANT then
           
            set this = thistype(GetUnitId(u))
           
            if OrderType[orderId] == OrderType.QUEUE then
                set .dirtyTrick = true
            elseif not .dirtyTrick or not (orderId == ORDER_move) or not (GetOrderPointX() == .x) or not  (GetOrderPointY() == .y) and itemValidate(.whichItem) then
                set .dirtyTrick = false
                call .destroy()
            endif
        endif
        set u = null
    endmethod

 

[Bannar]

Thanks for your feedback guys!

The unit will keep moving towards the location and the system will stop when the unit reaches the position, because the order is no longer move.

@Jampion precisely! I don't stop the movement, just deallocate the PeriodicData instance.

However, the item-type check might be indeed a better solution than comparing item reference with invalid one (null).

 

[Bannar]

Updated the fidelity of periodic callback as discussed.
Updated documentation for AddSmoothItemPickupCondition function.

 

[Jampion]

Item removal has been taken care of by using GetItemTypeId(data.item) == 0.
If the item is killed, the system will still fire an event.
To stop it if the item is dead, use GetWidgetLife(data.item) <= 0.405.
@IcemanBo you should probably add this in your library too.

 

[Bannar]

Added new function, IsItemPickupable to ExtensionMethods.
SmoothItemPickup has been adjusted to reflect changes made.

Edit: Also added IsItemAlive function as an extension method.

 

[IcemanBo]

Right, I'll also need to copy Powerup, and UnitAlive check.

 

[Jampion]

I found another problem. If you use shift to order the unit to pick up the item several times it will fire the event several times.
This is what is supposed to happen, as the unit is basically reordered to pick up the item again.
However if the event makes the item unavailable there must only fire the first event.
If the item is removed, it works correctly, but if it is hidden or killed you will get this bug.

For the shift orders, the Test function will run immediatly and return true, because the unit already has the correct position.
FireEvent is delayed by 0 seconds, so the item could already have been made unavailable (most likely by the first event).
If you add IsItemPickupable condition before OnNullTimer is started with a timer, it should fix the problem.
You can also add it to FireEvent to not fire the event if the item is not available. This would also prevent this for any potential bugs we don't know yet.

 

[Bannar]

Thank you Jampion, once again.
You are right. Not only periodic callback, but also OnNullTimer event firing should be guarded by IsItemPickupable.
On top of that, I've added UnitAlive check in OnNullTimer since not only item, but unit itself could be invalidated during that period.

I try to keep my scripts coherent and SmoothItemPickup is no exception. FireEvent function is solely responsible for event firing and handling cascade event invocations. It is caller responsibility to invoke it in right circumstances.
That's why OnNullTimer (caller) has been updated instead : )

I've added you to the credits section.

 

[Jampion]

On top of that, I've added UnitAlive check in OnNullTimer since not only item, but unit itself could be invalidated during that period.

Gave me an idea to test something.
If you use a dummy unit to cast warstomp on the unit in the event and use a lot of shift orders, you will get funny behaviour. The first event fires normally and warstomp prevents the unit from picking up the item a second time. Now the shift orders run all at the same time and the event fires multiple times.
In theory one would expect, that the events would come one after another with a stun in between.
One could probably fix this, if only one PeriodicData exists per unit. It's not a big issue, so if fixing it would create too much overhead that would reduce the overall performance of the system I would leave it like that.

I found a worse problem, of which I don't know how to solve it. The system orders the unit to move towards a point between the unit and the item. This point is inside the pick up range.
If this point is blocked by an object, the unit will instead move towards a point between the unit and the blocking object. This point is no longer inside the pick up range, so the event will not fire.
Let's say the unit is 200 range away from the item and another unit stands in between the unit and the item. The unit will not move, as it is already as close to the target point as it can get.

 

[Bannar]

One could probably fix this, if only one PeriodicData exists per unit. It's not a big issue, so if fixing it would create too much overhead that would reduce the overall performance of the system I would leave it like that.

System forbids multiple PeriodicData instances to be running for the same unit. Check function OnTargetOrder, this part to be precise:

        if not table.has(GetHandleId(argUnit)) then
            if ongoing.empty() then
                call TimerStart(looper, 0.031250000, true, function OnCallback)
            endif
            set data = PeriodicData.create(argUnit, range)
        else
            set data = table[GetHandleId(argUnit)]
        endif
        set data.item = argItem

I'll think about second problem tomorrow after work, thanks!

 

[Jampion]

Yeah true, was stupid by me. PeriodicData is not the problem. It's multiple NullTimers.
It's all the consecutive shift orders that happen at the same, when the first event already fired. This means the unit is already close to the item and no periodic data is used. Instead the Test function returns true and a NullTimer is started.
In general if you use multiple shift orders, multiple events should fire.
The problem with the null timer is, that all events happen after all orders have already been given. In theory it should be: order - event - order - event ...
You decided to use a null timer because:

// Ensures order is finished before item is picked up.
// Fixes the issue with unit moving towards the item location, rather than stopping

So it makes sense to use a null timer and the problem that arises with it is small and can be handled by the user of the system, if needed.
In other words: I would leave it like it is.

 

[BloodForBlood]

Can you make a backwards compatibility(for 1.26a,1.27 or ETC)?

 

[Bannar]

It's very simple. Replace set collision = BlzGetUnitCollisionSize(argUnit) with: set collision = GetUnitCollision(argUnit)
And append requirement at the top: GetUnitCollision.

The reason I'm not doing this by default is recent movement in WE development. And there will be even more changes in the future.
Think what will happen when code arrays become reality..

 

[BloodForBlood]

It's very simple. Replace set collision = BlzGetUnitCollisionSize(argUnit) with: set collision = GetUnitCollision(argUnit)
And append requirement at the top: GetUnitCollision.

The reason I'm not doing this by default is recent movement in WE development. And there will be even more changes in the future.
Think what will happen when code arrays become reality..

Oh,yeah the get unit collision size.Thanks

EDIT:
It doesn't work on 1.26a.I used Nestharus' "GetUnitCollision" but still doesn't work.While I got full inventory, I tried to pick other items(non-rune) and it gave me an error("Inventory is full") which is normal.But my hero stopped, It didn't move to the item..
Is that a bug or am I missing something?

 

[Bannar]

If what you say is true, then the order of execution and event fire is different in 1.26 than in patches 1.28+.
Currently, order "stop" that will be auto invoked for unit attempting to pickup item with full inventory will happen BEFORE artificial move is triggered. The only exception for the lack of auto "stop" order is situation when unit has ongoing order queue. Instead of order "stop", the next order in the queue would be dequeued and issued.

If my guess is indeed a reality (I don't have 1.26 and I will not be downgrading my game to check this out), then snippet's artificial move order is invoked before the auto "stop" and is getting halted because of that.

Honestly though, the EasyStackNSplit, the original system by that included similar but not as robust "smooth-pickup" feature worked just fine. It was released years ago..

 

[BloodForBlood]

If what you say is true, then the order execution and event fire are different in 1.26 from patches 1.28+.
Currently, order "stop" that will be auto invoked for unit attempting to pickup item with full inventory will happen BEFORE artificial move is triggered. The only exception for the lack of auto "stop" order is situation when unit has ongoing order queue. Instead of order "stop", the next order in the queue would be dequeued and issued.

If my guess is indeed a reality (I don't have 1.26 and I will not be downgrading my game to check this out), then snippet's artificial move order is invoked before the auto "stop" and is getting halted because of that.

Honestly though, the EasyStackNSplit, the original system by that included similar but not as robust "smooth-pickup" feature worked just fine. It was released years ago..

Well, yeah.Good bye SmoothItemPickup!
I won't be updating my version of Warcraft 3 because of Memory Hack...

 

[TriggerHappy]

Looks good and is useful. However you don't link or mention ExtensionMethods in the documentation.

Maybe provide a simple example which clearly shows off the API. The two systems you linked as examples are fairly big.

 

[Bribe]

@Bannar are you still around to check up on these resources of yours, or have you given up on scripting?

 

[Bannar]

I've not given up. There is priority list behind the scenes, unfortunately. Be aware that I've updated and re-checked most of my stuff internally few months ago: crojewsk/war3libs.

Perhaps we could have a short call to discuss the direction for the coding-subsection.

 

[Bribe]

Approved as a solid vJass library for legacy users