Shattering Frost - v2.0.1

[Moderator]

16 Dec 2011
Bribe: The changes look much better. Rating changed to 4/5 (Recommended).

 

[Mr_Bean]

Update:
- Fixed a few issues with the code.
- Fixed a few things with the dummy caster.
- In the demo map, if you kill a ghoul, you will now get gold.
- You can now configure the delay between when the spell is cast and when the units are stunned.

The player did not get credit because I didn't turn the Gives Bounty flag on. I have changed that now

 

[Maker]

What I meant with the credit was the actual kill. Try displaying unit name of killing unit.

This is important if there is kill count for heroes. You don't have to change it, but I prefer to have the hero deal all the damage.

 

[Mr_Bean]

Surely if there is a kill count they would count all the kills made by the player, not just by the hero? Like if the hero can summon an Infernal, for example. The Infernal can kill units, which would ideally add to the kill count.

 

[OxygenD]

    private function Timer_Actions takes nothing returns nothing
        local timer t = GetExpiredTimer()
        local Data D = Data(GetTimerData(t))
        local real x = GetUnitX(D.target)
        local real y = GetUnitY(D.target)
        local unit u
        call GroupEnumUnitsInRange(bj_lastCreatedGroup, x, y, RANGE, Condition(function Group_Conditions))
        loop
            set u = FirstOfGroup(bj_lastCreatedGroup)
            exitwhen u == null
            if (u != D.target) then
                set bj_lastCreatedUnit = CreateUnit(GetOwningPlayer(D.caster), DUMMY_ID, x, y, 0)
                call UnitAddAbility(bj_lastCreatedUnit, DUMMY_SPELL_ID)
                call SetUnitAbilityLevel(bj_lastCreatedUnit, DUMMY_SPELL_ID, D.level)
                call IssueTargetOrder(bj_lastCreatedUnit, "thunderbolt", u)
                call UnitApplyTimedLife(bj_lastCreatedUnit, 'BTLF', 5)
            endif
            call GroupRemoveUnit(bj_lastCreatedGroup, u)
        endloop
        call ReleaseTimer(t)
        set t = null
        call D.destroy()
    endfunction

Dont use bj_lastCreatedUnit and bj_lastCreatedGroup its just globals variables. In your variant you can use just local group GroupName. Create this local group and change all bj_lastCreatedGroup to GroupName. Do the same with the unit.

 

[Ruke]

library ShatteringFrost requires TimerUtils
    globals
        // Important Configurables:
        private constant integer SPELL_ID = 'A000'          // Change A000 to the raw code of the "Shattering Frost" spell.
        private constant integer DUMMY_SPELL_ID = 'A001'    // Change A001 to the raw code of "Shattering Frost [dummy]" spell.
        private constant integer DUMMY_ID = 'h001'          // Change h001 to the raw code of your dummy unit.
        
        // Other Configurables:
        private constant real RANGE = 250                   // Area of effect (how close units have to be to the target to get stunned).
        private constant real SHARD_DELAY = 0.5             // The delay between the ice blast and when the units get stunned. I wouldn't make it higher than 0.5 seconds.
    endglobals
    
    private function Group_Conditions takes unit target, unit caster returns boolean
        return (IsUnitEnemy(target, GetOwningPlayer(caster))) and (IsUnitType(target, UNIT_TYPE_DEAD) == false)
    endfunction
    
    // End Of Configurables.
    // Don't change anything past this point!
    
    private struct Data
        unit caster
        unit target
        integer level
        
        method destroy takes nothing returns nothing
            set this.caster = null
            set this.target = null
            call this.deallocate()
        endmethod
        
        static method Timer_Actions takes nothing returns nothing
            local timer t = GetExpiredTimer()
            local thistype D = GetTimerData(t)
            local real x = GetUnitX(D.target)
            local real y = GetUnitY(D.target)
            local unit u
            local unit dummy
            
            call GroupEnumUnitsInRange(bj_lastCreatedGroup, x, y, RANGE, null)
            
            loop
                set u = FirstOfGroup(bj_lastCreatedGroup)
                exitwhen u == null
                
                if (u != D.target and Group_Conditions(u, D.caster)) then
                    set dummy = CreateUnit(GetOwningPlayer(D.caster), DUMMY_ID, x, y, 0)
                    
                    call UnitAddAbility(dummy, DUMMY_SPELL_ID)
                    call SetUnitAbilityLevel(dummy, DUMMY_SPELL_ID, D.level)
                    call IssueTargetOrder(dummy, "thunderbolt", u)
                    call UnitApplyTimedLife(dummy, 'BTLF', 5)
                    
                    set dummy = null
                endif
                
                call GroupRemoveUnit(bj_lastCreatedGroup, u)
            endloop
            
            call ReleaseTimer(t)
            set t = null
            call D.destroy()
        endmethod

        static method create takes nothing returns thistype
            local timer t = NewTimer()
            local thistype D = thistype.allocate() 
            
            set D.caster = GetTriggerUnit()
            set D.target = GetSpellTargetUnit()
            set D.level = GetUnitAbilityLevel(D.caster, SPELL_ID)
            
            call SetTimerData(t, D)
            call TimerStart(t, SHARD_DELAY, true, function thistype.Timer_Actions)
            set t = null
            
            return D
        endmethod
        
        static method Conditions takes nothing returns boolean
            if (GetSpellAbilityId() == SPELL_ID) then
                call thistype.create()
            endif
            
            return false
        endmethod
        
        static method onInit takes nothing returns nothing
            local trigger t = CreateTrigger()
            call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
            call TriggerAddCondition(t, Condition(function thistype.Conditions))
        endmethod
    endstruct    
endlibrary

• Use library instead scope (then, make it requires TimerUtils or the libraries that you will use later ).
  
• The "Actions" function is unnesesary, you can make all of these actions into the create method.
  
• Use the "destroy" method to null your variables (only who really needs null) and then, use deallocate (if you're using allocate).
  
• Use "thistype" keyword instead the name of the struct (is the same, but better e_e).
  
• Don't use "bj_lastCreatedUnit" like this.
  
• When you enumerate units into a group, don't put a filter condition, make the filter in the loop .

call TimerStart(t, SHARD_DELAY, true, function Timer_Actions)

Why true? the actions only executes one time.

local Data D = Data(GetTimerData(t))

->

local Data D = GetTimerData(t)

Null t when you create the trigger

You will never (in this case) destroy the trigger, so, what's the point?

---

Nice spell .

Greetings

 

[Magtheridon96]

Dont use bj_lastCreatedUnit and bj_lastCreatedGroup its just globals variables. In your variant you can use just local group GroupName. Create this local group and change all bj_lastCreatedGroup to GroupName. Do the same with the unit.

It's actually the opposite. I encourage using the global group instead of creating and destroying a group in the same thread. It's cleaner and more efficient

There's no need to use bj_lastCreatedUnit though.
Creating a local variable for that is faster (Locals are theoretically faster than globals and they don't get slower with more characters as much as globals do).

If u is a global and v is a local, u is faster and v is slower.
If uu is a global and vv is a local, vv is faster and uu is slower.

You can imagine the following formulas:
This is only an example.

globalLookup = 3x + 5
localLookup = x + 8

x is the number of characters.

With 1 character, globalLookup = 8 and localLookup = 9 (globalLookup is faster)
With any other number of characters, the localLookup would be faster.

This is purely an example that shows a special case when globals are faster than locals.

This still doesn't give you a reason to use dynamic group handles in any function because function calls are DRASTICALLY slow relative to any variable lookup

But, I wouldn't recommend using one-character long names for global variables, so locals are technically always going to be faster than globals.

----------
TL;DR?

Use a local unit; Keep the bj_lastCreatedGroup.

 

[Mr_Bean]

Thanks guys!

Update:
- Replaced bj_lastCreatedUnit with a local variable.
- Used Ruke's suggestions.
- You can now change the delay between the casting of the spell and the stun. It is initially set to 0.5/0.4/0.3 seconds (suggested by Bribe).
- You can now change the range of the stun per level. It is initially set to 200/250/300 (suggested by Bribe).

 

[mckill2009]

- I suggest put "thunderbolt" as a global so that it can be easily adjusted if the user doesnt want that...
- I never tried this spell but I think T32 is a good option than TimerUtils...
- I dont think it's necessary to put the destroy method...call thig.deallocate() is fine...

 

[Ruke]

- I dont think it's necessary to put the destroy method...call thig.deallocate() is fine...

Do you have a problem with the destroy method? xD

As did you say in your spell, if it's not a requirement (I have questions about this) but it's recomended, why shouldn't we do it?

 

[mckill2009]

Do you have a problem with the destroy method? xD

As did you say in your spell, if it's not a requirement (I have questions about this) but it's recomended, why shouldn't we do it?

I posted in the other thread...

it may leak but very minimal, I've created a test with nulling a unit and not...

here's the result: creates unit every 0.03/sec, units created 1500...

Ram 101,432 = nulls the unit with your method

Ram 101,498 = w/o nulling them but with .destroy()

maybe 1500 isnt that much but I believe even if 10000, results is not a big deal...

EDIT: SOrry for double posting...

EDIT2: New test 3000 units...

Ram 134396 = no nulling
Ram 134664 = nulls with your method

 

[Mr_Bean]

Update - v2.0:
- Completely rewrote everything from scratch. It made me want to cry when I saw how badly I used to code.

 

[Lambdadelta]

^ Lol :3

New code is very readable. Well done on presentation.

 

[Mr_Bean]

Thanks. I was very shocked when I looked at the old code, because I thought I would have done something slightly better back then :S

 

[Maker]

-Method onHit has unused local unit dummy variable
-I think dead units can have over 0.405 life. Use not(IsUnitType(unit, UNIT_TYPE_DEAD) or GetUnitTypeId(unit) == 0)
-Increase follow through time so the caster completes the cast animation
-Pausing a unit should be avoided, it pauses buffs

 

[Adiktuz]

When I saw it, I was thinking like: "I saw this a long time ago"

than I saw the upload date... XD...

anyway, it was really nice for you to rewrite the code and make it better...

 

[Mr_Bean]

Haha, you actually remembered it? :O

 

[Losam]

Great job, it was well written, developed (not to mention that the description of the spell was well done).
In my opinion deserves 5/5.

 

[Adiktuz]

yeah, there's only quite a few spells of this idea here on hive....

 

[Mr_Bean]

Update - v2.0.1:
- Removed an unused local variable.
- Fixed the unit filter to check properly for dead units.
- The original target is now stunned instead of paused (because pausing a unit pauses its buffs).

Thanks to Maker for the above suggestions.

EDIT: Added screenshots to illustrate what happens when you cast the spell.

 

[ShadyRO]

I get an error while trying to play the map with this spell in it.

It says : Fatal error! Exception 0xC00000000005 access violation at 00:23000000004 the instruction 0x00000000004 referenced memory at 0x0000000004 could not be writen, click ok to terminate the application, wtf ?

 

[Map Designer]

the Spell is quite cool.

May I suggestion one thing though? perhaps you could configure maximum number of ice shards that will stun enemies around target?

as you know, many of AOE spells have maximum damage. if so many units are surrounding the target, they will all be hit by shards and get stunned/damaged, with no limit to the maximum number of targets. I would say 4/5 targets max is kinda balanced.

also, if more than this limit is around the target unit, then Random 5 must be chosen.

dont you agree?

 

[Pyrogasm]

All you have to do is modify private static method explode to create a shard for only N units instead of all the units.

private static ...
  local integer i = 0
  //...
  loop
    exitwhen temp == null or i >= 5 //add the second half of this line
    //...
    if ...
      set i = i+1
      //...
    endif
  endloop
//...
endfunction

FirstOfGroup is functionally random afaik but if you really care you can replace it with GroupPickRandomUnit instead.