[JASS] GetRandomUnit

[Spellbound]

Is there an efficient way of picking a random unit in range matching certain conditions? I'm trying to create a bouncing attack a la Huntress, using BPower's Missile for projectiles.

Some context:

So I have this code (which doesn't work btw because my loop is nonsense):

much red

        private static method onFinish takes Missile this returns boolean
            
            local unit FoG  = null
            local unit u    = null
            local real x    = GetUnitX(this.target)
            local real y    = GetUnitY(this.target)
            local integer i
            local integer imax
            
            local real h        = GetUnitFlyHeight(this.target) + 50
            local real angle
            local Missile m
            
            set udg_NextDamageType = udg_DamageTypeBounce
            call UnitDamageTarget( this.source, this.target, this.damage, true, true, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL, null )
            call ConditionalTriggerExecute( udg_ClearDamageEvent )
            
            if this.data > 0 then

                call GroupEnumUnitsInRange(grp,x,y,mAoE[this.data],null)
                set imax = CountUnitsInGroup(grp)
                loop
                    set FoG=FirstOfGroup(grp)
                    exitwhen i>=imax
                    set i = i + 1
                    if (not UnitAlive(FoG)) or IsUnitAlly(FoG, GetOwningPlayer(this.source)) or FoG == this.target or (IsUnitOwnedByPlayer(FoG, this.owner)) then
                        call GroupRemoveUnit(grp,FoG)
                    endif
                    
                endloop
                
                if IsUnitGroupEmptyBJ(grp) == false then
                    set u = GroupPickRandomUnit(grp)
                    set angle = Atan2(GetUnitY(FoG)-y,GetUnitX(FoG)-x)
                        
                    set x = x + Cos(angle)
                    set y = y + Sin(angle)
                    
                    set m               = Missile.create(x, y, h + 50, angle, 2000, 50)
                    set m.speed         = this.speed
                    //set m.acceleration  = 0
                    set m.scale         = this.scale
                    set m.model         = this.model
                    set m.arc           = this.arc
                    set m.source        = this.source
                    set m.damage        = this.damage * .85
                    set m.target        = FoG
                    set m.data          = this.data - 1
                    set mAoE[m.data]    = mAoE[this.data]
                    call launch(m)
                    
                    set u = null
                    call groupClear(grp)
                endif
                
            endif
            
            return true   
        endmethod

As you can see, plenty of red flags. I'm told them Blizzard.j functions are bad, and for my purposes, they are as I need to be able to squeeze out the most amount of performance since this method will happen -A LOT-.

Using the First of Group way, it kinda works, but I have this little problem where it will always pick the same unit, which throws out the randomness of a bounce attack out the window. Can anyone show me another way to enumerate a random unit in range matching conditions?

 

[DracoL1ch]

set imax = CountUnitsInGroup(grp)
                loop
                    set FoG=FirstOfGroup(grp)
                    exitwhen i>=imax
                    set i = i + 1
                    if (not UnitAlive(FoG)) or IsUnitAlly(FoG, GetOwningPlayer(this.source)) or FoG == this.target or (IsUnitOwnedByPlayer(FoG, this.owner)) then
                        call GroupRemoveUnit(grp,FoG)
                    endif
                    
                endloop

once unit will not pass this condition (if he won't fulfil conditions and stay in group) rest loop will make 0 sense since you'll always get the same unit from now on.

wc3 able to handle thousands operations, you don't have to reinvite the wheel. it should probably be fine.

but if you are sure that this code will lag, you should use condition (filter) instead of "null", so you don't have to scan group twice.

regarding random - you can use whatever method you want, starting from simple enumerating group

set target=FirstOfGroup(g)
loop
set u=FirstOfGroup(g)
exitwhen u==null
if GetRandomInt(0,1)==1 then
set target=u
endif
call GroupRemoveUnit(g,u)
endloop

 

[Bribe]

        private static method onFinish takes Missile this returns boolean
            
            local unit u    = null
            local unit array units
            local real x    = GetUnitX(this.target)
            local real y    = GetUnitY(this.target)
            local integer i
            
            local real angle
            local Missile m
            
            set udg_NextDamageType = udg_DamageTypeBounce
            call UnitDamageTarget( this.source, this.target, this.damage, true, true, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL, null )
            call TriggerEvaluate(udg_ClearDamageEvent)
            
            if this.data > 0 then

                call GroupEnumUnitsInRange(grp, x, y, mAoE[this.data], null)
                loop
                    set u = FirstOfGroup(grp)
                    exitwhen u == null
                    call GroupRemoveUnit(grp, u)
                    if u != this.target and UnitAlive(u) and IsUnitEnemy(u, GetOwningPlayer(this.source)) then
                        set units[i] = u
                        set i = i + 1
                    endif
                endloop
                
                if i > 0 then
                    set u = [GetRandomInt(0, i - 1)]
                    set angle = Atan2(GetUnitY(u)-y,GetUnitX(u)-x)
                    
                    set x = x + Cos(angle)
                    set y = y + Sin(angle)
                    
                    set m               = Missile.create(x, y, GetUnitFlyHeight(this.target) + 100.00, angle, 2000, 50)
                    set m.speed         = this.speed
                    //set m.acceleration  = 0
                    set m.scale         = this.scale
                    set m.model         = this.model
                    set m.arc           = this.arc
                    set m.source        = this.source
                    set m.damage        = this.damage * .85
                    set m.target        = u
                    set m.data          = this.data - 1
                    set mAoE[m.data]    = mAoE[this.data]
                    call launch(m)
                    
                    set u = null
                endif
                
            endif
            
            return true   
        endmethod

 

[Spellbound]

Getting a syntax error here: set u = [GetRandomInt(0, i - 1)]

I'm guessing it's set u = units[GetRandomInt(0, i - 1)] instead?

set imax = CountUnitsInGroup(grp)
                loop
                    set FoG=FirstOfGroup(grp)
                    exitwhen i>=imax
                    set i = i + 1
                    if (not UnitAlive(FoG)) or IsUnitAlly(FoG, GetOwningPlayer(this.source)) or FoG == this.target or (IsUnitOwnedByPlayer(FoG, this.owner)) then
                        call GroupRemoveUnit(grp,FoG)
                    endif
                    
                endloop

once unit will not pass this condition (if he won't fulfil conditions and stay in group) rest loop will make 0 sense since you'll always get the same unit from now on.

Yeah, my loop is bad (like I said in the OP ).

 

[Wietlol]

I'm told them Blizzard.j functions are bad,

They are but you have to understand the reason.
The problem about JASS is that function calls are quite heavy.
Every Blizzard.j function can be written by hand instead of calling the function (unlike Common.j) so you could save the function call operation and use the actions directly.

In some cases:

function SetUnitAbilityLevelSwapped takes integer abilcode, unit whichUnit, integer level returns integer
    return SetUnitAbilityLevel(whichUnit, abilcode, level)
endfunction

these things occur that you call a function that directly links to another one with the same attributes but then just in a different order.
These are just useless and you should use the natives instead.

But other functions:

function ModuloReal takes real dividend, real divisor returns real
    local real modulus = dividend - I2R(R2I(dividend / divisor)) * divisor

    // If the dividend was negative, the above modulus calculation will
    // be negative, but within (-divisor..0).  We can add (divisor) to
    // shift this result into the desired range of (0..divisor).
    if (modulus < 0) then
        set modulus = modulus + divisor
    endif

    return modulus
endfunction

Are just about implemenation and generic usage.
These functions arent really that bad to be used and even recommended instead of writing the code directly.

\but if you are sure that this code will lag, you should use condition (filter) instead of "null", so you don't have to scan group twice.

You are aware that you contradict yourself there right?

Anyway the most efficient way of looping through a group is a FirstOfGroup iteration.

    local unit FoG
    loop
        set FoG = FirstOfGroup(g)
        exitwhen FoG == null
        call GroupRemoveUnit(g, FoG)
        
        //Actions
    endloop

This is the standard FirstOfGroup iteration that will loop through group g and does //Action for every unit.

This is my implementation of GroupGetRandomUnit:

    globals
        unit bf_pickedUnit
    endglobals
    function GroupGetRandomUnit takes group g returns unit
        local unit FoG
        local group swap = CreateGroup()
        local integer count = 0
        local integer random
        set bf_pickedUnit = null
        
        loop
            set FoG = FirstOfGroup(g)
            exitwhen FoG == null
            call GroupRemoveUnit(g, FoG)
            
            set count = count +1
            call GroupAddUnit(swap, FoG)
        endloop
        
        set random = GetRandomInt(0, count)
        set count = 0
        
        loop
            set FoG = FirstOfGroup(swap)
            exitwhen FoG == null
            call GroupRemoveUnit(swap, FoG)
            call GroupAddUnit(g, FoG)
            
            set count = count +1
            if count == random then
                set bf_pickedUnit = FoG
            endif
        endloop
        
        call DestroyGroup(swap)
        set swap = null
        return bf_pickedUnit
    endfunction

This will not empty the group nor destroy it.
You can do a more efficient way of looping through the group but that will clear/destroy the group.
You can add your preferred condition in here.

@Bribe:
set u = [GetRandomInt(0, i - 1)]
I find that statement very interesting...
set u = units[GetRandomInt(0, i - 1)]

 

[Bribe]

Getting a syntax error here: set u = [GetRandomInt(0, i - 1)]

I'm guessing it's set u = units[GetRandomInt(0, i - 1)] instead?

Yes, sorry buddy. I had an earlier draft but somehow when I was editing I must have retyped that statement incorrectly.

And, as Wietlol said, some BJ functions are good. R/IMaxBJ, R/IMinBJ, ModuloInteger/Real, some others like CustomVictoryBJ and SetPlayerAllianceBJ have their uses. Some cinematic ones are good.

 

[Spellbound]

        private static method onFinish takes Missile this returns boolean
            
            local unit u    = null
            local unit array units
            local real x    = GetUnitX(this.target)
            local real y    = GetUnitY(this.target)
            local integer i
            
            local real angle
            local Missile m
            
            set udg_NextDamageType = udg_DamageTypeBounce
            call UnitDamageTarget( this.source, this.target, this.damage, true, true, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL, null )
            call TriggerEvaluate(udg_ClearDamageEvent)
            
            if this.data > 0 then

                call GroupEnumUnitsInRange(grp, x, y, mAoE[this.data], null)
                loop
                    set u = FirstOfGroup(grp)
                    exitwhen u == null
                    call GroupRemoveUnit(grp, u)
                    if u != this.target and UnitAlive(u) and IsUnitEnemy(u, GetOwningPlayer(this.source)) then
                        set units[i] = u
                        set i = i + 1
                    endif
                endloop
                
                if i > 0 then
                    set u = [GetRandomInt(0, i - 1)]
                    set angle = Atan2(GetUnitY(u)-y,GetUnitX(u)-x)
                    
                    set x = x + Cos(angle)
                    set y = y + Sin(angle)
                    
                    set m               = Missile.create(x, y, GetUnitFlyHeight(this.target) + 100.00, angle, 2000, 50)
                    set m.speed         = this.speed
                    //set m.acceleration  = 0
                    set m.scale         = this.scale
                    set m.model         = this.model
                    set m.arc           = this.arc
                    set m.source        = this.source
                    set m.damage        = this.damage * .85
                    set m.target        = u
                    set m.data          = this.data - 1
                    set mAoE[m.data]    = mAoE[this.data]
                    call launch(m)
                    
                    set u = null
                endif
                
            endif
            
            return true   
        endmethod

Right, I was having some problems with Missile and was unable to test this code until now. Besides the set u = [GetRandomInt(0, i - 1)] part, the local integer i had to be initialised, otherwise it kept picking the same unit over and over again. Ironically, initialising variables was the exact problem I was having with Missile.

Thanks for the code. The bounce works nicely now

 

[LeP]

Here is a cute way to get a random unit without an array. I have done it in the Enum callback but you can ofcourse just do it with a FirstOfGroup-loop. But i'm not even sure it's faster coz one call to GetRandomInt for every unit instead of just assigning to some array.
But i also dont care.

library RandomUnit

globals
    private unit U
    private integer C
endglobals

private function SelectUnitRandom takes nothing returns boolean
    // for C = 0 this is always true as GetRandomInt(0, 0) returns 0
    if GetRandomInt(0, C) == C then
        set U = GetFilterUnit()
    endif
    set C = C +1
    return true
endfunction

function GetRandomUnit takes group g, real x, real y, real r, boolexpr filter returns unit
    local boolexpr f = And(filter, Condition(function SelectUnitRandom))
    set U = null
    set C = 0
    call GroupEnumUnitsInRange(g, x, y, r, f)
    call DestroyBoolExpr(f)
    set f = null
    return U
endfunction

endlibrary

Use it like this:

function aFilter takes nothing returns boolean
    return GetFilterUnit() != SomeUnit and not IsUnitType(GetFilterUnit(), UNIT_TYPE_UNDEAD) // and other conditions
endfunction

local unit u = GetRandomUnit(Group, X, Y, Range, Filter(function aFilter))

If one doesn't need the group one can simply use one constant group and put into the library.

 

[Bribe]

While I like not using a unit array, it is necessary to have a balanced probability pool. The total probability needs to be taken into account before GetRandomInt is called.

 

[LeP]

While I like not using a unit array, it is necessary to have a balanced probability pool. The total probability needs to be taken into account before GetRandomInt is called.

'no'
https://en.wikipedia.org/wiki/Reservoir_sampling (for us k = 1)
Should've included this in the OP though.

e:
And indeed as Bribe said there already is something like this in Bliazzard.j: GroupPickRandomUnit.
So if you're a fan of composeable and modular programming you can just go ahead and use GroupEnumUnitsInRange(yourfilter) + GroupPickRandomUnit, but i guess it isn't zomg optimzed.
It would definitly reduce your 9 lines of code to two (+ your filter func).
With that i guess we mentioned every way to pick a random unit in here?
And we've come full circle as you already used GroupPickRandomUnit in your code

 

[Bribe]

The units picked first have mire than a 1/number of units chance.

 

[LeP]

The units picked first have mire than a 1/number of units chance.

There might be a bug in my or blizzards code but reservoir sampling is a known and proven method. If you don't trust the math you can check it for small numbers like 2, 3 and 4 yourself.

e:
But as i said, the array version might be better in jass.

 

[Bribe]

Yeah, you're right. For some reason I thought the whole group was being re-sampled each iteration. This will have the same results.