[vJASS] Group

[Darkdread]

function GroupRandomUnitPick takes group g returns unit
    local integer i=0
    if tg == null then
        set tg=CreateGroup()
    endif
    loop
        set temp[i]=FirstOfGroup(g)
        exitwhen temp[i]==null
        call BJDebugMsg(GetUnitName(temp[i]))
        call GroupAddUnit(tg, temp[i])
        call GroupRemoveUnit(g, temp[i])
        set i=i+1
    endloop
    set tempg = g       //nothing
    set g = tg          //something
    set tg = tempg      //nothing
    return temp[GetRandomInt(0,i-1)]
endfunction

The units in group g became non-existent after one call?

NOTE: temp is a unit array and tempg is a group variable

 

[Ceday]

I don't understand what is the difference between tg and tempg.

Your "set g = tempg" or "set g = tg" trick won't work because when you set g = tempg, anything that effects tempg also effects g.

Using the Blizzard function for picking a random unit is currently better than this method also.

function GroupRandomUnitPick takes group g returns unit
    local integer i=0
    local integer random
    if tempg == null then
        set tempg=CreateGroup()
    else
        call GroupClear(tempg)
    endif
    
    loop
        set temp[i]=FirstOfGroup(g)
        exitwhen temp[i]==null
        call BJDebugMsg(GetUnitName(temp[i]))
        call GroupAddUnit(tempg, temp[i])
        call GroupRemoveUnit(g, temp[i])
        set i=i+1
    endloop


    set random = i
    // You need to add all units to the original group one by one again
    loop
        set i = i - 1
        call GroupAddUnit(g, temp[i])
        exitwhen i == 0
    endloop
    
    return temp[GetRandomInt(0,random-1)]
endfunction

 

[Darkdread]

I read the swap tutorial here http://www.hiveworkshop.com/forums/...on-using-first-group-loop-enumeration-223140/ so I tried to use it lol

 

[PurgeandFire]

First of Loop group enumerations are best when you don't need the units in the group afterward. If you need to preserve units, I would just use a ForGroup(). See:
http://www.hiveworkshop.com/forums/2380111-post5.html

In some cases, you can use a First of group and readd the units to a copy group, but I would only do that if you have a lot of globals to pass where it would be inconvenient to use ForGroup(). In your case, ForGroup() is just easier (see that post).

As for your code, the loop first runs and removes all the units from "g" and puts them in "tg". tempg is made to point to "g", so tempg actually just contains the empty group. g is assigned to the full group tg, so now "g" is restored to its original container. Then tg is sent to the empty group. In the end:
tempg -> empty group
g -> filled group
tg -> empty group

g is a local variable in that function, so there isn't really a point in assigning it to the filled group. I assume you want "tempg" to point to the filled group? If so, just use set tempg = tg and then set tg = null. But your GroupPickRandomUnit empties the original group, so you should be wary of that.

 

[Darkdread]

^ I thought if I called this function my group wouldn't be emptied because I set the variable "g" back to filled. This function doesn't actually re-fill the group I use to call? I assume it does not re-fill from your comment, thanks for your time! Fancy meeting u here too lol

 

[PurgeandFire]

It doesn't refill.

When you are assigning things to a non-primitive (anything that isn't a string, integer, real, boolean, or 'code' type), you are just moving a pointer to "point" to the object in memory. Here is a quick example:

local unit A = GetTriggerUnit()
local unit B = A
call KillUnit(A)

What do you think happens here?

When you use variables on non-primitives, they just act as pointers. So by assigning B to A, you are just making it so that both "A" and "B" "point" to the triggering unit. But if you kill the unit pointed to by A, it means that the unit pointed to by B.

This is kinda how it would look like symbolically:

A ----> [ Triggering Unit ]
B ----------^

After it is killed:

A ----> [ Killed triggering unit ]
B --------------^

Similarly, just switching a group to point to a filled group won't fill the original group. You have to keep track of the objects you created, and keep in mind what points to what.

 

[Dr Super Good]

The only problem with the original implementation is that the group is passed by value (handle index) rather than reference to value (pointer to handle index). As such you perform a meaningless swap with a local variable (from the parameter) rather than with the variable that was used as the argument.

Since JASS does not support handle references and also does not support bulk group copying you have no choice but to either use a very slow "unwind" loop to place everything back into the original group before function return or use a "ForGroup" loop.