1. Are you planning to upload your awesome spell or system to Hive? Please review the rules here.
    Dismiss Notice
  2. The Aftermath has been revealed for the 19th Terraining Contest! Be sure to check out the Results and see what came out of it.
    Dismiss Notice
  3. Melee Mapping Contest #3 - Results are out! Congratulate the winners and check plenty of new 4v4 melee maps designed for this competition!
    Dismiss Notice
  4. The winners of our cinematic soundtrack competition have been decided! Step by the Music Contest #11 - Results to check the entries and congratulate the winners!
    Dismiss Notice

Cluster Rockets v1.1.0

Submitted by Ofel
This bundle is marked as approved. It works and satisfies the submission rules.
  • Description

Tinker deploys his rocket pads and fire all the rockets to any targeted unit or any units near the targeted location. Each rocket will explode if collide with any enemy units. The explosion causes damage to nearby units.
Level 1 - 32 rockets, 60 explosion damage.
Level 2 - 48 rockets, 65 explosion damage.
Level 3 - 64 rockets, 70 explosion damage.

Channeling
  • Changelog

--- v1.0.0 ---
First public release version.

--- v1.0.1 ---
Optimized scripts based on some review.

--- v1.0.2 ---
Optimized scripts and fixed a small leak based on reviews.

--- v1.0.3 ---
Optimized scripts and a little performance fix based on review.
Changed tooltip and hotkey.
New ability icon.

--- v1.0.4 ---
Fixed one fatal problem: spell freezes due to bad indexing.

--- v1.0.5 ---
Fixed another fatal problem: can't cast at the center of map.

--- v1.0.6 ---
Optimized unit target acquiring.

--- v1.0.7 ---
Fixed bug: rockets stop when casting rapidly.

--- v1.1.0 ---
Adapted to game patch 1.30. No more dummy units.
New property settings.
Improved ground targeting.
Added target effect on units.
Changed configuration variables.​
  • Credits

Some of the code are adapted from vJASS libraries and Wurst.
Credits to Nestharus, D.O.G, and Wareditor.

Icon by darkdeathknight.
Target effect model by ChirusHighwind.​
  • Other

You can find importing instruction and some Q&A inside the map.
Thanks also to Tank-Commander, Almia, Meatmuffin, BPower, Kam and ILH for their reviews that helped me discover things need to be fixed.​

Keywords:
Rocket, Explosive, Collision, Cluster.
Previews
Contents

Cluster Rockets (Map)

Reviews
Moderator
11:09, 31st May 2016 Tank-Commander: Thanks for the great submission, just have a few notes on some issues. See my post
  1. 11:09, 31st May 2016
    Tank-Commander: Thanks for the great submission, just have a few notes on some issues. See my post
     
  2. Tank-Commander

    Tank-Commander

    Spell Reviewer

    Joined:
    May 26, 2009
    Messages:
    1,544
    Resources:
    44
    Packs:
    1
    Spells:
    41
    Tutorials:
    2
    Resources:
    44
    Just went over very quickly (haven't quite finished going through)

    - You never flush your hashtable
    - Your check unit exists function is flawed: a unit can have x and y co-ordinates of 0 and still exist albeit very unlikely, you can check if the UnitTypeId is null (which will be the case if it doesn't exist)
    - This is a very weird hybrid of using a Linked List and a Hashtable, why?
    - You only need one initialisation function, they can be combined
     
  3. Almia

    Almia

    Joined:
    Apr 24, 2012
    Messages:
    4,839
    Resources:
    35
    Spells:
    30
    Tutorials:
    4
    JASS:
    1
    Resources:
    35
    GetWidgetLife cannot detect whether or not a unit(widget) is alive in some circumstances.
    You can just check if the UnitTypeId is 0

    Check whether the unit exist in the map should be removed. It is just equal to UnitAlive

    FlyAbilityID is not recommended to become a constant. Users might change it and break the spell :V
     set rFacing = GetRandomReal(0, 360) * bj_DEGTORAD

    ->
     set rFacing = GetRandomReal(-bj_PI, bj_PI) 


    As for Tank-Commander's comment
    - The reason his hashtable is not flushed is because it is being used for unit collision values. But I think the keys used for channeling check shall be flushed.

    - " This is a very weird hybrid of using a Linked List and a Hashtable, why?" Hashtable was used for collision check(Again) and for channeling check. He doesn't use a Unit Indexer so he doesn't want to do a linear search for it :V

    Overall, I liked it :D

    I should test this tomorrow

    opinion : I think you should use the Barrage missile model :V
     
  4. Ofel

    Ofel

    Joined:
    Mar 29, 2012
    Messages:
    428
    Resources:
    10
    Spells:
    10
    Resources:
    10
    You mean the custom script inside the configuration?
    Nah, I think it's not neat :3
    It's also made for if the user want to reconfigure the spell for some reason, they can just put it at the end of their new configuration (that's the bonus maybe).

    Done.

    I was thinking that in worst case, that the default crow ability has been somehow edited. So the user can use this to rescue the spell. *\o/*

    Forgot this one. Done.

    It's just boolean and integer. Right?
     
  5. Almia

    Almia

    Joined:
    Apr 24, 2012
    Messages:
    4,839
    Resources:
    35
    Spells:
    30
    Tutorials:
    4
    JASS:
    1
    Resources:
    35
    you need to flush it for the sake of leaks i guess
     
  6. BPower

    BPower

    Joined:
    Mar 18, 2012
    Messages:
    1,745
    Resources:
    21
    Spells:
    15
    Tutorials:
    1
    JASS:
    5
    Resources:
    21
    You leak a local group handle in function ClsR_AcquireNewTarget also in function ClsR_TerminateRocket. and in the periodic function.

    You eventually leak a local unit when you exit the FoG iteration once a proper
    target unit was found.

    Very cool spell.
     
  7. Ofel

    Ofel

    Joined:
    Mar 29, 2012
    Messages:
    428
    Resources:
    10
    Spells:
    10
    Resources:
    10
    I did destroy the group below
    endloop
    .

    But I thought local unit is already nulled when FoG return null, isn't?
     
  8. Meatmuffin

    Meatmuffin

    Joined:
    Jul 25, 2014
    Messages:
    446
    Resources:
    9
    Maps:
    2
    Spells:
    7
    Resources:
    9
    It is not when you exit the group with
    exitwhen true


    Set the unit to null before you do that.
     
  9. Ofel

    Ofel

    Joined:
    Mar 29, 2012
    Messages:
    428
    Resources:
    10
    Spells:
    10
    Resources:
    10
    That! I missed.
    Gonna fix soon.

    Thanks.
    Should I null both local group and unit?
     
  10. Meatmuffin

    Meatmuffin

    Joined:
    Jul 25, 2014
    Messages:
    446
    Resources:
    9
    Maps:
    2
    Spells:
    7
    Resources:
    9
    I doubt it, but I prefer to use global variables when it comes to unit groups
    instead of local ones -- they are just more reliable and you don't have to
    create/destroy them, you just refer to them when you're using GroupEnum.
     
  11. Ofel

    Ofel

    Joined:
    Mar 29, 2012
    Messages:
    428
    Resources:
    10
    Spells:
    10
    Resources:
    10
    Yup. I might try to save as many locals as possible next time.
     
  12. BPower

    BPower

    Joined:
    Mar 18, 2012
    Messages:
    1,745
    Resources:
    21
    Spells:
    15
    Tutorials:
    1
    JASS:
    5
    Resources:
    21
    It's true that the FoG loop automatically nulls the local unit handle,
    if exitwhen u == null is the exit condition. Otherwise not.

    Yes you should null the local group handle.
     
  13. Dat-C3

    Dat-C3

    Joined:
    Mar 15, 2012
    Messages:
    2,443
    Resources:
    10
    Models:
    1
    Maps:
    5
    Spells:
    3
    Tutorials:
    1
    Resources:
    10
    Very cool, way better then the default cluster rockets.

    4/5 - Will raise my rating to 5/5 after I finish reading through the triggers. =)
     
  14. Tank-Commander

    Tank-Commander

    Spell Reviewer

    Joined:
    May 26, 2009
    Messages:
    1,544
    Resources:
    44
    Packs:
    1
    Spells:
    41
    Tutorials:
    2
    Resources:
    44
    I've gone over the code a bit more thoroughly now and noted a few things:

    - You use a lot of local unit groups and destroy them, since you always utilise them in a way that leaves them empty you do not need to do this and can use one global unit group for all uses (adding one global but removing all the lines creating/destroying local groups)
    - Please give your local variables meaningful names using single letters is very unhelpful; while you do explain what they are in most functions (some are not) having to scroll back up every two seconds isn't that great
    - You have the number for bj_DEGTORAD (57.2957795) instead of the constant bj_DEGTORAD in your code, seems a bit pointless
    - When you recycle with a linked list you don't need to null all the unit variables, it will not leak regardless (as the variable will be reused)
    - Having a GUI configuration does not a GUI spell make; this is a pure JASS submission
    - udg_ClsR_trig can be a local making
    Code (vJASS):
        if udg_ClsR_trig != null then
            call DestroyTrigger(udg_ClsR_trig)
        endif
    redundant
    - this makes the spell uncastable on the line x == 0 of any map (again unlikely but it is a problem)
    Code (vJASS):
    if GetOrderTarget() != null or GetOrderPointX() != 0 then


    I'd note not all of these are mandatory (the ones towards the top particularly) but it would be helpful and would be slight improvements in my opinion (notably in terms of readability) The spell is great and I'd like to approve it once these things have been solved
     
  15. Ofel

    Ofel

    Joined:
    Mar 29, 2012
    Messages:
    428
    Resources:
    10
    Spells:
    10
    Resources:
    10
    Gonna fix this ASAP.
     
  16. IcemanBo

    IcemanBo

    Joined:
    Sep 6, 2013
    Messages:
    5,936
    Resources:
    22
    Maps:
    3
    Spells:
    11
    Template:
    1
    Tutorials:
    4
    JASS:
    3
    Resources:
    22
    Pretty much was Tank already mentioned--and also
    null
    the local
    rect
    in the init.
     
  17. Tank-Commander

    Tank-Commander

    Spell Reviewer

    Joined:
    May 26, 2009
    Messages:
    1,544
    Resources:
    44
    Packs:
    1
    Spells:
    41
    Tutorials:
    2
    Resources:
    44
    I noticed this has been updated to v1.0.3 but the one critical issue (the GetOrderPointX() != 0 problem) still remains the spell is very near perfect bar that code-wise
     
  18. Ofel

    Ofel

    Joined:
    Mar 29, 2012
    Messages:
    428
    Resources:
    10
    Spells:
    10
    Resources:
    10
    Can the one problem be ignored? I can't figure out the solution.
     
  19. jakeZinc

    jakeZinc

    Joined:
    Aug 13, 2013
    Messages:
    1,366
    Resources:
    20
    Spells:
    20
    Resources:
    20
    And still theres a bug occuring ( cant post screenshot atm ), the current missiles just paused the whole spell instances ( the debug expiration timer didnt also work on missiles ), it is frequently visible by lowering the missile speed and rapidly casting it alternatively on the ally and enemy unit.