[JASS] GetSpellAbilityId() returning 0..?

[Cokemonkey11]

scope knife initializer i
    globals
        private constant integer KNIFECODE='A004'
    endglobals
    
    private function c takes nothing returns boolean
        return GetSpellAbilityId()==KNIFECODE
    endfunction
    
    private function a takes nothing returns nothing
         call BJDebugMsg("rawr")
    endfunction
    
    private function i takes nothing returns nothing
        local trigger t=CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(t,EVENT_PLAYER_UNIT_SPELL_EFFECT)
        call TriggerAddCondition(t,Condition(function c))
        call TriggerAddAction(t,function a)
    endfunction
endscope

Any reason the debug message wouldn't be displayed? The spell is based on banish.

 

[Pharaoh_]

Why do you use scope, when the only thing you set it just an integer global, which should not be that way? Won't only this do the job?

private function c takes nothing returns boolean
         return GetSpellAbilityId() == 'A004'
endfunction

 

[Cokemonkey11]

There's actually something like 50 lines of actions, and more globals; I just simplified it for the question in the thread.

And no. KNIFECODE is a constant for a reason.

 

[The Reborn Devil]

Try to check if the initializer runs. I've had problems with that sometimes.
You should also null the local trigger.

 

[Cokemonkey11]

The initializer runs.

Check this too...:

 

[Element of Water]

I can guarantee there's nothing wrong with that scope you posted. Either you've got the wrong ability code, or something else in the trigger is mucking it up.

 

[xxdingo93xx]

Try to check if the initializer runs. I've had problems with that sometimes.
You should also null the local trigger.

No he shouldn't null the local trigger. Why? Because.

 

[Element of Water]

It doesn't matter wither way. It makes NO difference whatsoever. Null it if you want, don't null if you want.

 

[The Reborn Devil]

No he shouldn't null the local trigger. Why? Because.

Umm..

First of all, always null handle local variables when they reach the end of their 'scope', always. Plenty of times, it is not necessary to null a local. But turning the nulling into a mechanical process while trivializing it is better than having to give thought to such a lame thing... Just to save a couple of lines of codee...

 

[Vulcano]

Anyway, if the handle held by the variable is not EVER destroyed, then you don't have to null it.

your trigger is never destroyed so no nulling needed.
End discussion because this is probably one of the smallest leaks there is .

 

[The Reborn Devil]

You don't have to null it, but it's good coding practice.

 

[SanKakU]

hey...wait a second...are you serious it's good to null your triggers? i thought doing so would be bad. if it's good then i should do it too.

 

[GhostWolf]

It's not good and it's not bad, it doesn't effect you at all.
Why people say it's 'good' is because it gives you more practice with good coding, because in other cases it IS good.

 

[PurplePoot]

It gives you more practice with bad coding. Being messy in situations where you don't have to be is as stupid as being neat where it causes problems.

 

[Sophismata]

It's a good habit to get into. Unless you know why you should be nulling or not nulling your variables, always null them at the end of their scope.

The obscure bugs people are thinking about relate to timers and dynamic triggers.

 

[PurplePoot]

It's a better idea to just understand your code to the fullest extent possible with the current community knowledge as well as additional personal analysis. Lack of understanding inevitably causes crashing and burning no matter how careful you are.

 

[TriggerHappy]

Tell us what happens when you do this;

scope knife initializer i
    globals
        private constant integer KNIFECODE='A004'
    endglobals

    private function c takes nothing returns boolean
        if GetSpellAbilityId()==KNIFECODE then
            call BJDebugMsg("Spell runs")
            return false
        endif
        call BJDebugMsg(I2S(GetSpellAbilityId()) + " != " + I2S(KNIFECODE)
        return false
    endfunction

    private function i takes nothing returns nothing
        local trigger t=CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(t,EVENT_PLAYER_UNIT_SPELL_EFFECT)
        call TriggerAddCondition(t,Condition(function c))
    endfunction
endscope

 

[PurplePoot]

Are you considering the possibility of other spells being cast?

 

[Cokemonkey11]

I fixed it, all I did was change the base spell from banish to aerial shackles, and the funny thing was...

There was no interferance with another spell, because the only other spell the unit had was stomp.