• 🏆 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!

[JASS] Spell doesn't damage.

Status
Not open for further replies.

howl2000

howl2000

howl2000

Level 6
Joined
Sep 13, 2008
Messages
261
I made this spell its supposed to deal damage to all living enemies in three areas in front of the caster. The script executes, but no damage is dealt.
If anyone can help me fix I'd appreciate it. This is my second scripted spell so give me a break if something is inefficient.

JASS: 
function Trig_ArcaneWave_Conditions takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'A002' ) ) then
        return false
    endif
    return true
endfunction

function Trig_ArcaneWave_Actions takes nothing returns nothing
    local group g1 = CreateGroup()
    local group g2 = CreateGroup()
    local group g3 = CreateGroup()
    local location l1 = GetUnitLoc(GetSpellAbilityUnit())
    local location l2 = PolarProjectionBJ(l1, 256.00, GetUnitFacing(GetSpellAbilityUnit()))
    local location l3 = PolarProjectionBJ(l2, 512.00, GetUnitFacing(GetSpellAbilityUnit()))
    local location l4 = PolarProjectionBJ(l3, 768.00, GetUnitFacing(GetSpellAbilityUnit()))
    local unit u1 = GetSpellAbilityUnit()
    local unit u2
    local real d1 = ( GetHeroLevel(u1) / 4 )
    local real d2 = ( 100 + ( GetHeroStatBJ(bj_HEROSTAT_INT, u1, true) * ( d1 + 1 ) ) )
    set g1 = GetUnitsInRangeOfLocAll(150.00, l2)
    set g2 = GetUnitsInRangeOfLocAll(150.00, l3)
    set g3 = GetUnitsInRangeOfLocAll(150.00, l4)
    call TriggerSleepAction( 0.33 )
    loop
        set u2 = FirstOfGroup(g1)
        if ( not ( IsUnitEnemy(u2, GetOwningPlayer(u1)) == true ) ) then
            if ( not ( IsUnitAliveBJ(u2) == true ) ) then
                exitwhen u2 == null
                call GroupRemoveUnit(g1, u2)
                call UnitDamageTargetBJ( u1, u2, d2, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC )
            else
            endif
        else
            call GroupRemoveUnit(g1 , u2)
        endif
    endloop
    call TriggerSleepAction( 0.33 )
    loop
        set u2 = FirstOfGroup(g2)
        if ( not ( IsUnitEnemy(u2, GetOwningPlayer(u1)) == true ) ) then
            if ( not ( IsUnitAliveBJ(u2) == true ) ) then
                exitwhen u2 == null
                call GroupRemoveUnit(g2, u2)
                call UnitDamageTargetBJ( u1, u2, d2, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC )
            else
            endif
        else
            call GroupRemoveUnit(g2 , u2)
        endif
    endloop
    call TriggerSleepAction( 0.33 )
    loop
        set u2 = FirstOfGroup(g3)
        if ( not ( IsUnitEnemy(u2, GetOwningPlayer(u1)) == true ) ) then
            if ( not ( IsUnitAliveBJ(u2) == true ) ) then
                exitwhen u2 == null
                call GroupRemoveUnit(g3, u2)
                call UnitDamageTargetBJ( u1, u2, d2, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC )
            else
            endif
        else
            call GroupRemoveUnit(g3 , u2)
        endif
    endloop
    call RemoveLocation(l1)
    set l1 = null
    call RemoveLocation(l2)
    set l2 = null
    call RemoveLocation(l3)
    set l3 = null
    call RemoveLocation(l4)
    set l4 = null
    call DestroyGroup(g1)
    set g1 = null
    call DestroyGroup(g2)
    set g2 = null
    call DestroyGroup(g3)
    set g3 = null
    set u1 = null
    set u2 = null
endfunction

//===========================================================================
function InitTrig_ArcaneWave takes nothing returns nothing
    set gg_trg_ArcaneWave = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_ArcaneWave, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_ArcaneWave, Condition( function Trig_ArcaneWave_Conditions ) )
    call TriggerAddAction( gg_trg_ArcaneWave, function Trig_ArcaneWave_Actions )
endfunction
 

howl2000

howl2000

howl2000

Level 6
Joined
Sep 13, 2008
Messages
261
Bump.

The spell worked before I added in the second if in each loop, but I needed to make the spell only damage enemies.
 

howl2000

howl2000

howl2000

Level 6
Joined
Sep 13, 2008
Messages
261
Marcelo Hossomi said:
not ( IsUnitEnemy(u2, GetOwningPlayer(u1)) == true )

IsUnitEnemy() returns true if the unit IS an enemy. So, if you put not before it, then it will affect only ALLIED.
Click to expand...


Thx for the tip, but I removed not and it changed nothing. Also it didn't damage teammates in either case. I also removed the not in the is unit dead part. That didn't help either.

This is the original working script before the if functions were added.
JASS: 
function Trig_ArcaneWave_Conditions takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'A002' ) ) then
        return false
    endif
    return true
endfunction

function Trig_ArcaneWave_Actions takes nothing returns nothing
    local group g1 = CreateGroup()
    local group g2 = CreateGroup()
    local group g3 = CreateGroup()
    local location l1 = GetUnitLoc(GetSpellAbilityUnit())
    local location l2 = PolarProjectionBJ(l1, 256.00, GetUnitFacing(GetSpellAbilityUnit()))
    local location l3 = PolarProjectionBJ(l2, 512.00, GetUnitFacing(GetSpellAbilityUnit()))
    local location l4 = PolarProjectionBJ(l3, 768.00, GetUnitFacing(GetSpellAbilityUnit()))
    local unit u1 = GetSpellAbilityUnit()
    local unit u2
    local real d1 = ( GetHeroLevel(u1) / 4 )
    local real d2 = ( 100 + ( GetHeroStatBJ(bj_HEROSTAT_INT, u1, true) * ( d1 + 1 ) ) )
    set g1 = GetUnitsInRangeOfLocAll(180.00, l2)
    set g2 = GetUnitsInRangeOfLocAll(180.00, l3)
    set g3 = GetUnitsInRangeOfLocAll(180.00, l4)
    call TriggerSleepAction( 0.33 )
    loop
        set u2 = FirstOfGroup(g1)
                exitwhen u2 == null
                call GroupRemoveUnit(g1, u2)
                call UnitDamageTargetBJ( u1, u2, d2, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC )
    endloop
    call TriggerSleepAction( 0.33 )
    loop
        set u2 = FirstOfGroup(g2)
                exitwhen u2 == null
                call GroupRemoveUnit(g2, u2)
                call UnitDamageTargetBJ( u1, u2, d2, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC )
    endloop
    call TriggerSleepAction( 0.33 )
    loop
        set u2 = FirstOfGroup(g3)
                exitwhen u2 == null
                call GroupRemoveUnit(g3, u2)
                call UnitDamageTargetBJ( u1, u2, d2, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC )
    endloop
    call RemoveLocation(l1)
    set l1 = null
    call RemoveLocation(l2)
    set l2 = null
    call RemoveLocation(l3)
    set l3 = null
    call RemoveLocation(l4)
    set l4 = null
    call DestroyGroup(g1)
    set g1 = null
    call DestroyGroup(g2)
    set g2 = null
    call DestroyGroup(g3)
    set g3 = null
    set u1 = null
    set u2 = null
endfunction

//===========================================================================
function InitTrig_ArcaneWave_Copy takes nothing returns nothing
    set gg_trg_ArcaneWave_Copy = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_ArcaneWave_Copy, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_ArcaneWave_Copy, Condition( function Trig_ArcaneWave_Conditions ) )
    call TriggerAddAction( gg_trg_ArcaneWave_Copy, function Trig_ArcaneWave_Actions )
endfunction
 
Last edited:

Lord_BoNes

Lord_BoNes

Lord_BoNes

Level 6
Joined
Sep 5, 2007
Messages
264
NEVER put a GroupRemoveUnit() inside an if... In the case that the if DOESN'T activate, then the unit will stay in the group (still in the FirstOfGroup() slot) creating an infinate loop. After about 8000 executions, Wc3 figures it out and stop it dead in its tracks.

The same thing applies to exitwhen. Don't put them inside if statements. It's a bad practice.

Your code SHOULD look like:
JASS: 
    loop
        set u2 = FirstOfGroup(g1)
        exitwhen u2 == null

        if (IsUnitEnemy(u2, GetOwningPlayer(u1)) != true) then
            if (IsUnitAliveBJ(u2) != true) then
                call UnitDamageTargetBJ( u1, u2, d2, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC )
            endif
        endif

        call GroupRemoveUnit(g1, u2)
    endloop

This code is also easier for me to read, but that's just my opinion.

Hope this has been helpful :thumbs_up:
 

Marcelo Hossomi

Marcelo Hossomi

Marcelo Hossomi

Level 7
Joined
Dec 31, 2005
Messages
712
Lord_BoNes said:
NEVER put a GroupRemoveUnit() inside an if... In the case that the if DOESN'T activate, then the unit will stay in the group (still in the FirstOfGroup() slot) creating an infinate loop. After about 8000 executions, Wc3 figures it out and stop it dead in its tracks.

The same thing applies to exitwhen. Don't put them inside if statements. It's a bad practice.
Click to expand...

Said everything.
 

Dr Super Good

Dr Super Good

Dr Super Good

Spell Reviewer
Level 64
Joined
Jan 18, 2005
Messages
27,202
You can combine booleans (eg from conditional statmenets) to return a boolean from 1 opperation thus avoiding an extra if statement.

JASS: 
        if ( not ( IsUnitEnemy(u2, GetOwningPlayer(u1)) == true ) ) then
            if ( not ( IsUnitAliveBJ(u2) == true ) ) then

Can be optimized down to
JASS: 
 if not IsUnitEnemy(u2, GetOwningPlayer(u1)) and not IsUnitAliveBJ(u2) then

Note this is not fixing your simple selection statments, this is purly telling you how to optimize them.

It also works for returning booleans.
 
Last edited:

howl2000

howl2000

howl2000

Level 6
Joined
Sep 13, 2008
Messages
261
Ooh Dr Super Good messed up. He capitolized his if statement lol. jk
Thx for all your help everyone. I optimized my loops and got the spell working.

Here it is if you're wondering how it turned out.
JASS: 
function Trig_ArcaneWave_Conditions takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'A002' ) ) then
        return false
    endif
    return true
endfunction

function Trig_ArcaneWave_Actions takes nothing returns nothing
    local group g1 = CreateGroup()
    local group g2 = CreateGroup()
    local group g3 = CreateGroup()
    local location l1 = GetUnitLoc(GetSpellAbilityUnit())
    local location l2 = PolarProjectionBJ(l1, 256.00, GetUnitFacing(GetSpellAbilityUnit()))
    local location l3 = PolarProjectionBJ(l1, 512.00, GetUnitFacing(GetSpellAbilityUnit()))
    local location l4 = PolarProjectionBJ(l1, 768.00, GetUnitFacing(GetSpellAbilityUnit()))
    local unit u1 = GetSpellAbilityUnit()
    local unit u2
    local real d1 = ( GetHeroLevel(u1) / 4 )
    local real d2 = ( 100 + ( GetHeroStatBJ(bj_HEROSTAT_INT, u1, true) * ( d1 + 1 ) ) )
    set g1 = GetUnitsInRangeOfLocAll(155.00, l2)
    set g2 = GetUnitsInRangeOfLocAll(155.00, l3)
    set g3 = GetUnitsInRangeOfLocAll(155.00, l4)
    call TriggerSleepAction( 0.22 )
    loop
        set u2 = FirstOfGroup(g1)
        exitwhen u2 == null
        call GroupRemoveUnit(g1, u2)
        if IsUnitEnemy(u2, GetOwningPlayer(u1)) and GetUnitState(u2, UNIT_STATE_LIFE) > 0 then
        call UnitDamageTargetBJ( u1, u2, d2, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC )
        else
        endif
    endloop
    call TriggerSleepAction( 0.22 )
    loop
        set u2 = FirstOfGroup(g2)
        exitwhen u2 == null
        call GroupRemoveUnit(g2, u2)
        if IsUnitEnemy(u2, GetOwningPlayer(u1)) and GetUnitState(u2, UNIT_STATE_LIFE) > 0 then
        call UnitDamageTargetBJ( u1, u2, d2, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC )
        else
        endif
    endloop
    call TriggerSleepAction( 0.22 )
    loop
        set u2 = FirstOfGroup(g3)
        exitwhen u2 == null
        call GroupRemoveUnit(g3, u2)
        if IsUnitEnemy(u2, GetOwningPlayer(u1)) and GetUnitState(u2, UNIT_STATE_LIFE) > 0 then
        call UnitDamageTargetBJ( u1, u2, d2, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC )
        else
        endif
    endloop
    call RemoveLocation(l1)
    set l1 = null
    call RemoveLocation(l2)
    set l2 = null
    call RemoveLocation(l3)
    set l3 = null
    call RemoveLocation(l4)
    set l4 = null
    call DestroyGroup(g1)
    set g1 = null
    call DestroyGroup(g2)
    set g2 = null
    call DestroyGroup(g3)
    set g3 = null
    set u1 = null
    set u2 = null
endfunction

//===========================================================================
function InitTrig_ArcaneWave takes nothing returns nothing
    set gg_trg_ArcaneWave = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_ArcaneWave, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_ArcaneWave, Condition( function Trig_ArcaneWave_Conditions ) )
    call TriggerAddAction( gg_trg_ArcaneWave, function Trig_ArcaneWave_Actions )
endfunction
I learned alot from you guys.
Trigger does exactly what I wanted and will be easy to add to later thanks to everyones help.
 

Lord_BoNes

Lord_BoNes

Lord_BoNes

Level 6
Joined
Sep 5, 2007
Messages
264
I'm not picking here, but...

I think:
JASS: 
if (IsUnitEnemy(u2, GetOwningPlayer(u1)) and GetUnitState(u2, UNIT_STATE_LIFE) > 0) then
    call UnitDamageTargetBJ( u1, u2, d2, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC )
endif

Is better than:
JASS: 
if IsUnitEnemy(u2, GetOwningPlayer(u1)) and GetUnitState(u2, UNIT_STATE_LIFE) > 0 then
call UnitDamageTargetBJ( u1, u2, d2, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC )
else
endif

I know that it's only a small thing, and it's purely cosmetic. But, it does make it easier to read, and it's a good habit to get into for down the track.

Also, your condition function could be optimised:
JASS: 
function Trig_ArcaneWave_Conditions takes nothing returns boolean
    return (GetSpellAbilityId() == 'A002')
endfunction
Because the statement inside the brackets is an "==" statement, it becomes a true/false (boolean) statement, and can be directly returned.

Good job on the coding though :thumbs_up:
 
Status
Not open for further replies.

Similar threads

nGy
  • Locked
[vJASS] Please help me, otimize my spell
Replies
19
Views
6K
nGy
nGy
flacidTurtle
  • Locked
[JASS] Anyone know why this trigger loops?
Replies
15
Views
990
Maker
Maker
NightElf125
  • Locked
Jass Spell Issue
Replies
5
Views
1K
Dr Super Good
Dr Super Good
Garfield1337
  • Locked
Would this leak?
Replies
2
Views
720
Garfield1337
Garfield1337
deathismyfriend
  • Locked
[vJASS] optimizing minigame trigger
Replies
4
Views
619
deathismyfriend
deathismyfriend
Top