Item Removal System v3.0.0.2

[Moderator]

12th Dec 2015
IcemanBo: For long time as NeedsFix. Rejected.

IcemanBo: http://www.hiveworkshop.com/forums/spells-569/item-removal-system-v3-0-0-2-a-256794/#post2640239

Older

11:04, 12th Sep 2014
PurgeandFire: Review:
http://www.hiveworkshop.com/forums/2586456-post11.html

 

[Vinz]

Very useful for dungeon crawlers and RPGs where players neglect picking up items/loot while fighting monsters.

4/5

 

[BPower]

Alloc is not listed as requirement, it's just written as a comment. The url of Alloc is not up to date.
I recommend you use Alloc as an optional requirement, because the only valid link to it is on GitHub at the moment.
This block doesn't make any sense.

            if not ProtectedItems.has( id) then
                call ProtectedItems.remove( id)
            endif

 

[Daffa]

Uh, is there any system like this before, I think I saw at least one before this...

 

[Almia]

Vexorian's TomeSentinel? I think you never heard that one.

 

[deathismyfriend]

Alloc is not listed as requirement, it's just written as a comment. The url of Alloc is not up to date.
I recommend you use Alloc as an optional requirement, because the only valid link to it is on GitHub at the moment.
This block doesn't make any sense.

            if not ProtectedItems.has( id) then
                call ProtectedItems.remove( id)
            endif

Thanks BPower didn't know about the little mistake in my code.

I forgot to remove Alloc as I never use the actual struct instances in this system. I will update when I can with the fixes and any new features I can think of or anyone else thinks of.

Uh, is there any system like this before, I think I saw at least one before this...

No idea. I didn't look. My old version of this was on the jass section but I removed it. Then I put it here.

Vexorian's TomeSentinel? I think you never heard that one.

Not sure who you are referring that statement to ?

 

[Chaosy]

Useful and simple to use. Perfect.

 

[deathismyfriend]

Thanks Chaosy

 

[Kazeon]

static method deProtectItem takes item itm returns nothing
deProtect seems childish. deprotect is better, personal taste.

            local integer id = GetHandleId( itm)
            if not isNotInTable(GetItemTypeId(itm), id) then
                call addItemToRemove( itm, TIME) // This adds the Manipulated item with a standard time.
            endif
            set unitHasItem.boolean[GetHandleId(itm)] = false

You seem to forgot something there

And does it automatically remove items that are created by trigger?

 

[deathismyfriend]

Thanks I didn't notice that mistake.

Also no it does not register items made through a trigger.

If you want these items added you need to call addItemToRemove( item, time)

 

[PurgeandFire]

Review:

• unitDataX/Y isn't really needed. You should just store the unit in the getNewItemDelay, and then use GetUnitX() and GetUnitY() in getNewItems. Better yet--you can avoid using hashtables at all, and just use an array and a static timer (which is better than having to create/destroy a timer each time):
  
  Code

          globals
              private timer newItems = CreateTimer()
              private unit array newItemUnits
              private integer newItemCount = 0
          endglobals
  
          //... later in the code ...
  
          private static method getNewItems takes nothing returns nothing
              loop
                  exitwhen newItemCount == 0
                  call MoveRectTo(ItemRect, GetUnitX(newItemUnits[newItemCount]), GetUnitY(newItemUnits[newItemCount]))
                  call EnumItemsInRect(ItemRect, null, function thistype.addEnumItems)
                  set newItemCount = newItemCount - 1
              endloop
          endmethod
  
          private static method getNewItemDelay takes nothing returns boolean
              set newItemCount = newItemCount + 1
              set newItemUnits[newItemCount] = GetTriggerUnit()
              call TimerStart(newItems, 0, false, function thistype.getNewItems)
          endmethod

• I'm a little iffy on the addItemToRemove/removeItem/cleanItemTable. Is it necessary to use all those hashtable functions? Couldn't you use a struct to store it, wait the duration, and then clean up? (correct me if I'm wrong though, I'm only looking at a glance).

Otherwise, it looks good.

 

[Kazeon]

Exactly, I was about to suggest you to use something like item indexer, but I haven't seen any approved one :/

 

[deathismyfriend]

An array with static timer would mean another table to keep track of several items going off at the same time. Also it would mean extra checks and a faster timer to make sure no items were missed.

UnitX/Y is needed. When I add the unit and try to use the unit it causes a bug. Whenever a unit gets removed before the item is checked its location reverts to 0,0. I need the time to make sure that the item gets dropped since an item being dropped is not immediate. I can use the structs in this case though. So I will switch it out to get rid of the tables that will no longer be needed.

Also it seems that there was a bug when uploading. The old map was uploaded not the new one. I hope to make changes soon. I think it would be better to stick with multiple timers though.

 

[IcemanBo]

static method setItemExpiration takes item itm, real seconds returns nothing
    // * this function allows you to add items when created by a trigger and adds a specific time to the item * /
    local integer id = GetHandleId( itm)
    if ProtectedItems.has(id) then
        call ProtectedItems.remove( id)
    endif
    if not unitHasItem.boolean[GetHandleId(itm)] then
        call addItemToRemove(itm, seconds)
    endif
endmethod

...

static method forceItemExpiration takes item itm, real seconds returns nothing
    set ItemTable[GetHandleId(itm)].boolean[3] = true
    call addItemToRemove(itm, seconds)
endmethod

^There is not explained when to use which one.

static method getItemExpiration takes item itm returns real
    // * this function allows you to add items when created by a trigger and   adds a specific time to the item * /
    return TimerGetRemaining(ItemTable[GetHandleId( itm)].timer[2])
endmethod

^Read the comment. It is wrong.

Is it explained at a place what these booleans should state? ItemTable[id].boolean[0/3]

At the moment you potentially create/destroy many timers, so you could consider using TimerUtils.
But why not just index needed items with help of an array and decrement a countdown for each item[index]? Then only one global timer was needed.
Table still could be used to work with itemTypes, but the whole removal structure will probably be improved with looping through an array.

^If not, you still could take actual usage of structs to bind data, like PnF suggested.

I'm certain using an ItemIndexer would also have benefits. (especially the usage of onIndex())

onInit it's okay that you do, but the triggers don't need to be nulled.
onInit you could store the player into a local variable when iterating.

It is more readable writing the whole name instead of making abbreviation and cutting 1 or 2 letters for names.

As this is not the jass section, it would be helpful for most users to provide a short demo.