• 🏆 Texturing Contest #33 is OPEN! Contestants must re-texture a SD unit model found in-game (Warcraft 3 Classic), recreating the unit into a peaceful NPC version. 🔗Click here to enter!
  • It's time for the first HD Modeling Contest of 2024. Join the theme discussion for Hive's HD Modeling Contest #6! Click here to post your idea!

Fire Storm_v2

Fire Storm

This was the spell I used for the Zephyr Challenge #6. This is version 2, fixed allot of the things to optimize it.

This was created from scratch, not edit from the entry.

Requires - JNGP (vJass)

JASS:
scope FireStorm initializer Init

globals

    private constant integer SpellId    = 'A000'
        //This is the spell's raw code.
    private constant integer EntityUnit = 'n000'
        //This is the dummy unit
    private constant real InitDamage    = 100
        //This is the initial damage that the fire ball will deal
    private constant real LvlInc        = 25
        //This is the damage increase per level
    private constant real MaxDist       = 500
        //This is the maximum distance that the fire balls will move away from the cast point
    private constant real MinDist       = 100
        //This is the minimum distance when created
    private constant real MaxHeight     = 600
        //This is the maximum height the fire balls will travel
    private constant real UnitTime      = 6
        //This is the dummy units timed life
    private constant real DetectDist    = 100
        //This is the distance that an unit will be detected.
    private constant string ModelPath   = "Abilities\\Weapons\\RedDragonBreath\\RedDragonMissile.mdl"
        //This is the fire ball's model path
    private constant string Effect      = "Abilities\\Weapons\\RedDragonBreath\\RedDragonMissile.mdl"
        //This is the colision effect
    private constant string BirthEffect = "Abilities\\Spells\\Other\\Incinerate\\FireLordDeathExplode.mdl"
        //This is the birth effect
        
    private integer counter     = 0
    private timer TIM           = CreateTimer()
    private firedata array Data

endglobals

struct firedata

    unit caster = null
    unit entity = null
    real a      = 0
    real x      = 0
    real y      = 0
    real z      = 0
    real d      = 0
    real xC     = 0
    real yC     = 0
    effect s    = null
    
    static method create takes unit u, real x, real y returns firedata
    
        local firedata dat = firedata.allocate()
        
        set dat.caster = u
        set dat.xC     = x
        set dat.yC     = y
        set dat.d      = GetRandomReal(MinDist, MaxDist)
        set dat.a      = GetRandomReal(0, 360)
        set dat.x      = x + dat.d * Cos(dat.a * bj_DEGTORAD)
        set dat.y      = y + dat.d * Sin(dat.a * bj_DEGTORAD)
        set dat.entity = CreateUnit(GetOwningPlayer(u), EntityUnit, dat.x, dat.y, dat.a)
        set dat.s      = AddSpecialEffectTarget(ModelPath, dat.entity, "origin")
        
        
        call DestroyEffect(AddSpecialEffect(BirthEffect, dat.x, dat.y))
        call UnitApplyTimedLife(dat.entity, 'BFLT', UnitTime)
        
        if counter == 0 then
            call TimerStart(TIM, 0.03, true, function firedata.onLoop)
        endif
        
        set counter          = counter + 1
        set Data[counter -1] = dat
        
        return dat
    
    endmethod
    
    static method damageArea takes unit u, real x, real y returns nothing
    
       
        local group g = CreateGroup()
        local unit  n = null  
        local real  d = (GetUnitAbilityLevel(u, SpellId) * LvlInc) + InitDamage
    
        call GroupEnumUnitsInRange(g, x, y, DetectDist, null)
    
        loop
    
            set n = FirstOfGroup(g)
            exitwhen n == null
            if IsUnitEnemy(n, GetOwningPlayer(u)) == true then
                call UnitDamageTarget(u, n, d, true, false, ATTACK_TYPE_MAGIC, DAMAGE_TYPE_NORMAL, null)
                call DestroyEffect(AddSpecialEffectTarget(Effect, n, "chest"))
            endif
            call GroupRemoveUnit(g, n)
    
        endloop
    
        call DestroyGroup(g)
        
        set g = null
       
    endmethod
    
    static method detectUnit takes unit u, real x, real y returns integer
    
        local group    g = CreateGroup()
        local integer  i = 0
        local unit     n = null   
    
        call GroupEnumUnitsInRange(g, x, y, DetectDist, null)
    
        loop
    
            set n = FirstOfGroup(g)
            exitwhen n == null
            if IsUnitEnemy(n, GetOwningPlayer(u)) == true then
                if GetUnitState(n, UNIT_STATE_LIFE) > 0 then
                    call DestroyGroup(g)
                    set g = null
                    set n = null
                    return 0
                endif
            endif
            call GroupRemoveUnit(g, n)
            
            call DestroyGroup(g)
            set n = null
            set g = null
    
        endloop
    
        call DestroyGroup(g)
        set g = null
    
        return 1
    
    endmethod
    
    static method onLoop takes nothing returns nothing
    
        local firedata dat
        local integer i = 0
        
        loop
        
            exitwhen i >= counter
            set dat = Data[i]
            
            //This is the onLoop actions
            
            set dat.a = dat.a + 5
            set dat.x = dat.xC + dat.d * Cos(dat.a * bj_DEGTORAD)
            set dat.y = dat.yC + dat.d * Sin(dat.a * bj_DEGTORAD)
            
            call SetUnitX(dat.entity, dat.x)
            call SetUnitY(dat.entity, dat.y)
            
            set dat.z = dat.z + 20
            call SetUnitFlyHeight(dat.entity, dat.z, 40)
            
            if firedata.detectUnit(dat.entity, dat.x, dat.y) == 0 then
            
                call KillUnit(dat.entity)
                call firedata.damageArea(dat.caster, dat.x, dat.y)
            
            endif
            
            //End of onLoop actions
            
            if GetWidgetLife(dat.entity) < 0.405 then
            
                set Data[i] = Data[counter - 1]
                set counter = counter - 1
                call DestroyEffect(AddSpecialEffectTarget(Effect, dat.entity, "origin"))
                
                call dat.destroy()
            
            endif
            
            set i = i + 1
        
        endloop
        
        if counter == 0 then
            call PauseTimer(TIM)
        endif
    
    endmethod
    
    method onDestroy takes nothing returns nothing
    
        local unit u = .entity
        call DestroyEffect(.s)
        call TriggerSleepAction(.1)
        call RemoveUnit(u)
        
        set u = null
    
    endmethod

endstruct

private function FireStormConditions takes nothing returns boolean

    return GetSpellAbilityId() == SpellId
    
endfunction

private function FireStormActions takes nothing returns nothing

    local integer i = 0
    local unit    u = GetTriggerUnit()
    local real    x = GetSpellTargetX()
    local real    y = GetSpellTargetY()

    loop
    
        exitwhen i == 20
        set i = i + 1
        call TriggerSleepAction(.1)
        call firedata.create(u, x, y)
    
    endloop
    
    set u = null

endfunction

private function Init takes nothing returns nothing
    
    local trigger MainTrigger = CreateTrigger()

    call TriggerRegisterAnyUnitEventBJ( MainTrigger, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( MainTrigger, Condition( function FireStormConditions ) )
    call TriggerAddAction( MainTrigger, function FireStormActions )

    set MainTrigger = null
    
endfunction

endscope

Credits :

Silvenon - I learned allot from his code.
xxdingo93xx - Visual Improvements
Dynasti - Code Improvements and bug fix
PurgeandFire111 - Code Improvements and bug fix



Keywords:
fire, storm, fire storm
Contents

Xiliger Test Map (Map)

Reviews
17:18, 12th Mar 2010 The_Reborn_Devil: The coding looks ok, but in the method detectUnit you can in some cases destroy the group g twice. Also, I would recommend using a group recycler as you seem to be using groups a lot. And although separating...

Moderator

M

Moderator

17:18, 12th Mar 2010
The_Reborn_Devil:

The coding looks ok, but in the method detectUnit you can in some cases destroy the group g twice. Also, I would recommend using a group recycler as you seem to be using groups a lot. And although separating the code into several functions makes it easier to read, it also makes it less efficient as NewGen only inlines functions with only one line in them. Since there's nothing really bad I approve this.


Status: Approved
Rating: Useful
 
Level 18
Joined
Oct 18, 2007
Messages
930
Well I'm unable to test the map for sum reason, so I will rate this by it's code.

I'm giving it a 3/5.

Reason(s):
  • You shouldn't use libraries for Spells, use scopes
  • Why use a library/scope when nothings private? If you made 2 spells with the init function it would mess up.
  • Don't use TriggerSleepAction or Waits. It acts weird etc in multiplayer and is unnecceseary in singleplayer if you know how to use a timer
  • You use a variable that is not needed, "Trigger". You can just make a local one and null it, saves memory ;)
  • JASS:
    if GetUnitState(dat.entity, UNIT_STATE_LIFE) < 1 then
    Don't use < 1 or > 0. As it will bug up. Use the correct value of .405

How you can improve:
  • Make your spell functions, globals, structs Private( But only if it isn't going to be used by another trigger or function outside the library/scope. For this, just make a unique named function that is not private named for example:
    JASS:
    function MySpellName_AccesAnyNameHere takes anything returns whateveryou'dlike
  • Read up on some vJass/Jass guides on how to compose your coding in an efficient way
  • Try to minimalize unecceseary variables etc..
  • Fix Bugs.

~Dynasti
 
Look inside the hidden tags for the fix for the compile error, and for other leak/normal cleanups:

JASS:
function FireStormConditions takes nothing returns boolean

    if GetSpellAbilityId() == SpellId then
        return true
    else
        return false
    endif
    
endfunction
Should be:
JASS:
function FireStormConditions takes nothing returns boolean
    return GetSpellAbilityId() == SpellId
endfunction
Change:
JASS:
call damageArea(dat.caster, dat.x, dat.y)
To:
JASS:
call firedata.damageArea(dat.caster, dat.x, dat.y)
Else it won't compile. =D

JASS:
    static method damageArea takes unit u, real x, real y returns nothing
    
       
        local group g = CreateGroup()
        local unit  n = null  
        local real  d = (GetUnitAbilityLevel(u, SpellId) * LvlInc) + InitDamage
    
        call GroupEnumUnitsInRange(g, x, y, DetectDist, null)
    
        loop
    
            set n = FirstOfGroup(g)
            exitwhen n == null
            if IsUnitEnemy(n, GetOwningPlayer(u)) == true then
                call UnitDamageTarget(u, n, d, true, false, ATTACK_TYPE_MAGIC, DAMAGE_TYPE_NORMAL, null)
                call DestroyEffect(AddSpecialEffectTarget(Effect, n, "chest"))
            endif
            call GroupRemoveUnit(g, n)
    
        endloop
    
        call DestroyGroup(g)
        set g = null //add this
       
    endmethod
You need to null the group, cleared groups are not null, they are just empty.

In this method:
JASS:
    static method detectUnit takes unit u, real x, real y returns integer
You should add this:
JASS:
set g = null //add this at the end for the same reasons above
Also change this:
JASS:
                if GetUnitState(n, UNIT_STATE_LIFE) > 0 then
                    call DestroyGroup(g)
                    return 0
                endif
To this:
JASS:
                if GetUnitState(n, UNIT_STATE_LIFE) > 0 then
                    call DestroyGroup(g)
                    set g = null 
                    set n = null //since you are returning before the loop is finished
                    return 0
                endif
Also, this:
JASS:
if GetUnitState(n, UNIT_STATE_LIFE) > 0 then
Should be:
JASS:
if GetWidgetLife(n) > 0.405 then
But it doesn't really make a noticeable difference except in specific situations. (You can actually use a native declared from the .ai, but oh well)

There are other things, like using TSA in a loop rather than a timer, using FirstOfGroup() instead of ForGroup(), but those are a little annoying to fix.


Nice job otherwise. (Sorry if you did fix some of these things, I just based it on your code posted, I didn't actually download the map) =D

EDIT: Just tested the map, looks awesome.
 
Last edited:
Level 15
Joined
Jul 6, 2009
Messages
889
You shouldn't use libraries for Spells, use scopes
What is wrong with using libraries for spells?

1) Common practise to use CAPITALS for global constants.

2) I don't see any documentation for the spell. How do I import it? What steps do I follow?

3)
JASS:
call TimerStart(TIM, 0.03, true, function firedata.onLoop)

0.03 should be editable.

4)
JASS:
         loop
    
            set n = FirstOfGroup(g)
            exitwhen n == null
            if IsUnitEnemy(n, GetOwningPlayer(u)) == true then
                call UnitDamageTarget(u, n, d, true, false, ATTACK_TYPE_MAGIC, DAMAGE_TYPE_NORMAL, null)
                call DestroyEffect(AddSpecialEffectTarget(Effect, n, "chest"))
            endif
            call GroupRemoveUnit(g, n)
    
        endloop
Why don't you damage the enemies directly in the conditional filter, you then solve 4 and this one at the same time.

5) Dynamic groups!! Use GroupUtils for your group needs.

6) Trigger's that stay around don't need to be nulled (MainTrigger).

I'd say this is badly coded, but the spell itself, I must say is really good! IT's sheer eyecandy. I'm gonna actually try turning it into ice to see what things I get =D
 
Last edited:
Level 15
Joined
Jul 6, 2009
Messages
889
This spell has not been approved in the resource section and is not guaranteed to be working.

Yet it's approved lol.

Since there's nothing really bad I approve this.

:razz: Nothing really bad?

The So Called Rules said:
Performance
Submissions must be bugless, leakless, lagless on a computer that runs Warcraft III smoothly, and fully multiinstanceable. Spells using non-recycling "indexing systems" will not be accepted.

Even if it is faster it still leaks...

There is a WHOLE lot of stuff could be optimized in this spell [which I won't point out one by one as I'm too lazy to type :mwahaha:].

But anyways:

The things I mentioned +

1) Merging the Conditions block with the Actions block, I just heard it's better and I've been doing so for a long time :grin:

2) if GetUnitState(n, UNIT_STATE_LIFE) > 0 then

Using: GetWidgetLife() is faster apparently, but IsUnitType() is better incase you increase the maximum HP or a dead unit or something like that?

3) Those groups really need to be fixed.... dynamic groups :hohum:

4) The missiles should face the correct angle when rotating!!

I don't know why this was approved in it's current state :zip:
 
Top