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

[JASS] Local Variable Problems T_T

Status
Not open for further replies.

NightSong

NightSong

NightSong

Level 4
Joined
Feb 25, 2008
Messages
58
I'm making a spell called Bloodfire, which deals a certain amount of damage, and then heals the caster for a percentage of the target's remaining health. I got the spell to work using GUI, but it used globals so I tried converting it to custom text and changing all the udg_ variables to local real variables. However, this didn't work (much to my surprise) and I've tried many times to fix it to no avail. Here are the GUI and JASS versions:

  • Bloodfire GUI
    • Events
      • Unit - A unit Starts the effect of an ability
    • Conditions
      • (Ability being cast) Equal to Bloodfire
    • Actions
      • Set TaintMultiplier = 1.00
      • If (((Target unit of ability being cast) has buff Taint (Level 1)) Equal to True) then do (Set TaintMultiplier = 1.20) else do (Do nothing)
      • If (((Target unit of ability being cast) has buff Taint (Level 2)) Equal to True) then do (Set TaintMultiplier = 1.30) else do (Do nothing)
      • If (((Target unit of ability being cast) has buff Taint (Level 3)) Equal to True) then do (Set TaintMultiplier = 1.40) else do (Do nothing)
      • If (((Target unit of ability being cast) has buff Taint (Level 4)) Equal to True) then do (Set TaintMultiplier = 1.50) else do (Do nothing)
      • Set BloodfireDamage = (TaintMultiplier x (40.00 + (50.00 x (Real((Level of (Ability being cast) for (Triggering unit)))))))
      • Unit - Cause (Triggering unit) to damage (Target unit of ability being cast), dealing BloodfireDamage damage of attack type Chaos and damage type Normal
      • Set BloodfireHeal = ((Life of (Target unit of ability being cast)) x (0.05 x (Real((Level of (Ability being cast) for (Triggering unit))))))
      • Unit - Set life of (Triggering unit) to ((Life of (Triggering unit)) + BloodfireHeal)
JASS: 
function Bloodfire_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A008'
endfunction

function Bloodfire_Func002001 takes nothing returns boolean
    return ( UnitHasBuffBJ(GetSpellTargetUnit(), 'B001') == true )
endfunction

function Bloodfire_Func003001 takes nothing returns boolean
    return ( UnitHasBuffBJ(GetSpellTargetUnit(), 'B002') == true )
endfunction

function Bloodfire_Func004001 takes nothing returns boolean
    return ( UnitHasBuffBJ(GetSpellTargetUnit(), 'B003') == true )
endfunction

function Bloodfire_Func005001 takes nothing returns boolean
    return ( UnitHasBuffBJ(GetSpellTargetUnit(), 'B000') == true )
endfunction

function Bloodfire_Actions takes nothing returns nothing
    local real taintmultiplier = 1.0
    if ( Trig_Bloodfire_Func002001() ) then
    local real taintmultiplier = 1.2
    else
    if ( Trig_Bloodfire_Func003001() ) then
    local real taintmultiplier = 1.3
    else
    if ( Trig_Bloodfire_Func004001() ) then
        local real taintmultiplier = 1.4
    else
    if ( Trig_Bloodfire_Func005001() ) then
        local real taintmultiplier = 1.5
    local real BloodfireDamage = ( TaintMultiplier * ( 40.00 + ( 50.00 * I2R(GetUnitAbilityLevelSwapped(GetSpellAbilityId(), GetTriggerUnit())) ) ) )
    call UnitDamageTargetBJ( GetTriggerUnit(), GetSpellTargetUnit(), BloodfireDamage, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_NORMAL )
    local real BloodfireHeal = ( GetUnitStateSwap(UNIT_STATE_LIFE, GetSpellTargetUnit()) * ( 0.05 * I2R(GetUnitAbilityLevelSwapped(GetSpellAbilityId(), GetTriggerUnit())) ) )
    call SetUnitLifeBJ( GetTriggerUnit(), ( GetUnitStateSwap(UNIT_STATE_LIFE, GetTriggerUnit()) + BloodfireHeal ) )
    endif
    endif
    endif
    endif
endfunction

//===========================================================================
function InitTrig_Bloodfire takes nothing returns nothing
    set gg_trg_Bloodfire = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Bloodfire, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_Bloodfire, Condition( function Bloodfire_Conditions ) )
    call TriggerAddAction( gg_trg_Bloodfire, function Bloodfire_Actions )
endfunction

NOTE: I'm planning on using this spell in the current hero contest going on, so if it's against the rules for me to get help then.. well, I dunno. Anyways, in the case that it's fine thanks in advance!
-NS
 

Eleandor

Eleandor

Eleandor

Level 21
Joined
Aug 21, 2005
Messages
3,699
locals can only be declared on the very beginning of a function.

JASS: 
function Bloodfire_Actions takes nothing returns nothing
    local real taintmultiplier = 1.0
    local real BloodfireDamage
    local real BloodfireHeal
    if ( Trig_Bloodfire_Func002001() ) then
        set taintmultiplier = 1.2
    elseif ( Trig_Bloodfire_Func003001() ) then
        set taintmultiplier = 1.3
    elseif ( Trig_Bloodfire_Func004001() ) then
        set taintmultiplier = 1.4
    elseif ( Trig_Bloodfire_Func005001() ) then
        set taintmultiplier = 1.5
    endif
    set BloodfireDamage = ( TaintMultiplier * ( 40.00 + ( 50.00 * I2R(GetUnitAbilityLevelSwapped(GetSpellAbilityId(), GetTriggerUnit())) ) ) )
    call UnitDamageTargetBJ( GetTriggerUnit(), GetSpellTargetUnit(), BloodfireDamage, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_NORMAL )
    set BloodfireHeal = ( GetUnitStateSwap(UNIT_STATE_LIFE, GetSpellTargetUnit()) * ( 0.05 * I2R(GetUnitAbilityLevelSwapped(GetSpellAbilityId(), GetTriggerUnit())) ) )
    call SetUnitLifeBJ( GetTriggerUnit(), ( GetUnitStateSwap(UNIT_STATE_LIFE, GetTriggerUnit()) + BloodfireHeal ) )
endfunction
 

Earth-Fury

Earth-Fury

Earth-Fury

Level 11
Joined
Feb 18, 2004
Messages
394
Problem 1: functions like function Bloodfire_Func002001 takes nothing returns boolean

You can use the return value of these functions directly in an if-then-else:

JASS: 
if ( Trig_Bloodfire_Func002001() ) then
// Should be:
if UnitHasBuffBJ(GetSpellTargetUnit(), 'B001') then
== true is redundant and unnecessary.

Removing that crap yeilds:
JASS: 
function Bloodfire_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A008'
endfunction

function Bloodfire_Actions takes nothing returns nothing
    local real taintmultiplier = 1.0
    if UnitHasBuffBJ(GetSpellTargetUnit(), 'B001') then
    local real taintmultiplier = 1.2
    else
    if UnitHasBuffBJ(GetSpellTargetUnit(), 'B002') then
    local real taintmultiplier = 1.3
    else
    if UnitHasBuffBJ(GetSpellTargetUnit(), 'B003') then
        local real taintmultiplier = 1.4
    else
    if UnitHasBuffBJ(GetSpellTargetUnit(), 'B000') then
local real taintmultiplier = 1.5
    local real BloodfireDamage = ( TaintMultiplier * ( 40.00 + ( 50.00 * I2R(GetUnitAbilityLevelSwapped(GetSpellAbilityId(), GetTriggerUnit())) ) ) )
    call UnitDamageTargetBJ( GetTriggerUnit(), GetSpellTargetUnit(), BloodfireDamage, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_NORMAL )
    local real BloodfireHeal = ( GetUnitStateSwap(UNIT_STATE_LIFE, GetSpellTargetUnit()) * ( 0.05 * I2R(GetUnitAbilityLevelSwapped(GetSpellAbilityId(), GetTriggerUnit())) ) )
    call SetUnitLifeBJ( GetTriggerUnit(), ( GetUnitStateSwap(UNIT_STATE_LIFE, GetTriggerUnit()) + BloodfireHeal ) )
    endif
    endif
    endif
    endif
endfunction

//===========================================================================
function InitTrig_Bloodfire takes nothing returns nothing
    set gg_trg_Bloodfire = CreateTrigger( )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Bloodfire, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_Bloodfire, Condition( function Bloodfire_Conditions ) )
    call TriggerAddAction( gg_trg_Bloodfire, function Bloodfire_Actions )
endfunction

Problem 2: You can only declare locals at the top of a function.

Problem 3: You use "TaintMultiplier" and "taintmultiplier". JASS is case sensitive, so those two names are not the same.

Problem 4: You try redeclare locals halfway through the function instead of using the set statement. Fixing the above 3 yields:
JASS: 
function Bloodfire_Actions takes nothing returns nothing
    local real taintMultiplier = 1.0
    local real bloodfireDamage
    local real bloodfireHeal
    if UnitHasBuffBJ(GetSpellTargetUnit(), 'B001') then
    set taintMultiplier = 1.2
    else
    if UnitHasBuffBJ(GetSpellTargetUnit(), 'B002') then
    set taintMultiplier = 1.3
    else
    if UnitHasBuffBJ(GetSpellTargetUnit(), 'B003') then
        set taintMultiplier = 1.4
    else
    if UnitHasBuffBJ(GetSpellTargetUnit(), 'B000') then
set taintmultiplier = 1.5
    set bloodfireDamage = ( taintMultiplier * ( 40.00 + ( 50.00 * I2R(GetUnitAbilityLevelSwapped(GetSpellAbilityId(), GetTriggerUnit())) ) ) )
    call UnitDamageTargetBJ( GetTriggerUnit(), GetSpellTargetUnit(), bloodfireDamage, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_NORMAL )
    set bloodfireHeal = ( GetUnitStateSwap(UNIT_STATE_LIFE, GetSpellTargetUnit()) * ( 0.05 * I2R(GetUnitAbilityLevelSwapped(GetSpellAbilityId(), GetTriggerUnit())) ) )
    call SetUnitLifeBJ( GetTriggerUnit(), ( GetUnitStateSwap(UNIT_STATE_LIFE, GetTriggerUnit()) + bloodfireHeal ) )
    endif
    endif
    endif
    endif
endfunction

Problem 5: The lack of proper indenting hides a logic error:
JASS: 
function Bloodfire_Actions takes nothing returns nothing
    local real taintMultiplier = 1.0
    local real bloodfireDamage
    local real bloodfireHeal
    if UnitHasBuffBJ(GetSpellTargetUnit(), 'B001') then
        set taintMultiplier = 1.2
    else
        if UnitHasBuffBJ(GetSpellTargetUnit(), 'B002') then
            set taintMultiplier = 1.3
        else
            if UnitHasBuffBJ(GetSpellTargetUnit(), 'B003') then
                set taintMultiplier = 1.4
            else
                if UnitHasBuffBJ(GetSpellTargetUnit(), 'B000') then
                    set taintMultiplier = 1.5
                    set bloodfireDamage = ( TaintMultiplier * ( 40.00 + ( 50.00 * I2R(GetUnitAbilityLevelSwapped(GetSpellAbilityId(), GetTriggerUnit())) ) ) )
                    call UnitDamageTargetBJ( GetTriggerUnit(), GetSpellTargetUnit(), bloodfireDamage, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_NORMAL )
                    set bloodfireHeal = ( GetUnitStateSwap(UNIT_STATE_LIFE, GetSpellTargetUnit()) * ( 0.05 * I2R(GetUnitAbilityLevelSwapped(GetSpellAbilityId(), GetTriggerUnit())) ) )
                    call SetUnitLifeBJ( GetTriggerUnit(), ( GetUnitStateSwap(UNIT_STATE_LIFE, GetTriggerUnit()) + bloodfireHeal ) )
                endif
            endif
        endif
    endif
endfunction

Most of the code will only be executed if the unit has only the final buff!

Problem 6: You don't use elseif. Fixing the logic error and using elseif, we end up with:
JASS: 
function Bloodfire_Actions takes nothing returns nothing
    local real taintMultiplier = 1.0
    local real bloodfireDamage
    local real bloodfireHeal

    if UnitHasBuffBJ(GetSpellTargetUnit(), 'B000') then
        set taintMultiplier = 1.5
    elseif UnitHasBuffBJ(GetSpellTargetUnit(), 'B003') then // Will only be executed if the if and all previosue elseif's in the if block are false.
        set taintMultiplier = 1.4
    elseif UnitHasBuffBJ(GetSpellTargetUnit(), 'B002') then
        set taintMultiplier = 1.3
    elseif UnitHasBuffBJ(GetSpellTargetUnit(), 'B001') then
        set taintMultiplier = 1.2
    endif
    
    set bloodfireDamage = ( taintMultiplier * ( 40.00 + ( 50.00 * I2R(GetUnitAbilityLevelSwapped(GetSpellAbilityId(), GetTriggerUnit())) ) ) )
    call UnitDamageTargetBJ( GetTriggerUnit(), GetSpellTargetUnit(), bloodfireDamage, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_NORMAL )
    set bloodfireHeal = ( GetUnitStateSwap(UNIT_STATE_LIFE, GetSpellTargetUnit()) * ( 0.05 * I2R(GetUnitAbilityLevelSwapped(GetSpellAbilityId(), GetTriggerUnit())) ) )
    call SetUnitLifeBJ( GetTriggerUnit(), ( GetUnitStateSwap(UNIT_STATE_LIFE, GetTriggerUnit()) + bloodfireHeal ) )
endfunction

Problem 7: GetTriggerUnit()[icode=jass] and [icode=jass]GetSpellTargetUnit() and GetUnitAbilityLevelSwapped() are called a lot. Using more locals:
JASS: 
function Bloodfire_Actions takes nothing returns nothing
    local unit caster = GetTriggerUnit()
    local unit target = GetSpellTargetUnit()
    local integer abilityLevel = GetUnitAbilityLevel(caster, GetSpellAbilityId())

    local real taintMultiplier = 1.0
    local real bloodfireDamage
    local real bloodfireHeal
    
    if UnitHasBuffBJ(target, 'B000') then
        set taintMultiplier = 1.5
    elseif UnitHasBuffBJ(target, 'B003') then
        set taintMultiplier = 1.4
    elseif UnitHasBuffBJ(target, 'B002') then
        set taintMultiplier = 1.3
    elseif UnitHasBuffBJ(target, 'B001') then
        set taintMultiplier = 1.2
    endif
    
    set bloodfireDamage = (taintMultiplier * ( 40.00 + ( 50.00 * I2R(abilityLevel) ) ) )
    call UnitDamageTargetBJ(caster, target, bloodfireDamage, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_NORMAL )
    set bloodfireHeal = (GetUnitStateSwap(UNIT_STATE_LIFE, target) * ( 0.05 * I2R(abilityLevel) ) )
    call SetUnitLifeBJ(caster, ( GetUnitStateSwap(UNIT_STATE_LIFE, caster) + bloodfireHeal ) )
endfunction

Problem 8: Blizzard.j functions. (BJ functions) are evil. The GUI uses a lot of functions from blizzard.j. BJ functions are written in JASS, and most of them are very poorly coded. (And useless!)

Lets go over the ones you use:

JASS: 
function UnitHasBuffBJ takes unit whichUnit, integer buffcode returns boolean
    return (GetUnitAbilityLevel(whichUnit, buffcode) > 0)
endfunction

From the above, we can see that to know if a unit has a buff, you check if the ability level for the buffs rawcode is above 0. We will inline this later.

JASS: 
function UnitDamageTargetBJ takes unit whichUnit, unit target, real amount, attacktype whichAttack, damagetype whichDamage returns boolean
    return UnitDamageTarget(whichUnit, target, amount, true, false, whichAttack, whichDamage, WEAPON_TYPE_WHOKNOWS)
endfunction

Almost the same function as the native. However, the native has two extra paramiters:
JASS: 
native UnitDamageTarget takes unit whichUnit, widget target, real amount, boolean attack, boolean ranged, attacktype attackType, damagetype damageType, weapontype weaponType returns boolean

So your call to UnitDamageTarget BJ can go:
JASS: 
// from this:
call UnitDamageTargetBJ( caster, target, bloodfireDamage, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_NORMAL )
// to this:
call UnitDamageTarget(caster, target, bloodfireDamage, true, false, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_NORMAL, WEAPON_TYPE_WHOKNOWS)

Another BJ function you use is:
JASS: 
function GetUnitStateSwap takes unitstate whichState, unit whichUnit returns real
    return GetUnitState(whichUnit, whichState)
endfunction

A good example of a useless BJ function. We will inline this later...

JASS: 
function SetUnitLifeBJ takes unit whichUnit, real newValue returns nothing
    call SetUnitState(whichUnit, UNIT_STATE_LIFE, RMaxBJ(0,newValue))
endfunction

An especially useless function, as you can use:
JASS: 
native SetWidgetLife takes widget whichWidget, real newLife returns nothing


Now, after ininling all of those, and using GetWidgetLife and SetWidgetLife instead of GetUnitState and SetUnitState we have:
JASS: 
function Bloodfire_Actions takes nothing returns nothing
    local unit caster = GetTriggerUnit()
    local unit target = GetSpellTargetUnit()
    local integer abilityLevel = GetUnitAbilityLevel(caster, GetSpellAbilityId())

    local real taintMultiplier = 1.0
    local real bloodfireDamage
    local real bloodfireHeal
    
    if GetUnitAbilityLevel(target, 'B000') > 0 then
        set taintMultiplier = 1.5
    elseif  GetUnitAbilityLevel(target, 'B003') > 0 then
        set taintMultiplier = 1.4
    elseif  GetUnitAbilityLevel(target, 'B002') > 0 then
        set taintMultiplier = 1.3
    elseif  GetUnitAbilityLevel(target, 'B001') > 0 then
        set taintMultiplier = 1.2
    endif
    
    set bloodfireDamage = (taintMultiplier * ( 40.00 + ( 50.00 * I2R(abilityLevel) ) ) )
    call UnitDamageTarget(caster, target, bloodfireDamage, true, false, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_NORMAL, WEAPON_TYPE_WHOKNOWS)
    set bloodfireHeal = (GetWidgetLife(target)) * ( 0.05 * I2R(abilityLevel) )
    call SetWidgetLife(caster, GetWidgetLife(caster) + bloodfireHeal)
endfunction

Problem 9: Useless local variables. You only use bloodfireDamage and heal once. so why do they need to even be variables? Removing them and inlining, we get:
JASS: 
function Bloodfire_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A008'
endfunction

function Bloodfire_Actions takes nothing returns nothing
    local unit caster = GetTriggerUnit()
    local unit target = GetSpellTargetUnit()
    local integer abilityLevel = GetUnitAbilityLevel(caster, GetSpellAbilityId())

    local real taintMultiplier = 1.0
    
    if GetUnitAbilityLevel(target, 'B000') > 0 then
        set taintMultiplier = 1.5
    elseif  GetUnitAbilityLevel(target, 'B003') > 0 then
        set taintMultiplier = 1.4
    elseif  GetUnitAbilityLevel(target, 'B002') > 0 then
        set taintMultiplier = 1.3
    elseif  GetUnitAbilityLevel(target, 'B001') > 0 then
        set taintMultiplier = 1.2
    endif
    
    call UnitDamageTarget(caster, target, taintMultiplier * (abilityLevel * 50.00 + 40.00), true, false, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_NORMAL, WEAPON_TYPE_WHOKNOWS)
    call SetWidgetLife(caster, GetWidgetLife(caster) + GetWidgetLife(target) *  0.05 * abilityLevel)
	
    set caster = null
    set target = null // See the notes below as to why these two lines are here.
endfunction

function InitTrig_Bloodfire takes nothing returns nothing
    set gg_trg_Bloodfire = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(gg_trg_Bloodfire, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(gg_trg_Bloodfire, Condition(function Bloodfire_Conditions))
    call TriggerAddAction(gg_trg_Bloodfire, function Bloodfire_Actions)
endfunction

Small note: I2R() isn't needed, as JASS lets you use both integers and reals in an equation. If any reals are used, the result of the equation will be a real type value.

Another small note: You must always set local variables that are of a type extending from handle, like units, locations, groups, etc. to null at the end of a function, or before the function returns a value. this fixes a small memory leak.

I'm very sleepy now and this is a long post. Yeah. Damn Pyrogasm and his large rewriting of scripts... i picked up the habbit.
 

Blue_Jeans

Blue_Jeans

Blue_Jeans

Level 6
Joined
Jun 30, 2006
Messages
230
Very good post, Earth-Fury, very good... However, I hate seeing ability/unit/buff codes just lying around like that. I always use variables... that way you have the configuration for the thing at the top of your function. You don't have to go hunting for it if you want to change it.

Also, that way you can use a word to relate its meaning. 'B001' doesn't mean much, but BID_BLOOD_FIRE does... I use UID for Unit ID, AID for Ability ID, BID for Buff ID... etc... but I also use them as global variables that get set on spell Init... and that would take JassNewGen for easy declaration. Everyone ought to have JassNewGen, but not many do, it seems.
 

PurplePoot

PurplePoot

PurplePoot

Level 40
Joined
Dec 14, 2005
Messages
10,532
Also, that way you can use a word to relate its meaning. 'B001' doesn't mean much, but BID_BLOOD_FIRE does... I use UID for Unit ID, AID for Ability ID, BID for Buff ID... etc... but I also use them as global variables that get set on spell Init... and that would take JassNewGen for easy declaration. Everyone ought to have JassNewGen, but not many do, it seems.
Click to expand...
Comments are your friends.
 

Blue_Jeans

Blue_Jeans

Blue_Jeans

Level 6
Joined
Jun 30, 2006
Messages
230
PurplePoot said:
Comments are your friends.
Click to expand...

If I have to use the rawcode more than once, I don't want to write a comment more than once. There are times you use the rawcode 3 or 4 or more times... in such cases, there are usually more than one rawcode you are dealing with. If they are written as BID_BUFF_NAME, then it is far easier to know what is going on in a glance...

But each man to his own, eh?
 
Status
Not open for further replies.

Similar threads

Flame_Phoenix
  • Locked
[JASS] Spell local variable problem
Replies
21
Views
1K
PurplePoot
PurplePoot
MultiGod
  • Locked
[JASS] Help me!
Replies
9
Views
606
MultiGod
MultiGod
Maximilianx
  • Locked
[JASS] This is bizzare.
Replies
2
Views
608
Maximilianx
Maximilianx
xD.Schurke
  • Locked
[JASS] Is there somethin i should rework?
Replies
3
Views
470
spiwn
spiwn
RedFlash
  • Locked
[JASS] Spell doesn't seem to work
Replies
2
Views
605
RolePlaynGamer
RolePlaynGamer
Top