[vJASS] only for player

[Aniki]

only for player - "convenient" ONLY_FOR* block for doing stuff for only a specific player or everyone but a specific player

It simply caches the result of GetLocalPlayer() == Player(i) into hopefully well named arrays.
It tries to also work for both "player nambers" (set pn = GetPlayerId(p)) and player variables of course (set p = Player(i)).

So this in my opinion improves readability and saves some unneeded GetLocalPlayer() calls and comparisons.

Usage:

if ONLY_FOR_PN[i] then
// if ONLY_FOR_P[GetTriggerPlayer()] then
    call MultiboardDisplay(...)
    call StartSound(...)
    set tt = CreateTextTag(...)
    call CameraSetupApply(...)
    call SetCameraBounds(...)
    call ClearSelection()
    call SelectUnit(...)
    // etc.
endif

set fx_path = "Abilities\\..."
// if FOR_EVERYONE_BUT_PN[i] then
if FOR_EVERYONE_BUT_P[GetTriggerPlayer()] then
    set fx_path = ""
endif
call AddSpecialEffect(fx_path, x, y)

library OnlyForPlayer initializer only_for_player_init

globals
    player array PLAYER
    player LOCAL_PLAYER_P
    integer LOCAL_PLAYER_PN

    boolean array ONLY_FOR_PN
    boolean array FOR_EVERYONE_BUT_PN
endglobals

struct ONLY_FOR_P extends array
    static method operator[] takes player p returns boolean
        return ONLY_FOR_PN[GetPlayerId(p)]
    endmethod
endstruct

struct FOR_EVERYONE_BUT_P extends array
    static method operator[] takes player p returns boolean
        return FOR_EVERYONE_BUT_PN[GetPlayerId(p)]
    endmethod
endstruct

function only_for_player_init takes nothing returns nothing
    local integer i

    set i = 0
    loop
        exitwhen i >= bj_MAX_PLAYERS

        set PLAYER[i] = Player(i)
        set ONLY_FOR_PN[i] = GetLocalPlayer() == PLAYER[i]
        set FOR_EVERYONE_BUT_PN[i] = not ONLY_FOR_PN[i]

        set i = i + 1
    endloop

    set LOCAL_PLAYER_P = GetLocalPlayer()
    set LOCAL_PLAYER_PN = GetPlayerId(LOCAL_PLAYER_P)
endfunction

endlibrary

 

[Bribe]

The naming conventions are displeasurable. The only thing that one may want to be cached is GetLocalPlayer(), though not for speed issues but so it can be accessed by GUI.

I recommend allowing this to be graveyarded. While it may be a fun thing to have written, it is counter-intuitive, probably doesn't have better performance and is an overall unnecessary resource.

 

[BPower]

I agree with Bribe in all points.

 

[DracoL1ch]

in fact I doubt there are any difference which you can sense. literally 1 microsecond vs 1 microsecond. So it's just matter of writing.

 

[Aniki]

In my opinion the difference between:

if GetLocalPlayer() == GetTriggerPlayer() then
    ...
endif

and

if ONLY_FOR_P[GetTriggerPlayer()] then
    ...
endif

is exactly the same as calling the "swap" version of a native function:

function GetUnitAbilityLevelSwapped takes integer abilcode, unit whichUnit returns integer
    return GetUnitAbilityLevel(whichUnit, abilcode)
endfunction

call GetUnitAbilityLevelSwapped('A000', hero)

instead of the native function directly

call GetUnitAbilityLevel(hero, 'A000')

That is, why should you make the extra unneeded function call (in the 1st case it's the call to GetLocalPlayer() [and the == operator], in the 2nd it's the swap function itself: GetUnitAbilityLevelSwapped), when you just don't have to.

 

[looking_for_help]

In my opinion the difference between:

if GetLocalPlayer() == GetTriggerPlayer() then
    ...
endif

and

if ONLY_FOR_P[GetTriggerPlayer()] then
    ...
endif

That is, why should you make the extra unneeded function call (in the 1st case it's the call to GetLocalPlayer() [and the == operator], in the 2nd it's the swap function itself: GetUnitAbilityLevelSwapped), when you just don't have to.

I disagree. In the first if, you have three things evaluated: GetLocalPlayer, GetTriggerPlayer and operator==.

In the second if, you have GetTriggerPlayer, GetPlayerId and operator[] evaluated.

So in both cases you have 2 functions and 1 operator evaluated. GetPlayerId might be even slightly slower than GetLocalPlayer due to taking one parameter (in comparsion to no parameter). Those are micro-optimizations that just obfuscate the code (and I really doubt it has any measurable impact).

So I would expect this method even being slower than the standard version.

 

[Aniki]

In the second if, you have GetTriggerPlayer , GetPlayerId and operator[] evaluated.

That's true, but only for debug mode.

In non-debug mode/release:

if ONLY_FOR_P[GetTriggerPlayer()] then

// would be translated into something like this:

if (ONLY_FOR_PN[GetPlayerId((GetTriggerPlayer()))]) then // INLINED!!

// and of course if "GetPlayerId(GetTriggerPlayer())" was already computed/known it simplifies again

if ONLY_FOR_PN[pn] then

Those are micro-optimizations that just obfuscate the code

It's meant to be exactly the opposit actually, i.e a code readability improvement, the
speed part is just a nice "bonus".

 

[edo494]

that still has to evaluate GetPlayerId(), GetTriggerPlayer() and operator[]. The player variable optimization can be applied to the native too, while arguably a little bit slower, kind of hints the intention a little bit more than ONLY_FOR_PN, since ONLY_FOR_PN could potentially be renamed to anything, but GetLocalPlayer is always going to be GetLocalPlayer.

 

[Aniki]

operator[] is:

function s__ONLY_FOR_P__staticgetindex takes player p returns boolean
    return ONLY_FOR_PN[GetPlayerId(p)]
endfunction

this expression (release mode):
ONLY_FOR_PN[ GetPlayerId(GetTriggerPlayer()) ]
doesn't use operator[] (s__ONLY_FOR_P__staticgetindex)

The player variable optimization can be applied to the native too

How?

ONLY_FOR_PN could potentially be renamed to anything

Sure, if the mapper wants a another name, it's his call (he has the ultimate authority),
he could rename it to whatever he wants.

 

[edo494]

well you can cache the right-side of the == operator from function call to variable, even tho it doesnt make much sense from your example, but if its more complex expression, it would.

The fact that the operator[] compiles to function makes your version even slower than the native, because now you have 3 function calls fighting against 2 function calls and boolean comparision, which I will put 10€ bet is faster than function call, not even native.

 

[looking_for_help]

That's true, but only for debug mode.

In non-debug mode/release:

if ONLY_FOR_P[GetTriggerPlayer()] then

// would be translated into something like this:

if (ONLY_FOR_PN[GetPlayerId((GetTriggerPlayer()))]) then // INLINED!!

No, you posted it yourself. In your inlined version you have exactly three things evaluated: GetTriggerPlayer, GetPlayerId and native operator[] (in comparison to native operator==). Thats exactly the same amount as with the standard approach.

How?

By caching the local player variable? I.e.

if myLocalPlayer == GetTriggerPlayer() then

// or by using an array for all players like you did:

if Players[2] == GetTriggerPlayer() then

It's meant to be exactly the opposit actually, i.e a code readability improvement, the
speed part is just a nice "bonus".

I highly disagree. You are using the name ONLY_FOR_P. The term "for" in a programming context refers to some kind of loop. So I guess most people would expect some kind of iteration behind this keyword.

This library could be simplified like this:

library PlayersLib
    struct Players extends array
         private static integer array playerArray
         static method operator[] takes integer playerIndex returns player
             return playerArray[playerIndex]
         endmethod
         // initialization
    endstruct
endlibrary

and being used like shown above. However, just storing a few values in an array and provide a getter for those is not enough for a system.

 

[Bribe]

Not to mention Nestharus already made a player array system and ascertained the speed benefit was almost nonexistent.

 

[Aniki]

No, you posted it yourself. In your inlined version you have exactly three things evaluated: GetTriggerPlayer , GetPlayerId and native operator[] (in comparison to native operator== ). Thats exactly the same amount as with the standard approach.

if (ONLY_FOR_PN[GetPlayerId((GetTriggerPlayer()))]) then // INLINED!!

That's not my "inlined version" it's what jasshelper would output in release mode (when you release a map, not when you are developing it) for the following:

if ONLY_FOR_P[GetTriggerPlayer()] then

which is just syntactic sugar for:

if s__ONLY_FOR_P__staticgetindex(GetTriggerPlayer())

which in release mode would become:

if (ONLY_FOR_PN[ GetPlayerId((GetTriggerPlayer()))] ) then // INLINED!!

and there is no operator[] (s__ONLY_FOR_P__staticgetindex) here.

ONLY_FOR_PN[ ... ] is an array lookup.

 

[looking_for_help]

Please re-read what I posted, I don't want to repeat everything again.

I know this is the inlined version from the release mode compilation.

In release mode you have two natives called and one native operator (which is in the first case an array lookup and in the second case an integer equal operator). Which is the same amount for the standard approach.

 

[Aniki]

In release mode you have two natives called and one native operator (which is in the first case an array lookup and in the second case an integer equal operator). Which is the same amount for the standard approach.

So you are saying that array lookups are somewhat similar to the binary operator == (integer comparison)... Okay, I guess an array lookup can be though as an operator, an unary operator.

As DracoL1ch stated eariler:

in fact I doubt there are any difference which you can sense. literally 1 microsecond vs 1 microsecond. So it's just matter of writing.

It is only a matter of writing, but your suggestion:

The term "for" in a programming context refers to some kind of loop. So I guess most people would expect some kind of iteration behind this keyword.

is perplexing to me.

I don't see how:

if ONLY_FOR_PN[pn] then
endif

can be confused as a loop, I guess if there was no "if ONLY_", it might be.

 

[BPower]

I love the JASS code discussions, they broaden one's mind.
No seriously it's always a very high quality conversation.
And you can learn a lot about coding and the way you should
re-consider what you actually coded.

For the end user it never pays off though.
I vote for graveyarding. It's a code snippet as useless as a snippet can be.

Edit:
Honestly an useful snippet for player handles should store all playing
players in a doubly linked list, eventually also create a force based lists.
Could be extended to ease alliance settings for a single players or entire player list.
Could fire some related custom events.
Meanwhile you can allow playerArray[this] for needless micro optimization.
udg_LocalDude = GetLocalPlayer() is cool because GUI would require custom script: in some cases.

 

[looking_for_help]

So you are saying that array lookups are somewhat similar to the binary operator == (integer comparison)...

Sure, both are operators.

I personally wouldn't have included those into a comparsion, but you mentioned the operator== in your second post, so the intention was to show that you safe nothing with this method, not even operators.

It is only a matter of writing, but your suggestion:

is perplexing to me.

I don't see how:

if ONLY_FOR_PN[pn] then
endif

can be confused as a loop, I guess if there was no "if ONLY_", it might be.

Without looking at the implementation, I don't understand what ONLY_FOR_PN should mean at all. What is "PN"? Why "ONLY"?

The thing is you have an object here (the array, respectivly the static class wrapping around it) but you name it like an action (== a function). This is confusing. From something named ONLY_FOR_PN I would expect it performs some action on some object. It sounds like "iterate over all players, but only those who fullfill the condition I pass". Like some FOR_EACH macros out there in other programming languages. On the other hand it is used like an object (using the operator[] instead of the operator()), so that looks conflicting.

Having something like

if Players[2] == GetTriggerPlayer() then
// or maybe
if Players.player2 == GetTriggerPlayer() then

is just much easier to read (which doesn't mean I would use such a thing).

 

[Wietlol]

This reminds me of a small feature of my (too) big ass library.

    function BF_INIT_GlobalVariables takes nothing returns nothing
        set LOCAL_PLAYER = GetLocalPlayer()
        set LOCAL_PLAYER_ID = GetPlayerId(LOCAL_PLAYER)
        set IS_LOCAL_PLAYER[LOCAL_PLAYER_ID] = true
    endfunction

Basically,
GetLocalPlayer() put in LOCAL_PLAYER
GetPlayerId(GetLocalPlayer()) put in LOCAL_PLAYER_ID
(GetPlayerId(GetLocalPlayer()) == x) put in IS_LOCAL_PLAYER[x]

 

[edo494]

I vote for graveyarding, this is just way too simple and inlinable to any resource within 2 minutes

 

[BPower]

I vote for graveyarding, this is just way too simple and inlinable to any resource within 2 minutes

Yes, I share edo's opinon.

 

[Bribe]

Likewise. It is conceptually interesting, but the potential isn't there. Graveyarded.

 

[Troll-Brain]

Then maybe this one also should be graveyarded :
http://www.hiveworkshop.com/forums/jass-resources-412/snippet-localhelper-215108/