[vJASS] my cast system

[machris13]

i would like to have a feedback on my cast system..
it works fine for me but i dont know if it is really efficient..

library Cast initializer onInit

globals
    private integer PLAYER_ID
    private string interrupted
    boolean array busy[1] //this is a checker
endglobals

function Cast takes real casttime, unit caster returns nothing
    
    local real i = 0
    local real UnitX = GetUnitX(caster)
    local real UnitY = GetUnitY(caster)
    local real UnitAcq = GetUnitAcquireRange(caster)
    
    set PLAYER_ID = GetPlayerId(GetOwningPlayer(caster))
    set busy[PLAYER_ID] = true
    set busy[PLAYER_ID] = false
    debug call BJDebugMsg(I2S(PLAYER_ID))
    
    call SetUnitAcquireRange(caster, 0.0)
    call IssueImmediateOrder(caster, "holdposition")
    loop
        if GetUnitX(caster) != UnitX or GetUnitY(caster) != UnitY or busy[PLAYER_ID] != false then
            debug call BJDebugMsg(interrupted)
            call SetUnitAcquireRange(caster, UnitAcq)
            call IssuePointOrder(caster, "attack", GetUnitX(caster), GetUnitY(caster))
            set caster = null
            return
        endif
        call TriggerSleepAction(0.1)
        set i = i + 0.1
        exitwhen i == casttime
    endloop
    debug call BJDebugMsg("done!")
    set caster = null
        

endfunction

private function onInit takes nothing returns nothing
    set interrupted = "Interrupted!"
endfunction

endlibrary

That is the script, and this is how to use:

scope Spell initializer my

globals
    private constant integer MY_SPELL = 'A000'
endglobals

private function action takes nothing returns nothing

    local unit caster = GetTriggerUnit()
    local real casttime = 2.0
    
    call Cast(casttime, caster)
    //actions later
    set caster = null
    
endfunction

private function condition takes nothing returns boolean

    return GetSpellAbilityId() == MY_SPELL
    
endfunction

private function my takes nothing returns nothing
    local trigger t = CreateTrigger(  )
    call TriggerAddAction(t, function action)
    call TriggerAddCondition(t, function condition)
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
endfunction

endscope

this is still not finished, it still prevents the unit to cast but its easy to fix..i just want it to prove if it works really..

 

[Adiktuz]

uses waits so its inefficient, specially since waits are inaccurate and cannot actually be lower than .27 seconds...

also its not MUI, only MPI... since you use a check that uses the playerId... and also why set it to true then set it to false right after...

and you can set the string value at the globals block...

boolean array busy [1] is bad... remove the [1]...

on the sample trigger, the usage of the variable actually just worsen the code, since you would only use GetTriggerUnit() once, its better to use it directly... the local variable for the cast time is useless too...

you don't need to null caster and not to say that the null inside the loop is unnecessary as you also null it at the end...

This would really need a lot of changes and optimizations before it can be good...

also, there are already good casting systems out there... so I suggest that you just use them...

 

[machris13]

any other things to replace waits?

i set it to true then to false so that it would cancel any cast that are currently happening.

i was making a very simple one that fits my knowldege

boolean array busy [1] is bad... remove the [1]...

why? i made it array so all players can have it covered

and you can set the string value at the globals block...

if i do that what will i put in onInit? XD

as you also null it at the end...

i put null at both side because the spell can be interupted and will skip all remaining actions

 

[Adiktuz]

Timed Loops ofc...

Why not set it to false directly? coz there's no point on doing this:

busy = true
busy = false

/*
  that is the same as this actually, as it is almost instant and wc3 is single threaded so other things will only get executed after the instance of this trigger is finished...
*/

busy = false

I mean just remove the [1], it will still be an array without it... the [1] just increases init time...

its a library so you can remove onInit...

Overall, the method ur using is inefficient and isn't MUI... at least MPI...

 

[machris13]

busy = false

if i do this there will be leakage..i mean the caster can cast 2 channels at one time..

busy = true //cancels his previous cast if there is
busy = false //proceeds on his next cast

Timed Loops ofc...

how?

Overall, the method ur using is inefficient and isn't MUI... at least MPI...

so can you re write the script to mui?

 

[Adiktuz]

I don't see how he can cast two channels at same time because the time between the set = true and false is almost instant, and since wc3 is single threaded, it won't really affect other triggers or scripts until the current instance of the current trigger is finished... anyway, it won't matter that much if it really happens or not as you would really need to rewrite this...

as for rewrite, ofc I can but I'm so busy right now you know...

You don't know how to do timed loops? its basically using a timer to run a loop... (the same as a periodic trigger on GUI)

I suggest using (TimedLoop + Hashtables + UnitGroup) or (Index + TimerUtils)

one more, merge the actions into the conditions coz its faster than using Cond + Actions...

 

[machris13]

I don't see how he can cast two channels at same time because the time between the set = true and false is almost instant, and since wc3 is single threaded, it won't really affect other triggers or scripts until the current instance of the current trigger is finished...

oh yeah i see it now

as for rewrite, ofc I can but I'm so busy right now you know...

please when you have time


i will search a bit about that timedloop..i hope there where tuts out there..
thanks for the feedback i will try rewrite my script

 

[Adiktuz]

and learn not to use actions, just merge it with the conditions...

something like this:

scope Spell initializer my

// Since you only use 'A000' once, you might just want to use the value directly...

private function condition takes nothing returns boolean
    if GetSpellAbilityId() == 'A000' then
        call Cast(2.0, GetTriggerUnit())
    endif
    return false
endfunction

private function my takes nothing returns nothing
    local trigger t = CreateTrigger(  )
    call TriggerAddCondition(t, function condition)
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
endfunction

endscope

as for my time, I really won't have the time I think as I'm loaded with two projects... and if I'll rewrite this, I might as well just create my own cast system, which I don't want to do, as there is already an awesome one here... http://www.hiveworkshop.com/forums/...3-2-a-143066/?prev=search=casting&d=list&r=20

 

[machris13]

oh ok.. i dont quite understand that return false

as there is already an awesome one here... Spell Engine v0.3.2

this one is cool but i what i want is to learn how to do it..

 

[Adiktuz]

return false because its useless to return true when there's no action for the trigger...

and you need it coz conditions need to return a boolean... (so if you omit the return false, it would be SYNTAX ERROR)

anyway, I suggest trying to learn how to use http://www.hiveworkshop.com/forums/...3-2-a-143066/?prev=search=casting&d=list&r=20

coz it would enable you to do lots of things...

if you don't want to use it, try to look at its code, and see how its done... ^_^

 

[machris13]

this is too complicated..

but ok i will try to learn the script..
thanks anyways!

 

[Adiktuz]

yeah, that one was a bit advance... ^_^

but you could surely do it, just put some effort... ^.^

I suggest looking only on the main part as that's the thing you need... or maybe just try to look up on spells/systems on the Spells section that uses timed loops...