[vJASS] Willl This Cause A Desync?

[Mr_Bean]

Hello,

I would like to know whether the following snippet would cause a desync:

private static method checkCast takes nothing returns boolean
    local player owner = GetTriggerPlayer()
    
    if GetTriggerEventId() == EVENT_PLAYER_UNIT_SPELL_CHANNEL then
        if IsPlayerEnemy(GetLocalPlayer(), owner) or (IsPlayerAlly(GetLocalPlayer(), owner) and not GetPlayerAlliance(owner, GetLocalPlayer(), ALLIANCE_SHARED_VISION)) then
            call UnitShareVision(GetTriggerUnit(), GetLocalPlayer(), true)
        endif
    elseif GetTriggerEventId() == EVENT_PLAYER_UNIT_SPELL_ENDCAST then
        if IsPlayerEnemy(GetLocalPlayer(), owner) or (IsPlayerAlly(GetLocalPlayer(), owner) and not GetPlayerAlliance(owner, GetLocalPlayer(), ALLIANCE_SHARED_VISION)) then
            call UnitShareVision(GetTriggerUnit(), GetLocalPlayer(), false)
        endif
    endif
    
    set owner = null
    return false
endmethod

It is meant to share the caster's visibility with all players when it begins casting and reset the vision back to normal if it cancels the spell or finishes it. It works fine, but I want to be sure it won't desync.

Also, would it be better to store GetLocalPlayer() in a local variable or not?

Thanks

Mr_Bean

 

[Troll-Brain]

Yes it should, because the action will be performed only for some players' computers and not for the others, which means on the computer X the vision will be shared for the player A but on the computer Y the player A will be considered to have not this shared vision, so it's likely to desync in some cases, like an attack.

Since you call GetLocalPlayer() several times it is better to use a variable, not only for "efficiency" but more important for readability.
Note that instead of a local variable you could use a global one, it's even better.

Here to fix the possible desync, instead of using GetLocalPlayer, use a loop for all the possible human players.

 

[Mr_Bean]

Cool, thanks. Is this better:

private static method shareVisibility takes nothing returns nothing
    local player enumP = GetEnumPlayer()
    
    if GetPlayerSlotState(enumP) == PLAYER_SLOT_STATE_PLAYING and GetPlayerController(enumP) == MAP_CONTROL_USER then
        if not GetPlayerAlliance(source, enumP, ALLIANCE_SHARED_VISION) then
            call UnitShareVision(enumU, enumP, share)
        endif
    endif
    
    set enumP = null
endmethod

private static method checkCast takes nothing returns boolean
    if GetSpellAbilityId() == SPELL_ID then
        
        set source = GetTriggerPlayer()
    
        if GetTriggerEventId() == EVENT_PLAYER_UNIT_SPELL_CHANNEL then
            set share = true
        elseif GetTriggerEventId() == EVENT_PLAYER_UNIT_SPELL_ENDCAST then
            set share = false
        endif
        
        call ForceEnumPlayers(enumF, null)
        set enumU = GetTriggerUnit()
        call ForForce(enumF, function thistype.shareVisibility)
        call ForceClear(enumF)
        
    endif
    
    return false
endmethod

source, enumF and enumU are globals.

 

[Troll-Brain]

It's better in the sense that all is synced, so no desync possible.
Also i suppose ForceEnum... work as GroupEnum, i mean first it clears the group and then eventually add players, so ForceClear shouldn't be necessary here.

 

[Mr_Bean]

Is that how GroupEnum works? :/
I always thought that GroupEnum adds units to the group ... you learn something every day!

Thanks man

 

[Troll-Brain]

I'm 100 % sure that's how GroupEnum works yes.