• Listen to a special audio message from Bill Roper to the Hive Workshop community (Bill is a former Vice President of Blizzard Entertainment, Producer, Designer, Musician, Voice Actor) 🔗Click here to hear his message!
  • Read Evilhog's interview with Gregory Alper, the original composer of the music for WarCraft: Orcs & Humans 🔗Click here to read the full interview.

[Snippet] [+] UnitInventory

This snippet will track a unit's inventory.
It won't cache all the items and put them in order by slot number though.
It will however track item raw codes in slots.

Code

JASS:
/********************************************
*
*   UnitInventory
*   v2.0.0.0
*   By Magtheridon96
*
*   - Tracks unit inventory data.
*   - Makes your life a bit easier.
*   - This won't win any benchmarks.
*
*   Requires:
*   ---------
*   
*       - UnitIndexer by Nestharus
*           - hiveworkshop.com/forums/jass-resources-412/system-unit-indexer-172090/
*       - RegisterPlayerUnitEvent by Magtheridon96
*           - hiveworkshop.com/forums/jass-resources-412/snippet-registerplayerunitevent-203338/
*       - Table by Bribe
*           - hiveworkshop.com/forums/jass-resources-412/snippet-new-table-188084/
*       - OrderEvent by Bribe
*           - hiveworkshop.com/forums/jass-resources-412/snippet-order-event-190871/
*
*       Optional:
*       ---------
*
*           - TimerUtils by Vexorian
*               - wc3c.net/showthread.php?t=101322
*
*               (If you use my version of TimerUtils, this will take advantage of that)
*
*   API:
*   ----
*
*       - struct UnitInventory extends array
*
*           - readonly integer size
*               - The size of the inventory
*           - readonly integer empty
*               - The number of empty slots
*           - readonly integer full
*               - The number of taken slots
*
*           - static method operator [] takes unit whichUnit returns thistype
*               - Put a unit in between the brackets and you will get the instance
*
*           - method getItem takes integer slot returns item
*               - Returns the item in a given slot
*
*           - method getItemId takes integer slot returns integer
*               - Returns the item raw code of an item in a given slot
*
*           - method hasItem takes item it returns boolean
*               - Determines whether a unit has a certain item or not
*
*           - method hasItemId takes integer id returns boolean
*               - Determines whether a unit has a certain item type (raw code) or not
*
*           - method refresh takes nothing returns nothing
*               - Recalculates the size of the inventory. If you have an inventory upgrade, you would need to call this.
*
********************************************/
library UnitInventory requires UnitIndexer, RegisterPlayerUnitEvent, Table, OrderEvent, optional TimerUtils
    
    globals
        private constant boolean NO_INVENTORY_UPGRADE = true
    endglobals
    
    private module Init
        private static method onInit takes nothing returns nothing
            local integer i = 852002
            call RegisterPlayerUnitEvent(EVENT_PLAYER_UNIT_PICKUP_ITEM, function thistype.pickup)
            call RegisterPlayerUnitEvent(EVENT_PLAYER_UNIT_DROP_ITEM, function thistype.drop)
            loop
                call RegisterOrderEvent(i, function thistype.switch)
                exitwhen i == 852007
                set i = i + 1
            endloop
        endmethod
    endmodule
    
    struct UnitInventory extends array
    
        readonly integer size
        readonly integer empty
        readonly integer full
        
        private Table types
        private Table hasTypes
        
        method getItem takes integer slot returns item
            return UnitItemInSlot(GetUnitById(this), slot)
        endmethod
        
        method getItemId takes integer slot returns integer
            return this.types[slot]
        endmethod
        
        method hasItem takes item it returns boolean
            return UnitHasItem(GetUnitById(this), it)
        endmethod
        
        method hasItemId takes integer id returns boolean
            return this.hasTypes.boolean[id]
        endmethod
        
        static if NO_INVENTORY_UPGRADE then
            private static method filter takes unit u returns boolean
                return UnitInventorySize(u) > 0
            endmethod
        endif
        
        private static method index takes nothing returns boolean
            set thistype(GetIndexedUnitId()).size = UnitInventorySize(GetIndexedUnit())
            set thistype(GetIndexedUnitId()).empty = thistype(GetIndexedUnitId()).size
            set thistype(GetIndexedUnitId()).full = 0
            if thistype(GetIndexedUnitId()).types == 0 then
                set thistype(GetIndexedUnitId()).types = Table.create()
                set thistype(GetIndexedUnitId()).hasTypes = Table.create()
            endif
            return false
        endmethod
        
        private static method deindex takes nothing returns boolean
            call thistype(GetIndexedUnitId()).types.flush()
            return false
        endmethod
        
        implement UnitIndexStruct
        
        method refresh takes nothing returns nothing
            local integer oldSize = this.size
            set this.size = UnitInventorySize(GetUnitById(this))
            set this.empty = this.empty + this.size - oldSize
        endmethod
        
        private method updateData takes nothing returns nothing
            local integer slot = 0
            local integer id = 0
            call this.hasTypes.flush()
            loop
                set id = GetItemTypeId(UnitItemInSlot(GetUnitById(this), slot))
                set this.types[slot] = id
                set this.hasTypes.boolean[id] = true
                exitwhen slot == 5
                set slot = slot + 1
            endloop
        endmethod
        
        private static method pickup takes nothing returns nothing
            local thistype this = GetUnitUserData(GetTriggerUnit())
            set this.empty = this.empty - 1
            set this.full = this.full + 1
            call this.updateData()
        endmethod
        
        private static method drop takes nothing returns nothing
            local thistype this = GetUnitUserData(GetTriggerUnit())
            set this.empty = this.empty + 1
            set this.full = this.full - 1
            call this.updateData()
        endmethod
        
        static if not LIBRARY_TimerUtils then
            private static key dataK
            private static Table data = dataK
        endif
        
        private static method zero takes nothing returns nothing
            static if LIBRARY_TimerUtilsEx then
                call thistype(ReleaseTimer(GetExpiredTimer())).updateData()
            elseif LIBRARY_TimerUtils then
                call thistype(GetTimerData(GetExpiredTimer())).updateData()
                call ReleaseTimer(GetExpiredTimer())
            else
                call thistype(data[GetHandleId(GetExpiredTimer())]).updateData()
                call DestroyTimer(GetExpiredTimer())
            endif
        endmethod
        
        private static method switch takes nothing returns nothing
            static if LIBRARY_TimerUtils then
                call TimerStart(NewTimerEx(GetUnitUserData(GetTriggerUnit())), 0, false, function thistype.zero)
            else
                local timer t = CreateTimer()
                set data[GetHandleId(t)] = GetUnitUserData(GetTriggerUnit())
                call TimerStart(t, 0, false, function thistype.zero)
                set t = null
            endif
        endmethod
        
        implement Init
    endstruct

endlibrary

Demo

JASS:
struct Demo extends array

    private static unit hero
    
    private static method display takes nothing returns nothing
        local string s = ""
        local integer i = 0
        call ClearTextMessages()
        loop
            if UnitInventory[hero].getItem(i) != null then
                set s = s + GetObjectName(UnitInventory[hero].getItemId(i)) + "\n"
            else
                set s = s + "null\n"
            endif
            exitwhen i == 5
            set i = i + 1
        endloop
        call DisplayTimedTextToPlayer(GetLocalPlayer(), 0, 0, 60, s)
        call DisplayTimedTextToPlayer(GetLocalPlayer(), 0, 0, 60, "Empty slots: " + I2S(UnitInventory[hero].empty))
        call DisplayTimedTextToPlayer(GetLocalPlayer(), 0, 0, 60, "Occupied slots: " + I2S(UnitInventory[hero].full))
    endmethod
    
    private static method onInit takes nothing returns nothing
        set hero = CreateUnit(Player(0), 'Hpal', 0, 0, 270)
        call CreateItem('ciri', 0, 0)
        call CreateItem('ciri', 0, 0)
        call CreateItem('brac', 0, 0)
        call CreateItem('rwiz', 0, 0)
        call CreateItem('rwiz', 0, 0)
        call CreateItem('stel', 0, 0)
        call CreateItem('stel', 0, 0)
        call TimerStart(CreateTimer(), 3, true, function thistype.display)
    endmethod
    
endstruct

Feel free to comment and/or suggest any additions to the API. (No. Do not ask me to change the API.)
This won't win any benchmarks, but it will cache static data and provide you with wrappers to make your lives a bit easier and your codes a bit more readable.
 

Attachments

  • UnitInventory Testmap.w3x
    39 KB · Views: 100
Last edited:

LeP

LeP

Level 13
Joined
Feb 13, 2008
Messages
543
Non-descriptive var names: gy'd.
Stupid module-init: gy'd.
Non linked requirement 'OrderEvent': gy'd.
static-if outside of a function: gy'd.
No type-conversions: gy'd.
native type as name: gy'd.

Also use a boolean to check if the user wants to use the filter thingie.

It's not that these are all bad per se, but this is a public resource, and even if it's only hives resource section we still need some quality and good coding practice.
 
Non-descriptive var names: gy'd.

Who cares? It's a short script and there are only 3 variables that can easily be deciphered because under their declarations you have this:

JASS:
        method operator size takes nothing returns integer
            return s[this]
        endmethod
        
        method operator empty takes nothing returns integer
            return e[this]
        endmethod
        
        method operator full takes nothing returns integer
            return f[this]
        endmethod

As for i and k, no one would care.

Stupid module-init: gy'd.

Yes because module initialization will get this resource graveyarded.
What if I wanted to get the inventory of a unit within a module initialization?

Non linked requirement 'OrderEvent': gy'd.

You would remind me about that, you wouldn't tell me it deserves to be graveyarded -.-
Oh and thanks :)

No type-conversions: gy'd.

What?

native type as name: gy'd.

No.
Give me something better than UnitInventory[unit].item(0) and I'll change it.

Also use a boolean to check if the user wants to use the filter thingie.

Sure

static-if outside of a function: gy'd.

Irrelevant
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
- I care, it makes the code easier to follow/edit, for moderator, any user, and the author itself.

- We have jasshelper made by Cohadar for a proper vjass initializer order.

- getItem()
Anyway, in any real language you are not allowed to use reserved keywords for variable/function names, it's a terrible practice.
 
There are only 5 variables. It shouldn't take anyone more than 10 seconds to figure out what they're for -.-
It's not a problem at all.
I'll still change them.

We have jasshelper made by Cohadar for a proper vjass initializer order.

It's unofficial and I bet only 5 or so users have it installed.

getItem()

No.
 

LeP

LeP

Level 13
Joined
Feb 13, 2008
Messages
543
Non-descriptive var names: gy'd.

Who cares? It's a short script and there are only 3 variables that can easily be deciphered because under their declarations you have this:

JASS:
        method operator size takes nothing returns integer
            return s[this]
        endmethod
        
        method operator empty takes nothing returns integer
            return e[this]
        endmethod
        
        method operator full takes nothing returns integer
            return f[this]
        endmethod

As for i and k, no one would care.
I care. I spend enough time looking at code, better make that easier.
And this is not about lonng names but about descriptive ones.
Something like real x, real y or even timer t *can* be descriptive enough because we have conventions for things like position by x-y-coordinates and one-time timer. etc.
But a "global" int of name f. I have no idea what that could mean.

Stupid module-init: gy'd.

Yes because module initialization will get this resource graveyarded.
What if I wanted to get the inventory of a unit within a module initialization?
Well yes, it's a bad coding practice in a public resource: gy'd.

Non linked requirement 'OrderEvent': gy'd.

You would remind me about that, you wouldn't tell me it deserves to be graveyarded -.-
Oh and thanks :)
Well i wouldn't approve it, and if time goes by without an update: gy'd.

No type-conversions: gy'd.

What?
JASS:
local integer i = integer(this)
It originally comes from a time when we weren't sure if struct will always be represented by integer, but it's a good coding practice nonetheless. You know, a timer is not an integer, and neither is a UnitInventory.

native type as name: gy'd.

No.
Give me something better than UnitInventory[unit].item(0) and I'll change it.
What Troll-Brain said, or for example UnitInventory[unit][index].


static-if outside of a function: gy'd.

Irrelevant
Not irrelevant.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
There are only 5 variables. It shouldn't take anyone more than 10 seconds to figure out what they're for -.-
It's not a problem at all.

That's just a general rule, you can't really make exceptions, because human beings are so much subjective.


I'll still change them.

Good to know.


It's unofficial and I bet only 5 or so users have it installed.

Laziness/fear of users will never be an excuse.
You still can give the link to it.



It's still a terrible practice, getItem is much much much more like the jass convention.
 

LeP

LeP

Level 13
Joined
Feb 13, 2008
Messages
543
There are only 5 variables. It shouldn't take anyone more than 10 seconds to figure out what they're for -.-
It's not a problem at all.
I'll still change them.
Better use the same coding standard for all your scripts. I don't care if they're "big" or "small" projects. In the end you wont spend significant more time on writing but save a lot of time reading your code.

It's unofficial and I bet only 5 or so users have it installed.
Yeah, fuck progress. Better use normal jass.

Yeah, getItem would be way too descriptive. Can't do that to your users. In the end maybe they would feel good using your system.
 
It's still a terrible practice, getItem is much much much more like the jass convention.

Jass convention isn't that well-defined.
But w/e, I'll give you 2 wrappers, or I'll just go with LeP's idea.

Laziness/fear of users will never be an excuse.
You still can give the link to it.

Bleh, let's stay on the safe side.
Nestharus is having problems with Cohadar's JH (it crashes randomly at times)
Plus, you can't assume that every user is going to be using Cohadar's JH or wants to.

edit
Fuck it, are you people trolling me? -.-

edit
Not irrelevant.

I need a reason as to why it's relevant.
I need a static if for those globals. Why would I put them inside a function?

edit

It originally comes from a time when we weren't sure if struct will always be represented by integer, but it's a good coding practice nonetheless. You know, a timer is not an integer, and neither is a UnitInventory.

I'm using thistype, so it shouldn't a problem.

Well yes, it's a bad coding practice in a public resource: gy'd.

Making a resource less bug-prone is not a bad coding practice.

Yeah, fuck progress. Better use normal jass.

It's not progress:
Nestharus is having problems with Cohadar's JH (it crashes randomly at times)

edit
The method operator idea is unintuitive.
Want to know why?

JASS:
UnitInventory[unit][slot] -> item
UnitInventory[unit].itemId(1) -> integer

It's not clear whether the method operator should return the item or the itemId.
Also, I hate getItem. It's an ugly method name.
The method item is going to stay as is.
I need solid reasons as to why it's a bad coding practice.
 
Last edited:
Level 17
Joined
Apr 27, 2008
Messages
2,455
Jass convention isn't that well-defined.
But w/e, I'll give you 2 wrappers, or I'll just go with LeP's idea.

That doesn't mean you should use one convention that break every convention of any real language (jass included), "reserved" keywords are reserved.
It's not because that vJass in the background convert to it to jass that mean you should abuse it like this way.


Bleh, let's stay on the safe side.

If you're talking about module initializer i don't bother that much, i just personaly don't use them anymore if it's not needed.


Nestharus is having problems with Cohadar's JH (it crashes randomly at times)

So far, i have not seen any proof.

Plus, you can't assume that every user is going to be using Cohadar's JH or wants to.

Honestly i don't care about retarded/lazy people, or even the ones who use cJass.
These last ones could still edit the code for themselves.


edit
Fuck it, are you people trolling me? -.-

I'm not, i'm just really really sicked of all these vJass abuses, actually vJass is used like a textmacro ...
You're just the perfect example.


I need a reason as to why it's relevant.

About what ?

I need a static if for those globals. Why would I put them inside a function?

I have personnaly nothing against it, it's just a shame that static ifs look like too much as normal ifs.

Making a resource less bug-prone is not a bad coding practice.

What ?

It's not progress:

edit
The method operator idea is unintuitive.
Want to know why?

JASS:
UnitInventory[unit][slot] -> item
UnitInventory[unit].itemId(1) -> integer

It's not clear whether the method operator should return the item or the itemId.

Have we ever talked about method operators ?


Also, I hate getItem. It's an ugly method name.

I hardly see how it's ugly.

The method item is going to stay as is.
I need solid reasons as to why it's a bad coding practice.

It's not the first time that people say how that vJass abuses are fugly, i don't bother to repeat myself again and again.
You're just blind.

I like you as a person, but i also really hate how you write your codes.
That's senseless and fugly.
 
Half my post was made up of responses to you and LeP.
I guess I should start using quote boxes correctly (heh).

I hardly see how it's ugly.

It's mostly personal. (You'd know if you've worked with RecipeEngine and realized the ugliness of call RemoveItem(getItem(unit, integer)))

If you're talking about module initializer i don't bother that much, i just personaly don't use them anymore if it's not needed.

But it's needed. Units can have items in their inventory during map init.

Oh, and sorry if I seemed aggressive above, it's just that my blood pressure got really high because of how LeP phrased his post.

It's not the first time i explained that vJass abuses are fugly.
You're just blind.

OK OK.
I don't want this thread to become what that RawCodeIndexer thread has become: A monstrosity.
 

LeP

LeP

Level 13
Joined
Feb 13, 2008
Messages
543
Jass convention isn't that well-defined.
But w/e, I'll give you 2 wrappers, or I'll just go with LeP's idea.
Only one API-endpoint.


I'm using thistype, so it shouldn't a problem.
It never realy was a problem. (There was a time where i2h was no problem either.)
It is just that you don't use objects as integer. Well not before you use the
right function for conversion.

JASS:
UnitInventory[unit][slot] -> item
UnitInventory[unit].itemId(1) -> integer

It's not clear whether the method operator should return the item or the itemId.
Also, I hate getItem. It's an ugly method name.
The method item is going to stay as is.
I need solid reasons as to why it's a bad coding practice.

There allready is a function for getting a item type, you know?
And you know, polluting scope, "re-declaring" a native type. And wrong highlighting ofc.

And because of one guy having problems with the new jh, we all shouldn't use this glorious version?
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
It's mostly personal. (You'd know if you've worked with RecipeEngine and realized the ugliness of call RemoveItem(getItem(unit, integer)))

Aha, you've just pointed one more vJass silliness, implicit "this.", and no i'm not talking about implicit "this" (.member/method).

JASS:
struct s

    integer data

    method m takes integer data ...
        set data = data // yeah
    endmethod

endstruct

It's just better to always use "this.", or maybe just ".", but i'm not sure about this last one, maybe it's senseless.


But it's needed. Units can have items in their inventory during map init.

What i meant is that it is not needed with Cohadar's jasshelper.
 
Stupid module-init: gy'd.

Backward's compatibility with Vexorian's JassHelper doesn't deserve graveyard.

static-if outside of a function: gy'd.

Why?

No type-conversions: gy'd.

It's a matter of preference. I personally don't use type conversions because they get compiled with the "()", which is useless. I only add it for the "less_than" operand, or if I want to typecast to a different struct.

native type as name: gy'd.

It can be useful if it suits the occasion. For example in the New Table library I use those names and it seems to fit.
 

LeP

LeP

Level 13
Joined
Feb 13, 2008
Messages
543
Stupid module-init: gy'd.
Backward's compatibility with Vexorian's JassHelper doesn't deserve graveyard.

Well i for one like progress.

static-if outside of a function: gy'd.
Why?

I think static-ifs shouldn't alter toplevel declarations. They should not change
the semantic of a function either.


No type-conversions: gy'd.
It's a matter of preference. I personally don't use type conversions because they get compiled with the "()", which is useless. I only add it for the "less_than" operand, or if I want to typecast to a different struct.

Oh yeah, the extra parenthesis in your compiled map script are, like, going
to kill you. JassHelpers inliner also adds extra parenthesis, maybe you want
to stop inlining?

But jokes aside, a simple integer() realy helps you understanding
the code. As i said, you wouldn't use a unit or a timer
as a integer. You would use a function to get the integer representation of your
object. And struct are object at not integer on a highlevel view.
And the appropiat conversion from the highleven object to the low-level
representation is the integer() function.

So, a simple integer-type cast does not add any mali to your code but adds
clarity.

e: but i guess noones going to do it, so im meh with it.

native type as name: gy'd.
It can be useful if it suits the occasion. For example in the New Table library I use those names and it seems to fit.

In your table thing it actually kinda makes sense. And as i said, all my points
aren't necessary bad. But there is a clear difference between this script and
your table.
In your table, you use the typenames as actual types; you store values of type
item under the .item property of your table.
Here, it is used entirely different. Here you don't have to distinguish between
types.
 
Well i for one like progress.

Sure. Let's break backwards compatibility for every damn resource on this site and call it progress!

I think static-ifs shouldn't alter toplevel declarations. They should not change
the semantic of a function either.

That static if removes a Redundant Table declaration.
Why is this a con?

Oh yeah, the extra parenthesis in your compiled map script are, like, going
to kill you. JassHelpers inliner also adds extra parenthesis, maybe you want
to stop inlining?

But jokes aside, a simple integer() realy helps you understanding
the code. As i said, you wouldn't use a unit or a timer
as a integer. You would use a function to get the integer representation of your
object. And struct are object at not integer on a highlevel view.
And the appropiat conversion from the highleven object to the low-level
representation is the integer() function.

So, a simple integer-type cast does not add any mali to your code but adds
clarity.

By convention, this is used to refer to a struct instance.
I don't think integer(this) would make it any clearer than it already is.
Either way, I'm not using it as an integer, I'm using it as a struct instance.
integer(this) would actually make it less clear.

In your table thing it actually kinda makes sense. And as i said, all my points
aren't necessary bad. But there is a clear difference between this script and
your table.
In your table, you use the typenames as actual types; you store values of type
item under the .item property of your table.
Here, it is used entirely different. Here you don't have to distinguish between
types.

w/e
It's gone.
 
I think static-ifs shouldn't alter toplevel declarations. They should not change
the semantic of a function either.

I think it would be a good presentation for "when a library requirement should be optional" in which case, if you have to static-if some globals, maybe the code maker himself should decided which approach to natively support, rather than adding and endless sea of flavors.

Which I am mostly in favor of. Walking the fence makes for ugly code. Either pick a hashtable or pick the Table library. I think the Table library is a fine requirement, when you use the optimizer it's only a few dozen lines of code give or take.


As for "Cohadar's JassHelper" in terms of progress, I do support that idea as well.

The key is this: the code programmer should settle on something. Either Cohadar's JassHelper or not, either Table library or not...

But "module initializer" is mostly going to be used by people who are using Cohadar's JassHelper anyway (as a lot of us have switched over already).
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
Why not provide a linked list to iterate an all items owned by the unit.
For example it can be useful if you want to move all items before remove/kill the unit.
Sure you can still do it but it requires "more work".
And that would fit the point of this.

Now maybe it wouldn't be used that much, it's just a random suggestion, not really thought about it, i've just read what is the purpose of it.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
.next.getItem()
And eventually .prev

Where next and prev are obviously not integers but thistype.
Maybe it's not that intuitive but if an example is given in the main thread, it should be enough.
See how my UnitLL resource is popular, it uses the same syntax.

OFF-TOPIC

Oh and btw what the "[+]" in the thread title stands for ? You consider that it is more than a snippet but not a system ?
Personnaly i'm quite frustrated with this dual choice, for me a system = code + whatever else (sound or object editor stuff in general), no matter how complex or simple the resource it is, and "snippet" is for simples short codes.
But for more complex codes "snippet" doesn't really fit :/
 
It's not that it never got approved because the thread died, it's that it never got approved because I never have time to open up wc3 to review resources any more. When you count my commute with my work schedule that's 58 hours per week, with household responsibilities/getting ready for work each day that adds about 12 hours each week, with sleeping that is 60 hours per week (someone likes to sleep in on the weekends). We're looking at about 38 hours there now, we tend to watch movies and TV shows together each week suddenly we are down to 24 hours maximum additional time, sometimes as little as 12, if we go away for a day to her parent's house or to a mall or to some extra thing, which happens about once a week as it is, I'm down to between 8-12 hours per week tops!!! Often it's even 0!!! And I need that time for myself so I avoid going ape crazy (which I do about once per week as well!!)

I definitely have no time for moderating o_O
 
Top