1. Are you planning to upload your awesome spell or system to Hive? Please review the rules here.
    Dismiss Notice
  2. Updated Resource Submission Rules: All model & skin resource submissions must now include an in-game screenshot. This is to help speed up the moderation process and to show how the model and/or texture looks like from the in-game camera.
    Dismiss Notice
  3. DID YOU KNOW - That you can unlock new rank icons by posting on the forums or winning contests? Click here to customize your rank or read our User Rank Policy to see a list of ranks that you can unlock. Have you won a contest and still havn't received your rank award? Then please contact the administration.
    Dismiss Notice
  4. The poll for Hive's 12th Concept Art Contest is up! Go cast your vote for your favourite genie!
    Dismiss Notice
  5. Travel to distant realms and encounter scenes unknown to the common folk. The Greatest of Adventures is upon us with the 8th Cinematic Contest. Join in on a fun ride.
    Dismiss Notice
  6. The 18th Icon Contest is ON! Choose any ingame unit and give him/her Hero abilities. Good luck to all.
    Dismiss Notice
  7. Contestants are to create a scene set in the Stone Age. Come and see what you can come up with. We wish you the best of luck!
    Dismiss Notice
  8. Colour outside the lines! Techtree Contest #13 is a go. The contest is optionally paired.
    Dismiss Notice
  9. Greetings cerebrates, our Swarm needs new spawners that will have numerous children. Join the HIVE's 31st Modeling Contest - Spawners and Spawned! The contest is optionally paired.
    Dismiss Notice
  10. Check out the Staff job openings thread.
    Dismiss Notice
Dismiss Notice
60,000 passwords have been reset on July 8, 2019. If you cannot login, read this.

Chest System

Submitted by Chaosy
This bundle is marked as substandard. It may contain bugs, not perform optimally or otherwise be in violation of the submission rules.
My first vJASS system that I uploaded. Don't be too harsh.

Even though the system itself is fully vJASS, I tried to make the demo as GUI friendly as possible.

Code (vJASS):
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
 


demo examples

  • test
    • Events
      • Map initialization
    • Conditions
    • Actions
      • Custom script: local UnitChest c
      • Hashtable - Create a hashtable
      • Set hash = (Last created hashtable)
      • -------- - --------
      • Set u = Chest 0001 <gen>
      • Set item = Shadow Orb Fragment
      • Custom script: set c = UnitChest.create()
      • Custom script: call c.SetChest(udg_u)
      • Custom script: call c.addItem(udg_item, 100)
      • Custom script: call c.addGold(100)
      • Set item = Blood Key
      • Custom script: call c.addKey(udg_item)
      • Custom script: call SaveInteger(udg_hash, GetHandleId(c.chest), 1, c)

  • drop
    • Events
      • Unit - A unit Starts the effect of an ability
    • Conditions
      • (Ability being cast) Equal to Unlock Chest
      • (Unit-type of (Target unit of ability being cast)) Equal to Chest
    • Actions
      • Custom script: local unit u = GetSpellTargetUnit()
      • Custom script: local UnitChest c = LoadInteger(udg_hash, GetHandleId(u), 1)
      • Set u = (Triggering unit)
      • Custom script: call c.UnlockChest(udg_u)

  • removetest
    • Events
      • Player - Player 1 (Red) types a chat message containing remove as An exact match
    • Conditions
    • Actions
      • Custom script: local UnitChest c = udg_some_chest
      • Custom script: call c.removeItem(udg_item)

  • spawn
    • Events
      • Player - Player 1 (Red) types a chat message containing spawn as An exact match
    • Conditions
    • Actions
      • Custom script: local UnitChest c = UnitChest.create()
      • Set loc = (Random point in (Playable map area))
      • Set x = (X of loc)
      • Set y = (Y of loc)
      • Custom script: call c.spawn(udg_x, udg_y)
      • Set item = Claws of Attack +3
      • Custom script: call c.addItem(udg_item, 100)
      • Custom script: call SaveInteger(udg_hash, GetHandleId(c.chest), 1, c)

  • event
    • Events
      • Game - CS_event becomes Equal to 1.00
    • Conditions
    • Actions
      • Game - Display to (All players) the text: (A chest was opened by + ((Name of CS_unlocker) + ( and gained + ((String(CS_gold)) + ( gold and + ((String(CS_lumber)) + lumber))))))



Keywords:
Chaosy, system, vjass, vJASS, chest, drop, loot
Contents

Just another Warcraft III map (Map)

Reviews
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 7th Jan 2015 IcemanBo:...
  1. 15th Feb 2015
    IcemanBo: map is updateable again.

    23th April 2015
    IcemanBo: https://www.hiveworkshop.com/posts/2678108/



    Older

    7th Jan 2015
    IcemanBo:
    https://www.hiveworkshop.com/posts/2636884/

    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.
     
  2. Chaosy

    Chaosy

    Joined:
    Jun 9, 2011
    Messages:
    10,609
    Resources:
    18
    Maps:
    1
    Spells:
    11
    Tutorials:
    6
    Resources:
    18
    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

    Code (vJASS):
    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
     

    Attached Files:

    Last edited: Apr 23, 2015
  3. Quilnez

    Quilnez

    Joined:
    Oct 12, 2011
    Messages:
    3,250
    Resources:
    37
    Icons:
    2
    Tools:
    1
    Maps:
    7
    Spells:
    21
    Tutorials:
    2
    JASS:
    4
    Resources:
    37
    Very well then. ^)^

    Code (vJASS):
    struct UnitChest

    Sounds more like a unit attachment point for me

    Code (vJASS):
    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:
    Code (vJASS):
    // 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 :grin:
     
  4. Chaosy

    Chaosy

    Joined:
    Jun 9, 2011
    Messages:
    10,609
    Resources:
    18
    Maps:
    1
    Spells:
    11
    Tutorials:
    6
    Resources:
    18
    I think I get your point. It shouldn't be too hard to implement.

    edit: updated!
     
    Last edited: Oct 9, 2014
  5. BPower

    BPower

    Joined:
    Mar 18, 2012
    Messages:
    1,741
    Resources:
    21
    Spells:
    15
    Tutorials:
    1
    JASS:
    5
    Resources:
    21
    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.
     
  6. Chaosy

    Chaosy

    Joined:
    Jun 9, 2011
    Messages:
    10,609
    Resources:
    18
    Maps:
    1
    Spells:
    11
    Tutorials:
    6
    Resources:
    18
    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.
     
    Last edited: Oct 13, 2014
  7. deathismyfriend

    deathismyfriend

    Joined:
    Oct 24, 2012
    Messages:
    6,532
    Resources:
    14
    Spells:
    12
    Tutorials:
    2
    Resources:
    14
    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.
    Code (vJASS):

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

    Chaosy

    Joined:
    Jun 9, 2011
    Messages:
    10,609
    Resources:
    18
    Maps:
    1
    Spells:
    11
    Tutorials:
    6
    Resources:
    18
    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 :p.

    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+
     
  9. IcemanBo

    IcemanBo

    Joined:
    Sep 6, 2013
    Messages:
    6,165
    Resources:
    22
    Maps:
    3
    Spells:
    11
    Template:
    1
    Tutorials:
    4
    JASS:
    3
    Resources:
    22
    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()
     
  10. Chaosy

    Chaosy

    Joined:
    Jun 9, 2011
    Messages:
    10,609
    Resources:
    18
    Maps:
    1
    Spells:
    11
    Tutorials:
    6
    Resources:
    18
    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?

    Silly mistake, will make sure to fix it.

    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.

    Well, the hashtable isn't exactly a requirement, I just decided to use it for the demo.
     
    Last edited: Oct 20, 2014
  11. IcemanBo

    IcemanBo

    Joined:
    Sep 6, 2013
    Messages:
    6,165
    Resources:
    22
    Maps:
    3
    Spells:
    11
    Template:
    1
    Tutorials:
    4
    JASS:
    3
    Resources:
    22
    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.

    Code (vJASS):

    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..
     
  12. IcemanBo

    IcemanBo

    Joined:
    Sep 6, 2013
    Messages:
    6,165
    Resources:
    22
    Maps:
    3
    Spells:
    11
    Template:
    1
    Tutorials:
    4
    JASS:
    3
    Resources:
    22
    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
     
  13. Chaosy

    Chaosy

    Joined:
    Jun 9, 2011
    Messages:
    10,609
    Resources:
    18
    Maps:
    1
    Spells:
    11
    Tutorials:
    6
    Resources:
    18
    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.
     
    Last edited: Apr 11, 2015
  14. bowser499

    bowser499

    Joined:
    Jul 20, 2009
    Messages:
    782
    Resources:
    7
    Tools:
    1
    Maps:
    3
    Spells:
    3
    Resources:
    7
    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
     
  15. BPower

    BPower

    Joined:
    Mar 18, 2012
    Messages:
    1,741
    Resources:
    21
    Spells:
    15
    Tutorials:
    1
    JASS:
    5
    Resources:
    21
    thats not true
     
  16. pred1980

    pred1980

    Joined:
    Mar 19, 2010
    Messages:
    844
    Resources:
    1
    Maps:
    1
    Resources:
    1
    Why??
     
  17. Quilnez

    Quilnez

    Joined:
    Oct 12, 2011
    Messages:
    3,250
    Resources:
    37
    Icons:
    2
    Tools:
    1
    Maps:
    7
    Spells:
    21
    Tutorials:
    2
    JASS:
    4
    Resources:
    37
    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?
    Code (vJASS):
        method SetChest takes unit u returns nothing
            set this.chest = u
        endmethod


    - Why this is not done in the create method?
    Code (vJASS):
        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:
     
  18. Chaosy

    Chaosy

    Joined:
    Jun 9, 2011
    Messages:
    10,609
    Resources:
    18
    Maps:
    1
    Spells:
    11
    Tutorials:
    6
    Resources:
    18
    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.
     
  19. BPower

    BPower

    Joined:
    Mar 18, 2012
    Messages:
    1,741
    Resources:
    21
    Spells:
    15
    Tutorials:
    1
    JASS:
    5
    Resources:
    21
    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
    Code (vJASS):
    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.
    Code (vJASS):
        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.