Scope question

[WereElf]

Alright, I have a trigger:

scope SEF

    globals
        boolean array SB
        boolean array EB
        boolean array FB
        unit array SU
        unit array EU
        unit array FU
        unit array SEF_C
        real array SEF_X
        real array SEF_Y
        timer array SEF_T
    endglobals

    function SEF_Init takes nothing returns boolean
        local unit c = GetTriggerUnit()
        local unit t = GetSpellTargetUnit()
        local integer utid = GetUnitTypeId(t)
        if utid  == 'odoc' or utid == 'oshm' or utid == 'ospw' then
            
        endif
        return false
    endfunction

function Trig_TEST_Actions takes nothing returns nothing
    call UnitRemoveAbilityBJ( 'A02J', gg_unit_o000_0181 )
    call UnitAddAbilityBJ( 'A02J', gg_unit_o000_0181 )
endfunction

//==============================================================================
    function InitTrig_SEF takes nothing returns nothing
        set gg_trg_SEF = CreateTrigger(  )
        call TriggerRegisterPlayerEventEndCinematic( gg_trg_SEF, Player(0) )
        call TriggerAddCondition( gg_trg_SEF, Condition( function SEF_Init ) )
    endfunction
    
endscope

My question is: Should I keep the InitTrig inside the scope?
And... When/How do I use "initializers"?

 

[GIMLI_2]

yes, leave it inside the scope.

initializer call functions at map initialization, you want to call your InitTrig at map initialization to create the trigger and the event/actions, so do sth like that:

scope SEF initializer InitTrig_SEF

    globals
        boolean array SB
        boolean array EB
        boolean array FB
        unit array SU
        unit array EU
        unit array FU
        unit array SEF_C
        real array SEF_X
        real array SEF_Y
        timer array SEF_T
    endglobals

    function SEF_Init takes nothing returns boolean
        local unit c = GetTriggerUnit()
        local unit t = GetSpellTargetUnit()
        local integer utid = GetUnitTypeId(t)
        if utid  == 'odoc' or utid == 'oshm' or utid == 'ospw' then
            
        endif
        return false
    endfunction

function Trig_TEST_Actions takes nothing returns nothing
    call UnitRemoveAbilityBJ( 'A02J', gg_unit_o000_0181 )
    call UnitAddAbilityBJ( 'A02J', gg_unit_o000_0181 )
endfunction

//==============================================================================
    function InitTrig_SEF takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerRegisterPlayerEventEndCinematic( t, Player(0) )
        call TriggerAddCondition( t, Condition( function SEF_Init ) )
        set t = null
    endfunction
    
endscope

 

[WereElf]

Alright, thanks
I'll ask again when I have more vJASS questions.

But could you give me some basic vJASS tips?
So far I've only used libraries, globals declarations, and textmacros. This will be my 1-st scope. I know about structs. Still haven't needed to use any.
So.. anything else, that's pretty useful in vJASS?

EDIT: And.. would declaring them like that:

        boolean array SB = true
        boolean array EB = true
        boolean array FB = true

set only SB[0], EB[0] and FB[0] to true?

 

[GIMLI_2]

I'm a beginner as you are

EDIT: And.. would declaring them like that:
Jass:
boolean array SB = true
boolean array EB = true
boolean array FB = true
set only SB[0], EB[0] and FB[0] to true?

i dont think that works. create a global boolean array in the globals block and set them in the initialization function separatly

 

[EdgeOfChaos]

I'm also somewhat a beginner with vJASS, but I do know how to program other languages and am in school for software engineering. I can give some general coding tips:

- Keep scope as low as possible. Everything should be private unless there is a good reason to have it otherwise.

- In the globals section of the code, write named constants in. For example:

private constant integer MY_ABILITY = 'A000'
private constant real RANGE = 500.
private constant real DAMAGE = 120.

Not only does this help in readability, but if everything is a constant, you can change how the ability works without even going into the code.
For example, these lines:

    if utid  == 'odoc' or utid == 'oshm' or utid == 'ospw' then
    call UnitRemoveAbilityBJ( 'A02J', gg_unit_o000_0181 )
    call UnitAddAbilityBJ( 'A02J', gg_unit_o000_0181 )

could do with named constants replacing nonsense identifiers like 'A02J'.

- Your code should be self-revealing. As in, good names for variables and functions. Variables like u, x, y, i are sometimes OK in locals or loops, but for global variables, things like SB, FB, EB, should never happen. Same goes for struct name - SEF is not a good name. Remember, code readability is at least as important as functionality.

- Specific to VJASS: null your units. For example in your init trigger, you should do

set c = null
set t = null

at the end of the trigger. If you don't, unit pointers leak because blizzard's coding language is shitty. Only applies to locals.

- If you ever need to have a boolean array ALL initialized to true, it may mean that you're doing some bad coding. Since there are only 2 values, you can let them all become false and then just reverse all your identifiers to use false as their normal value instead of true. And rename the variable accordingly.

 

[WereElf]

Well, I kind a posted the trigger while I was still getting started.
Now it looks more like this (still only about 50% done):

scope StormEarthFire //initializer InitTrig_SEF

    globals
        boolean array SB // Storm boolean - a storm unit has been absorbed
        boolean array EB // Earth boolean - an earth unit has been absorbed
        boolean array FB // Fire boolean - a fire unit has been absorbed
        unit array SU // Storm unit
        unit array EU // Earth unit
        unit array FU // Fire unit
        unit array SEF_U // Caster
        real array SEF_X // X of cast point
        real array SEF_Y // Y of cast point
        timer array SEF_T // Cooldown timers
        integer array SEF_C // Storm Earth and Fire count - counts how many units have been absorbed
        integer array TC // time count
        integer array Current_Type
        effect array SEF_E
    endglobals
    
    private function Fade takes nothing returns nothing
        local timer t = GetExpiredTimer()
        local integer id = GetHandleId(t)
        local unit c = LoadUnitHandle(udg_Table, id, 'unit')
        local unit u = LoadUnitHandle(udg_Table, id, 'targ')
        local real dist = LoadReal(udg_Table, id, 'dist')
        local real x = GetUnitX(u)
        local real y = GetUnitY(u)
        local real xc = GetUnitX(c)
        local real yc = GetUnitY(c)
        local real h = y-yc
        local real w = x-xc
        local integer i = GetPlayerId(GetOwningPlayer(u))
        if IsUnitAliveBJ( u ) then
            if h*h+w*w <= 10000.00 then
                call RecTimer(t)
                call ShowUnit( c , false )
                call UnitRemoveAbility( u , 'A02J' )
                if Current_Type[i] == 1 then
                    call UnitAddAbility( u , 'A02L' )
                elseif Current_Type[i] == 2 then
                    call UnitAddAbility( u , 'A02K' )
                elseif Current_Type[i] == 3 then
                    call UnitAddAbility( u , 'A02M' )
                endif
                set SEF_C[i] = SEF_C[i] + 1
                set Current_Type[i] = 0
                if SEF_C[i] < 3 then
                    call UnitAddAbility( u , 'A02J' )
                else
                    call UnitAddAbility( u , 'A02N' )
                endif
            else
                call SetUnitVertexColor(c, 255, 255, 255, (255*R2I(h*h+w*w)/250000)-10)
            endif
        else
            call RecTimer(t)
        endif
        set t = null
        set c = null
        set u = null
    endfunction
    
    private function PeriodicCheck takes nothing returns nothing
        local timer t = GetExpiredTimer()
        local integer id = GetHandleId(t)
        local unit c = LoadUnitHandle(udg_Table, id, 'cast')
        local unit u = LoadUnitHandle(udg_Table, id, 'targ')
        local integer i = GetPlayerId(GetOwningPlayer(c))
        set TC[i] = TC[i] + 1
        if IsUnitDeadBJ(c) or IsUnitDeadBJ(u) then
            set TC[i] = 0
            call DestroyEffect(SEF_E[i])
            call RecTimer(t)
            if Current_Type[i] == 1 then
                set SU[i] = null
                set SB[i] = false
            elseif Current_Type[i] == 2 then
                set EU[i] = null
                set EB[i] = false
            elseif Current_Type[i] == 3 then
                set FU[i] = null
                set FB[i] = false
            endif
            set Current_Type[i] = 0
        elseif TC[i] >= 20 then
            call DestroyEffect(SEF_E[i])
            set TC[i] = 0
            call SetUnitInvulnerable( u , true )
            call RecTimer(t)
            call SlideToUnit( u , c , 400.00 , function Fade)
        endif
        set t = null
        set c = null
        set u = null
    endfunction
    
    function SEF_Init takes unit u, unit c returns nothing
        local timer t = GetFreeTimer()
        local real x = GetUnitX(u)
        local real y = GetUnitY(u)
        local real xc = GetUnitX(c)
        local real yc = GetUnitY(c)
        local integer id = GetHandleId(t)
        set SEF_E[GetPlayerId(GetOwningPlayer(c))] = AddSpecialEffectTarget( "Abilities\\Spells\\Items\\StaffOfSanctuary\\Staff_Sanctuary_Target.mdl", u, "chest")
        call SaveUnitHandle(udg_Table, id, 'cast', c)
        call SaveUnitHandle(udg_Table, id, 'targ', u)
        call SetUnitFacing( u, bj_RADTODEG*Atan2(yc-y,xc-x))
        call PauseUnit( u , true )
        call TimerStart( t, 0.10, true, function PeriodicCheck)
    endfunction
    
    function SEF_Cast takes integer i returns nothing
    
    endfunction

    function SEF_Check takes nothing returns boolean
        local unit c = GetTriggerUnit()
        local unit t = GetSpellTargetUnit()
        local integer utid = GetUnitTypeId(t)
        local player p = GetOwningPlayer(c)
        local integer i = GetPlayerId(p)
        if GetSpellAbilityId() == 'A02J' then
            if utid  == 'odoc' then
                if not FB[i] then
                    set SEF_U[i] = c
                    set FU[i] = t
                    set FB[i] = true
                    set Current_Type[i] = 3
                    call SEF_Init(t, c)
                else
                    call DisplayTextToPlayer(p, 0, 0, "You already have a chosen caster for the Fire part of the spell.")
                endif
            elseif utid == 'oshm' then
                if not SB[i] then
                    set SEF_U[i] = c
                    set SU[i] = t
                    set SB[i] = true
                    set Current_Type[i] = 1
                    call SEF_Init(t, c)
                else
                    call DisplayTextToPlayer(p, 0, 0, "You already have a chosen caster for the Storm part of the spell.")
                endif
            elseif utid == 'ospw' then
                if not EB[i] then
                    set SEF_U[i] = c
                    set EU[i] = t
                    set EB[i] = true
                    set Current_Type[i] = 2
                    call SEF_Init(t, c)
                else
                    call DisplayTextToPlayer(p, 0, 0, "You already have a chosen caster for the Earth part of the spell.")
                endif
            else
                call UnitRemoveAbility( c , 'A02J' )
                call UnitAddAbility( c , 'A02J' )
                call DisplayTextToPlayer(p, 0, 0, "You need to target a Shaman, Witch Doctor or a Spirit Walker with this spell")
            endif
        elseif GetSpellAbilityId() == 'A02N' then
            set SEF_X[i] = GetSpellTargetX()
            set SEF_Y[i] = GetSpellTargetY()
            call SEF_Cast(i)
        endif
        set c = null
        set t = null
        set p = null
        return false
    endfunction
    
    private function OnDeath takes nothing returns boolean
        local unit u = GetTriggerUnit()
        local integer i = GetPlayerId(GetOwningPlayer(u))
        if u == SEF_U[i] and SEF_C[i] > 0 then
            if Current_Type[i] == 1 then
                call SetUnitInvulnerable( SU[i] , false )
                call SetUnitVertexColor(SU[i], 255, 255, 255, 255)
                call PauseUnit( SU[i], false)
                set SB[i] = false
                set SU[i] = null
            elseif Current_Type[i] == 2 then
                call SetUnitInvulnerable( EU[i] , false )
                call SetUnitVertexColor( EU[i], 255, 255, 255, 255)
                call PauseUnit( EU[i], false)
                set EB[i] = false
                set EU[i] = null
            elseif Current_Type[i] == 3 then
                call SetUnitInvulnerable( FU[i] , false )
                call SetUnitVertexColor( FU[i], 255, 255, 255, 255)
                call PauseUnit( FU[i], false)
                set FB[i] = false
                set FU[i] = null
            endif
            if SB[i] then
                call SetUnitX( SU[i], GetUnitX(u))
                call SetUnitY( SU[i], GetUnitY(u)+75)
                call ShowUnit( SU[i], true )
                call SetUnitInvulnerable( SU[i] , false )
                call SetUnitVertexColor( SU[i], 255, 255, 255, 255)
                call PauseUnit( SU[i], false)
                set SB[i] = false
                set SU[i] = null
            endif
            if EB[i] then
                call SetUnitX( EU[i], GetUnitX(u)-64.952)
                call SetUnitY( EU[i], GetUnitY(u)-37.5)
                call ShowUnit( EU[i], true )
                call SetUnitInvulnerable( EU[i] , false )
                call SetUnitVertexColor( EU[i], 255, 255, 255, 255)
                call PauseUnit( EU[i], false)
                set EB[i] = false
                set EU[i] = null                
            endif
            if FB[i] then
                call SetUnitX( FU[i], GetUnitX(u)+64.952)
                call SetUnitY( FU[i], GetUnitY(u)-37.5)
                call ShowUnit( FU[i], true )
                call SetUnitInvulnerable( FU[i] , false )
                call SetUnitVertexColor( FU[i], 255, 255, 255, 255)
                call PauseUnit( FU[i], false)
                set FB[i] = false
                set FU[i] = null                
            endif
            set SEF_C[i] = 0
        endif
        return false
    endfunction
    
//==============================================================================
    function InitTrig_SEF takes nothing returns nothing
        local integer i = 0
        local trigger t = CreateTrigger()
        set gg_trg_SEF = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ( gg_trg_SEF, EVENT_PLAYER_UNIT_SPELL_EFFECT )
        call TriggerAddCondition( gg_trg_SEF, Condition( function SEF_Check ) )
        call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_DEATH )
        call TriggerAddCondition( t, Condition( function OnDeath ))
        loop
            exitwhen i > 11
            set SB[i] = false
            set EB[i] = false
            set FB[i] = false
            set i = i + 1
        endloop
        set t = null
    endfunction
    
endscope

But I'll take your tips into account

Now I'm wondering how to get rid of the unit's shadow while it's fading...

 

[Chaosy]

Scopes are pretty useless, they do nothing that a library does not as far as I know.

 

[WereElf]

Well, libraries go to the map's header, while scopes don't do that.
I use libraries only if other triggers need to call functions from the library. While scopes are for skills like Storm Earth and Fire. When I just need to declare some global variables for them, without using the variable editor.

I don't know if there are any other differences between libraries and scopes

 

[Chaosy]

The code going into the header got no downside, if anything it's better because you wont get the undeclared function error which can happen otherwise.

There is no benefit with coding a spell in a scope instead of a lib.

 

[WereElf]

Well, perhaps you are right. But had I made it into a library, than I would've needed to put "requires Sliding", when it's a scope - I can avoid that.
And I kind a don't see the point of trurning it into a library when it doesn't need to be one xP

 

[EdgeOfChaos]

If nothing else in the map is using those functions (they're all private), there's no need for them to be in the header with a library. Scopes can call all functions from libraries, whereas libraries cannot call other libraries functions if they are placed above them (seems like the placement of libraries in the actual code is kind of random).

Methods that are called in other places should go in the header. If a method is not called in any other place, it should go near the bottom of your code, which is what scopes do, because other functions shouldn't even know they exist. In this same way, you can technically turn all of your variables into full globals, but that's not a good thing to do. If a function is not using them, they don't need to know about them - which is why we have things like class level variables and local variables.

And @WereElf, every function and global variable in that trigger should be private (even the initializer and trigger code). And you should almost never put comments on your variables. If you have to write a comment for your variable, its name is probably bad. Some examples:

unit array SEF_U // Caster
timer array SEF_T // Cooldown timers
unit array SU // Storm unit
unit array EU // Earth unit
unit array FU // Fire unit

Would be a lot better as:

private unit array caster
private timer array cooldown
private unit array storm
private unit array earth
private unit array fire

Don't worry about this interfering with another "caster" from another trigger! That's the beauty of the private keyword - it cannot interfere with them. You can have 500 different variables all called caster, as long as they're in different scopes/libs and declared private!

 

[WereElf]

If I was making some kind of system - I would've given better names and would've made the variables private, but since this is just a skill for my map, and I'm not planing to make it a resource - I'll keep it like this.
I'm not planing to make other triggers use "SEF_" variables.
As you can see I have few private functions, but that's because they have "common" names, and I may call other functions (in other scopes/libraries) the same way.

Anyways.. can you explain "Methods"? What are they, and what are they used for?

 

[EdgeOfChaos]

Methods are like functions, except they are used inside structs. I don't think you need or can use them inside scopes/libs.

Sure, I understand you're not planning on making it a system. But let's say it doesn't work, and you come to the hive for help - people will read your code and not immediately understand what's going on. Or if you come back to this in 1 month and have to edit your code, it will take time to understand it.

I'm not planing to make other triggers use "SEF_" variables.

If this were a GUI trigger, then you would be correct in prefixing it with something related to your system because GUI has no class level variables, just full globals. In vJASS there is no need for prefixing, because the vJASS compiler takes care of it for you.

If you write "private unit array caster" or something, the compiler literally will go through and rename that variable to SEF_______caster before compiling. The only thing doing it manually does it reduce code readabilty. It has the same effect of, say, prefixing your local variables and them commenting them, which is a waste of time.

 

[Flux]

There is no benefit with coding a spell in a scope instead of a lib.

Uhmm, no, there are benefits in using scopes. If your spell is using functions from other libraries and does not have an API (list of custom functions to be used by the coder), then declaring it as a scope is better than declaring it as a library with tons of requirements.

 

[WereElf]

By the way, when I make a public variable declaration, when calling it from functions, outside the scope, do I need to use the scope's name as prefix, or do I just use the name as I've written it?

scope Test
    globals
        public trigger Fade = CreateTrigger()
    endglobals
endscope

function Something takes nothing returns nothing
    call TriggerEvaluate(Fade) /* or */ call TriggerEvaluate(Test_Fade)
endfunction

 

[Chaosy]

Uhmm, no, there are benefits in using scopes. If your spell is using functions from other libraries and does not have an API (list of custom functions to be used by the coder), then declaring it as a scope is better than declaring it as a library with tons of requirements.

I don't see how the API is related.

Using one keyword instead of another just because you don't want to write requires X gotta be the laziest excuse I have heard in a while xd
If you code a spell that requires other systems (aka 99 of all jass spells) you will just get invalid errors if someone else tries to use it.
It will yell undeclared function instead of missing lib.

 

[EdgeOfChaos]

Why is that being lazy? The scope keyword is written for that purpose. Writing unneeded code because you don't want to replace the word "library" with "scope" is very silly.

About public
Inside the same library/scope, you do not need an identifier
Outside the library/scope, you do need the identifier
Example from the VJASS Manual

    library cookiesystem
        public function ko takes nothing returns nothing
            call BJDebugMsg("a")
        endfunction

        function thisisnotpublicnorprivate takes nothing returns nothing
             call ko()
             call cookiesystem_ko() //cookiesystem_ preffix is optional
        endfunction
    endlibrary

    function outside takes nothing returns nothing
         call cookiesystem_ko() //cookiesystem_ preffix is required
    endfunction

 

[Flux]

I don't see how the API is related.

Using one keyword instead of another just because you don't want to write requires X gotta be the laziest excuse I have heard in a while xd
If you code a spell that requires other systems (aka 99 of all jass spells) you will just get invalid errors if someone else tries to use it.
It will yell undeclared function instead of missing lib.

Coders tend to take the easiest path to a task. Example, that's why coders use loops instead of manually coding each loop action one by one. Being lazy is enough reason to use scope. And what if you decided to add a system to your map that your spell will use, then you have to manually add that to the requirements. It's just too much of hassle and your argument is invalid. And you can always include the requirement in the documentation of the spell written inside a scope, something like "Requires X". And I thought were talking about scripting your map spells not scripting a public spell to be uploaded.