vJass Spell Questions - Making this spell (Corpse Explosion) better

01-09-2009, 09:27 PM

I have only recently started learning vJass, so I really need some assistance in finding out how to improve my n00bish code. But enough of that, here's a spell I've just made, a Corpse Explosion (or Cadaver Blast as I've called it). The spell targets a point, if there is a corpse there it will explode, if not the caster will be paused so it won't cast the spell and lose mana.
What I'm interested is, do you notice any ways to improve this? If you do and have some free time, please post your revised code (using structs for example, since I'm not quite sure how).

Thanks in advance!

JASS:

scope CadaverBlast initializer Init


globals
    private constant integer SPELLID     = 'A001'
    private constant real RANGE          = 400.0
    private constant real DAMAGEBASIC    = 125.0
    private constant real DAMAGEPERLEVEL = 50.0
    private constant string EFFECTAREA   = "Models\\SFX\\CadaverBlast.mdl"  //Blizzards beta model :D
    private constant string EFFECTTARGET = "Abilities\\Spells\\Other\\Stampede\\MissileDeath.mdl"
endglobals

private function Damage takes integer level returns real
    return (DAMAGEBASIC-DAMAGEPERLEVEL)+DAMAGEPERLEVEL*level
endfunction

private function TargetCheckDead takes nothing returns boolean
    return (GetWidgetLife(GetFilterUnit()) < 0.405) and (IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE) == false) and (IsUnitType(GetFilterUnit(), UNIT_TYPE_MECHANICAL) == false)
endfunction

private function TargetCheckAlive takes nothing returns boolean
    return (GetWidgetLife(GetFilterUnit()) > 0.405) and (IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE) == false) and (IsUnitType(GetFilterUnit(), UNIT_TYPE_MAGIC_IMMUNE) == false) and (IsUnitType(GetFilterUnit(), UNIT_TYPE_MECHANICAL) == false)
endfunction
//===========================================================================
private function Conditions takes nothing returns boolean
    return GetSpellAbilityId()==SPELLID
endfunction
 
private function Actions takes nothing returns nothing
    local unit caster = GetTriggerUnit()
    local integer level = GetUnitAbilityLevel(caster, SPELLID)
    local location loc = GetSpellTargetLoc()
    local real x = GetLocationX(loc)
    local real y = GetLocationY(loc)
    local group g = CreateGroup()                                // Group containig the corpse
    local unit u                                                 // Corpse Targeted   
    local group ge                                               // Group containing enemy units around the corpse
    local unit v                                                 // Unit in group ge
    local player p = GetOwningPlayer(caster)
        call GroupEnumUnitsInRange(g, x, y, 100.0, Condition(function TargetCheckDead))
        set u = FirstOfGroup(g)
        if u==null then 
            call PauseUnit(caster, true)
            call PauseUnit(caster, false)
            call IssueImmediateOrder(caster, "stop")
        else
            call RemoveUnit(u)
            set u = null
            call DestroyEffect(AddSpecialEffect(EFFECTAREA, x, y))
            set ge = CreateGroup()
            call GroupEnumUnitsInRange(ge, x, y, RANGE, Condition(function TargetCheckAlive))
            loop
                set v = FirstOfGroup(ge)
                exitwhen v==null
                call GroupRemoveUnit(ge, v)
                if IsUnitEnemy(v, p) then
                    call UnitDamageTarget(caster, v, Damage(level), false, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_UNIVERSAL, null)
                    call DestroyEffect(AddSpecialEffectTarget(EFFECTTARGET, v, "chest"))
                endif
                set v = null
            endloop
            call DestroyGroup(ge)
            set ge = null
        endif
    call RemoveLocation(loc)
    set loc = null
    set caster = null
    call DestroyGroup(g)
    set g = null
endfunction

//===========================================================================
private function Init takes nothing returns nothing
    local trigger CadaverBlastTrig = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(CadaverBlastTrig, EVENT_PLAYER_UNIT_SPELL_CAST )
    call TriggerAddCondition(CadaverBlastTrig, Condition( function Conditions))
    call TriggerAddAction(CadaverBlastTrig, function Actions)
    set CadaverBlastTrig = null
endfunction


endscope

01-09-2009, 09:39 PM

Bobo_The_Kodo

Quote:

call TriggerRegisterAnyUnitEventBJ(CadaverBlastTrig, EVENT_PLAYER_UNIT_SPELL_CAST )

Effect, not cast

Quote:

local trigger CadaverBlastTrig = CreateTrigger()
call TriggerRegisterAnyUnitEventBJ(CadaverBlastTrig, EVENT_PLAYER_UNIT_SPELL_CAST )
call TriggerAddCondition(CadaverBlastTrig, Condition( function Conditions))
call TriggerAddAction(CadaverBlastTrig, function Actions)
set CadaverBlastTrig = null

Might as well just call the variable 't'. Also, you won't need to null it

Quote:

using structs for example, since I'm not quite sure how

No need to use structs, if it's all inside one function

Quote:

private function Damage takes integer level returns real
return (DAMAGEBASIC-DAMAGEPERLEVEL)+DAMAGEPERLEVEL*level
endfunction

JASS:

private constant function Damage takes integer level returns real
    return (DAMAGEBASIC-DAMAGEPERLEVEL)+DAMAGEPERLEVEL*level
endfunction

constant makes it get inlined

Quote:

(GetWidgetLife(GetFilterUnit()) < 0.405)

use

JASS:

 IsUnitType( GetFilterUnit(), UNIT_TYPE_DEAD )

JASS:

 GetWidgetLife( GetFilterUnit() ) == 0

0.405 is only when the target dies, not what their life is at after they die

Quote:

set v = null

No need to null v

Quote:

call RemoveUnit(u)

It would be better to instead do:

JASS:

 call ShowUnit( u, false )

because if that unit is currently in another group or something, and you remove it, then that group might screw up from a FirstOfGroup loop

Quote:

set ge = CreateGroup()
call GroupEnumUnitsInRange(ge, x, y, RANGE, Condition(function TargetCheckAlive))

Why not just reuse 'g' ?

JASS:

            call GroupEnumUnitsInRange(g, x, y, RANGE, Condition(function TargetCheckAlive))

( Group Enum clears all units currently in the group )

01-09-2009, 09:40 PM	#3
Anitarf	Technically, the actual spell effect should occur on a different event. Also, use a static group instead of creating&destroying them. Shouldn't the explosion occur where the corpse was, rather than where the spell target point is?

01-09-2009, 09:58 PM

HyperActive

Thanks Bobo_The_Kodo, that's very useful.

Quote:

Technically, the actual spell effect should occur on a different event.

Only with the EVENT_PLAYER_UNIT_SPELL_CAST am I able to stop the ability from being cast. Thus prevent cooldown and mana loss.

Quote:

Also, use a static group instead of creating&destroying them.

I'll look into it.

Quote:

Shouldn't the explosion occur where the corpse was, rather than where the spell target point is?

Yeah, I've thought of that, but thought that, since I'm using a 100 aoe to detect the corpse, it won't be much of a problem. Bit now, come to think of it, I'm gonna correct that. Any unnecessary error is one error too much.

01-09-2009, 10:14 PM

Bobo_The_Kodo

Quote:

Only with the EVENT_PLAYER_UNIT_SPELL_CAST am I able to stop the ability from being cast. Thus prevent cooldown and mana loss.

Ya, but then the players can order their dude to use the spell, then press stop when he has almost finished casting it, and it won't cost mana / cooldown.

01-09-2009, 10:26 PM	#6
Flame_Phoenix	Ok, it seems to me you really should read this tutorial: http://www.wc3campaigns.net/showthread.php?t=103877 If you already master the concepts of "free global declaration", "scope" and "private" than skip to section 4 "Starting our spell". I know if you read it to the end, that it will help.

01-09-2009, 10:54 PM

Anitarf

Quote:

Originally Posted by HyperActive

Only with the EVENT_PLAYER_UNIT_SPELL_CAST am I able to stop the ability from being cast. Thus prevent cooldown and mana loss.

I was talking about the spell effect, not the spell casting conditions; obviously you need your current event for the latter, that still doesn't prevent you from using a proper event for the former.

01-10-2009, 04:57 AM	#8
HyperActive	I see. Ok, I'll fiddle a little more with the spell, trying to improve it with your suggestions.

01-10-2009, 10:19 AM	#9
Flame_Phoenix	Again, you realy should read the tutorial...

01-10-2009, 05:07 PM

#10

chobibo

Just setup a separate trigger that executes before mana is used, this is just a rough example:

JASS:

globals
    group tempGroup
    constant integer ABILITY_ID='A000'
endglobals

constant function SpellFilter takes nothing returns boolean
    return GetSpellAbilityId()==ABILITY_ID
endfunction


function StopExecute takes nothing returns nothing
    // if there are no targets then stop the order here
endfunction


function SpellMain takes nothing returns nothing
    // Do your stuff
endfunction


function InitTrig_CorpseExplosion takes nothing returns nothing
    local trigger trg
    
    set trg=CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(trg, EVENT_PLAYER_UNIT_SPELL_CAST)
    call TriggerAddCondition(trg, Condition(function SpellFilter))
    call TriggerAddAction(trg, function StopExecute)
    
    set trg=CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(trg, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(trg, Condition(function SpellFilter))
    call TriggerAddAction(trg, function SpellMain)
    
    set tempGroup=CreateGroup()
    
endfunction

Of course, you need to code the error messages manually.

01-10-2009, 06:02 PM

#11

Flame_Phoenix

Quote:

chobibo

Your own code can be improved ...

01-10-2009, 06:49 PM	#12
chobibo	Huh? what do you mean?