Jass code to check player with most units in group.

02-04-2006, 09:14 AM

This is a simple code for checking which playing has the most units in the area. But it doesn't work. Any ideas?

Code:

function CheckUnit takes nothing returns nothing
    local trigger ti = GetTriggeringTrigger()
    local integer tempint = GetHandleInt(ti, I2S(GetPlayerId(GetOwningPlayer(GetEnumUnit()))))
    set tempint = tempint + 1
    call SetHandleInt(ti, I2S(GetPlayerId(GetOwningPlayer(GetEnumUnit()))), tempint)
    set ti = null
endfunction

function MostUnits takes group checkgroup returns player
    local trigger ti = GetTriggeringTrigger()
    local player tempplayer
    local integer tempi = 0
    local integer tempint = 0
    local integer loopint = 0
    call ForGroupBJ( checkgroup,  function CheckUnit )
    loop
        set tempi = GetHandleInt(ti, I2S(loopint))
        if tempi > tempint then
           set tempint = tempi
           set tempplayer = Player(loopint)
        endif
        exitwhen loopint == 15
        set loopint = loopint + 1
    endloop
    call FlushHandleLocals(ti)
    return tempplayer
endfunction

02-04-2006, 11:51 AM

Vexorian

Some changes:

JASS:

function CheckUnit takes nothing returns nothing
 local trigger ti = GetTriggeringTrigger()
 local string k=I2S(GetPlayerId(GetOwningPlayer(GetEnumUnit())))

    call SetHandleInt(ti, GetHandleInt(ti,k)+1 )
    set ti = null
endfunction

I2S(GetPlayerId(GetOwningPlayer(GetEnumUnit()))) is kind of a lot of calls, calling stuff like that twice or more times in the same process is a bad habit. Also you where using an integer variable for something that was used once. This way it simplifies the code even more and uses a variable for the call that should use a variable there.

I don't think using handle variables on the trigger is a great idea. I think you are making this trigger for your own map? then make an array, Handle Variables are too slow, and you are picking every unit in the group this would cause some lag issues if the group is big. If you can't make an array use one of the many BJ arrays out there or use CSCache's tables, a table would at least remove the extra time caused by getting the handle id of the same trigger twice per unit.

JASS:

function MostUnits takes group checkgroup returns player
    local trigger ti = GetTriggeringTrigger()
    local integer playerid=0
    local integer n
    local integer max = -1
    local integer i = 0

    loop
        exitwhen(i>15)
        call SetHandleInt(ti,I2S(i),0)
        set i=i+1
    endloop
    set i=0
    call ForGroup( checkgroup,  function CheckUnit )
    loop
        set n = GetHandleInt(ti, I2S(i))
        if n > max then
           set max = n
           set playerid = i
        endif
        exitwhen i == 15
        set i = i + 1
    endloop
    call FlushHandleLocals(ti)
    return Player(playerid)
endfunction

You are not using bj_wantDestroyGroup, so there is no reason to use ForGroupBJ instead of ForGroup. The names you gave to the variables are too big AND undescriptive. If you are going to make such big variable names at least make them descriptive. Otherwise use smaller names.

Something you were not doing is setting handle variables to 0 before calling the Enum.

I changed the max (tempint) initial value to -1 instead of 0. In case everyplayer in the group had 0 units there, the function would just halt the thread.

I changed it so it saves the player's id instead of a player variable.

The logic itself should have worked, unless the group was always empty when you used it. Or if the problem was that sometimes it didn't work correctly then I don't know what the problem was

02-04-2006, 09:06 PM

Jazradel

Hidden information:

I just tried the trigger and it always returns Player 1. I'm begining to think my Handle Vars are stuffed up somehow. Thanks for your help though.

02-04-2006, 09:09 PM	#4
Vexorian	in the code yourself posted : call ForGroupBJ( checkgroup, function CheckUnit ) I was going to say FlushHandleLocals but I didn't know if you weren't attaching any other information to the trigger.

02-04-2006, 11:19 PM

Jazradel

This is the current code. Can anyone see any problems? I added a BJDebugMsg in the CheckUnit function and nothing happened, although I know the group works.

Code:

function CheckUnit takes nothing returns nothing
    local integer i = GetPlayerId(GetOwningPlayer(GetEnumUnit()))
    set bj_randDistChance[i] = bj_randDistChance[i] + 1
endfunction

function MostUnits takes group checkgroup returns player
    local integer playerid = -1
    local integer max = 0
    local integer i = 0
    loop 
        set bj_randDistChance[i] = 0
        exitwhen i == 15
        set i = i + 1
    endloop
    call ForGroup( checkgroup,  function CheckUnit )
    set i=0
    loop
        if bj_randDistChance[i] > max then
           set max = bj_randDistChance[i]
           set playerid = i
        endif
        exitwhen i == 15
        set i = i + 1
    endloop
    if playerid == -1 then
        return null
    else
        return Player(playerid)
    endif
endfunction

02-04-2006, 11:28 PM	#6
Vexorian	again first value of max should be -1 . Add a BJDebugMsg at the beginning of the function and see if it is getting called correclty. In case it is then add another BJDebugMsg before the enum

02-05-2006, 12:24 AM	#7
Jazradel	I got it working. Thanks Vex. And max should be 0 otherwise it would return Player 1(Red) if there weren't any units in the group.