Help With GetRandomUnit()

01-06-2010, 10:29 PM

JASS:

function GetRandomPlayerHero takes nothing returns unit 
    local integer i = 0
    local unit t    = null
    loop
        set t = PlayerHero[GetRandomInt(0, 7)]
        if( GetUnitAbilityLevel(t, 'A028') > 0 ) then
            set t = null
        endif
    exitwhen t != null
    endloop
    return t
endfunction

I want to be able to get a random unit from an array of units, but only if the unit satisfied the condition (condition being they don't have a certain ability). This is the way I'm doing it now, but it isn't the best way, I'm sure it can be optimized.

I might also add that the array is of size 8, (0-7), and may not always be filled depending on how many players are in the game.

01-06-2010, 10:57 PM

grim001

Quote:

Originally Posted by thehellman

I might also add that the array is of size 8, (0-7), and may not always be filled depending on how many players are in the game.

The way you are doing it now will probably take the least number of operations on average. Your alternative is to build a local unit array filled with the existent units meeting the critera and use a counter to keep track of how many units you add to it. Then you'd pick a random hero from this new, compact array. The problem is that you would need to loop over this array again and null all the unit references. You'd also have to store the picked unit in a local unit variable before cleaning the array, and since this is what you need to return, the unit variable would need to be a parameter so that it doesn't leak a reference. Then you'd need a wrapper function to hide the null unit parameter. Overall, it'll probably wind up performing worse 99% of the time than your existing function.

It would look something like this:

JASS:

function GetRandomPlayerHero_Sub takes unit u returns unit 
    local unit array heroes
    local integer heroes_n = -1
    local integer i = 0
        loop
            exitwhen i > 7
            if GetUnitAbilityLevel(PlayerHero[i], 'A028') > 0 then //This would also filter out null units
                set heroes_n = heroes_n + 1
                set heroes[heroes_n] = PlayerHero[i]
            endif
            set i = i + 1
        endloop
        if heroes_n == -1 then
            return null
        endif
        set u = heroes[GetRandomInt(0, heroes_n)]
        set i = 0
        loop
            exitwhen i > heroes_n
            set heroes[i] = null
            set i = i + 1
        endloop
    return u
endfunction

function GetRandomPlayerHero takes nothing returns unit
    return GetRandomPlayerHero_Sub(null)
endfunction

Alternatively, if you instead used an array of pointers to the hero units, you'd have a much easier time since you could avoid using unit references directly:

JASS:

struct UnitPointer //example struct holding your hero units
    unit u
endstruct

function GetRandomPlayerHero_Sub takes nothing returns unit 
    local UnitPointer array heroes
    local integer heroes_n = -1
    local integer i = 0
        loop
            exitwhen i > 7
            if GetUnitAbilityLevel(PlayerHero[i].u, 'A028') > 0 then //This would also filter out null units
                set heroes_n = heroes_n + 1
                set heroes[heroes_n] = PlayerHero[i] //PlayerHero is a UnitPointer array now
            endif
            set i = i + 1
        endloop
        if heroes_n == -1 then
            return null
        endif
    return heroes[GetRandomInt(0, heroes_n)].u
endfunction

01-06-2010, 11:54 PM

weaaddar

Grim, that super duper overkill::

JASS:

function GetRandomPlayerHero takes nothing returns unit 
    local integer i = 0
    local boolean found = false
    loop
        set i = GetRandomInt(0, 7)
        if( GetUnitAbilityLevel(PlayerHero[i], 'A028') == 0 ) then
            set found = true
        endif
    exitwhen found
    endloop
    return PlayerHero[i]
endfunction

01-07-2010, 12:26 AM	#4
thehellman	Thanks, I think I'll use the second method. +rep to both. EDIT: Actually the above function doesn't work. It'll return a null hero, I just added a small null checker.

01-07-2010, 02:39 AM	#5
weaaddar	The hellman what you should do is instead of making it 0,7 figure out how many players you have and just add there heroes to the array. That way you'd only get null if there are no players without the ability.

01-07-2010, 04:30 AM

grim001

Well your function still has a random element, which makes it an upgraded version of the original function. I was just demonstrating what it would look like to remove the random element, but I don't recommend doing it that way since it is overkill and would usually be slower. The fastest way would be to just keep a compact array of eligible heroes and just pick from it with a random integer, and the simplest/preferred method would be what weaaddar posted.

01-07-2010, 04:36 AM	#7
weaaddar	Ah the easiest approach maybe to forgoe the array and just use the BJ function, like GroupPickRandomUnit ofcourse its probably poor for efficiency.

01-07-2010, 10:52 AM

Anitarf

You could just use a global array instead of a local one to avoid leaks (you could also use a local integer array that stores the indexes of the PlayerHero array instead of the heroes directly).

This is faster than the looping random checker in cases where only a few players have a valid hero. Most importantly, in the case where there are no valid heroes, this version of the function won't suffer from an infinite loop like the old one, it will just return null.

If such cases are not possible in your game, then it doesn't matter so much which version of the function you use.

JASS:

globals
    private unit array TempHero
endglobals

function GetRandomPlayerHero takes nothing returns unit 
    local integer i = 0
    local integer j = 0
    loop
        if PlayerHero[i]!=null and GetUnitAbilityLevel(PlayerHero[i], 'A028')==0 then
            set TempHero[j]=PlayerHero[i]
            set j=j+1
        endif
        set i=i+1
        exitwhen i>=8
    endloop
    if j==0 then
        return null
    endif
    set i = GetRandomInt(0, j-1)
    return TempHero[i]
endfunction