Chest System

[Moderator]

15th Feb 2015
IcemanBo: map is updateable again.

23th April 2015
IcemanBo: http://www.hiveworkshop.com/forums/spells-569/chest-system-258181/index2.html#post2678108

Older

7th Jan 2015
IcemanBo:
http://www.hiveworkshop.com/forums/spells-569/chest-system-258181/#post2636884

10:19, 8th Oct 2014
TriggerHappy:

1. Wrap this in a library.
2. Privatize as much as possible.
3. Constants are usually capitalized (MAX_INDEX)
4. "Chest" is kind of generic. ItemChest might be better.
5. The system is very simple and all it does is basically roll a random number for you and pick an item from an array.
6. This system only allows ~9 instances, which is terrible. You should be using a hashtable or a unit indexer.

 

[Chaosy]

System rejected on my request, I don't indend to update this. But it's still fully working. So if someone wants to use it go ahead.

code

library unitChestLib
/*
    Chest System by Chaosy
    
    Desp:
    The system easily allows you to give random drop chances to a unit when it is unlock. Even though
    the system is designed for chests to drop 'random' items once unlocked, it can also be used as a general
    system to drop items from every unit in the map.
    
    Methods:
        method spawn takes real x, real y returns nothing
        method SetChest takes unit u returns nothing
        method addItem takes integer i, integer chance returns nothing
        method addKey takes integer i returns nothing
        method removeKey takes nothing returns nothing   
        method addGold takes integer i returns nothing
        method removeGold takes integer i returns nothing
        method addLumber takes integer i returns nothing
        method removeLumber takes integer i returns nothing
        method removeItem takes integer someItem returns nothing
        method UnlockChest takes unit u returns nothing
*/

// -- Config --

globals
    //maxindex allowed, with other words a chest can drop 999 items by default
    //note that you can extend the variable above the normal cap (8191)
    private constant integer MAX_INDEX = 999
    //unit type of the chests used
    private constant integer CHEST_TYPE = 'h000'
    //animation played when opened
    private constant string OPEN_ANIMATION = "stand alternate"
endglobals

// -- System --

struct UnitChest

    private integer current = 0
    private integer gold = 0
    private integer lumber = 0
    private integer neededKey = 0
    private integer array items[MAX_INDEX]
    private integer array dropChance[MAX_INDEX]
    unit chest
    
    
    static method create takes nothing returns thistype
        return thistype.allocate()
    endmethod
    
    method spawn takes real x, real y returns nothing
        set .chest = CreateUnit(Player(PLAYER_NEUTRAL_PASSIVE), CHEST_TYPE, x, y, 0)
    endmethod
    
    method SetChest takes unit u returns nothing
        set .chest = u
    endmethod
    
    method addItem takes integer i, integer chance returns nothing
        set .current = .current + 1
        set .items[.current] = i
        set .dropChance[.current] = chance
        if .current >= MAX_INDEX then
            call BJDebugMsg("Error - Chest System: Max index reached, you might want to re-config the system.")
        endif
    endmethod
    
    method addKey takes integer i returns nothing
        set .neededKey = i
    endmethod
    
    method removeKey takes nothing returns nothing   
        set .neededKey = 0
    endmethod
    method addGold takes integer i returns nothing
        set .gold = .gold + i
    endmethod
    
    method removeGold takes integer i returns nothing
        set .gold = .gold - i
    endmethod
    
    method addLumber takes integer i returns nothing
        set .lumber = .lumber + i
    endmethod
    
    method removeLumber takes integer i returns nothing
        set .lumber = .lumber - i
    endmethod
    
    method removeItem takes integer someItem returns nothing
        local integer i = 0
        local integer i2
        loop
        exitwhen i > .current
            if .items[i] == someItem then
                set i2 = i + 1
                loop
                    exitwhen i2 > .current
                    set .items[i2] = .items[i2 - 1]
                    set .dropChance[i2] = .dropChance[i2 -1]
                    set i2 = i2 + 1
                endloop
                set .current = .current - 1
            endif
            set i = i + 1
        endloop
    endmethod
    
    method UnlockChest takes unit u returns nothing
        local integer rng
        local integer i = 0
        local integer i2 = 0
        local real x = GetUnitX(.chest)
        local real y = GetUnitY(.chest)
        local player p = GetOwningPlayer(u)
        loop
            exitwhen i2>= 6
                if GetItemTypeId(UnitItemInSlotBJ(u, i2)) == .neededKey or .neededKey == 0 then
                    set i2 = 6
                    set udg_CS_gold = .gold
                    set udg_CS_lumber = .lumber
                    set udg_CS_chest = .chest
                    set udg_CS_unlocker = u
                    set udg_CS_event = 1
                    set udg_CS_event = 0
                    call UnitAddAbility(.chest, 'Aloc')
                    call QueueUnitAnimation(.chest, OPEN_ANIMATION)
                    if .gold > 0 then
                        call SetPlayerState(p, PLAYER_STATE_RESOURCE_GOLD, GetPlayerState(p, PLAYER_STATE_RESOURCE_GOLD) + .gold)
                    endif
                    if .lumber > 0 then
                        call SetPlayerState(p, PLAYER_STATE_RESOURCE_LUMBER, GetPlayerState(p, PLAYER_STATE_RESOURCE_LUMBER) + .lumber)
                    endif
                    loop
                        exitwhen i > .current
                        set rng = GetRandomInt(1,100)
                        if rng < .dropChance[i] then
                            call CreateItem(.items[i], x, y)
                        endif
                        set i = i + 1
                    endloop
                endif
            set i2 = i2 + 1
        endloop
    endmethod
endstruct
endlibrary

changelog:
-made variables private
-wrapped the code into a library
-changed various names
-debug message added
-added spawn and setchest methods
-updated demo triggers
-added add/remove key methods
-added add/remove gold/lumber methods
-removed comments from demo triggers
-improved one demo trigger a little
-changed most method names
-removed floating texts (texttag) completely
-added an event with GUI support to detect when a chest is unlocked.
-added a new demo trigger for the event
-improved API

credits:
Chriz. - model for chest in the test map

 

[Kazeon]

My first vJASS system that I uploaded. Don't be too harsh.

Very well then. ^)^

struct UnitChest

Sounds more like a unit attachment point for me

method ChestAddItem takes integer i, integer chance returns nothing

Variable names for the parameter (e.g. i) should be more informative for the user. For that function, as example, my suggestion is itemId or itemID seems nice enough.
_______________________________

Actually, I have a better idea and concept for you. Your system's name inspired me about a chest/treasure system. Yeah, your system is actually more like an item-drop system which is might be useful for RPGs.

But my concept is a bit different here, where the system will allow user to create a treasure with it's own item set at give coordinate, each item has their own spawn chance.

So the system will contain functions like:

// Create treasure instance at x, y
static method create takes real x, real y returns thistype

// Add item to the chest
method add takes integer itemId, real chance returns nothing

// Open the treasure
method open/unlock takes nothing returns nothing
    // This function will contains something like this
    call PlayItemAnimation(.item, "open")
    // And then create all the items at .x and .y

Well, it's quite hard to describe, but I hope you get my point

 

[Chaosy]

I think I get your point. It shouldn't be too hard to implement.

edit: updated!

 

[BPower]

In general a nice concept, I've also made such a system for a map about 1 year ago.
Here are some things you should consider.

This should be your creator function, you may take UnitTypeId as first argument, if you want to use multiple chest models:
function CreateChest takes real x, real y, real facing returns Chest

Some chest stay closed until you use something like a key, so
add a method/method operator method operator locked= takes boolean flag returns nothing

chest models normally have an open animation, which definitly should be played, furthermore you could play a sound like the "slam door" sound file.

You need a trigger which determines how to interact with chest, for example if the are selected + range check of the unit which is under controll.
This should somehow be configurable, because it will be very map specific.

Now if a chest is opened once you may add 'Aloc' so it stays the way it is.

The market place unit in the object editor doesn't use an healthbar, which is also very nice for chest (maybe).

In our jass section there should be a library named IPool (Integer Pool), which really fits your needs concerning setting up item pools.
It's very heavy on Table instances, but works fine.

Chest can have a general item pool, but also a specific one. Think about chests associated to quests.

 

[Chaosy]

Righty, righty. Added the option to make a chest require a key to open. You can also add lumber/gold to the chest now.

edit: I got an idea on how to add item pools (so you don't need to re-create over and over).. I will need to think a little about it before I add it though.

 

[deathismyfriend]

You should remove the BJs.

Also you can use item pools as suggested or you can use an item drop system. Like mine. Just when the chest is opened kill it if it is a unit. If it is not a unit when it dies add unit with item pool or item table if using my system to it then kill and remove unit so the items will be dropped.

In your create method you can change it to this I believe.

    static method create takes nothing returns thistype
        return thistype.allocate()
    endmethod

 

[Chaosy]

Yeah, that sounds about right. Normally it wouldn't work though, since normally you use this.someVariable(s) in between the allocation and the return which should cause a error if I am understanding it correctly .

For the item pool, I don't like to force people to use other resources unless it's something rather advanced.

Killing the chest is a good suggestion! I think I will add that as a constant when I update this next time.

Thanks for your feedback. rep+

 

[IcemanBo]

method Spawn, and method SetChest could be merged with create method.

Once a chest is unlocked, the instance should be destroyed, or?

DropChance should be a real. And in your UnlockChest method you should take a new random number for drop chance for each item.
Now you only make it once, and use the generated number for all item drops. Imagine a chest can drop 10 items with DropChance 90%. Then, if your number becomes randomly higher 90, so no item will be created at all. (also "rng" is a bit confusing as variable name)

If I have a look in method ChestRemoveItem I think attaching values to itemId might be useful, or having something like a list.
Or a simple solution would be to keep your method, but instead of the second loop just to move last item instance to removed position to fill the gap, just like it's done in dynamic indexing.

Whenever you use the word "item" in code, you actually could say "itemId", to signal that it's actually only an integer (rawcode) and not the actual item handle.

Hashtable can be initialisized in your system code:
static hashtable hash = InitHashtable()

 

[Chaosy]

method Spawn , and method SetChest could be merged with create method.

The reason I didn't combine them was because I wanted to allow the user to 'convert' an existing unit to a chest within the system. So I thought that in order to make it as flexible as possible I would create a separate method for spawning the chest.
So now you can create a instance of the chest and then just set a existing unit to act like a chest with SetChest Method. Or am I wrong to do that?

And in your UnlockChest method you should take a new random number for drop chance for each item.

Silly mistake, will make sure to fix it.

or having something like a list.

Do you remember the last time I tried to create a list?! it was a disaster lol. On the indexing, I will see if I can translate your words to.. whatever language my brain speaks.

Hashtable can be initialisized in your system code:

Well, the hashtable isn't exactly a requirement, I just decided to use it for the demo.

 

[IcemanBo]

So now you can create a instance of the chest and then just set a existing unit to act like a chest with SetChest Method. Or am I wrong to do that?

It's not really wrong, I think.. but it's just my personal preference. ^^
Me, I would like to see the create function just like BPower said, and then automatically set the chest the created unit.
It seems simple and clean for me.

static method create takes player p, integer uType, real x, real y, real angle returns thistype
    local thistype this = thistype.allocate()
    set this.x = x
    set this.y = y
    set this.facing = angle
    set this.chest = CreateUnit(p, uType, x, y, angle) 
    return this
endmethod

Edit: There still could be the method "SetChestUnit" as addition of course..

 

[IcemanBo]

The uploaded map is not up to date. The code in description is not the same as in your map's. Update it.

Still some notes to the (newer) code:

• 
  It doesn't make much sense there is pre-defined constant unit type for chests, if the chest unit can be changed to any unit via method.
  Just immediatly let user let define the unit or unittype on creation.
• For namings of parameter or your members, don't mixup item and itemType. It might be confusing. Integer is always itemType.
• If MAX_INDEX is reached, don't add any more items.
• In method ChestRemoveItem no nested loop is needed. Just move the last instance to fill the gap. It's like dynamic indexing.
• In method UnlockChest the GetRandomInt function should be moved inside the loop. It should be evaluated seperatly for each new item.
• Use natives to avoid not needed function calls if you can.
• I don't necessarily see texttags are linked with creating items at a chest. Maybe you should overthink this feature, or maybe it's better to make it optional.
• As map is not up to date I could not test it myself, but the loop in UnlockChest seems strange regarding integer i2.

If the API is simple I can imagine this system might be useful.
Still, for now: Needs Fix

 

[Chaosy]

I am going to update this since I am about to use it for my own project.

*It does actually make sense if the user doesn't want to spawn the chest with triggers. Then you have to select the unit that's pre-placed on the map.
*So basically I should change the name from "items" to "itemTypes" ?
*fixed.
*There is no way to get that by default. So I have to store the last used instance, right?
*fixed
*If I understand you correctly I don't got any of those in the system itself. However I think there are one of those in the demo triggers.
*Hm, I guess you're right. Will fix.

I lost my old file, or deleted it by mistake. Heck if I know. But I copied the triggers from the main post and updated it with new triggers.

edit: I found a few mistakes in the code, gotta fix 'em quick.

 

[bowser499]

To start with, here is my own review.

Originality: To be honest, that's actually the rough remake of Blizzard's mob drop system with some more advanced features. Nothing more.

Coding: When looking through the code, I've pointed out some mistakes. Here are the most general ones:

• It's better to make private constant string OPEN_ANIMATION an integer which will use animation index because in some cases the string animation may act not like you want it to act. But it never happens so with indexes.
• Just a recommendation: place the array members of a struct below all non-array members. This will later help you to see all arrays easier.
• In static method create it definetly is a lot better to do return thistype.allocate() directly than using a local variable.
• Instead of using this.member just use .member. Shorter and better.
• Name your methods better. Use addKey instead of ChestAddKey, shorter and understandable.
• This has memory leaks. Please null your handles, especially in UnlockChest method of your struct.
• It's better to use GetWidgetX and GetWidgetY instead of GetUnitX and GetUnitY.
• It is bad to use BJ functions like UnitItemInSlotBJ or SetTextTagVelocityBJ.
• Regarding texttags, it's much better to make their colors configurable in globals block.
• Why do we need local integer rng? It is better to use it directly in the condition and get the same result.
• Regarding drop chances, you can get rid of that array by using a single itempool object. You can manipulate item weights and it's lots more flexible to handle.
• Make unit chest a readonly member.
• Please add more documentation for this system. Users can't get all by themselves.

3/5: Useful

 

[BPower]

It's better to use*GetWidgetX*and*GetWidgetY*instead of*GetUnitX*andGetUnitY

thats not true

 

[pred1980]

In static method create it definetly is a lot better to do return thistype.allocate() directly than using a local variable.

Why??

 

[Kazeon]

In static method create it definetly is a lot better to do return thistype.allocate() directly than using a local variable.

It's even better not to write the constructor at all (the create method) since you do nothing there.

I have several questions:
- Why this method exists? Any case user want to change the chest unit?

    method SetChest takes unit u returns nothing
        set this.chest = u
    endmethod

- Why this is not done in the create method?

    method ChestSpawn takes real x, real y returns nothing
        set this.chest = CreateUnit(Player(PLAYER_NEUTRAL_PASSIVE), CHEST_TYPE, x, y, 0)
    endmethod

- Why u no use [Snippet] IPool? You can extend the functionality so that the chest could spit any kind of integer: is that number of gold, number of lumber, unit id, item id, or anything. Then you can provide an event when a chest is opened, so user is free to determine their own actions, instead of limited only to increase lumber/gold. Read IPool and you will understand what I want here.

- And again, just remove the texttag, and provide an event when a chest is opened instead.

Ah, I forgot I have posted my idea before:

_______________________________

Actually, I have a better idea and concept for you. Your system's name inspired me about a chest/treasure system. Yeah, your system is actually more like an item-drop system which is might be useful for RPGs.

But my concept is a bit different here, where the system will allow user to create a treasure with it's own item set at give coordinate, each item has their own spawn chance.

So the system will contain functions like:

// Create treasure instance at x, y
static method create takes real x, real y returns thistype

// Add item to the chest
method add takes integer itemId, real chance returns nothing

// Open the treasure
method open/unlock takes nothing returns nothing
    // This function will contains something like this
    call PlayItemAnimation(.item, "open")
    // And then create all the items at .x and .y

Well, it's quite hard to describe, but I hope you get my point

 

[Chaosy]

Why this method exists? Any case user want to change the chest unit?

Because if a chest is preplaced I still have to create an instance of the object and I don't want to spawn a new chest unit in that case.

I am kinda against forcing people to use systems if I can avoid it. But oh well, I will add Ipool and fix as many suggested things as possible sometime today.

 

[BPower]

I will add some ideas of mine, what you should change in the current version.

1. Use a more generic API. Currently it looks like

set chest = ChestUnit.create()// Chest fits better that ChestUnit
call chest.ChestAddItem(x)// ?? looks pretty weird

call chest.addItem(x)// better

2. The item pool could be of type itempool. link:unit/itempool

3.

    method SetChest takes unit u returns nothing
        set this.chest = u
    endmethod

You should create the chest unit in static method create.

4. The gold and lumber bounty could use a system like blizzards bounty system.
constant base + roll or simply a random number in range of x to x1.

5. Texttags should be created locally for the player who is recieving the bounty.
Size, fadeout, color, .... should be configurable.

 

[bowser499]

thats not true

Prove it with some facts, please.
My benchmarking tells me other thing.

It's even better not to write the constructor at all (the create method) since you do nothing there.

True but tell that to JassHelper v0.A.2.B of mine which states that create method must be declared.

- Why this method exists? Any case user want to change the chest unit?

    method SetChest takes unit u returns nothing
        set this.chest = u
    endmethod

I wonder, why can't we tell 'for the sake of completion'?

- Why this is not done in the create method?

    method ChestSpawn takes real x, real y returns nothing
        set this.chest = CreateUnit(Player(PLAYER_NEUTRAL_PASSIVE), CHEST_TYPE, x, y, 0)
    endmethod

Reasonable.

- Why u no use [Snippet] IPool? You can extend the functionality so that the chest could spit any kind of integer: is that number of gold, number of lumber, unit id, item id, or anything. Then you can provide an event when a chest is opened, so user is free to determine their own actions, instead of limited only to increase lumber/gold. Read IPool and you will understand what I want here.

Seriously. My opinion is that people mustn't use snippets for making other snippets. Snippets are provided just for usage of them in spells or map but I think it's fair enough if the person wants to do it his own, unique way. Let's not force people to use templates.

1. Use a more generic API. Currently it looks like

set chest = ChestUnit.create()// Chest fits better that ChestUnit
call chest.ChestAddItem(x)// ?? looks pretty weird

call chest.addItem(x)// better

2. The item pool could be of type itempool. link:unit/itempool

3.

    method SetChest takes unit u returns nothing
        set this.chest = u
    endmethod

You should create the chest unit in static method create.

4. The gold and lumber bounty could use a system like blizzards bounty system.
constant base + roll or simply a random number in range of x to x1.

5. Texttags should be created locally for the player who is recieving the bounty.
Size, fadeout, color, .... should be configurable.

These are mostly the things which were said by me in the post above.

 

[BPower]

Prove it with some facts, please.
My benchmarking tells me other thing.

Here is my source Link. Since you mentioned it so straight foreward, I would like to see your tests proofing TriggerHappys thread, which I linked should be re-worked.

 

[Chaosy]

I am glad that you guys have so many suggestions, makes my rookie life easier. I've hold unto some of the suggestions because of split opinions though so I will wait to implement them.

Why do we need local integer rng ? It is better to use it directly in the condition and get the same result.

Because variables are faster than using direct calls? Or am I wrong?

Make unit chest a readonly member.

Doesn't that make it un-modifiable? I have enver used that keyword before so I don't know what it does exactly. I can guess by logic though.

It is bad to use BJ functions like UnitItemInSlotBJ or SetTextTagVelocityBJ .

Why? Those are what I consider 'good' BJ functions since they actually do something useful. It makes the code 4 lines shorter at the price of one extra function call. I'd say it's an acceptable trade.

I changed most method names, but aren't methods supposed to be capitalized? I think I read that in the jass structure rules. (it's not called that but I don't remember the correct name JPAG or something -.-)

 

[bowser499]

Because variables are faster than using direct calls? Or am I wrong?

Every variable requires extra space in a memory. Direct call is faster than assigning it to a variable, at least in your case this is true since you only need it once.

Doesn't that make it un-modifiable? I have enver used that keyword before so I don't know what it does exactly. I can guess by logic though.

It can still be modified by your setChest method. It is modifiable inside the struct but is not outside of it.

Why? Those are what I consider 'good' BJ functions since they actually do something useful. It makes the code 4 lines shorter at the price of one extra function call. I'd say it's an acceptable trade.

Own function are tons better. UnitItemInSlotBJ don't do too-much-useful operation.

 

[BPower]

This is a more or less hardcoded version of what you should try to achieve.
What can you do different( better) than the code below.
- remove all that texttag, sound, locust etc... stuff
- users can run this code onChestOpen trigger evaluation.
- So you have to provide wrapper function to get TriggerChestPlayer, TriggerChestUnit and TriggerChestResource
- TriggerChestResource doesn't have to be gold or lumber, but any integer, if you leave PlayerState manipulation to the user onChestOpen event.

Take whatever you need from this piece of code. Ask questions if you have.

library Chest /* v1.0
*************************************************************************************
*
*   Create units, acting like treasure chests.
*
*************************************************************************************
*
*   */uses/*
*
*       */ Table                 /* Works with Bribes and Vexorians Table.
*       */ optional ErrorMessage /* [url]https://github.com/nestharus/JASS/tree/master/jass/Systems/ErrorMessage[/url] 
*       */ optional Init         /* -
*
************************************************************************************
*
*   1. Import instruction
*   ¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯
*       Copy the Chest script into your map.
*
*   2. API
*   ¯¯¯¯¯¯
*       API goes here
*
*   3. Configuration
*   ¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯
*/

    globals
        private constant boolean ADD_LOCUST_WHEN_OPEN       = true// Comment goes here
        private constant string PREFIX_RECIEVE_GOLD         = "|cffffcc00"
        private constant string PREFIX_RECIEVE_LUMBER       = "|cff006400"
        private constant real TEXT_TAG_HEIGHT               = 0.023
        private constant real TEXT_TAG_HEIGHT_OFFSET_GOLD   = 32.
        private constant real TEXT_TAG_HEIGHT_OFFSET_LUMBER = 80.
        private constant real TEXT_TAG_VELOCITY_X           = 16*0.0005546875*Cos(90*bj_DEGTORAD)
        private constant real TEXT_TAG_VELOCITY_Y           = 16*0.0005546875*Sin(90*bj_DEGTORAD)
        private constant real TEXT_TAG_LIFESPAN             = 2.5
        private constant real TEXT_TAG_FADEPOINT            = 1.75
    endglobals

// Configurate how you detect in your map an existing chest key. 
    private function UnitHasRequiredKey takes unit whichUnit, integer whichKey returns boolean
        local integer index   = 0
        loop
            if (GetItemTypeId(UnitItemInSlot(whichUnit, index)) == whichKey) then
                return true 
            endif

            set index = index + 1
            exitwhen index >= bj_MAX_INVENTORY        
    
        endloop
        return false
    endfunction

//Script code. Do not edit anything below.
static if not LIBRARY_Init then 
    private module CHEST_INITIALIZER
        private static method onInit takes nothing returns nothing
            call thistype.init()
        endmethod
    endmodule
endif

    struct Chest
        private static sound error          = null
        private static sound onOpenSound    = null
    
        readonly static trigger openEvent   = CreateTrigger()
        readonly static thistype eventIndex = 0
        private static Table table          = 0
        private static integer goldSlot     = -1
        private static integer lumberSlot   = -2
        readonly unit chestUnit
        private itempool pool
        private Table chestTable
        private integer itemCounter 
        private boolean isOpen
        integer requiredKey
        
        method destroyEx takes boolean wantDestroyPool returns nothing
            call deallocate()
            static if Table.flush2D.exists then
                call table.flush(GetHandleId(chestUnit))
            else
                call table.remove(GetHandleId(chestUnit))
            endif
            call chestTable.destroy()
            if (wantDestroyPool) then
                call DestroyItemPool(pool)
            endif
            set pool      = null
            set chestUnit = null
        endmethod
        
        private method manipulateResource takes unit forUnit, playerstate whichPlayerState, integer childKey, string prefix, real offset returns nothing
            local player forPlayer = GetOwningPlayer(forUnit)
            local integer amount   = chestTable[childKey]    
            local string text      = prefix + "+"   

            if (amount != 0) then
                if (amount < 0) then
                    set text = prefix + "-"
                endif
                set text = text + I2S(amount)
                    
                call SetPlayerState(forPlayer, whichPlayerState, GetPlayerState(forPlayer, whichPlayerState) + amount) 
                
                if (GetLocalPlayer() == forPlayer) then
                    set bj_lastCreatedTextTag = CreateTextTag()
                    call SetTextTagText(bj_lastCreatedTextTag, text, TEXT_TAG_HEIGHT)
                    call SetTextTagPosUnit(bj_lastCreatedTextTag, forUnit, offset)
                    call SetTextTagVelocity(bj_lastCreatedTextTag, TEXT_TAG_VELOCITY_X, TEXT_TAG_VELOCITY_Y)
                    call SetTextTagPermanent(bj_lastCreatedTextTag, false)
                    call SetTextTagLifespan(bj_lastCreatedTextTag, TEXT_TAG_LIFESPAN)
                    call SetTextTagFadepoint(bj_lastCreatedTextTag, TEXT_TAG_FADEPOINT)
                    call SetTextTagVisibility(bj_lastCreatedTextTag, true)
                endif
                set bj_lastCreatedTextTag = null
            endif
        endmethod
        
        // Uses operating unit as argument.
        method open takes unit trysToOpen, integer itemDropAmount, boolean ignoreKey returns boolean
            local thistype prev = eventIndex
            local real x        = GetUnitX(chestUnit)
            local real y        = GetUnitY(chestUnit)
            if (isOpen) then
                return false
            endif
            
            // Check for wrong usage.
            static if DEBUG_MODE and LIBRARY_ErrorMessage then
                call ThrowError((trysToOpen == null), "Chest", "open", "unit trysToOpen", this, "Invalid Unit Argument (null)!")
                call ThrowError((itemDropAmount < 0), "Chest", "open", "itemDropAmount",  this, "Invalid Integer Argument (<0)!")
            endif
            
            // Check if a key is required
            if (requiredKey != 0) and not (ignoreKey) and not (UnitHasRequiredKey(trysToOpen, requiredKey)) then
                // Error message.
                if GetLocalPlayer() == GetOwningPlayer(trysToOpen) then
                    call StartSound(error)
                    call ClearTextMessages()
                endif
                call DisplayTimedTextToPlayer(GetOwningPlayer(trysToOpen), 0.52, 0.96, 2., "\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n|cffffcc00Required key:|r " + GetObjectName(requiredKey))
                return false
            endif
            set isOpen = true

            // Visuals + Sounds
            call SetUnitAnimationByIndex(chestUnit, 0)
            
            static if ADD_LOCUST_WHEN_OPEN then
                call UnitAddAbility(chestUnit, 'Aloc')
                // Makes sense to me.
                call SetTerrainPathable(x, y, PATHING_TYPE_WALKABILITY, false)
            endif
            
            call AttachSoundToUnit(onOpenSound, chestUnit)
            call SetSoundVolume(onOpenSound, 50)
            call StartSound(onOpenSound)

            // Generate Items.
            loop
                exitwhen (itemDropAmount == 0)
                call PlaceRandomItem(pool, x, y)
                set itemDropAmount = itemDropAmount - 1
            endloop
            
            call manipulateResource(trysToOpen, PLAYER_STATE_RESOURCE_GOLD,   goldSlot,   PREFIX_RECIEVE_GOLD, TEXT_TAG_HEIGHT_OFFSET_GOLD)
            call manipulateResource(trysToOpen, PLAYER_STATE_RESOURCE_LUMBER, lumberSlot, PREFIX_RECIEVE_LUMBER, TEXT_TAG_HEIGHT_OFFSET_LUMBER)

            loop
                exitwhen (itemCounter == 0)
                set itemCounter = itemCounter - 1
                call CreateItem(chestTable[itemCounter], x, y)
            endloop
        
            // Fire Event.
            set thistype.eventIndex = this
            call TriggerEvaluate(thistype.openEvent)
            set thistype.eventIndex = prev
            
            // Return boolean 
            return true
        endmethod
        
        static method operator [] takes unit whichUnit returns thistype
            static if DEBUG_MODE and LIBRARY_ErrorMessage then
                call ThrowWarning((whichUnit == null),        "Chest", "[]", "whichUnit", 0, "Invalid Unit (null)!")
                call ThrowWarning((not table.has(whichUnit)), "Chest", "[]", "whichUnit", 0, GetUnitName(whichUnit) + " Is Not A ChestUnit!")
            endif
        
            return table[GetHandleId(whichUnit)]
        endmethod
        
        method addLumber takes integer minLumber, integer maxLumber returns nothing
            static if DEBUG_MODE and LIBRARY_ErrorMessage then
                call ThrowError((minLumber > maxLumber), "Chest", "addLumber", "min > max", this, "Passed In Minimum Lumber > Passed In Maximum Lumber!")
            endif
            
            set chestTable[thistype.lumberSlot] = GetRandomInt(minLumber, maxLumber)
        endmethod
        
        method addGold takes integer minGold, integer maxGold returns nothing
            static if DEBUG_MODE and LIBRARY_ErrorMessage then
                call ThrowError((minGold > maxGold), "Chest", "addGold", "min > max", this, "Passed In Minimum Gold > Passed In Maximum Gold!")
            endif
            
            set chestTable[thistype.goldSlot] = GetRandomInt(minGold, maxGold)
        endmethod
        
        method addItemIdExplicit takes integer itemid returns nothing
            static if DEBUG_MODE and LIBRARY_ErrorMessage then
                call ThrowError((itemid == 0), "Chest", "addItemIdExplicit", "itemid", this, "Invalid Integer Argument (0)!")
            endif            
            
            set chestTable[itemCounter] = itemid
            set itemCounter             = itemCounter + 1
        endmethod
        
        method applyItemPool takes itempool whichItempool returns nothing
            static if DEBUG_MODE and LIBRARY_ErrorMessage then
                call ThrowError((whichItempool == null), "Chest", "applyItempool", "itempool", this, "Invalid Itempool (null)!")
                call ThrowWarning((pool != null),        "Chest", "applyItempool", "itempool", this, "Over-writting Itempool Handle!")
            endif
            
            set pool = whichItempool
        endmethod
        
        static method create takes player forWho, integer unitid, real x, real y, real face returns thistype
            local thistype this = thistype.allocate()
            
            set chestUnit       = CreateUnit(forWho, unitid, x, y, face)
            set chestTable      = Table.create()
            set table[GetHandleId(chestUnit)] = this   
            set requiredKey     = 0
            set itemCounter     = 0
            set isOpen          = false
            call PauseUnit(chestUnit, true)
            
            static if DEBUG_MODE and LIBRARY_ErrorMessage then
                call ThrowError((GetUnitTypeId(chestUnit) == 0), "Chest", "create", "unitid", unitid, "Invalid unitid! Created Chest Unit Is null!")
            endif
            
            return this
        endmethod

        private static method init takes nothing returns nothing
            set Chest.table = Table.create()
            set onOpenSound = CreateSound("Sound\\Ambient\\DoodadEffects\\DoorSlam1.wav", false, false, true, 10, 10, "CombatSoundsEAX")
            set error       = CreateSoundFromLabel("InterfaceError", false, false, false, 10, 10)
        endmethod
        
        implement optional CHEST_INITIALIZER
        implement optional Init
        
    endstruct
    
    function GetTriggerChest takes nothing returns Chest
        return Chest.eventIndex
    endfunction

endlibrary

 

[IcemanBo]

Well there is quite some of critique/suggestions by other users, but let' just work slowly.

 //maxindex allowed, with other words a chest can drop 999 items by default
    //note that you can extend the variable above the normal cap (8191)
    private constant integer MAX_INDEX = 999
    //unit type of the chests used
    private constant integer CHEST_TYPE = 'h000'
    //animation played when opened
    private constant string OPEN_ANIMATION = "stand alternate"

You note the systems can work for any unit in map. Then the config seems kind of pointless to act as static variables.

Atm I don't really agree with the coding. Neighter with the concept, nor with efficiency.

Actually, read stuff suggested by others already.
For example BPower's code. You don't need to make like him, but you should understand the concept.
Also I stress his point again:

- remove all that texttag, sound, locust etc... stuff

 

[Chaosy]

Meh. Seems like too much work. Sad as it is since I considered it as one of my better creations.

Place it as rejected instead, I wont update this.

 

[IcemanBo]

Done.