• 🏆 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!
  • ✅ The POLL for Hive's Texturing Contest #33 is OPEN! Vote for the TOP 3 SKINS! 🔗Click here to cast your vote!

[JASS] Unit Group Problem (Struct Related)

Status
Not open for further replies.
Level 4
Joined
Apr 2, 2006
Messages
32
Having a slight problem with a spell I've created with NewGen here.

It's meant to send the target unit flying backwards.
When it comes to a stop, a bunch of units are grouped within a small radius of that unit,
and are then knocked back in the 'Knockback_Aftershock' function.

All of that works, apart from the last part - the grouped-units aren't knocked back.
All I am able to tell from what is happening here, is that the grouped-units
are moved just once, and then the spell comes to a halt.

I've tried debugging in that non-working function, and it reads 0 for both groups.
Strange though, because when I try debugging in the previous function,
it reads above 0 (if there are units in the group).

So you should've been able to pick up what I'm trying to do here.
I'm using the simple "Loop & exitwhen unit of group = null" method,
with the extra duplicate group adding all removed units back into the one
thats had units removed from it inside the loop.

I've got no idea how to fix this.
Is there any other sort of method I can use that enables me to have access
to a struct's members when looping through a unit group?

JASS:
scope Knockback

globals
    //Knockback effects config
    private constant integer ID   = 'AOcl'
    private constant real damage  = 4
    private constant real tdelay  = 0.03
    private constant real brake   = 0.975
    private constant real maxdist = 700
    private constant real power   = 25
    private constant string SFX   = "Abilities\\Spells\\Undead\\Impale\\ImpaleHitTarget.mdl"
    private constant string SFX2  = "Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl"

    //Knockback after-shock config
    private constant real damage2  = 325
    private constant real brake2   = 0.9
    private constant real maxdist2 = 445
    private constant real AoE      = 300   
    private constant real power2   = 10 
    private constant string SFX3   = "Abilities\\Spells\\Human\\ThunderClap\\ThunderClapCaster.mdl"
endglobals

struct Knockback_Data
    unit u   
    unit T  
    real ang = 0
    real mxd = maxdist
    real pwr = power
endstruct

struct Knockback_Aftershock_Data
    group g 
    group g2
    real mxd2 = 0
    real pwr2 = 0
    real OX   = 0
    real OY   = 0
endstruct

function Knockback_Filter takes nothing returns boolean
    return IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(GetTriggerUnit())) and GetWidgetLife(GetFilterUnit()) > 0.405
endfunction

function Knockback_Aftershock takes nothing returns nothing
    local timer tt                     = GetExpiredTimer()
    local Knockback_Aftershock_Data d2 = Knockback_Aftershock_Data(GetHandleInt(tt, "d2"))
    local group g                      = d2.g
    local group g2                     = d2.g2
    local real mxd2                    = d2.mxd2
    local real pwr2                    = d2.pwr2
    local real OX                      = d2.OX
    local real OY                      = d2.OY
    local unit p 

        call BJDebugMsg(I2S(CountUnitsInGroup(d2.g)))
        call BJDebugMsg(I2S(CountUnitsInGroup(d2.g2)))

    if mxd2 > 0 then
        loop
            set p      = FirstOfGroup(g)
            exitwhen p == null
            call GroupRemoveUnit(g, p)
            call SetUnitPosition(p, GetUnitX(p) + pwr2 * Cos(Atan2(GetUnitY(p)-OY, GetUnitX(p)-OX)), GetUnitY(p) + pwr2 * Sin(Atan2(GetUnitY(p)-OY, GetUnitX(p)-OX)))
            call DestroyEffect(AddSpecialEffect(SFX2, GetUnitX(p), GetUnitY(p)))
        endloop

        call GroupAddGroup(g2, d2.g)

        set d2.pwr2 = d2.pwr2 * brake2
        set d2.mxd2 = d2.mxd2 - d2.pwr2
    else
        call d2.destroy()
        call PauseTimer(tt)
        call FlushHandleLocals(tt)
        call DestroyTimer(tt)
    endif


    set tt = null
    set g  = null
    set g2 = null
endfunction

function Knockback_Effects takes nothing returns nothing
    local timer t          = GetExpiredTimer()
    local Knockback_Data d = Knockback_Data(GetHandleInt(t, "d"))
    local unit u           = d.u
    local unit T           = d.T
    local real ang         = d.ang
    local real mxd         = d.mxd
    local real pwr         = d.pwr
    local real polarX      = GetUnitX(T) + pwr * Cos(ang)
    local real polarY      = GetUnitY(T) + pwr * Sin(ang)
    local Knockback_Aftershock_Data d2
    local group g
    local boolexpr b
    local timer tt

    if mxd > 0 then
        call SetUnitPosition(T, polarX, polarY)
        call DestroyEffect(AddSpecialEffect(SFX, polarX, polarY))
        call DestroyEffect(AddSpecialEffect(SFX2, polarX, polarY)) 
        call UnitDamageTarget(u, T, damage, false, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_NORMAL, null)
        set d.pwr = d.pwr * brake
        set d.mxd = d.mxd - d.pwr
    else
        set g  = CreateGroup()
        set b  = Condition(function Knockback_Filter)
        set d2 = Knockback_Aftershock_Data.create()
        set tt = CreateTimer()

        call GroupEnumUnitsInRange(g, GetUnitX(T), GetUnitY(T), AoE, b)

        set d2.g       = g
        set d2.g2      = g
        set d2.mxd2    = maxdist2
        set d2.pwr2    = power2
        set d2.OX      = GetUnitX(T)
        set d2.OY      = GetUnitY(T) 

        call SetHandleInt(tt, "d2", d2)
        call TimerStart(tt, tdelay, true, function Knockback_Aftershock)

        call DestroyEffect(AddSpecialEffect(SFX3, GetUnitX(T), GetUnitY(T))) 
        
        call DestroyBoolExpr(b)
        set g  = null
        set b  = null
        set tt = null

        call PauseUnit(T, false)
        call d.destroy()
        call PauseTimer(t)
        call FlushHandleLocals(t)
        call DestroyTimer(t)
    endif

    set t = null
    set u = null
    set T = null
endfunction

function Knockback_Actions takes nothing returns nothing 
    local Knockback_Data d = Knockback_Data.create()
    local unit u           = GetTriggerUnit()
    local unit T           = GetSpellTargetUnit()
    local real X1          = GetUnitX(u)
    local real Y1          = GetUnitY(u)
    local real X2          = GetUnitX(T)
    local real Y2          = GetUnitY(T) 
    local real ang         = Atan2(Y2-Y1, X2-X1)
    local timer t          = CreateTimer()

    call PauseUnit(T, true)

    set d.u   = u
    set d.T   = T
    set d.ang = ang

    call SetHandleInt(t, "d", d)
    call TimerStart(t, tdelay, true, function Knockback_Effects)

    set u = null
    set T = null
endfunction

function Knockback_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == ID
endfunction

function InitTrig_Knockback_Copy takes nothing returns nothing
    local trigger t = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( t, Condition( function Knockback_Conditions ) )
    call TriggerAddAction( t, function Knockback_Actions )
    set t = null
endfunction

endscope
 
Level 13
Joined
Nov 22, 2006
Messages
1,260
Wow, a nice piece of code.......very good triggering, nice job.

Why do you use a local trigger in InitTrig_Knockback_Copy function when you can use a global one that is created?

You forgot to nullify the timer in Knockback_Actions. Actually it's better to use Vexorian's CSSafety library which prevents a lot of timer bugs and also you don't have to nullify them.

There is no real need to do this:

JASS:
local unit u                                   = d.u local
unit T                                          = d.T
local real ang                                = d.ang
local real mxd                                = d.mxd
local real pwr                                = d.pwr


Why don't you just use d.T, d.ang, d.mxd and d.pwr as normal variables? You don't have to make new locals and then equalize them. That way you'd save some space.

Also, it's more efficient to have Cos(ang) and Sin(ang) in a variable so you don't call them every 0.03 seconds (put them in the Knockback_Data struct).

It seems like boolexprs that are not generated with And(), Or() or Not() don't, so there's not need in destroying them in this case.

Now here's the problem, I think:

JASS:
loop
    set p = FirstOfGroup(g)
    exitwhen p == null
    call GroupRemoveUnit(g, p)
    call SetUnitPosition(p, GetUnitX(p) + pwr2 * Cos(Atan2(GetUnitY(p)-OY, GetUnitX(p)-OX)), GetUnitY(p) + pwr2 * Sin(Atan2(GetUnitY(p)-OY, GetUnitX(p)-OX)))
    call DestroyEffect(AddSpecialEffect(SFX2, GetUnitX(p), GetUnitY(p)))
endloop
call GroupAddGroup(g2, d2.g)


This line messes that up:

set d2.g = g set d2.g2 = g

It should be like this:

JASS:
local group g2 = CreateGroup()
// ...
// ...
call GroupAddGroup(d2.g, g2)
loop
    set p = FirstOfGroup(g2)
    exitwhen p == null
    call GroupRemoveUnit(g2, p)
    call SetUnitPosition(p, GetUnitX(p) + pwr2 * Cos(Atan2(GetUnitY(p)-OY, GetUnitX(p)-OX)), GetUnitY(p) + pwr2 * Sin(Atan2(GetUnitY(p)-OY, GetUnitX(p)-OX)))
    call DestroyEffect(AddSpecialEffect(SFX2, GetUnitX(p), GetUnitY(p)))
endloop
call DestroyGroup(g2)


I hope you understand what stuff you have to change, because I don't have time to explain. You did it too complicated when it could be much simpler.

Crap gotta go, I'll finish later
 

Attachments

  • CSSafety.txt
    1.6 KB · Views: 145
Level 40
Joined
Dec 14, 2005
Messages
10,532
You can easily get rid of all the locals in the init. (except the struct, and the timer if you don't want to put it inside the struct)

By the way, you can clean up that group loop a lot, and storing only 1 group;

JASS:
        loop
            set p      = FirstOfGroup(g)
            exitwhen p == null
            call GroupRemoveUnit(g, p)
            call SetUnitPosition(p, GetUnitX(p) + pwr2 * Cos(Atan2(GetUnitY(p)-OY, GetUnitX(p)-OX)), GetUnitY(p) + pwr2 * Sin(Atan2(GetUnitY(p)-OY, GetUnitX(p)-OX)))
            call DestroyEffect(AddSpecialEffect(SFX2, GetUnitX(p), GetUnitY(p)))
        endloop

        call GroupAddGroup(g2, d2.g)

Becomes (removing g2 altogether) (also faster and cleaner than silvenon's code)

JASS:
//g is equal to dg.g
//local group g = d2.g
        set d2.g = CreateGroup()
        loop
            set p      = FirstOfGroup(g)
            exitwhen p == null
            call GroupRemoveUnit(g, p)
            call GroupAddUnit(d2.g,p)
            call SetUnitPosition(p, GetUnitX(p) + pwr2 * Cos(Atan2(GetUnitY(p)-OY, GetUnitX(p)-OX)), GetUnitY(p) + pwr2 * Sin(Atan2(GetUnitY(p)-OY, GetUnitX(p)-OX)))
            call DestroyEffect(AddSpecialEffect(SFX2, GetUnitX(p), GetUnitY(p)))
        endloop
        call DestroyGroup(g)


Silvenon, boolexprs still "leak" in a sense, (without And/Or/Not) but they're stored in a similar way to strings, and the same boolexpr can be reused if it wasn't destroyed.
 
Level 13
Joined
Nov 22, 2006
Messages
1,260
(also faster and cleaner than silvenon's code)

I was in a real hurry and I didn't have time to understand the whole code and think of a most efficient way.

Silvenon, boolexprs still "leak" in a sense, (without And/Or/Not) but they're stored in a similar way to strings, and the same boolexpr can be reused if it wasn't destroyed.

Somebody said that destroying boolexprs is "dangerous", whatever that meant.
 
Level 4
Joined
Apr 2, 2006
Messages
32
Thanks guys, that helped, and it now works.

My only problem now, is that, for some reason rather the filter isn't working properly.
Units are grouped regardless whether they're an enemy to the trig unit or not.

Is there a way to reference a struct in a filter function?
If not, is there a way to fix this?
 
Level 13
Joined
Nov 22, 2006
Messages
1,260
Ah, here we have the problem:

JASS:
function Knockback_Filter takes nothing returns boolean
    return IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(GetTriggerUnit())) and GetWidgetLife(GetFilterUnit()) > 0.405
endfunction


GetTriggerUnit() is null in this case, because you're creating the boolexpr in a function that is not _Actions, so there's no event response. The easy way out of that would be something like this:

JASS:
function Knockback_Filter takes nothing returns boolean
    return IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(bj_ghoul[100])) and GetWidgetLife(GetFilterUnit()) > 0.405
endfunction

// ...
// ...
// Knockback_Actions function
set bj_ghoul[100] = GetTriggerUnit()


bj_ghoul is a global unit variable already existing, so you don't have to create a new one. It's MUI, because the boolexpr is created instantly, so you don't have to worry about that.
 
Level 13
Joined
Nov 22, 2006
Messages
1,260
No, silvenon, GetTriggerUnit() and such pass through functions that aren't threaded.

However, since the timer creates a new thread, GetTriggerUnit() isn't automatically passed.

I don't know what 'threaded' means, but what I wanted to say is that in this case GetTriggerUnit() is null. In what other cases the event responses do pass?

Oh, by the way, you can use Filter() instead of Condition() ;)

........but you said that there is no difference
 
Level 40
Joined
Dec 14, 2005
Messages
10,532
1) Threaded - in a new thread; runs parallel to the old one, on its own stack.

Meaning things like waits don't interfere between threads, and thread crashes only affect the thread they crash in.

2) Filter saves map size ;)

I just personally have a habit of using Condition for Condition and Filter for Filter, like they're intended to.
 
Level 13
Joined
Nov 22, 2006
Messages
1,260
Omg, ok Pooty, then I'm gonna be careful so I don't have 3 extra letters in my scripts, that will greatly decrease my map size.

If you're so worried about the map size, why don't you name your functions this way:

JASS:
function PootyRulz takes nothing returns boolean
    return true
endfunction


Into:

JASS:
function PR takes nothing returns boolean
    return true
endfunction


This way I saved a "lot" of space :p

I hope you were kidding..... :)
 
Level 40
Joined
Dec 14, 2005
Messages
10,532
No, I was actually serious. (If you use a lot of things like that, not just Filter vs Condition but other stuff, you can actually save quite a few KB or so on)

And you didn't save any space that way, thanks to old man optimizer ;)

It's the same with GetSpellAbilityUnit vs GetTriggerUnit, and so on, and so forth.

It doesn't make enough of a difference that you should relearn that unless you're really scratching your 4mb limit, though.
 
Status
Not open for further replies.
Top