I honestly cant get my around this problem! (simplest of loops)

Hey. I have a loop which is malfunctioning bigtime, even though it really is the simplest thing ever:


Code:

function Trig_Init_Func1 takes nothing returns nothing
    call RemoveUnit( GetEnumUnit() )
endfunction

function Trig_Init_Actions takes nothing returns nothing
    local integer i = 1
    loop
        exitwhen i > 4
            if (GetPlayerSlotState(ConvertedPlayer(i)) == PLAYER_SLOT_STATE_PLAYING ) then
            call SetPlayerStateBJ( ConvertedPlayer(i), PLAYER_STATE_RESOURCE_GOLD, 50 )
            set i = i + 1
        else
            call ForGroupBJ( GetUnitsOfPlayerAll(Player(i)), function Trig_Init_Func1 )
            set i = i + 1
        endif
    endloop
endfunction

//===========================================================================
function InitTrig_Init takes nothing returns nothing
    set gg_trg_Init = CreateTrigger(  )
    call TriggerAddAction( gg_trg_Init, function Trig_Init_Actions )
endfunction

As you may see, I want to remove all units of any player that is no playing;
Code:

            call ForGroupBJ( GetUnitsOfPlayerAll(Player(i)), function Trig_Init_Func1 )

It worked fine before, but I made a few changes here and there, and now it does not work anymore! One or two players whose units shouldn't be there are there anyways. What is wrong?|||The Player function should take the player id, not the converted player id. In other words, replace this:


Code:

call ForGroupBJ( GetUnitsOfPlayerAll(Player(i)), function Trig_Init_Func1 )

...with this:


Code:

call ForGroupBJ( GetUnitsOfPlayerAll(Player(i - 1)), function Trig_Init_Func1 )

Also, if you're going to use ForGroupBJ instead of the more efficient ForGroup, you might as well take advantage of its added functionality by setting bj_wantDestroyGroup to true. Your code currently leaks a unit group for every absent player (a minor leak, yes, but still...). The code you want is:


Code:

function Trig_Init_Func1 takes nothing returns nothing
    call RemoveUnit( GetEnumUnit() )
endfunction

function Trig_Init_Actions takes nothing returns nothing
    local integer i = 1
    loop
        exitwhen i > 4
        if (GetPlayerSlotState(ConvertedPlayer(i)) == PLAYER_SLOT_STATE_PLAYING ) then
            call SetPlayerStateBJ( ConvertedPlayer(i), PLAYER_STATE_RESOURCE_GOLD, 50 )
        else
            set bj_wantDestroyGroup = true
            call ForGroupBJ( GetUnitsOfPlayerAll(Player(i - 1)), function Trig_Init_Func1 )
        endif
        set i = i + 1
    endloop
endfunction

//===========================================================================
function InitTrig_Init takes nothing returns nothing
    set gg_trg_Init = CreateTrigger(  )
    call TriggerAddAction( gg_trg_Init, function Trig_Init_Actions )
endfunction

(I moved set i = i + 1 out of the if statement for slightly neater code, put it back in if you want)|||hey thanks for the super quick reply!

So if I got this right, all that was wrong (besides the leaks) was that i didnt use "i -1" instead of "i" when I was to remove the units?

On another note; what makes a BJ function less efficient than a non-BJ one? And what does BJ mean? And one last thing: In what case would anyone want to use a BJ over a non-BJ? I've heard a lot of smacktalk about BJ's :p

edit: tested it, and it works beatifully. Thanks!|||BJs are functions that are defined in the blizzard.j file.

It's not so much that BJs are slower than non-BJs, but rather that natives are faster than functions, and all BJs are functions (but not all functions are BJs). All natives are hardcoded in C++, while functions are written in JASS. C++ is much faster than JASS, which is the main reason that functions are slower than natives.

Take the ForGroupBJ function:


Code:

function ForGroupBJ takes group whichGroup, code callback returns nothing
    local boolean wantDestroy = bj_wantDestroyGroup
    set bj_wantDestroyGroup = false

    call ForGroup(whichGroup, callback)

    if (wantDestroy) then
        call DestroyGroup(whichGroup)
    endif
endfunction

(Edited out comments)

As you may be able to see, all functions are reduceable to natives, conditionals and/or assignments. Even if C++ wasn't faster than JASS, functions would still be slower than natives (although the difference would be marginal), because of this simple fact - you're adding extra calls in to the mix. Take GetKillingUnitBJ as an example:


Code:

function GetKillingUnitBJ takes nothing returns unit
    return GetKillingUnit()
endfunction

So, if you were to put 'local unit u = GetKillingUnitBJ()', the interpreter would call GetKillingUnitBJ, THEN call GetKillingUnit, then return the value from that to the local unit 'u'. Simply putting GetKillingUnit would be faster, as it cuts a function call from the sequence.

I try to use natives instead of functions (which include, but are not limited to the BJs) whenever possible, but there are some BJs/functions that are convenient (as opposed to GetKillingUnitBJ and similiar BJs that are useless, as they only call a native). A short list that come to mind are:

ModuloReal

ModuloInteger

Many of the multiboard BJs

TriggerRegisterAnyUnitEventBJ (in SOME cases)

CinematicModeExBJ

CinematicFadeBJ

Bleh, that's enough, but there are more. For the most part, though, BJs (and other functions) should be avoided if efficiency is desired. Keep in mind that even the above listed functions are inefficient, but are nonetheless sometimes worth using because they save time JASSing.|||Quote:








what makes a BJ function less efficient than a non-BJ one? And what does BJ mean? And one last thing: In what case would anyone want to use a BJ over a non-BJ? I've heard a lot of smacktalk about BJ's




The BJ functions call the natives (non-BJ functions) that do the same thing. The reason that most of them exist is that they rearrange the order of the arguments into what Blizzard feels is "a more natural order" in the GUI functions that read from them. The "smacktalk" primarily comes from snobby JASS coders who see no purpose in a function that only serves to rearrange arguments for GUI (since they don't use GUI).

The efficiency savings is kind of a load of crap, since computers today aren't going to be taxed by the difference between one function calling a native and calling a native directly. It's one extra (useless) step, but your average cell phone wouldn't notice the "efficiency difference", let alone a modern computer.|||Quote:








The BJ functions call the natives (non-BJ functions) that do the same thing. The reason that most of them exist is that they rearrange the order of the arguments into what Blizzard feels is "a more natural order" in the GUI functions that read from them. The "smacktalk" primarily comes from snobby JASS coders who see no purpose in a function that only serves to rearrange arguments for GUI (since they don't use GUI).

The efficiency savings is kind of a load of crap, since computers today aren't going to be taxed by the difference between one function calling a native and calling a native directly. It's one extra (useless) step, but your average cell phone wouldn't notice the "efficiency difference", let alone a modern computer.




Firstly, I do use GUI, although I admit that I use JASS more.

Secondly, there are plenty of BJ functions that don't rearrange the arguments at all - they do nothing but call the appropriate native. This is most noteable in the GetXXXXUnitBJ functions, but there are plenty of others too.

Thirdly, I agree with you that in a single script that is not run frequently, the efficiency gains are negligible. However, I can say from experience that the losses in efficency DO stack up if you have enough triggers, or loops on a short timer, or commonly executed triggers that leak (especially if that leak is in the Conditions function). I will say, though, that the efficiency gains that really make a difference are not those that come from inlining functions that call a single native (the example you used). No JASSer would change a single function to a native and expect a noticeable change. But they do stack up.|||Quote:








Firstly, I do use GUI, although I admit that I use JASS more.




My comments regarding snobs and/or JASS users was not directed at you or anyone in particular - more the culture of JASS users who feel compelled to slight people who don't exclusively work in JASS out of some perceived sense that better coding = better map-making.


Quote:








Secondly, there are plenty of BJ functions that don't rearrange the arguments at all - they do nothing but call the appropriate native. This is most noteable in the GetXXXXUnitBJ functions, but there are plenty of others too.




Read carefully, I said most - and how I described them is their ultimate purpose. It is the primary reason why Blizzard created them. The fact that there are also bj functions that don't rearrange the arguments probably just says something about the laziness of Blizzard.


Quote:








No JASSer would change a single function to a native and expect a noticeable change. But they do stack up.




The efficiency savings for something like bj vs. native is so small that you could only notice the difference in terms of microseconds if you absolutely slamming that function constantly and had dozens of functions doing so - and I can't really think of any reasonable map design that would call for such a setting... people who code notice the slight difference between these functions (my argument never included anything but bj functions vs. natives) by mass looping and running performance tests - not from regular/standard usage.|||hey, sorry for taking long to respond. I just got a cat and it takes up most of my time. Just wanted to say thanks for your replies :P