How to make this simple spell MUI? Thanks.

[Geshishouhu]

Hi all, I am making a spell that does periodical damage to a group of units. My code is like this:

globals
    unit XZGH_unit=null
    hashtable hash=InitHashtable()
    real counter
    constant real period=1.
    constant real duration=10.
endglobals

function callback takes nothing returns nothing
    local group g=LoadGroupHandle(hash,GetHandleId(XZGH_unit),StringHash("group"))
    local unit u
    local timer t=GetExpiredTimer()
    if counter<=0 then
        call PauseTimer(t)
        call DestroyTimer(t)
    else
        loop    
            set u=FirstOfGroup(g)
            exitwhen u==null
            call GroupRemoveUnit(g,u)
            call UnitDamageTarget(XZGH_unit,u,100.,true,false,ATTACK_TYPE_MAGIC,DAMAGE_TYPE_MAGIC,null) 
        endloop
        call FlushChildHashtable(hash,GetHandleId(XZGH_unit))
        call DestroyGroup(g)
        set counter=duration-period
    endif
    set g=null
    set t=null
endfunction

function actions takes nothing returns boolean
    local group g=CreateGroup()
    local real x=GetSpellTargetX()
    local real y=GetSpellTargetY()
    local timer t=CreateTimer()
    if GetSpellAbilityId() == 'A621' then
        set XZGH_unit=GetTriggerUnit()
        call GroupEnumUnitsInRange(g,x,y,800.,null)
        call SaveGroupHandle(hash,GetHandleId(XZGH_unit),StringHash("group"),g)
        set counter=duration
        call TimerStart(t,period,true,function callback)
    endif
    set g=null
    set t=null
    return false
endfunction

function InitTrig_XZGHui takes nothing returns nothing
    local trigger t=CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition(t, Condition(function actions))
    set t=null
endfunction

Apparently, this spell is not MUI. I have two questions:
First, regardless of efficiency, how to make it MUI?
Second, how to make it more efficient?
Thanks a lot.

 

[Maker]

You save the handle of the group and then destroy the group right away. No wonder it will not work correctly.

 

[Geshishouhu]

You save the handle of the group and then destroy the group right away. No wonder it will not work correctly.

Oops, thanks for pointing that out. Edited my trigger post.
But I think the reason that it is non-MUI is because of the this global unit variable "XZGH_unit". Is there a way to make it MUI? Thanks a lot.

BTW: can I null the local variable "g" in the "action" function?

 

[deathismyfriend]

Yes you can null g.
You always have to null all local variables that are handles.

 

[chobibo]

What's the script supposed to do? I don't think you should attach data to a unit if you are going to use a timer callback to process the data, try attaching to the timer.

 

[Geshishouhu]

Yes you can null g.
You always have to null all local variables that are handles.

Thanks, I will keep that in mind.

What's the script supposed to do? I don't think you should attach data to a unit if you are going to use a timer callback to process the data, try attaching to the timer.

It is a spell that causes damage to a group of units.
So do you mean that I have to use vJass? I only know attaching datas to a timer in vJass. :/

 

[chobibo]

No, of course not. You can attach data to timers in jass. You can use hashtables for that.
e.g. SaveHandle(hashtable,timer_key,child_key,HandleData)

EDIT: That was just a pseudo-code.

 

[Geshishouhu]

No, of course not. You can attach data to timers in jass. You can use hashtables for that.
e.g. SaveHandle(hashtable,timer_key,child_key,HandleData)

EDIT: That was just a pseudo-code.

Oh, I didn't know that. Thanks a lot. So can I attach the casting unit to the timer?

 

[chobibo]

Yes, and the group that is related to the casting unit too. You could also cross attach it to the casting unit if you need to retrieve the group using the unit.

• timer <-caster [hash, timer_key, child_key, caster]
• timer <-caster target group [hash, timer_key, child_key, caster_target_group]
• caster <- caster target group [hash, caster_key, child_key, caster_target_group]

You could also attach the timer, so if you need to immediately stop it, you can retrieve it.
• caster <- timer [hash, caster_key, child_key, timer]

 

[Geshishouhu]

Yes, and the group that is related to the casting unit too. You could also cross attach it to the casting unit if you need to retrieve the group using the unit.

• timer <-caster [hash, timer_key, child_key, caster]
• timer <-caster target group [hash, timer_key, child_key, caster_target_group]
• caster <- caster target group [hash, caster_key, child_key, caster_target_group]

You could also attach the timer, so if you need to immediately stop it, you can retrieve it.
• caster <- timer [hash, caster_key, child_key, timer]

Thanks for the detailed answer, chobibo! I want to give you 10 Reps if I could.

 

[BPower]

in actions you create a timer, not matter if the SpellAbilityId matches or not. If the AbilityId doesn't fit you don't destroy the timer.

Edit: For group g it's the same. Here is a solid solution:

function run takes unit u, real x, real y returns nothing
    local group g=CreateGroup()
    local timer t=CreateTimer()
    set XZGH_unit=u// <-- ???
    call GroupEnumUnitsInRange(g,x,y,800.,null)
    call SaveGroupHandle(hash,GetHandleId(XZGH_unit),StringHash("group"),g)
    set counter=duration
    call TimerStart(t,period,true,function callback)
    set g=null
    set t=null
endfunction
function actions takes nothing returns boolean
    if GetSpellAbilityId() == 'A621' then
        call run(GetTriggerUnit(), GetSpellTargetX(), GetSpellTargetY())
    endif
    return false
endfunction

Also you create alot of local groupa. What about using one global group in callback?

 

[muzzel]

This is still far away from MUI...

- As already mentioned, you dont need the global "XZGH_unit".

- You want your spell to be periodic, but in the first iteration of the timer you remove all the target units from the group and destroy the group. You even flush the child hashtable! This way the other iterations wont do anything. (This is actually not a MUI problem, its a general problem with your code)

- Your "counter" variable is global, to keep it MUI you need to use a local counter and attach its value to the timer (by hashtable, just like the group).

- btw, using stringhashs as child keys is quite useless, just use constant integers

This is how your code should look like: (didnt test it, i hope its working)

globals
    unit caster = null
    hashtable hash=InitHashtable()
    constant real period=1.
    constant real duration=10.
endglobals

function cb()
    call UnitDamageTarget(caster, GetEnumUnit(), 100.,true,false,ATTACK_TYPE_MAGIC,DAMAGE_TYPE_MAGIC,null)

function callback takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local integer handleId = GetHandleId(t)
    local group g = LoadGroupHandle(hash, handleId, 1)
    local real counter = LoadReal(hash, handleId, 3)
    local unit u
    if counter<=0 then
        call PauseTimer(t)
        call DestroyTimer(t)
        call FlushChildHashtable(hash,, handleId)
        call DestroyGroup(g)
    else
        set caster = LoadUnitHandle(hash, handleId, 2)
        call ForGroup(g, function cb)
        call SaveReal(hash, handleId, 3, counter-period)
    endif
    set g=null
    set t=null
endfunction

function actions takes nothing returns boolean
    local group g = null
    local real x=GetSpellTargetX()
    local real y=GetSpellTargetY()
    local integer handleId
    local timer t = null
    if GetSpellAbilityId() == 'A621' then
        set t = CreateTimer()
        set handleId = GetHandleId(t)
        set g = CreateGroup()
        call GroupEnumUnitsInRange(g,x,y,800.,null)
        call SaveGroupHandle(hash, handleId, 1, g)
        call SaveUnitHandle(hash, handleId, 2, GetTriggerUnit())
        call SaveReal(hash, handleId, 3, duration)
        call TimerStart(t,period,true,function callback)
        set g=null
        set t=null
    endif
    return false
endfunction

function InitTrig_XZGHui takes nothing returns nothing
    local trigger t=CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition(t, Condition(function actions))
    set t=null
endfunction

 

[Geshishouhu]

in actions you create a timer, not matter if the SpellAbilityId matches or not. If the AbilityId doesn't fit you don't destroy the timer.

Edit: For group g it's the same. Here is a solid solution:

function run takes unit u, real x, real y returns nothing
    local group g=CreateGroup()
    local timer t=CreateTimer()
    set XZGH_unit=u// <-- ???
    call GroupEnumUnitsInRange(g,x,y,800.,null)
    call SaveGroupHandle(hash,GetHandleId(XZGH_unit),StringHash("group"),g)
    set counter=duration
    call TimerStart(t,period,true,function callback)
    set g=null
    set t=null
endfunction
function actions takes nothing returns boolean
    if GetSpellAbilityId() == 'A621' then
        call run(GetTriggerUnit(), GetSpellTargetX(), GetSpellTargetY())
    endif
    return false
endfunction

Also you create alot of local groupa. What about using one global group in callback?

Thanks for both of you. I have solved the problem. One example is better than thousand words.