1. Updated Resource Submission Rules: All model & skin resource submissions must now include an in-game screenshot. This is to help speed up the moderation process and to show how the model and/or texture looks like from the in-game camera.
    Dismiss Notice
  2. DID YOU KNOW - That you can unlock new rank icons by posting on the forums or winning contests? Click here to customize your rank or read our User Rank Policy to see a list of ranks that you can unlock. Have you won a contest and still haven't received your rank award? Then please contact the administration.
    Dismiss Notice
  3. Ride into the sunset with the 32nd Modeling Contest.
    Dismiss Notice
  4. This adventure has come to an end. Congratulate our heroes in the 16th Mini Mapping Contest Results.
    Dismiss Notice
  5. From the gates of hell, the 5th Special Effect Contest Results have emerged.
    Dismiss Notice
  6. Race against the odds and Reforge, Don't Refund. The 14th Techtree Contest has begun!
    Dismiss Notice
  7. Check out the Staff job openings thread.
    Dismiss Notice
Dismiss Notice
60,000 passwords have been reset on July 8, 2019. If you cannot login, read this.

[System] Item Cleanup

Discussion in 'JASS Resources' started by Tirlititi, Aug 11, 2010.

  1. Tirlititi

    Tirlititi

    Joined:
    Jul 11, 2010
    Messages:
    396
    Resources:
    12
    Models:
    6
    Maps:
    2
    Spells:
    3
    JASS:
    1
    Resources:
    12
    As you already know, powerups are not properly cleaned when they are picked. I have also found two other bugs with that :
    1) Items are not properly cleaned when they are attacked to death (not only powerups are concerned),
    2) Items are not destroyed when you use "RemoveItem" on a dead item. Indeed, the model is properly hid but the handle still exists and leaks.
    So, in order to properly destroy an item, you have to set its life to more than 0.405 before trying to remove it.

    This librairy cleans the dead items every 15 seconds ; that takes care of all the leak caused by items. There are two case in which items won't be cleaned when you use it : if you set an dead item's life to more than 0.405 or if you kill an item when it is carried. In those case, you should remove the item manually (you can properly remove an item if it was carried when it died).

    You could disguss the use of a periodic cleanup but you have to know that doing a system which cleans items when they die is much more complicated. With this, you can just put the script in your trigger editor and items will be automatically cleaned.

    WorldBounds Library
    Code (vJASS):

    library ClearItems requires optional WorldBounds
    /*
        This library fixes the leak and the lag caused by unremoved items,
        including powerups and manually-destroyed items.

        The dead items are periodically removed. You can adjust the period by changing
        the constant CLEANING_PERIOD. Note that items' death animations need some time
        to play so adjust the DEATH_TIME delay accordingly.

        If you don't know exactly what you are doing, you shouldn't change the life
        of a dead item; the items are no longer usable after their death but
        you can still change their life.  If you set their life to more than 0.405,
        they won't be properly cleaned. You should also remove items manually
        if you kill them when they are carried by a unit.
    */

       
        globals
            // Interval between item-cleanups.
            private constant real CLEANING_PERIOD = 15
       
            // Time for the item's death animation, optimized for tomes and runes.
            private constant real DEATH_TIME = 1.5
        endglobals
       
        globals
            private keyword S
            private integer N = 0
            private code s_code
            private boolexpr s_bool
            private timer s_timer = CreateTimer()
            private item array I
        endglobals
       
        private function DeleteItems takes nothing returns nothing
            loop
                set N = N - 1
                call SetWidgetLife(I[N], 1)
                call RemoveItem(I[N])
                set I[N] = null
                exitwhen (N == 0)
            endloop
            call TimerStart(s_timer, CLEANING_PERIOD - DEATH_TIME, true, s_code)
        endfunction
       
        private function CleanItems takes nothing returns boolean
            if (GetWidgetLife(GetFilterItem()) < 0.405) then
                set I[N] = GetFilterItem()
                set N = N + 1
            endif
            return false
        endfunction
       
        private function SweepItems takes nothing returns nothing
            static if (LIBRARY_WorldBounds) then
                call EnumItemsInRect(WorldBounds.world, s_bool, null)
            else
                call EnumItemsInRect(S.world, s_bool, null)
            endif
            if (N > 0) then
                call TimerStart(s_timer, DEATH_TIME, false, function DeleteItems)
            endif
        endfunction
       
        private struct S extends array
            static if (not LIBRARY_WorldBounds) then
                static rect world
            endif
            static method onInit takes nothing returns nothing
                static if (not LIBRARY_WorldBounds) then
                    set world = GetWorldBounds()
                endif
                set s_code = function SweepItems
                set s_bool = Filter(function CleanItems)
                call TimerStart(s_timer, CLEANING_PERIOD, true, s_code)
            endmethod
        endstruct
       
    endlibrary
     


    Thanks to Troll-Brain and Bribe who helped me to do it.
     
    Last edited: Jun 16, 2011
  2. watermelon_1234

    watermelon_1234

    Joined:
    Nov 18, 2007
    Messages:
    1,066
    Resources:
    10
    Spells:
    9
    JASS:
    1
    Resources:
    10
    Comments on second library:
    In function CleanupDeadItems, why don't you just use
    GetWorldBounds()
    directly instead of using a variable?
    I think you could also do that in the init function for the timer.

    Also, couldn't you have done something like this?
    Code (vJASS):
    private function CleanItem takes nothing returns nothing
        if GetWidgetLife(GetEnumItem())<0.405 then
        // Set the item's life to more than 0.405 in order to properly destroy it.
            call SetWidgetLife(GetEnumItem(),1)
            call RemoveItem(GetEnumItem())
        endif
    endfunction
    ...
    call EnumItemsInRect(r,null,function CleanItem)
     
    If you don't follow that suggestion, you should at least fix IsItemDead since it's using
    GetEnumItem
    instead of
    GetFilterItem
    .
     
  3. Tirlititi

    Tirlititi

    Joined:
    Jul 11, 2010
    Messages:
    396
    Resources:
    12
    Models:
    6
    Maps:
    2
    Spells:
    3
    JASS:
    1
    Resources:
    12
    Well, GetWorldBounds doesnt' return a variable so it causes leak. I know the function is called just once but I am used to make as few leak as I can.
    For The CreateTimer, you're right even if it doesn't change a lot ^^.

    Thanks, I thought I fixed it cause I already saw it.
    Using a filter and a callback makes the code more readable in my opinion. If you convince me that it is more important than it looks, I'll follow that suggestion :razz:.
     
  4. watermelon_1234

    watermelon_1234

    Joined:
    Nov 18, 2007
    Messages:
    1,066
    Resources:
    10
    Spells:
    9
    JASS:
    1
    Resources:
    10
    Code (vJASS):
    native GetWorldBounds takes nothing returns rect
    ?
    I just suggested that since I looked at LightLeaklessDamageDetect which uses GetWorldBounds directly.
     
  5. busterkomo

    busterkomo

    Joined:
    Jun 17, 2007
    Messages:
    1,423
    Resources:
    1
    Tutorials:
    1
    Resources:
    1
    GetWorldBounds() == GetWorldBounds() will return false. It does in fact leak. Just use a global. Also, the local timer variable isn't needed in the GUI friendly version. I find this to have few very, quirky uses, but the decision is up to a JASS moderator.
     
  6. azlier

    azlier

    Joined:
    Oct 3, 2008
    Messages:
    354
    Resources:
    4
    JASS:
    4
    Resources:
    4
    This would be much more helpful if it didn't require a custom creation function to utilize, and such a thing is very difficult to pull off.

    Also, I do believe there's one or two scripts out there that do the exact same thing. One is on WC3C. Exact copies of scripts for the sake of competition are never a good thing.
     
  7. Tirlititi

    Tirlititi

    Joined:
    Jul 11, 2010
    Messages:
    396
    Resources:
    12
    Models:
    6
    Maps:
    2
    Spells:
    3
    JASS:
    1
    Resources:
    12
    Well, as I said, the powerups bug is known but the script in wc3c doesn't fix the leak.
    Just put try to put a "BJDebugMsg(GetHandleId)" after the destruction of the item, and you'll see that it is not removed.

    I'm not specially for using such kind of librairies (and it's worse when it has already been done) but :
    1) The previous librairy only fix the half of what it is said to fix,
    2) This kind of strange bugs would normally be fixed/replaced by Blizzard but I'm not sure that they will actually release new patches for now.

    I would fully understand that you think these librairies are not enough usefull or simple in use to be approved but I think this bug is enough hard to guess to deserve a place where it is said.
     
  8. azlier

    azlier

    Joined:
    Oct 3, 2008
    Messages:
    354
    Resources:
    4
    JASS:
    4
    Resources:
    4
    Hmm. True, Vexorian's script only looks for the most common problem.

    Well then. Maybe the CreateItem function could have some name other than 2? Something that's still trivial to change, but actually gives some sort of description of what it does?
     
  9. Tirlititi

    Tirlititi

    Joined:
    Jul 11, 2010
    Messages:
    396
    Resources:
    12
    Models:
    6
    Maps:
    2
    Spells:
    3
    JASS:
    1
    Resources:
    12
    I'm ready to receive suggestions :grin:.

    I don't like "Ex" since it doesn't take more arguments than the native function. "Debug" would be nice for me but I think it overtones the debug mode.

    Maybe "NewItem" or something like that?
     
  10. busterkomo

    busterkomo

    Joined:
    Jun 17, 2007
    Messages:
    1,423
    Resources:
    1
    Tutorials:
    1
    Resources:
    1
    Just use CreateItemSafe or CreateItemEx. This would be way better if you could reference return values in hooks, but whatever. Oh, and once again use a global variable in the GUI friendly version to store GetWorldBounds().
     
  11. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,158
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    In the top script, you leak the local pointer 'it' within the CreateItemSafe function - the item pointer should be better off as a global.

    >> Just put try to put a "BJDebugMsg(GetHandleId)" after the destruction of the item, and you'll see that it is not removed.

    This is not a valid test. Handle pointers in JASS are 'smart', as in the handle id is not recycled until all pointers to it are wiped. Since you still have the variable pointing to the handle and pass that pointer into GetHandleId, the returned result will definitely still match the would-be decaying handle id.

    Unless you grabbed the item another way, such as from an enumerator, and the item still appeared, my above argument stands.
     
  12. Tirlititi

    Tirlititi

    Joined:
    Jul 11, 2010
    Messages:
    396
    Resources:
    12
    Models:
    6
    Maps:
    2
    Spells:
    3
    JASS:
    1
    Resources:
    12
    Waow, I didn't believe you when I read but it does leak. That's awesome. I fixed that.

    You can still enum "removed-when-dead" items and its datas (name, item-type, etc...) are not cleared so I confirm the bug. But thanks to warn (I thought only the "==null" was unsafe).
     
  13. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,158
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    Was that the instant it was removed or was it after a wait period?
     
  14. Tirlititi

    Tirlititi

    Joined:
    Jul 11, 2010
    Messages:
    396
    Resources:
    12
    Models:
    6
    Maps:
    2
    Spells:
    3
    JASS:
    1
    Resources:
    12
  15. azlier

    azlier

    Joined:
    Oct 3, 2008
    Messages:
    354
    Resources:
    4
    JASS:
    4
    Resources:
    4
    You should use simple item-in-map enumeration exclusively to clear dead items It appears to be the best method, as it doesn't require the user to change their function calls. It's also not terribly inefficient.

    Once you get that first script out of the way, you can increase the efficiency and compactness of the second one by jamming it all into the filter function and eliminating the second function. Like how lots of people use conditions instead of actions.

    Just do it every 30 seconds to few minutes and everything will be fine.
     
  16. Nestharus

    Nestharus

    Joined:
    Jul 10, 2007
    Messages:
    6,146
    Resources:
    8
    Spells:
    3
    Tutorials:
    4
    JASS:
    1
    Resources:
    8
    all in the filter function >.> like was suggested in another post

    Code (vJASS):

    private function IsItemDead takes nothing returns boolean
        return GetWidgetLife(GetFilterItem())<0.405
    endfunction

    private function CleanItem takes nothing returns nothing
        // Set the item's life to more than 0.405 in order to properly destroy it.
        call SetWidgetLife(GetEnumItem(),1)
        call RemoveItem(GetEnumItem())
    endfunction
     


    store
    Filter(function IsItemDead)
    into a boolexpr and just use the var instead of converting it every time as
    Condition(function IsItemDead)
     
  17. Tirlititi

    Tirlititi

    Joined:
    Jul 11, 2010
    Messages:
    396
    Resources:
    12
    Models:
    6
    Maps:
    2
    Spells:
    3
    JASS:
    1
    Resources:
    12
    I updated.

    Is that version ok?

    By the way, "Filter(function Code)==Filter(function Code)" returns true, so we don't need to make a global filter, no?
     
  18. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,158
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    I made a script from your original code which cleans everything up and has it running as optimally as possible.

    Functions CreateItemSafe and NewItem do the same thing.

    Code (vJASS):

    library ClearItems

    /*
        This library fixes the leak and the lag caused by non-removed items.
        This includes powerups and items that are destroyed by killing them
        (by attacking them or setting their life to less than 0.405).
        In order to use it, items must be created by the unique functions

            function CreateItemSafe takes integer rawcode, real x, real y returns item
            function NewItem takes integer rawcode, real x, real y returns item        ** NewItem and CreateItemSafe do the same thing.

        This function creates the item and the trigger needed to clean it after its death.
        Items are removed after a few wait (DEATH_TIME) to let them show their death animation.
        The default wait is 1.5 second, which is enough for all the Blizzard item models.
        If you set this value too high, tomes and runes will display a bright point
        until their destruction.
    */


        globals
            private constant real DEATH_TIME = 1.5 // Set to the highest duration of items' death animations.
        endglobals
       
        globals
            private hashtable Hash = InitHashtable() // For typecasting and reverse-array lookups.
            private trigger Trig = CreateTrigger() // A single dynamic trigger to handle all death events.
            private trigger Exec = CreateTrigger() // A semi-dynamic trigger.
            private boolexpr Eval = null
        endglobals
       
        private struct data
           
            static integer Size = 0
            static data array Stack
            static item array Items
           
            /**
             * To deallocate properly, some array searching is required.
             */

           
            static method add takes item it returns item
                if (it != null) then
                    set Size = Size + 1
                    set Stack[Size] = data.allocate()
                    set Items[Stack[Size]] = it
                    call SaveInteger(Hash, 1, GetHandleId(it), Size)
                    call TriggerRegisterDeathEvent(Trig, it)
                endif
                return it
            endmethod
           
            static method pop takes integer i returns nothing
                local integer id
               
                // clear old material;
                set Items[Stack[i]] = null
                call Stack[i].deallocate()
                call DestroyTrigger(Trig)
               
                // update reverse-lookup data;
                if (i != Size) then
                    set Stack[i] = Stack[Size]
                    set id = GetHandleId(Items[Stack[i]])
                    call RemoveSavedInteger(Hash, 1, id)
                    call SaveInteger(Hash, 1, id, i)
                endif
               
                // shrink the stack;
                set Size = Size - 1
               
                // remake the trigger;
                set Trig = CreateTrigger()
                call TriggerAddCondition(Trig, Eval)
                if (Size > 0) then
                    set i = Size
                    loop
                        call TriggerRegisterDeathEvent(Trig, Items[Stack[i]])
                        exitwhen i == 1
                        set i = i - 1
                    endloop
                endif
            endmethod
           
            static method remove takes nothing returns nothing
                local item it = LoadItemHandle(Hash, 0, 0) // item was typecast from widget.
                local integer id = GetHandleId(it)
               
                call data.pop(LoadInteger(Hash, 1, id))
               
                // delete hashtable entries;
                call RemoveSavedHandle(Hash, 0, 0)
                call RemoveSavedInteger(Hash, 1, id)
               
                // Add 0.50 for margin of error;
                call TriggerSleepAction(DEATH_TIME + 0.50)
               
                // Remove the item completely;
                call SetWidgetLife(it, 1.0)
                call RemoveItem(it)
                set it = null
            endmethod

            /**
             * Fires when a powerup is used or any item is removed or killed.
             */

            static method onDeath takes nothing returns boolean
                call SaveWidgetHandle(Hash, 0, 0, GetTriggerWidget()) // widget will be typecast to item.
                call TriggerExecute(Exec)
                return false
            endmethod

            static method onPurchase takes nothing returns boolean
                call data.add(GetSoldItem())
                return false
            endmethod
           
            static method onPawned takes nothing returns nothing
                local integer id = GetHandleId(GetManipulatedItem())
                local integer i = LoadInteger(Hash, 1, id)
                if (i > 0) then
                    call data.pop(i)
                    call RemoveSavedInteger(Hash, 1, id)
                endif
            endmethod
           
            static method onInit takes nothing returns nothing
                call TriggerRegisterAnyUnitEventBJ(Exec, EVENT_PLAYER_UNIT_PAWN_ITEM)
                call TriggerAddAction(Exec, function data.onPawned)
               
                set Exec = CreateTrigger()
                call TriggerRegisterAnyUnitEventBJ(Exec, EVENT_PLAYER_UNIT_SELL_ITEM)
                call TriggerAddCondition(Exec, Condition(function data.onPurchase))
               
                call TriggerAddAction(Exec, function data.remove)
               
                set Eval = Condition(function data.onDeath)
                call TriggerAddCondition(Trig, Eval)
            endmethod
           
        endstruct
       
        function CreateItemSafe takes integer rawcode, real x, real y returns item
            return data.add(CreateItem(rawcode, x, y))
        endfunction
       
        function NewItem takes integer rawcode, real x, real y returns item
            return data.add(CreateItem(rawcode, x, y))
        endfunction

    endlibrary
     
     
    Last edited: Aug 26, 2010
  19. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,158
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    You should use TriggerSleepAction instead of timers, as you simply do not need those timers whatsoever.

    A global filterfunc just makes your script execute more quickly. It reduces the need to call the Filter() native over and over.

    This revision of your current library has been made as optimal and foolproof as possible.

    Code (vJASS):

    library ClearItems initializer init

    /*
        This library fixes the leak and the lag caused by unremoved items,
        including powerups and manually-destroyed items.

        The dead items are periodically removed. You can adjust the period by changing
        the constant CLEANING_PERIOD. Note that items' death animations need some time
        to play so adjust the DEATH_TIME delay accordingly.

        If you don't know exactly what you are doing, you shouldn't change the life
        of a dead item; the items are no longer usable after their death but
        you can still change their life.  If you set their life to more than 0.405,
        they won't be properly cleaned. You should also remove items manually
        if you kill them when they are carried by a unit.
    */


        globals
            // Interval between item-cleanups
            private constant real CLEANING_PERIOD = 15.0

            // Time for the item's death animation, optimized for tomes and runes
            private constant real DEATH_TIME = 1.5
        endglobals
       
        globals
            private item ToDelete = null
            private trigger Delete = CreateTrigger()
        endglobals

        private function DeleteItem takes nothing returns nothing
            local item it = ToDelete
            call TriggerSleepAction(DEATH_TIME + 0.5) // add 0.5 for margin of error.
            call SetWidgetLife(it, 1.0)
            call RemoveItem(it)
            set it = null
        endfunction

        private function CleanItems takes nothing returns boolean
            set ToDelete = GetFilterItem()
            if (GetWidgetLife(ToDelete) < 0.405) then
                call TriggerExecute(Delete)
            endif
            return false
        endfunction
       
        globals
            private rect WorldBounds
            private filterfunc Filt
        endglobals
       
        private function SweepItems takes nothing returns nothing
            call EnumItemsInRect(WorldBounds, Filt, null)
        endfunction

        private function init takes nothing returns nothing
            set WorldBounds = GetWorldBounds()
            set Filt = Filter(function CleanItems)
            call TimerStart(CreateTimer(), CLEANING_PERIOD, true, function SweepItems)
            call TriggerAddAction(Delete, function DeleteItem)
        endfunction

    endlibrary
     
     
    Last edited: Aug 26, 2010
  20. azlier

    azlier

    Joined:
    Oct 3, 2008
    Messages:
    354
    Resources:
    4
    JASS:
    4
    Resources:
    4
    It's not advisable to use TriggerSleepAction because it waits in real time. And it does so with inaccuracy at that. Which means if there's a sudden lag spike, the wait keeps ticking. That results in odd bugs, which is often enough to even desync a replay from what actually happened.