Need help for my first spell with vJass

[aggro]

well, i decided to learn vJass. Im doing some practice by converting my old gui spells into vJass.

thats the spell. basically, its proc chance increases with every missing hp and although it's a passive spell, it has 25 sec internal cooldown. It means it can proc only once every 25 sec. btw these "type: attack" and "modifiers" things are about my own attribute system. thats not necessary here, ignore it please.

I'm using Damage-Event-AIDS, Caster System, TimerUtils, GroupUtils.

thats the code:

scope Thirst initializer Starter

////////////////////////////////////////////////////////////////////////////////////////////

    globals
        private constant integer SPELL_ID = 'A02H'
        private timer cooldown
        private timer duration
        private trigger ta
        private trigger tb
        private trigger tc
        private real durationr = 8.00 + ( I2R(GetHeroStr(GetEventDamageSource(), true)) * 0.05 )
        private integer cooldownr = 0
    endglobals

////////////////////////////////////////////////////////////////////////////////////////////

    private function Actions takes nothing returns nothing
        local integer level = GetUnitAbilityLevel(GetEventDamageSource(), SPELL_ID )
        local integer roll = R2I(( ( ( GetUnitState(GetEventDamageSource(), UNIT_STATE_MAX_LIFE) - GetUnitState(GetEventDamageSource(), UNIT_STATE_LIFE) ) * 100.00 ) / GetUnitState(GetEventDamageSource(), UNIT_STATE_MAX_LIFE) ))
        local integer random = GetRandomInt(1, 100)
            if  ( random <= roll ) and ( GetUnitTypeId(GetEventDamageSource()) == 'U001' ) and (level>0) and (cooldownr==0) then
                call DisplayTimedTextToPlayer(GetOwningPlayer(GetEventDamageSource()),0,0,60,"test message")
                call CasterCastAbilityLevel( GetOwningPlayer(GetEventDamageSource()), 'A035', level, "unholyfrenzy", GetEventDamageSource(), true)
                call TimerStart(cooldown, 25.00, false, null)
                call TimerStart( duration, durationr, false, null )
                call TriggerSleepAction(durationr)
                call UnitRemoveAbility(GetEventDamageSource(), 'B006')
                set cooldownr = 25
                set cooldown=NewTimer()
                call SetTimerData(cooldown, cooldownr)
            endif
    endfunction

    private function Cooldown takes nothing returns nothing
        call EnableTrigger( ta )
    endfunction

    private function Duration takes nothing returns nothing
        call UnitRemoveAbility(GetEventDamageSource(), 'B006')
    endfunction

////////////////////////////////////////////////////////////////////////////////////////////

    private function Starter takes nothing returns nothing
        set ta=CreateTrigger()
        call TriggerAddAction(ta,function Actions)
        call Damage_RegisterEvent(ta)  
        
        set tb=CreateTrigger()
        call TriggerAddAction(tb,function Cooldown)
        call TriggerRegisterTimerExpireEvent( tb, cooldown ) 
        
        set tc=CreateTrigger()
        call TriggerAddAction(tc,function Duration)
        call TriggerRegisterTimerExpireEvent( tc, duration )  
    endfunction
endscope

(It's indented now.)

I'm stuck on making internal cooldown and buff ends(by the modifiers, buff duration is dynamic).

1. how to fix it?
2. how to make leakless and multi-doable this code?
3. is there anyway to optimize it? Because it looks very mess to me.

Thanks anyway.

 

[Crazy_Dead]

DOnt use BJ's. Just dont it.

GetUnitAbilityLevelSwapped(-.-)
//is
function GetUnitAbilityLevelSwapped takes integer abilcode, unit whichUnit returns integer
    return GetUnitAbilityLevel(whichUnit, abilcode)
endfunction

GetUnitStateSwap? GetUnitState?

 

[aggro]

thanks for advice. i converted BJs into natives.

so any advice about "internal cooldown" "buff ends" functions?

 

[Bribe]

It helps to not name things by number (t1, t2, t3) because it shows a disconnect of multi-user compatibility. Your indentation is screwed up, badly.

function foo takes nothing returns nothing
    //actions
    endfunction

You'd never get away with this kind of shit in real-world programming. I can't bother to read what your code is trying to do in its current state.

 

[hgkjfhfdsj]

- for the indentation, you can dl a jass editor like jasscraft to auto indent your text
- function Actions–save GetEventDamageSource as local or global to avoid so many fn calls
- multi-doable?
- your triggers/timers arent initialized
- function Duration–GetEventDamageSource() is null, perhaps this is the source of your error?

as for the internal timer thing, you can either attach a timer to each unit with some unit indexing system or you can have 1 timer to manage all. 1 timer per unit should be used if reseting cooldown etc is frequently used in your map, otherwise 1 timer to manage all instances would be more efficient. extracted is a portion of 'Effect' by executor. i suggest you read up on this recycle idea (by vexorian?), and include it in your code. this will also make your spell mui–right now i see that youre using 1 timer for maximum of 1 instance.

  //--
       private static method onRecycleTimer takes nothing returns nothing
         // recycle one instance and start the timer to recycle the next one ...
            local thistype this = .recycleStart
            set .recycleStart = .recycleStart.next
            call .destroy()
            if .recycleStart != 0 then
                call TimerStart(.recycleTimer,TimerGetRemaining(.NOW)-.recycleStart.schedule,false,function thistype.onRecycleTimer)
            else
                set .recycleEnd = 0
            endif
        endmethod
        private method recycle takes nothing returns nothing
            // add this instance to the recycle queue 
            call DestroyEffect(.sfx)
            set .sfx = null
            set .schedule   = TimerGetRemaining(.NOW)-RECYCLE_DELAY
            if .recycleStart == 0 then
                set .recycleStart  = this
                set .recycleEnd    = this
                set .next = 0
                call TimerStart(.recycleTimer,RECYCLE_DELAY,false,function thistype.onRecycleTimer)
            else
                set .recycleEnd.next    = this
                set .next               = 0
                set .recycleEnd         = this
            endif
        endmethod

EDIT: indented version...

scope Thirst initializer Starter
////////////////////////////////////////////////////////////////////////////////////////////
globals
    private constant integer SPELL_ID = 'A02H'
    private timer cooldown
    private timer duration
    private trigger t1
    private trigger t2
    private trigger t3
endglobals
////////////////////////////////////////////////////////////////////////////////////////////
private function Actions takes nothing returns nothing
    local integer level = GetUnitAbilityLevel(GetEventDamageSource(), SPELL_ID )
    local real cooldownr
    local real durationr = 8.00 + ( I2R(GetHeroStr(GetEventDamageSource(), true)) * 0.05 )
    local integer roll = R2I(( ( ( GetUnitState(GetEventDamageSource(), UNIT_STATE_MAX_LIFE) - GetUnitState(GetEventDamageSource(), UNIT_STATE_LIFE) ) * 100.00 ) / GetUnitState(GetEventDamageSource(), UNIT_STATE_MAX_LIFE) ))
    local integer random = GetRandomInt(1, 100)
    if  ( random <= roll ) and ( GetUnitTypeId(GetEventDamageSource()) == 'U001' )  then   
        call DisplayTimedTextToPlayer(GetOwningPlayer(GetEventDamageSource()),0,0,60,"test message")
        call CasterCastAbilityLevel( GetOwningPlayer(GetEventDamageSource()), 'A035', level, "unholyfrenzy", GetEventDamageSource(), true)
        call TimerStart(cooldown, 25.00, false, null)
        call TimerStart( duration, durationr, false, null )
        call DisableTrigger( t1 )  
    endif
endfunction

private function Cooldown takes nothing returns nothing
    call EnableTrigger( t1 )
endfunction

private function Duration takes nothing returns nothing
    call UnitRemoveAbility(GetEventDamageSource(), 'B006')
endfunction
////////////////////////////////////////////////////////////////////////////////////////////

private function Starter takes nothing returns nothing
    set t1=CreateTrigger()
    call TriggerAddAction(t1,function Actions)
    call Damage_RegisterEvent(t1)

    set t2=CreateTrigger()
    call TriggerAddAction(t2,function Cooldown)
    call TriggerRegisterTimerExpireEvent( t2, cooldown )

    set t3=CreateTrigger()
    call TriggerAddAction(t3,function Duration)
    call TriggerRegisterTimerExpireEvent( t3, duration )
endfunction

endscope

 

[aggro]

- for the indentation, you can dl a jass editor like jasscraft to auto indent your text

-- I fixed the code indents.

- function Actions–save GetEventDamageSource as local or global to avoid so many fn calls

-- i'll do that.

- multi-doable?

-- i mean, for example, five players have the same hero which has that spell and they cast that spell anytime without any bugs.

- your triggers/timers arent initialized

-- Actually thats my question. How to initialize the timers? I mean, when "cooldown" is counting down "function actions" wont work till "cooldown" is zero.

- function Duration–GetEventDamageSource() is null, perhaps this is the source of your error?

-- Function Duration–GetEventDamageSource() is null?? I didnt write any like that function. It doesnt gives any error. I just couldnt make the timers work.

 

[hgkjfhfdsj]

How to initialize the timers

private trigger t1 = CreateTrigger()
private timer cooldown = NewTimer()[/ljass]
// OR
set t1 = CreateTrigger() 
set duration = NewTimer()

//etc

Function Duration–GetEventDamageSource() is null?? I didnt write any like that function. It doesnt gives any error. I just couldnt make the timers work.

private function Duration takes nothing returns nothing
    call UnitRemoveAbility(GetEventDamageSource(), 'B006') //event response will not work.
endfunction

sorry, i meant that geteventdamagesource wont return anything--there is no event response for timer events, and as i was saying, attach 1 timer per unit that has the skill (if you dont want the schedule method). this will also make it mui/multi-doable

for fn duration to work properly, attach unit to timer and retrieve from that.