No Item Sharing v1.11

This bundle is marked as approved. It works and satisfies the submission rules.
  • Like
Reactions: Apheraz Lucent
JASS:
library NoItemSharing /*
    
                        No Item Sharing v1.11 by Flux
    -------------------------------------------------------------------------
    This System disable players from selling other player's item.
    It also allows other players from not using a certain player's item
    and/or disable its Stats Bonus. 
    It also allows to change icons and tooltips of Shared Items for easily 
    indicating the difference in-game.
    -------------------------------------------------------------------------
    
    */ requires /*
        */ optional Table /*  http://www.hiveworkshop.com/forums/jass-resources-412/snippet-new-table-188084/
        If not found, this system will use a hashtable instead. Hashtables is limited to
        256 per map. If your map is close to the hashtable limit, it is recommended
        to use Table
    
    NOTES:
          - Dropping a Shared Item in the ground will turn it into the Original Item
            thus Shared Item only appear in Inventory. That way, you don't need to edit the Text
            Description (ides) of the Shared Version decreasing the overall purple data fields 
            of the map.
          - The Original Version and the Shared Version must be manually created at Object Editor
          - Useful in AoS, Hero Arena and Hero Defense Maps by not allowing team feeding to a certain
            player who will carry the whole team.
          - Added Bonus: Coincidentally make the Treasure Chest Play an opening animation when dropped.
          - To fully understand how the system works, test with DEBUG_SYSTEM = true and run in Debug Mode.
    
    -----------------------------------------------------------------------------------------
        Item Shared Version can either be:
         - Disabled Shared: The Shared Item is unuseable/provides no stats and cannot be sold
                            It is recommended to use a Disabled Icon for items like this.
                            Demo Example: Boots of Speed, Claws of Attack.
         - Usabled Shared:  The Shared Item can be used but cannot be sold. 
                            Demo Example: Clarity Potion
    -----------------------------------------------------------------------------------------
    
     ******************************************************************
     ***************************** API: *******************************
     ******************************************************************
        
        function ItemShare.data takes integer origId, integer shareId returns nothing
            - Tells the system the the shared item type of origId is sharedId.
        
        function ItemShare.getOwner takes item it returns player
            - Returns the owner of a certain item, returns null if item has no owner.
            - Returns null if item type is not in Database.
        
        function ItemShare.setOwner item it, player p returns nothing
            - Set the new owner of an item.
            - It automatically transform the items based on the new owner. If the new
              owner is null, the item is automatically dropped and the first player to
              pick it will be the new owner.
              
     ******************************************************************
     
     
  CREDITS:
     Bribe - Table
    
*/
    //===============================================================================
    //============================= CONFIGURATION ===================================
    //===============================================================================
    //! textmacro ITEM_SHARING_INITIAL_DATA
        //*** Input Item rawcodes here **
        //call ItemShare.data(original Item Type, shared Item Type)
        call ItemShare.data('I000', 'I001')
        call ItemShare.data('I002', 'I003')
        call ItemShare.data('I004', 'I005')
        call ItemShare.data('I006', 'I007')
        //** Add more here **
        //call ItemShare.data(original Item Type, shared Item Type)
        //........
    //! endtextmacro
    
    globals
        //Show Debug Messages to help you understand how the system works.
        private constant boolean DEBUG_SYSTEM = true
    endglobals
    //===============================================================================
    //=========================== END CONFIGURATION =================================
    //===============================================================================
    
    struct ItemShare extends array
        //Integer: alternate type (uses itemTypeId)
        //Player: owner (uses GetHandleId)
        //Boolean: status (uses itemTypeId) - true if original, false if shared
        static if LIBRARY_Table then
            private static Table tb
            private static Table holder
        else
            private static hashtable hash = InitHashtable() 
            
        endif
        
        private static trigger pickTrg = CreateTrigger()
        private static trigger dropTrg = CreateTrigger()
        
        static if DEBUG_SYSTEM then
            private static constant string prefix = "|cffffcc00[NoItemSharing]|r: "
        endif
        
        static method data takes integer origId, integer shareId returns nothing
            static if LIBRARY_Table then
                set tb[origId] = shareId
                set tb[shareId] = origId
                set tb.boolean[origId] = true     
                set tb.boolean[shareId] = false
            else
                call SaveInteger(hash, origId, 0, shareId)
                call SaveInteger(hash, shareId, 0, origId)
                call SaveBoolean(hash, origId, 0, true)
                call SaveBoolean(hash, shareId, 0, false)
            endif
        endmethod
        
        private static method convert takes nothing returns nothing
            local timer t = GetExpiredTimer()
            local integer handleId = GetHandleId(t)
            local real x
            local real y
            
            static if LIBRARY_Table then
                local item it = tb.item[handleId]
                local integer itemType1 = GetItemTypeId(it)
                local integer itemType2 = tb[itemType1]
                local boolean shared = not tb.boolean[itemType1] and tb.boolean[itemType2]
                local integer id = GetHandleId(it)
                local player owner = tb.player[id]
            else
                local item it = LoadItemHandle(hash, handleId, 0)
                local integer itemType1 = GetItemTypeId(it)
                local integer itemType2 = LoadInteger(hash, itemType1, 0)
                local boolean shared = not LoadBoolean(hash, itemType1, 0) and LoadBoolean(hash, itemType2, 0)
                local integer id = GetHandleId(it)
                local player owner = LoadPlayerHandle(hash, id, 0)
            endif
            
            if shared and not IsItemOwned(it) then
                set x = GetItemX(it)
                set y = GetItemY(it)
                static if DEBUG_SYSTEM then
                    call BJDebugMsg(prefix + "|cffffcc00" + GetItemName(it) + "|r is converted to its original form")
                endif
                call RemoveItem(it)
                set it = CreateItem(itemType2, x, y)
                static if LIBRARY_Table then
                    call tb.player.remove(id)
                    set tb.player[GetHandleId(it)] = owner
                else
                    call RemoveSavedHandle(hash, id, 0)
                    call SavePlayerHandle(hash, GetHandleId(it), 0, owner)
                endif
            endif
            
            static if LIBRARY_Table then
                call tb.item.remove(handleId)
            else
                call RemoveSavedHandle(hash, handleId, 0)
            endif
            
            set it = null
            call DestroyTimer(t)
            set t = null
            set owner = null
        endmethod
        

        private static method onDrop takes nothing returns boolean
            local item it = GetManipulatedItem()
            local integer itemType1 = GetItemTypeId(it)
            local integer id = GetHandleId(it)
            local timer t
            
            static if LIBRARY_Table then
                local integer itemType2 = tb[itemType1]
                if not tb.boolean[itemType1] and tb.boolean[itemType2] then
                    static if DEBUG_SYSTEM then
                        call BJDebugMsg(prefix + GetPlayerName(GetTriggerPlayer()) + " has dropped a shared Item belonging to " + GetPlayerName(tb.player[GetHandleId(it)]))
                    endif
                    set t = CreateTimer()
                    set tb.item[GetHandleId(t)] = it
                    call TimerStart(t, 0.0, false, function thistype.convert)
                    set t = null
                else
                    static if DEBUG_SYSTEM then
                        call BJDebugMsg(prefix + GetPlayerName(GetTriggerPlayer()) + " has dropped an original Item belonging to " + GetPlayerName(tb.player[GetHandleId(it)]))
                    endif
                endif
            else
                local integer itemType2 = LoadInteger(hash, itemType1, 0)
                if not LoadBoolean(hash, itemType1, 0) and LoadBoolean(hash, itemType2, 0) then
                    static if DEBUG_SYSTEM then
                        call BJDebugMsg(prefix + GetPlayerName(GetTriggerPlayer()) + " has dropped a shared Item belonging to " + GetPlayerName(LoadPlayerHandle(hash, GetHandleId(it), 0)))
                    endif
                    set t = CreateTimer()
                    call SaveItemHandle(hash, GetHandleId(t), 0, it)
                    call TimerStart(t, 0.0, false, function thistype.convert)
                    set t = null
                else
                    static if DEBUG_SYSTEM then
                        call BJDebugMsg(prefix + GetPlayerName(GetTriggerPlayer()) + " has dropped an original Item belonging to " + GetPlayerName(LoadPlayerHandle(hash, GetHandleId(it), 0)))
                    endif
                endif
            endif
            
            static if LIBRARY_Table then
                call holder.unit.remove(id)
            else
                call RemoveSavedHandle(hash, id, 1)
            endif
            
            set it = null
            return false
        endmethod
        
        private static method onPick takes nothing returns boolean
            local item it = GetManipulatedItem()
            local unit u = GetTriggerUnit()
            local integer id = GetHandleId(it)
            local player trigPlayer = GetTriggerPlayer()
            local integer itemType1 = GetItemTypeId(it)
            
            static if LIBRARY_Table then
                local integer itemType2 = tb[itemType1]
                local boolean orig = tb.boolean[itemType1] and not tb.boolean[itemType2]
                local boolean shared = not tb.boolean[itemType1] and tb.boolean[itemType2]
                local player prevPlayer = tb.player[id]
            else
                local integer itemType2 = LoadInteger(hash, itemType1, 0)
                local boolean orig = LoadBoolean(hash, itemType1, 0) and not LoadBoolean(hash, itemType2, 0)
                local boolean shared = not LoadBoolean(hash, itemType1, 0) and LoadBoolean(hash, itemType2, 0)
                local player prevPlayer = LoadPlayerHandle(hash, id, 0)
            endif
            
            static if DEBUG_SYSTEM then
                call BJDebugMsg(prefix + GetPlayerName(trigPlayer) + " has picked |cffffcc00" + GetItemName(it) + "|r which originally belongs to " + GetPlayerName(prevPlayer))
            endif
            
            //If item is in the database
            if (orig or shared) then
                //If item has no owner
                if prevPlayer == null then
                    static if LIBRARY_Table then
                        set tb.player[id] = trigPlayer
                    else
                        call SavePlayerHandle(hash, id, 0, trigPlayer)
                    endif
                    static if DEBUG_SYSTEM then
                        call BJDebugMsg(prefix + GetPlayerName(trigPlayer) + " is the now the owner of |cffffcc00" + GetItemName(it) + "|r")
                    endif
                endif
                
                //orig -> shared will happen if a player happens to pick an item belonging to other players
                //shared -> orig will happen if you're the owner or if there is no owner or if you're the owner
                if (orig and trigPlayer != prevPlayer and prevPlayer != null) or (shared and (prevPlayer == null or trigPlayer == prevPlayer)) then
                    //Convert
                    call DisableTrigger(pickTrg)
                    call DisableTrigger(dropTrg)                
                    static if DEBUG_SYSTEM then
                        call BJDebugMsg(prefix + "|cffffcc00" + GetItemName(it) + "|r is converted")
                    endif
                    call RemoveItem(it)
                    set it = CreateItem(itemType2, 0, 0)
                    call UnitAddItem(u, it)
                    static if LIBRARY_Table then
                        call tb.player.remove(id)
                        set tb.player[GetHandleId(it)] = prevPlayer
                    else
                        call RemoveSavedHandle(hash, id, 0)
                        call SavePlayerHandle(hash, GetHandleId(it), 0, prevPlayer)
                    endif
                    call EnableTrigger(pickTrg)
                    call EnableTrigger(dropTrg)
                endif
                
                //Save Unit holding the item to have O(1) operation for unit dropping the item
                static if LIBRARY_Table then
                    set holder.unit[GetHandleId(it)] = u
                else
                    call SaveUnitHandle(hash, GetHandleId(it), 1, u)
                endif
            else
                static if DEBUG_SYSTEM then
                    call BJDebugMsg(prefix + "|cffffcc00" + GetItemName(it) + "|r is not in the database. It will treate it as a normal item")
                endif
            endif
            
            set it = null
            set u = null
            set trigPlayer = null
            set prevPlayer = null
            return false
        endmethod
        
        private static method onInit takes nothing returns nothing
            local integer i = 0
            loop
                call TriggerRegisterPlayerUnitEvent(pickTrg, Player(i), EVENT_PLAYER_UNIT_PICKUP_ITEM, null)
                call TriggerRegisterPlayerUnitEvent(dropTrg, Player(i), EVENT_PLAYER_UNIT_DROP_ITEM, null)
                set i = i + 1
                exitwhen i == bj_MAX_PLAYER_SLOTS
            endloop
            call TriggerAddCondition(pickTrg, Condition(function thistype.onPick))
            call TriggerAddCondition(dropTrg, Condition(function thistype.onDrop))
            static if LIBRARY_Table then
                set tb = Table.create()
                set holder = Table.create()
            endif
            //! runtextmacro ITEM_SHARING_INITIAL_DATA()
        endmethod
        
        static method getOwner takes item it returns player
            static if LIBRARY_Table then
                return tb.player[GetHandleId(it)]
            else
                return LoadPlayerHandle(hash, GetHandleId(it), 0)
            endif
        endmethod
        
        static method setOwner takes item it, player p returns nothing
            local integer itemType1 = GetItemTypeId(it)
            local unit u
            
            static if LIBRARY_Table then
                local integer itemType2 = tb[itemType1]
                local boolean orig = tb.boolean[itemType1] and not tb.boolean[itemType2]
                local boolean shared = not tb.boolean[itemType1] and tb.boolean[itemType2]
            else
                local integer itemType2 = LoadInteger(hash, itemType1, 0)
                local boolean orig = LoadBoolean(hash, itemType1, 0) and not LoadBoolean(hash, itemType2, 0)
                local boolean shared = not LoadBoolean(hash, itemType1, 0) and LoadBoolean(hash, itemType2, 0)
            endif
            
            if orig or shared then
                if p == null then  
                    static if LIBRARY_Table then
                        call tb.player.remove(GetHandleId(it))
                    else
                        call RemoveSavedHandle(hash, GetHandleId(it), 0)
                    endif
                else
                    static if LIBRARY_Table then
                        set tb.player[GetHandleId(it)] = p
                    else
                        call SavePlayerHandle(hash, GetHandleId(it), 0, p)
                    endif
                endif    
                
                //Refresh Item Status
                if IsItemOwned(it) then
                    static if LIBRARY_Table then
                        set u = holder.unit[GetHandleId(it)]
                    else
                        set u = LoadUnitHandle(hash, GetHandleId(it), 1)
                    endif
                    call UnitRemoveItem(u, it)
                    if p != null then
                        call UnitAddItem(u, it)
                    endif
                    set u = null
                endif
            endif
        endmethod
    endstruct

endlibrary


Changelog:
v1.00 - [7 July 2015]
- Initial Release

v1.01 - [7 July 2015]
- Uses Table instead to achieve O(1) operation

v1.02 [7 July 2015]
- Uses Table to store owner of Item instead of using ItemUserData
- Removed some unused variables
- Uses Disabled Icon on unuseable Shared Item.

v1.03 [13 July 2015]
- Fixed bug when obtaining an item not in the Database
- Fixed bug when passing a shared Item from player to another player (directly without dropping) who is also not the original owner.
- Added a lot of useful debug messages.
- Added a GetItemOwner API
- Improved test map demo.

v1.04 [27 July, 2015]
- Non-error messages placed in static ifs
- Used method Table.has to remove the +1 in PlayerId.

v1.05 [7 August, 2015]
- Made some code scripting changes like variable naming, debugging, etc.

v1.10 [28 May 2016]
- Completely rewrite the system.
- Made Table an optional requirement.
- Improved the coding.
- Changed API name.
- Added a function to allow users to change the owner of the item.

v1.11 [4 June 2016]
- ItemShare.setOwner now only works on items in the database.
- ItemShare.setOwner when item is owned is now an O(1) operation. It will no longer enumerate each unit to find the unit holding the item.
- Removed unnecessary disabling and re-enabling of triggers in method convert.

Keywords:
restrict, item, sharing, share
Contents

No Item Sharing (Map)

Reviews
Reviewer:KILLCIDE Time:12:06 Date:3 June 2016 Review:Link Status:Approved 19:46 - 28 May 2016 KILLCIDE: Status set to Pending on author's request. No Item Sharing v1.04 | Reviewed by BPower | 27.07.2015 Concept[/COLOR]] Allows...

Moderator

M

Moderator


Reviewer:
KILLCIDE

Time:
12:06

Date:
3 June 2016

Review:
Link

Status:
Approved


19:46 - 28 May 2016
KILLCIDE: Status set to Pending on author's request.



No Item Sharing v1.04 | Reviewed by BPower | 27.07.2015

[COLOR="gray"

[COLOR="gray"

[COLOR="gray"

[COLOR="gray"

Concept[/COLOR]]
126248-albums6177-picture66521.png
Allows to restrict items to one item owner.

Very useful for competitive teamplay maps, so items can't be shared,
abusive between multiple units. I.e. a supporter buys items for a carring hero.

Not really new in concept, hence a useful rating.
Code[/COLOR]]
126248-albums6177-picture66521.png
  • The system is MUI, leakless and working.
  • A function with unlocks an item owner would be useful. ( Allows a new owner )
  • For reasons of completeness I will mention a few things,
    but you don't have to apply changes in the code.
126248-albums6177-picture66523.png
  • I would still use the debug keyword, as debug only code should only be compiled in DEBUG_MODE.
  • PickUpTrigger & DropTrigger should be labeled pickUpTrigger & dropTrigger ( JPAG naming convention )
  • set itemName = GetItemName(it) is an not really required local. It's debug code only anyways.
  • The amount of locals you allocate in OnPickUp could be reduced ( itNew, itemInDatabase, itemName ), however it's no big deal as it is now.
  • function OnDrop doesn't really need a local integer for GetHandleId(item), you could pass that argument directly into function IsItemShared.
  • In ConvertInGround you should do exitwhen index == 0 and set index = index - 1 instead of using local integer i as loop index.
Demo Map[/COLOR]]
126248-albums6177-picture66523.png
  • The demo map demonstrates the system. Good job!
Rating[/COLOR]]
CONCEPTCODEDEMO MAPRATINGSTATUS
3/5
3.5/5
4/5
3.5/5
APPROVED
 
Level 9
Joined
Dec 3, 2011
Messages
366
Store data in the hashtable or http://www.hiveworkshop.com/forums/jass-resources-412/snippet-new-table-188084/
So that you can get shared/original id directly. Instead of use a timer I recommend you use Timer Utils or you store timer data in hashtable/table.
JASS:
local timer t = CreateTimer()
set ItemConvert_Table[GetHandleId(....)] = ....
call TimerStart(t,0, ...)

Use 0.0 instead of 0.01 with the timer.
 
Level 22
Joined
Feb 6, 2014
Messages
2,468
Store data in the hashtable or http://www.hiveworkshop.com/forums/jass-resources-412/snippet-new-table-188084/
So that you can get shared/original id directly. Instead of use a timer I recommend you use Timer Utils or you store timer data in hashtable/table.
JASS:
local timer t = CreateTimer()
set ItemConvert_Table[GetHandleId(....)] = ....
call TimerStart(t,0, ...)

Use 0.0 instead of 0.01 with the timer.

Thanks for the tip but I'm not really accustomed in using Hashtable. Besides, aren't Hashtable slower than indexing.

The Timer 0.0 or 0.01 won't matter because the human eye will not notice the difference. Even if I put 1.00 there, it will still work. That is just the delay for the item to be replaced.
 
Level 9
Joined
Dec 3, 2011
Messages
366
Thanks for the tip but I'm not really accustomed in using Hashtable. Besides, aren't Hashtable slower than indexing.

The Timer 0.0 or 0.01 won't matter because the human eye will not notice the difference. Even if I put 1.00 there, it will still work. That is just the delay for the item to be replaced.

You now that indexing/looping has a limit. Imagine, a map with 500 items. Indexing must be slower than hashtable (you know).

0 is true. What if two player pick up an item at the same time (time<0.01?).
 
Level 22
Joined
Feb 6, 2014
Messages
2,468
You now that indexing/looping has a limit. Imagine, a map with 500 items. Indexing must be slower than hashtable (you know).
Ok, but even if I used hashtable, I will still need to loop since I need to check if the itemType is equal to one of the itemShared[x].

0 is true. What if two player pick up an item at the same time (time<0.01?).

I tested it and it will still work. Besides that timer runs when unit drops an item not picked an item. I tried replacing 0.01 by 5.00 and dropping all items I can drop and it still works (though there is a 5 second delay before the item get converted). Still, I will change that to 0.0 in the next update.
 
Level 9
Joined
Dec 3, 2011
Messages
366
http://www.hiveworkshop.com/forums/jass-resources-412/snippet-new-table-188084/

JASS:
globals
private Table tb
endglobals

function SaveItemSharingData takes integer originalItem, integer sharedItem returns nothing
set tb[originalItem] = sharedItem
set tb[sharedItem] = originalItem
set tb.boolean[sharedItem] = true
set tb.boolean[originalItem] = false
endfunction

function AddNewItem takes nothing returns nothing
local timer t = GetExpiredTimer()
local integer it = tb[GetHandleId(...)]
...
...
...

endfunction

function onPickUp takes nothing returns boolean
local item i = ...
local unit u = ...
local player p = Player(tb[GetHandleId(i))
local timer t = CreateTimer()
if p != GetOwningPlayer(u) then
set tb[GetHandleId(t)] = GetItemTypeId(i)
call RemoveItem(i)
call TimerStart(t,0,false, ....)
endif
return false
endfunction

function init takes nothing returns nothing
set tb = Table.create()
call SaveItemSharingData(.... , ....)
endfunction

This would be better.

If an item is shared item, tb.boolean[GetItemTypeId(i)] will return true and reverse.
 
Level 22
Joined
Feb 6, 2014
Messages
2,468
http://www.hiveworkshop.com/forums/jass-resources-412/snippet-new-table-188084/

JASS:
globals
private Table tb
endglobals

function SaveItemSharingData takes integer originalItem, integer sharedItem returns nothing
set tb[originalItem] = sharedItem
set tb[sharedItem] = originalItem
set tb.boolean[sharedItem] = true
set tb.boolean[originalItem] = false
endfunction

function AddNewItem takes nothing returns nothing
local timer t = GetExpiredTimer()
local integer it = tb[GetHandleId(...)]
...
...
...

endfunction

function onPickUp takes nothing returns boolean
local item i = ...
local unit u = ...
local player p = Player(tb[GetHandleId(i))
local timer t = CreateTimer()
if p != GetOwningPlayer(u) then
set tb[GetHandleId(t)] = GetItemTypeId(i)
call RemoveItem(i)
call TimerStart(t,0,false, ....)
endif
return false
endfunction

function init takes nothing returns nothing
set tb = Table.create()
call SaveItemSharingData(.... , ....)
endfunction

This would be better!

Oh, i finally get what you mean. Yeah storing it would be better so I can get it directly. That way, I don't have to loop.
 
Level 22
Joined
Feb 6, 2014
Messages
2,468
Remove Global timer.

Create a local timer t

Store data to that timer:
JASS:
set tb.item[GetHandleId(timer)] = whichItem
set tb.unit[GetHandleId(item)] = whichUnit

No more index and global varibles

I agree on the no more global item variable.
You still haven't tell me why global timer is bad. Plus, when local timer, I have to destroy it.
 
Level 22
Joined
Feb 6, 2014
Messages
2,468
Your method is use a global timer and indexing item each time timer expired. Not safe and slower. Use local timer and save data, that's a better method. :ogre_love:

The timer is not periodic and is only used because of the delay of the placement of items in the ground. Since I need to replace the sharedItems on the ground but cannot do so immediately (because that items dropped are still not on the ground thus it has no x and y), I used a timer with 0.0 time . When the timer expires, the dropped sharedItems is now on the map (it now has x and y coordinate position) and now I can replace it. I don't see the point of making it a local timer.

I will wait what other people think about it because I don't see the point in making it a local timer. In fact I'm changing my mind about the indexing part. It is not slow and most of the time, the index is just equal to 1 so the loop only run once. The loop only runs more than once (index > 1) when items are dropped at the same time, like in the demo when you killed some heroes carrying a lot of items that drops on death.
 
Level 22
Joined
Feb 6, 2014
Messages
2,468
TimerStart(timer, 0.0) it isn't 0.0 second.
I don't get what you mean here? it isn't 0.0 second? what is?



Is convert on ground needed?

I just wanted it to be converted on ground because it may cause some inconsistency. Example, Player1 has an original version and dropped it (that item appears as an original version on the ground). Player 2 takes it and it is now a shared version. When Player 2 drops it, it will appear as a Shared Version on the ground when the mouse is hovered in that item. I just want it to always appear as the original version when on the ground. Perhaps I can make a static if to a constant boolean set by the downloader if he will use this feature or not.
 
Level 22
Joined
Feb 6, 2014
Messages
2,468
TimerStart(t,0.0,false, whichFunc) take a short time to execute whichFunc :ogre_frown:

So 0.0 is too short so I should changed it to 0.01? Btw, I initially had a 0.01 time on the timer and you said to use 0.0.
Using 0.0 works fine.

P/s: clear your globals

What globals to be exact? I nulled globalTempItem
set globalTempItem[i] = null in the ActualConversionInGround function.
 
Some notes:

What probably bothers me most after reading the code is that the system works with item creation and removal.
ItemTypeId also won't be the same anymore, as it's actually a new item type.
^With these 2 approaches I can see user's other code not be working correctly anymore,
as he might lose references without even knowing/ taking notice of it.

Config part should be marked/explained a bit better.
We can't really expect the normal user to go through code until he finds and identify the config.

local integer pN = GetPlayerId(GetTriggerPlayer()) + 1 //player number
^Why even + 1?

Instead of directly indexing all items being dropped, I would personaly attach
the item to the timer so you don't have to work with loops in the timer callback.

What do you think about making it library and adding API like "GetItemOwner"?

JASS:
//If the you happens to be the owner of the item you picked
            if ( pN == iN ) then
^Sorry, what?

JASS:
//if the item picked up is SHARED, then convert to ORIGINAL Version
    if (tb.boolean[itemType]) then
^How can this happen? In description is written that shared items only appear in inventory -> can't be picked up.

JASS:
//  This System sets the Custom Value of Items to the Player Number of the owner of that item.
^Outdated. :p

Just a note for you. When working with privates your names don't have to be very unique anymore.
Descriptive, yes. Unique, no. Just in case you would want to change something like "itemConversionTimer" -> "tim" , "clock", or anything similar.
On other site too short names also decrease readability sometimes, I can understand now because of easy context,
but me for example would not do "tb" but -> "table". 3 letters more don't hurt me.

function ConvertItemInInventory
->
function PickUp
============================
function ConvertItemInGround
->
function Drop

^I think such a concept is better, because the name directly implies
the correspondig event and so is easier to understand for the coder and reader.

JASS:
/*  HOW TO USE:
        1. Make the items (Original and Shared) in the Object Editor.
It probably may bother users a bit they have to create a dummy item for each item used in game.
Imagine a map might use 50+ items. Quite a lot of work. But we probably won't find an easy solution. :/

=======================================

Besides other notes I want to discuss the very first point, as I think it's important.
Please defend. Or anyone else.

Edit:

Uhm, actually, I also think the system does not handle item-giving from hero to hero?
 
Last edited:
Level 22
Joined
Feb 6, 2014
Messages
2,468
Some notes:

What probably bothers me most after reading the code is that the system works with item creation and removal.
ItemTypeId also won't be the same anymore, as it's actually a new item type.
^With these 2 approaches I can see user's other code not be working correctly anymore,
as he might lose references without even knowing/ taking notice of it.

What? Why won't it be the same? The item types are constant that are manually placed by the user depending on their map's item types.

Config part should be marked/explained a bit better.
We can't really expect the normal user to go through code until he finds and identify the config.

I placed a How to Use. You don't need need to go through the codes, just replace the item types in
JASS:
//INPUT ITEM RAWCODES HERE
        call SaveItemSharingData('I000', 'I001')
        call SaveItemSharingData('I002', 'I003')
        call SaveItemSharingData('I004', 'I005')
        call SaveItemSharingData('I006', 'I007')
        //ADD MORE HERE
        //call SaveItemSharingData(..., ...)

local integer pN = GetPlayerId(GetTriggerPlayer()) + 1 //player number
^Why even + 1?
Because iN = tb.integer[id] are initially zero and having a value of zero means it has no owner yet. It will conflict with Player 1 -> Player(0) if I did not use +1.

Instead of directly indexing all items being dropped, I would personaly attach
the item to the timer so you don't have to work with loops in the timer callback.
Ok.

What do you think about making it library and adding API like "GetItemOwner"?
Sure thing but I don't see the relation.

JASS:
//If the you happens to be the owner of the item you picked
            if ( pN == iN ) then
^Sorry, what?
The integer iN = owner of that item in integer form. If you picked an item and the converted player number is equal to iN, then you are the owner of that item.

JASS:
//if the item picked up is SHARED, then convert to ORIGINAL Version
    if (tb.boolean[itemType]) then
^How can this happen? In description is written that shared items only appear in inventory -> can't be picked up.
When a Unit directly dropped the Shared Item to another player's unit.
This is for Item giving from hero to hero

JASS:
//  This System sets the Custom Value of Items to the Player Number of the owner of that item.
^Outdated. :p
Noted.


Just a note for you. When working with privates your names don't have to be very unique anymore.
Descriptive, yes. Unique, no. Just in case you would want to change something like "itemConversionTimer" -> "tim" , "clock", or anything similar.
On other site too short names also decrease readability sometimes, I can understand now because of easy context,
but me for example would not do "tb" but -> "table". 3 letters more don't hurt me.
Okay sure.

function ConvertItemInInventory
->
function PickUp
============================
function ConvertItemInGround
->
function Drop

^I think such a concept is better, because the name directly implies
the correspondig event and so is easier to understand for the coder and reader.
Will do.

JASS:
/*  HOW TO USE:
        1. Make the items (Original and Shared) in the Object Editor.
It probably may bother users a bit they have to create a dummy item for each item used in game.
Imagine a map might use 50+ items. Quite a lot of work. But we probably won't find an easy solution. :/
Well there is no other way. They have no choice but to create another item. It is just copy paste from Object Manager.


Edit:

Uhm, actually, I also think the system does not handle item-giving from hero to hero?
It does.
 
Why won't it be the same?
I mean that if you use variables pointing on the item, it will have wrong result.
Also the HandleId will be a new one.
Actual item will be removed, a new one will be created -> variables will be point to null.
For item type I mean, for example you want to check if the hero has a potion[small], but now maybe he has the shared version of this item, but which he can also use.
The check would fail because the type id differs.

just replace the item types (in config)
I mean for example an explaination of the parameters would be useful.
If there exists a config it is always good if it's explained what to do and what does it affect.

initially zero
ok

I don't see the relation.
I just thought of if if user wants to read out the actual owner,
I mean it is anyway saved in already, so it would just allow to read it out.

When a Unit directly dropped the Shared Item to another player's unit.
This is for Item giving from hero to hero
ok

Achso, then sorry. Good:).
 
Level 22
Joined
Feb 6, 2014
Messages
2,468
I mean that if you use variables pointing on the item, it will have wrong result.
Also the HandleId will be a new one.
Actual item will be removed, a new one will be created -> variables will be point to null.
What variables will point to null? I didn't used the stoed variable Handle Id for the new item. However I forgot to null the tb.integer of the removed item.
JASS:
set itNew = CreateItem(tb[itemType], GetUnitX(u), GetUnitY(u))
call UnitAddItem(u, itNew)
set tb.integer[GetHandleId(itNew)] = iN //Store to the new item who is the owner


For item type I mean, for example you want to check if the hero has a potion[small], but now maybe he has the shared version of this item, but which he can also use.
The check would fail because the type id differs.

But determining whether an item is SHARED or ORIGINAL is dependent on
tb.boolean[itemType]. <- If that is TRUE, the it is shared, if that is FALSE, then it is original and the values are constant. For example you want to check if the hero has a potion having itemType of 'I001'. Then he dropped it and another player picked it. As that player picks the item, it will check who is the owner of that item. Since he is not the owner (pN != iN) and the itemType is 'I001', this if-condition is TRUE -> if not(tb.boolean[itemType]) then as defined earlier in SaveItemSharingData. Then the actions below would run
JASS:
if not(tb.boolean[itemType]) then
    call RemoveItem(it)
    set itNew = CreateItem(tb[itemType], GetUnitX(u), GetUnitY(u))
    call UnitAddItem(u, itNew)
    set tb.integer[GetHandleId(itNew)] = iN
    set itNew = null
endif
and the item gets converted into SHARED Version.

I will not submit this if it is not working. I fully tested it with every possibilities I can think of and it works fine, though if I miss something or if there is a bug, please tell.

EDIT: There is still an existing bug and I will update it as soon as I fully understand what you mean IcemanBo. The bug was if the acquired item is not defined in SaveItemSharingData. for example I picked an item with rawcode 'I016' but I did not put a call SaveItemSharingData('I016', ...), it will be removed as soon as it is picked. This is because of the default values of booleans which is equal to false thus the system assumed it is an original item and replaced by an undefined shared item.
 
Level 22
Joined
Feb 6, 2014
Messages
2,468
With variable, I mean this for example.

Ahh I get what you mean. If example I store the important item to a variable, that variable will no longer be pointing to that original item after the items are passed around to the players. Hmmm, quest items are mostly for RPGs and Campaigns and not for AoS and Hero Arenas that's why I completely overlook that part. But I will try to find a solution such that the original item is not completely removed.
 
Level 19
Joined
Mar 18, 2012
Messages
1,717
Quite known and useful concept for a system. It's often required to maintain a good
balance in team based maps like i.e. DotA.

OnItemDrop, OnItemPickUp, OnDrop, OnPickUp would be much more generic, better names than "ConvertItemInInventory" and "ConvertItemInGround" are.

local integer pN = GetPlayerId(GetTriggerPlayer()) + 1 //player number
I see what you do here, you can't save 0 as 0 represents "noOwner" in your system.
But there are different more intuitive ways to use an hashtable/Table:

Table covers the whole hashtable API including HaveSavedInteger.
The method in question is table.has(integer). It's better to use, as 0 can also be
a intentional value saved on a hashtable child key. Simply use table.remove(integer)
to remove the entry from the Table, so table.has(integer) returns false again.

Removing/Re-creating items vs hidding them somewhere on the map edge ...
It's not easy to say which is better to do. Basically it depends on other map systems.
If you have additional systems which rely on item HandleId or use ItemUserData,
removing an item can be critical. You should mention this somewhere in your header.

Your description system is outdated.

The Table instance tb could/should be created in your OnInit function.
Of course before calling InitializeData()

It's not a real problem at all, but itemConversionTimer is a quite long name.
Why not go with clock, tim, ...

set tb.integer[GetHandleId(itNew)] = iN and it= null could be moved out of the if then else condition.

Set to Need Fix for now.
 
Level 22
Joined
Feb 6, 2014
Messages
2,468
local integer pN = GetPlayerId(GetTriggerPlayer()) + 1 //player number
I see what you do here, you can't save 0 as 0 represents "noOwner" in your system.
But there are different more intuitive ways to use an hashtable/Table:

Table covers the whole hashtable API including HaveSavedInteger.
The method in question is table.has(integer). It's better to use, as 0 can also be
a intentional value saved on a hashtable child key. Simply use table.remove(integer)
to remove the entry from the Table, so table.has(integer) returns false again.

Hmmm, 0 will never be an intentional value so I guess I will have to stick to using this than using table.has(integer).
I don't see the harm done in using local integer pN = GetPlayerId(GetTriggerPlayer()) + 1 //player number

set tb.integer[GetHandleId(itNew)] = iN and it= null could be moved out of the if then else condition.
Why? What if the boolean condition is false, then that will still run
 
Last edited:
Level 22
Joined
Feb 6, 2014
Messages
2,468
Spell updated to v1.3

if table.has(...) then
endif
I know, but why use another function call when you can just compare the integer variable. Besides it works as it is intended.


Issues left:
IcemanBo said:
the system works with item creation and removal.
If a Quest Item (e.g. key) is in the ItemSharing Database, and it requires that specific item variable to proceed on a Quest, it can make the Quest impossible to achieve when the Quest Item is passed around players. This is because the Quest Item variable is pointing to a removed item.

However, as BPower stated:
BPower said:
Removing/Re-creating items vs hidding them somewhere on the map edge ...
It's not easy to say which is better to do. Basically it depends on other map systems.
This system is designed for Aos, Hero Arena, Hero Defense where team balance is critical. If an important quest item variable is present, then don't store it in the No Item Sharing System Database.
 
Level 22
Joined
Feb 6, 2014
Messages
2,468
That make your code hard to read and .....
Hmmm, will it? I have provided enough comments for that.

P/s: Your system must be extended.
Well I added one API so it's something. Besides, if I will have the system extended, it will not focus more on the No ITem Sharing but instead actual Item Systems. I want to focus on No Item Sharing and nothing else.
 
Your 1-timer method approach is a bit on the dark side. :D
But since you loop through all instances onExpire it should work in a real game without problems.
But I wonder if the timer is even needed. I mean items might be directly removed after drop,
and a new item might be created, why is indexing and starting a timer needed?

I would want to have noted that it won't be the actual item anymore, because you work with creation/removal.
 
Level 22
Joined
Feb 6, 2014
Messages
2,468
Your 1-timer method approach is a bit on the dark side. :D
But since you loop through all instances onExpire it should work in a real game without problems.
But I wonder if the timer is even needed. I mean items might be directly removed after drop,
and a new item might be created, why is indexing and starting a timer needed?

I would want to have noted that it won't be the actual item anymore, because you work with creation/removal.

I need the timer because i need to know the dropped item's xy coordinates which isn't available at the Unit Loses an Item Event.
Only 1 timer is needed because I looped all the possible dropped items and store them via global item array. And when the timer expires, I pick all the items stored ib the global item array.

I will put in the documentation about how this system works with item creation and removal in the next update.


EDIT: Hmmm, instead of looping, I can also use Table having an index (Child key) of the timer's handle id so each item has it's own timer like btdonald suggested. I may have to use TimerUtils when using this method for efficiency. With this, I no longer need globals but each dropped item has its own timer and hashtable call. Both methods work however I'm not sure which is better.
 
Last edited:
Level 22
Joined
Feb 6, 2014
Messages
2,468
Hm, okay haven't tested for my self, but I guess you tested it.
But can't you then maybe just create item at unit's x/y and with small offset if needed? (result would be like the same probably)

I thought of that too because it is simpler but what if the Unit's Drop item range is say 800. Then it would'nt be applicable anymore because the offset distance can be from 0 to 800.
 
Level 19
Joined
Mar 18, 2012
Messages
1,717
Non-Error messages should be either removed or placed in static ifs.
A static if behaves like a normal if then condition, but is evaluated during compile time.
If the condition doesn't match the code is not compiled.
It's very annyoing for map makers that each time they test their map, a system
tell them that it is working correct ( we expect that anyways ).
JASS:
static if DEBUG_MY_SYSTEM then
    call Msg("Info")
endif
debug call ClearTextMessages()
Remove this line.

This is working, but could be solved more elegant.
JASS:
        local integer iN = itemType.integer[handleId]           //RETRIEVE WHO IS THE OWNER, IF 0, THEN THERE IS NO OWNER
        local integer pN = GetPlayerId(GetTriggerPlayer()) + 1   
//.....
if iN == 0  then
Table API offers us: itemType.has(handleId) which equals HaveSavedInteger(table, parent, child)
That way you could remove the irritating Player-Id + 1

The system appears to work good,
 
Level 22
Joined
Feb 6, 2014
Messages
2,468
Even though it is approved, I want it to be perfect.

BPower said:
I would still use the debug keyword, as debug only code should only be compiled in DEBUG_MODE.
Done I think.

BPower said:
PickUpTrigger & DropTrigger should be labeled pickUpTrigger & dropTrigger ( JPAG naming convention )
Done

BPower said:
set itemName = GetItemName(it) is an not really required local. It's debug code only anyways.
Done

BPower said:
The amount of locals you allocate in OnPickUp could be reduced ( itNew, itemInDatabase, itemName ), however it's no big deal as it is now.
I managed to removed one boolean (isItemInDatabase) but I don't think that is what you meant. How can I still reduce it? Should I make it globals?

BPower said:
function OnDrop doesn't really need a local integer for GetHandleId(item), you could pass that argument directly into function IsItemShared.
But there is no GetHandleId(item) on function OnDrop.

BPower said:
In ConvertInGround you should do exitwhen index == 0 and set index = index - 1 instead of using local integer i as loop index.
Good call.
 
Top