Converting GUI into JASS

[GIMLI_2]

Hi everyone

i want to convert all of my GUI triggers into JASS, remove BJ functions, create local variables...

i am not familiar with JASS, but that is what i came up with:

GUI

• Pick Hero GUI
  • Ereignisse
    • Einheit - A unit Erwirbt einen Gegenstand
  • Bedingungen
    • (Unit-type of (Hero manipulating item)) Gleich Dummy_Select
  • Aktionen
    • Set TempPlayer = (Owner of (Triggering unit))
    • Set TempInt = (Player number of TempPlayer)
    • For each (Integer Int) from 1 to 7, do (Actions)
      • Schleifen - Aktionen
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • 'IF'-Bedingungen
            • (Item-type of (Item being manipulated)) Gleich GoblinIcon[Int]
          • 'THEN'-Aktionen
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • 'IF'-Bedingungen
                • Int Ungleich 7
              • 'THEN'-Aktionen
                • Set TempInt2 = Int
                • Set TempString = chosen
              • 'ELSE'-Aktionen
                • Set TempInt2 = (Random integer number between 1 and 6)
                • Set TempString = randomed
          • 'ELSE'-Aktionen
    • Set TempInt3 = (Random integer number between 1 and 8)
    • Set TempPoint = (Random point in (Load DialogIndex_Area of TempInt3 in RegionHash))
    • Einheit - Create 1 Goblin[TempInt2] for TempPlayer at TempPoint facing Vorgabe für Gebäude-Ausrichtung degrees
    • Set TempUnit1 = (Last created unit)
    • Auswahl - Select TempUnit1 for TempPlayer
    • Einheitengruppe - Add TempUnit1 to UnitGroup
    • Kamera - Pan camera for TempPlayer to TempPoint over 1.00 seconds
    • Spiel - Display to (All players) the text: (Player[TempInt] + (( |rhas + (TempString + |cff7ebff1)) + (Name of TempUnit1)))
    • Custom script: call RemoveLocation (udg_TempPoint)
    • Held - Create Item[0] and give it to TempUnit1

JASS

function PickHeroConditions takes nothing returns boolean
    if ( GetUnitTypeId(GetManipulatingUnit()) == 'h00D' ) then
    return true
    endif
    return false
endfunction

function PickHeroActions takes nothing returns nothing

    local player p = GetOwningPlayer(GetTriggerUnit())
    local unit u
    local integer player_id = GetPlayerId(p) + 1
    local integer unit_id
    local integer i = 1
    local string output
    local location spawn = GetRandomLocInRect(LoadRectHandle(udg_RegionHash, GetRandomInt(1, 8), udg_DialogIndex_Area))
    local real x = GetLocationX(spawn)
    local real y = GetLocationY(spawn)
    
    loop
        exitwhen i > 10
        if(GetItemTypeId(GetManipulatedItem()) == udg_GoblinIcon[i]) then
            if (i != 7) then
                set unit_id = i
                set output = "chosen"
            else
                set unit_id = GetRandomInt(1, 6)
                set output = "randomed"
            endif
        endif
        set i = i + 1
    endloop
    
    set u = CreateUnit(p, udg_Goblin[unit_id], x, y, bj_UNIT_FACING)
    call GroupAddUnit(udg_UnitGroup, u)
    
    if (GetLocalPlayer() == p) then
        call PanCameraToTimed(x, y, 1.00)
    endif
    
    if (GetLocalPlayer() == p) then
        call ClearSelection()
        call SelectUnit(u, true)
    endif
    
    call DisplayTextToForce(bj_FORCE_ALL_PLAYERS, udg_Player[player_id] + ((" |rhas " + (output + " |cff7ebff1")) + GetUnitName(u)))
    set bj_lastCreatedItem = CreateItem(udg_Item[0], x, y)
    call UnitAddItem(u,bj_lastCreatedItem)
    
    set u = null
    set p = null
    set spawn = null
    
endfunction

//===========================================================================
function InitTrig_Pick_Hero takes nothing returns nothing
    set gg_trg_Pick_Hero = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Pick_Hero, EVENT_PLAYER_UNIT_PICKUP_ITEM )
    call TriggerAddCondition( gg_trg_Pick_Hero, Condition( function PickHeroConditions ) )
    call TriggerAddAction( gg_trg_Pick_Hero, function PickHeroActions )
endfunction

the JASS trigger has the same ingame result as the GUI trigger now, but i want to know if there is anything to improve (and there is for sure) and i hope you guys can give me some tips for a beginner

thanks

 

[Wietlol]

    set bj_lastCreatedItem = CreateItem(udg_Item[0], x, y)
    call UnitAddItem(u,bj_lastCreatedItem)

Unless you use "Last created item" somewhere... which I can definately understand because you used GUI before, you could create a local variable of your item or even not use a variable at all.

    if (GetLocalPlayer() == p) then
        call PanCameraToTimed(x, y, 1.00)
    endif
    
    if (GetLocalPlayer() == p) then
        call ClearSelection()
        call SelectUnit(u, true)
    endif

Can you give me an example of when the first and the second if statements are not the same?

call DisplayTextToForce(bj_FORCE_ALL_PLAYERS, udg_Player[player_id] + ((" |rhas " + (output + " |cff7ebff1")) + GetUnitName(u)))

When displaying a message to all players, you could simply do "DisplayTextToPlayer(GetLocalPlayer(), stuff)"

    call TriggerAddCondition( gg_trg_Pick_Hero, Condition( function PickHeroConditions ) )
    call TriggerAddAction( gg_trg_Pick_Hero, function PickHeroActions )

Now that you are using JASS, you can move all the actions and conditions in one function.
Preferably in a condition because it is slightly faster because of how the threads work behind the scenes.
On the other hand, a condition is not able to use TriggerSleepAction() because of the same reason. (But you dont want to use it anyway.)

GetRandomLocInRect(LoadRectHandle(udg_RegionHash, GetRandomInt(1, 8), udg_DialogIndex_Area))

I havent taken a look into that function, but it should be rather easy to do that manually.
But if the code inside that function is neat and not filled with unnecessary stuff, you can keep using that.

if(GetItemTypeId(GetManipulatedItem())
...
if (GetLocalPlayer() == p) then
if (GetLocalPlayer() == p) then

These are redundant and can/should be changed into variables.
Doing these things inside a loop is almost always redundant.

    if ( GetUnitTypeId(GetManipulatingUnit()) == 'h00D' ) then
    return true
    endif
    return false

Whenever you use such things in boolean returns, you can just return the response from your boolean check:

     return GetUnitTypeId(GetManipulatingUnit()) == 'h00D'

 

[IcemanBo]

Moved to Trigger & Scripts forum!

function PickHeroConditions takes nothing returns boolean
    if ( GetUnitTypeId(GetManipulatingUnit()) == 'h00D' ) then
    return true
    endif
    return false
endfunction

You can directly return an expression which is or true or false. ->

function PickHeroConditions takes nothing returns boolean
    return GetUnitTypeId(GetManipulatingUnit()) == 'h00D'
endfunction

The trigger event is a PlayerUnit event, so you can use TriggeringPlayer()
local player p = GetOwningPlayer(GetTriggerUnit())
->
local player p = GetTriggerPlayer()

You could inline:
GetRandomLocInRect
and
DisplayTextToForce
Inline means you look what is inside the red function and write in yourself,
but in your own function. This way you don't unnecessarly call a function.

I cut the rest because I see someone else also postet already.

But two last things.
You don't need a location at all. You could go with reals only.
In your JASS trigger you loop from 1-10, in GUI from 1-7.

Edit:

In JASS you leak the location.

 

[GIMLI_2]

thanks for your answers!
i have some questions:

if(GetItemTypeId(GetManipulatedItem())
...
if (GetLocalPlayer() == p) then
if (GetLocalPlayer() == p) then

the GetLocalPlayer() isnt in a loop? i dont know what you mean there

about putting the actions into the conditions: how would i do that?

function PickHeroInit takes nothing returns nothing
    local trigger PickHero = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( PickHero, EVENT_PLAYER_UNIT_PICKUP_ITEM )
    call TriggerAddCondition( PickHero, Condition( function PickHeroConditions ) )
    call TriggerAddAction( PickHero, function PickHeroActions )
    set PickHero = null
endfunction

i tried to create a local trigger, but my trigger stopped working. i remembered doing this some years ago but i'm not sure.

you could create a local variable of your item or even not use a variable at all.

how would i do it without using a variable at all?

 

[Flux]

about putting the actions into the conditions: how would i do that?

function PickHeroInit takes nothing returns nothing
    local trigger PickHero = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( PickHero, EVENT_PLAYER_UNIT_PICKUP_ITEM )
    call TriggerAddCondition( PickHero, Condition( function PickHeroConditions ) )
    call TriggerAddAction( PickHero, function PickHeroActions )
    set PickHero = null
endfunction

You remove TriggerAddAction and put the whole code in function PickHeroConditions
But your code must be within a huge block of if-else.
Example:

function PickHeroConditions takes nothing returns boolean
//assign variables
if <your condition> then
  //copy the codes in your trigger actions
endif
return false
endfunction

how would i do it without using a variable at all?

If you only used the variable once, you can do something like this

call UnitAddItem(u, CreateItem(...))

 

[OmagnaT]

applied the responses in thread, though this can still be slightly refined

function PickHeroConditions takes nothing returns boolean
    local player p = GetTriggerPlayer()
    local unit u
    local integer player_id = GetPlayerId(p) + 1
    local integer unit_id
    local integer i = 1
    local integer itemid = GetItemTypeId(GetManipulatedItem())
    local string output
    local rect r = LoadRectHandle(udg_RegionHash, GetRandomInt(1, 8), udg_DialogIndex_Area)
    local real x = GetRandomReal(GetRectMinX(r), GetRectMaxX(r))
    local real y = GetRandomReal(GetRectMinY(r), GetRectMaxY(r))

    if GetUnitTypeId(GetManipulatingUnit()) == 'h00D' then
        loop
            exitwhen i > 10
            if  itemid == udg_GoblinIcon[i] then
                if i != 7 then
                    set unit_id = i
                    set output = "chosen"
                else
                    set unit_id = GetRandomInt(1, 6)
                    set output = "randomed"
                endif
            endif
            set i = i + 1
        endloop
    
        set u = CreateUnit(p, udg_Goblin[unit_id], x, y, bj_UNIT_FACING)
        call GroupAddUnit(udg_UnitGroup, u)
    
        if GetLocalPlayer() == p then
            call PanCameraToTimed(x, y, 1.00) 
            call ClearSelection()
            call SelectUnit(u, true)
        endif
    
        call DisplayTextToPlayer(GetLocalPlayer(), 0, 0, udg_Player[player_id] + ((" |rhas " + (output + " |cff7ebff1")) + GetUnitName(u)))

        set bj_lastCreatedItem = CreateItem(udg_Item[0], x, y)  // swap udg_Item[0] out for the raw code of the item (ex. 'I091')
        call UnitAddItem(u,bj_lastCreatedItem)
    endif

    call RemoveRect(r)
    set u = null
    set p = null
    set r = null
    set output = "" 
endfunction

function PickHeroActions takes nothing returns nothing
endfunction

//===========================================================================
function InitTrig_Pick_Hero takes nothing returns nothing 
    local integer index

    set index = 0
    set gg_trg_Pick_Hero = CreateTrigger(  )
    loop
        call TriggerRegisterPlayerUnitEvent(gg_trg_Pick_Hero, Player(index), EVENT_PLAYER_UNIT_PICKUP_ITEM, null)

        set index = index + 1
        exitwhen index == bj_MAX_PLAYER_SLOTS
    endloop
    
    call TriggerAddCondition( gg_trg_Pick_Hero, Condition( function PickHeroConditions ) )
    call TriggerAddAction( gg_trg_Pick_Hero, function PickHeroActions )
endfunction

 

[Wietlol]

the GetLocalPlayer() isnt in a loop? i dont know what you mean there

I said they were redundant and next to that, loops are a big cause of redundant function calls.

GetLocalPlayer() was used twice which makes the second one redundant and should be a variable instead.

But your code must be within a huge block of if-else.

Not necessarily...

function foo takes nothing returns boolean
    
    if not :condition: then
        return false
    endif
    
    //stuff goes here.
    
    return false
endfunction

        set bj_lastCreatedItem = CreateItem(udg_Item[0], x, y)  // swap udg_Item[0] out for the raw code of the item (ex. 'I091')
        call UnitAddItem(u,bj_lastCreatedItem)

->

call UnitAddItem(u, CreateItem(udg_Item[0], x, y))

call TriggerAddAction( gg_trg_Pick_Hero, function PickHeroActions )

Remove this.

 

[GIMLI_2]

thanks for your replies!

that's how my code looks now, and it still works ingame

function Trig_Pick_Hero_Conditions takes nothing returns boolean
    
        local player p = GetTriggerPlayer()
        local unit u
        local integer player_id = GetPlayerId(p) + 1
        local integer unit_id
        local integer i = 1
        local string s
        local rect r = LoadRectHandle(udg_RegionHash, GetRandomInt(1, 8), udg_DialogIndex_Area)
        local real x = GetRandomReal(GetRectMinX(r), GetRectMaxX(r))
        local real y = GetRandomReal(GetRectMinY(r), GetRectMaxY(r))
        local item h = GetManipulatedItem()
    
    if GetUnitTypeId(GetManipulatingUnit()) == 'h00D' then
    
        loop
            exitwhen i > 7
            if(GetItemTypeId(h) == udg_GoblinIcon[i]) then
                if (i != 7) then
                    set unit_id = i
                    set s = "chosen"
                else
                    set unit_id = GetRandomInt(1, 6)
                    set s = "randomed"
                endif
            endif
            set i = i + 1
        endloop
    
        set u = CreateUnit(p, udg_Goblin[unit_id], x, y, bj_UNIT_FACING)
        call GroupAddUnit(udg_UnitGroup, u)
    
        if (GetLocalPlayer() == p) then
            call PanCameraToTimed(x, y, 1.00)
            call ClearSelection()
            call SelectUnit(u, true)
        endif
    
        call DisplayTextToPlayer(GetLocalPlayer(), 0, 0 , udg_Player[player_id] + ((" |rhas " + (s + " |cff7ebff1")) + GetUnitName(u)))
        call UnitAddItem(u, CreateItem(udg_Item[0], x, y))
        
    endif
    
        call RemoveRect(r)
        set u = null
        set p = null
        set r = null
        set h = null
        set s = ""
    
    return false
    
endfunction


//===========================================================================
function InitTrig_Pick_Hero takes nothing returns nothing
    local integer index
    set index = 0
    set gg_trg_Pick_Hero = CreateTrigger(  )
    loop
        call TriggerRegisterPlayerUnitEvent(gg_trg_Pick_Hero, Player(index), EVENT_PLAYER_UNIT_PICKUP_ITEM, null)
        set index = index + 1
        exitwhen index == bj_MAX_PLAYER_SLOTS
    endloop
    call TriggerAddCondition( gg_trg_Pick_Hero, Condition( function Trig_Pick_Hero_Conditions ) )
endfunction

i also have a problem with setting up a global item array

globals
    item array Item[5]
endglobals

function InitActions takes nothing returns nothing
    set Item[0] = 'I000'
    set Item[1] = 'I002'
    set Item[2] = 'I001'
    set Item[3] = 'I003'
    set Item[4] = 'I005'
endfunction

function Initialization takes nothing returns nothing
    local trigger Init = CreateTrigger(  )
    call TriggerAddAction( Init, function InitActions )
    set trigger = null
endfunction

it says "cannot convert item to integer" or the other way around, cannot remember

 

[Wietlol]

GetItemTypeId(h)

That is a constant result in a loop... so redundant from the first iteration.
Also, you still use GetLocalPlayer() twice.

In your globals, you create an item array.
Items are not the same as item types.
And next to that, item types, unit types, destructable types, abilities, etc are all integers.
So you need an integer array instead of your item array.
I also recommend you to create a more unique name for your global variable.

One last thing: InitActions content should be inside Initialization.
It will save you a trigger and a thread
(There will also be a syntax error on "set trigger = null" because you should have done "set Init = null".)

(I am also not sure how you call "Initialization()".)

 

[GIMLI_2]

okay, i'm a bit confused about creating an initialization trigger

i also created my own function but it tells me that it expects a return? but i dont want to return anything

function MoveGoblin takes unit u takes real r returns nothing

    local rect rt = LoadRectHandle(udg_RegionHash, GetRandomInt(1, 8), udg_DialogIndex_Area)
    local real x = GetRandomReal(GetRectMinX(rt), GetRectMaxX(rt))
    local real y = GetRandomReal(GetRectMinY(rt), GetRectMaxY(rt))
    local player p = GetOwningPlayer(u)
    
    call SetUnitX(u, x)
    call SetUnitY(u, y)
    
    if (GetLocalPlayer() == p) then
        call PanCameraToTimed(x, y, r)
        call ClearSelection()
        call SelectUnit(u, true)
    endif
    
    call RemoveRect(rt)
    set rt = null
    set p = null
    
endfunction

edit: i solved the return thingy

 

[Wietlol]

You have written takes twice.
To have multiple parameters, you just separate them with a comma: function foo takes real r, integer i, boolean b returns nothing

In the init trigger, you just place all your stuff inside that "Initialization" function.
(However, I still don't know how you run it.)

 

[GIMLI_2]

You have written takes twice.
To have multiple parameters, you just separate them with a comma: function foo takes real r, integer i, boolean b returns nothing

In the init trigger, you just place all your stuff inside that "Initialization" function.
(However, I still don't know how you run it.)

i want to run it at map initialization

basically, i have a bunch of arrays in my initialization, and as you cannot set them in the globals block i need to set them in the initialization trigger?

 

[Flux]

Since you're already using JNGP, put the whole code inside a library or scope. Then follow it up with initializer.

scope yourSpell initializer OnInit
....
....
//Name must match what was written after 'initializer'
//This function will run at Map Initialization
function OnInit takes nothing returns nothing
...
endfunction

endscope

 

[Wietlol]

You may understand that something doesnt run out of itself.
When you convert a GUI trigger to JASS, you can see that it always has 3 functions.
1, the conditions
2, the actions
3, the init function.

The init functions is always "InitTrig_<trigger-name>" and takes no parameters and returns nothing.
This function runs on map loading (before map init) and can be used to create some stuff like triggers/timers, store some values in variables etc.
After that comes Map init, which is when you use the gg_trg_<trigger-name> variable and set "Run on Map Initialization" to true.
(Be aware that this has to have actions/conditions in order to do anything.)

In libraries, structs, etc, you can have a "initializer" or "onInit" function that will also run on map loading.
However, you should be carefull with what you do in there because all the map loadings run on one thread which means that the OP limit is a big problem when you have a lot of things running then. (But that is only for big maps )

 

[GIMLI_2]

okay thanks.

it looks like this now and it works:

globals
    integer array ItemCollection[5]
endglobals




scope Initialization initializer OnInit
    function OnInit takes nothing returns nothing
        set ItemCollection[0] = 'I000'
        set ItemCollection[1] = 'I002'
        set ItemCollection[2] = 'I001'
        set ItemCollection[3] = 'I003'
        set ItemCollection[4] = 'I005'
    endfunction
endscope

 

[Wietlol]

That would indeed work.
You could change OnInit to InitTrig_<trigger-name> but your current script will allow you to simply copy and paste it into other triggers because it is independent from the name.

It is just a matter of preference.

 

[GIMLI_2]

next problem xD

function FloatingText takes string s, player p returns nothing
    
    local real r = GetRandomReal(1.00, 360.00)
    local texttag t = CreateTextTag()
    local real x = GetCameraTargetPositionX()
    local real y = GetCameraTargetPositionY()
    
    call SetTextTagText(t, s, 15.00)
    call SetTextTagPos(t, x, y, 0)
    call SetTextTagVisibility(t, true)
    call SetTextTagColor(t, 255, 255, 255, 255)
    call SetTextTagVelocity(t, 80.00, r)
    call SetTextTagPermanent(t, false)
    call SetTextTagLifespan(t, 3.00)
    call SetTextTagFadepoint(t, 2.00)
    
    set t = null
    
endfunction

text does not show up

 

[Wietlol]

Your size exceeds the Texttag bounds.

I must have saved it somewere... lemme take a look.

EDIT:
When you take an in-depth look into the SetTextTagTextBJ function, you can see that the size is converted from texttagsize to height.
Those names are just the names used in the convertion function
The actual size is multiplied by 0.023 and divided by 10, which means that it is about 435 times as small as you actually tried to set it.
So what you have to do is multiply your number (15.00) by 0.0023 (0.0345) and you should be fine.

The maximum size of a texttag is 0.345 (150 in GUI or BJ function) according to my tests that I did on a combat text extension on my damage engine... this may change with graphical settings but I am not sure about that.

 

[GIMLI_2]

ok i fixed that, but it still does not show up. maybe i did sth wrong with the position of the floating text? i want to have the center of the current camera view

another thing. if i create a function "function f takes unit u returnst nothing" for example
do i need to null the unit u that is needed as input?

 

[Wietlol]

Parameters dont have to be nulled.

Try placing it slightly higher than 0... something like 100-200 would do.
Also, place a call BJDebugMsg("Test message") in it to check it is actually running.

 

[GIMLI_2]

debug msg shows up, still does not work :/

EDIT: fixed it, velocity had the same issues as the size

function FloatingText takes string s, player p returns nothing
    
    local real r = GetRandomReal(1.00, 360.00)
    local texttag t = CreateTextTag()
    local real x = GetCameraTargetPositionX()
    local real y = GetCameraTargetPositionY()
    
    call SetTextTagText(t, s, 0.0345)
    call SetTextTagPos(t, x, y, 100)
    if (GetLocalPlayer() != p) then
        call SetTextTagVisibility(t, false)
    endif
    call SetTextTagVelocity(t, 0.044375*Cos(r*bj_DEGTORAD), 0.044375*Sin(r*bj_DEGTORAD))
    call SetTextTagPermanent(t, false)
    call SetTextTagLifespan(t, 3.00)
    call SetTextTagFadepoint(t, 2.00)

    set t = null
    
endfunction

do i need to destroy the floating text? or is it enough to null it?

 

[Wietlol]

Take a good look in how "SetTextTagVelocity(t, r, r)" works.
The third parameter is NOT the angle.

 

[Zwiebelchen]

@Gimli: looks like your JASS knowledge is doing progress. Great to see someone who is actually trying to learn that thing instead of accepting horrible GUI in their maps.


Here's a suggestion for you, as you seem to have grasped the basics now:
Look into library and scope definitions. It allows you to define custom dependencies for your scripts, define an initialization order and overall adds a lot of neat structure to your code through privacy keywords.

The syntax looks like this

library MyLib initializer init uses OtherLib

function myPublicFunc takes nothing returns nothing
    //can also be called from outside the library
endfunction

private function myPrivateFunc takes nothing returns nothing
    //can only be called from inside the library
endfunction

private function init takes nothing returns nothing
    //runs at map init
endfunction

endlibrary

Do you see how you can define a custom function in the library declaration that will be run on map init without having to use InitTrig_?
That's neat and produces a lot cleaner code.

There is also a public keyword, but tbh not many people use it, as it pointlessly dumps the library name in front of your function with an underscore like this:
MyLib_MyPublicFunc

... hence why people just use functions without the public keyword, as they are essentially the same as public, but without the terrible underscore.


Scopes work very similar, but have some restrictions and are always initialized after all libraries.

As a rule of thumb: use scopes for spells and use libraries for systems and stuff that is required for your spells.

 

[GIMLI_2]

@zwiebelchen
Thanks! I just had too many issues with gui and finally decided to learn jass
About creating a library, what does "uses OtherLib" do?

And what ive seen in many spells, they create functions that return constant values instead of creating globals. Why is that better?

Currently im trying to make a spell but got stuck. In gui, i just used a unit indexer and had a cast trigger and a loop trigger with a periodic timer event. I could do that 1:1 in jass but there are better ways to do that, right?

 

[Wietlol]

when you create stuff in JASS and you want to call a function, you have to declare the function before you use it.
This will not work:

function foo1 takes nothing returns nothing
    call foo2()
endfunction

function foo2 takes nothing returns nothing
    
endfunction

But this will:

function foo1 takes nothing returns nothing
    
endfunction

function foo2 takes nothing returns nothing
    call foo1()
endfunction

This gets really complicated when you want to use functions from other triggers, because there is little that you can do about the order of how they are created.
In libraries, you can "use" or "require" another library by using the "uses" or "requires" keywords and it will place that used library above the one that uses it.

In other words:

library lib1 uses lib2
    function foo1 takes nothing returns nothing
        call foo2()
    endfunction
library

library lib2
    function foo2 takes nothing returns nothing
        
    endfunction
endlibrary

Will be:

//library lib2
function foo2 takes nothing returns nothing
    
endfunction
//endlibrary

//library lib1 uses lib2
function foo1 takes nothing returns nothing
    call foo2()
endfunction
//library

Using functions instead of globals is better because of how configurables work.
Lets say that you have a spell similar to Firebolt... can you guess how many configurables you need to make it cover pretty much everything?
I guess you will guess below 100... but I can come to 4k without problems.
And with configurables, you will never be prepared for in-game mechanics of the map that uses them.

So instead of creating million globals, you just make one function that will take the index number of your spell (preferably) and return the damage, speed, duration, cooldown, whatever.




In JASS, you dont need a new trigger for each periodic thing.
You can just use timers
But on the other hand, I recommend you to use a system.

 

[Flux]

About creating a library, what does "uses OtherLib" do?

It means it requires the library named "OtherLib" for your library to work. It's mainly because your library will use some functions created in the OtherLib. In terms of compilation, the "OtherLib" will written be above your library so that your library can call functions in "OtherLib"

And what ive seen in many spells, they create functions that return constant values instead of creating globals. Why is that better?

It is not actually better. Those "many spells" must have been written in JASS not vJASS. In Normal/Vanilla World Editor, you can't create a global via scripting e.g. globals...endglobals so JASS users end up using function for configuration instead of variables. Still vJASS spells have some configurables written in function format mainly due to being dependent on a variable e.g. damage dealth per level will be configured in a way like this

function Damage takes integer lvl returns real
   return lvl*100 + 100  //200, 300, 400
endfunction

Currently im trying to make a spell but got stuck. In gui, i just used a unit indexer and had a cast trigger and a loop trigger with a periodic timer event. I could do that 1:1 in jass but there are better ways to do that, right?

You could also do that in JASS, remember, the only difference between GUI and JASS is the way the code is presented. In terms of algorithm, it is mostly the same. However the advantage in vJASS is you can create structs which automatically have an allocate function (much like a Unit Indexer giving a custom value to a unit) that allows you to get a unique unused integer which you can use as an index of a spell. I highly advise reading this if you want to move to vJASS which in my opinion is a lot easier.

 

[Wietlol]

Instead of having

function Damage takes integer lvl returns real
   return lvl*100 + 100  //200, 300, 400
endfunction

You should be having

function GetDamage takes integer id returns real
   return GetLevel(id)*100 + 100  //200, 300, 400
endfunction

 

[Flux]

Instead of having

function Damage takes integer lvl returns real
   return lvl*100 + 100  //200, 300, 400
endfunction

You should be having

function GetDamage takes integer id returns real
   return GetLevel(id)*100 + 100  //200, 300, 400
endfunction

Uhmmm, why? I'm talking about the configuration part. My resources uses

function Damage takes integer lvl returns real
   return lvl*100 + 100  //200, 300, 400
endfunction

and mods have no problem with it.

 

[Wietlol]

Mods have no problem with it because mods dont care about the user's maps.
They shouldnt care either.

But let me take another example.
I want my spell to also deal damage based on my targets missing health.
So:

function GetDamage takes integer id returns real
    local real damageBase = 100
    local real damagePerLevel = 100 * GetLevel(id)
    local real damageByMissingHealth = 0.2 * (GetUnitState(GetTarget(id), UNIT_STATE_MAX_LIFE) - GetWidgetLife(GetTarget(id)))
    return damageBase + damagePerLevel + damageByMissingHealth
endfunction

So let me take a quick guess in Shadow Flux's function:

function Damage takes integer lvl returns real
    local real damageBase = 100
    local real damagePerLevel = 100 * lvl
    local real damageByMissingHealth = uhm... what?
    return damageBase + damagePerLevel + damageByMissingHealth
endfunction

If you think that it is bad because you have to call "GetLevel(id)" and "GetTarget(id)", then I recommend you to not use any configuration at all and ONLY write the stuff directly into your code.
However, it is harder to edit and it will require some searching to find what you are looking for.

 

[Flux]

Mods have no problem with it because mods dont care about the user's maps.
They shouldnt care either.

But let me take another example.
I want my spell to also deal damage based on my targets missing health.
So:

function GetDamage takes integer id returns real
    local real damageBase = 100
    local real damagePerLevel = 100 * GetLevel(id)
    local real damageByMissingHealth = 0.2 * (GetUnitState(GetTarget(id), UNIT_STATE_MAX_LIFE) - GetWidgetLife(GetTarget(id)))
    return damageBase + damagePerLevel + damageByMissingHealth
endfunction

So let me take a quick guess in Shadow Flux's function:

function Damage takes integer lvl returns real
    local real damageBase = 100
    local real damagePerLevel = 100 * lvl
    local real damageByMissingHealth = uhm... what?
    return damageBase + damagePerLevel + damageByMissingHealth
endfunction

He can also do this using my suggestion

//somewhere in his global config
private constant real DAMAGE_PER_MISSING = 0.2

//somewhere in his function config
private function Damage takes integer lvl returns real
   return lvl*100 + 100
endfunction


//somewhere in his onCast code
set level = GetUnitAbilityLevel(this.caster, SPELL_ID)
set this.staticDamage = Damage(level)

//somewhere in his damaging code
call UnitDamageTarget(this.caster, this.target, DAMAGE_PER_MISSING*GetUnitState(this.target, UNIT_STATE_MAX_LIFE) - GetWidgetLife(this.target) + this.staticDamage, ...)

 

[Wietlol]

Shadow Flux: "you won this round"

It is better to use functions that are generically loading the entire data of your spell's instance (by either sending in the id as parameter, by using it inside the struct so "this." can be used, by any other way) than making whatever you can design. (Except changing the code directly.)

But it is not bad to use variables or simple functions to load the data either.
You can use either methods, but if you want 100% generic stuff, you really have to link the entire data to your functions.

 

[GIMLI_2]

so as long as the function is in my struct, i can give it the id of a certain unit and can use it inside the function?

 

[Wietlol]

No, you would require the id of the instance of the spell...
when you use a struct, you can just use "this." but you should not be focussing on those things for now.

First, just make yourself comfortable with JASS and it's uses.
After that, you can make generic spells... or not.

 

[GIMLI_2]

ok

i have a really weird problem.
I made a periodic timer trigger which creates a random unit in a random point in a random location. it works fine, but it also creates those units outside the map boundary (and if they stack too much, they will pull themselves inside the boundary in the bottom left corner).
the function runs only once, that's not the problem. It's really confusing

 

[Wietlol]

"a random point in a random location"
I want to know how that works

Your random point should be within the map bounds...
So the random X should be higher than the minX and lower than the maxX.
The same for the Y axis.
If I could see how you wrote your random right now, I maybe could see where they are out of bounds or at least where they should be checked.

 

[GIMLI_2]

the thing is, it works fine. it just spawns a double at center of map obviously xD
what i mean with random point in random location:
i have 8 locations, one of them gets randomed, and in this one it will create a random point.

what could be the problem is that i create a unit at 0, 0 and then run a function that moves the unit (SetUnitX() SetUnitY()), maybe that creates 2 units?
one at 0, 0 and one at X, Y

local unit u = CreateUnit(Player(PLAYER_NEUTRAL_PASSIVE), Vehicle[i], 0, 0, 0.00)
...
call RandomLoc(u, false, 0.00, "Abilities\\Spells\\Human\\MassTeleport\\MassTeleportTarget.mdl")

function RandomLoc takes unit u, boolean b, real r, string s returns nothing

    
        local rect rt = LoadRectHandle(RegionHashtable, GetRandomInt(0, 7), 0/*udg_DialogIndex_Area*/)
        local real x = GetRandomReal(GetRectMinX(rt), GetRectMaxX(rt))
        local real y = GetRandomReal(GetRectMinY(rt), GetRectMaxY(rt))
        local player p = GetOwningPlayer(u)
        
        call SetUnitX(u, x)
        call SetUnitY(u, y)

        if (GetLocalPlayer() == p and b == true) then
            call PanCameraToTimed(x, y, r)
            call ClearSelection()
            call SelectUnit(u, true)
        endif
    
        if (s != "") then
            call AddSpecialEffect(s, x, y)
            call DestroyEffect( bj_lastCreatedEffect )
        endif

        set p = null
        set rt = null
        call RemoveRect(rt)
    
    endfunction

sth else: setting a units x and y ignores pathing? how can i avoid that?




EDIT: i changed the 0,0 to sth else and they do not spawn in the bottom left corner, so it has to do sth with that

 

[Wietlol]

First of all, you are not able to use "bj_lastCreatedEffect" that way.
You have to store the actual effect (created with AddSpecialEffect()) in a variable and use that variable.
In GUI functions, "bj_lastCreatedEffect" is the variable that is used.

Setting rt to null before removing it is quite stupid dont you think?
And what you are doing is making you unable to use that same rect (the one stored in the hashtable) for other things... so I guess you dont want to remove it.

"b == true" can just be "b"

SetUnitX/Y do indeed not check for pathing.
You could use "SetUnitPosition(u, x, y)" instead which does check that.

The removerect may be the cause of your problem... but I dont know for sure.

 

[GIMLI_2]

ok i fixed everything you mentioned.
still spawning units at center
it creates the unit, and when i move it, it looks like it copys the unit and moves the copy (or the other way around)

oh my god, im stupid xD i'm really sorry

found it

 

[Wietlol]

can you do a "call BJDebugMsg(I2S(rt))" just after the locals and give me the results?

 

[GIMLI_2]

i created the unit with the variable declaration, but i want to create it only every X seconds. i chose 2 seconds just for testing purpose and the trigger created 1 unit every second at 0, 0 and every 2 seconds, the unit got moved to the correct location.

such a stupid mistake


EDIT:
for triggers like this, which are neither spells nor systems, do i need to put it in a library/scope or is it ok this way? i also declared the global variable in the same trigger

globals
    integer VehicleTimer = 0
endglobals

function Trig_Vehicle_Conditions takes nothing returns boolean

        local integer i = GetRandomInt(0, 2)
        local unit u
    if (udg_Ev == false) then
        
        set VehicleTimer = VehicleTimer + 1
        if (VehicleTimer  == 2/*udg_Setting_Vehicle*/) then
            set u = CreateUnit(Player(PLAYER_NEUTRAL_PASSIVE), Vehicle[i], 0, 0, 0.00)
            call RandomLoc(u, false, 0.00, "Abilities\\Spells\\Human\\MassTeleport\\MassTeleportTarget.mdl")
            set VehicleTimer = 0
        endif
            
        
    endif
    
        set u = null
    
    return false
endfunction



//===========================================================================
function InitTrig_Vehicle takes nothing returns nothing
    set gg_trg_Vehicle = CreateTrigger(  )
    call TriggerRegisterTimerEvent( gg_trg_Vehicle, 1.00, true )
    call TriggerAddCondition( gg_trg_Vehicle, Condition( function Trig_Vehicle_Conditions ) )
endfunction

 

[Wietlol]

you can leave it like that, but most vJASSers use scopes.

 

[GIMLI_2]

ok

is it possible to return 2 values? i want my function to return a location (2 real values)

 

[Wietlol]

That is not possible.
You can place them in global variables though or you can use a struct that defines an object like a location.

 

[GIMLI_2]

is it ok to return a location then?

 

[IcemanBo]

You can write "return location", but you can not directly return 2 real values.
Then you woul need to store them somewhere where you have public access.

 

[GIMLI_2]

You can write "return location", but you can not directly return 2 real values.
Then you woul need to store them somewhere where you have public access.

ok


a trigger i just created, i'm not sure about the event, i think i've done sth wrong there probably. but it works

function Trig_Explosive_Mines_Conditions takes nothing returns boolean

        local unit u = GetTriggerUnit()
        local player p = GetOwningPlayer(u)
        local unit m

    if ( GetUnitTypeId(u) == 'n000' and  GetUnitTypeId(GoblinHero[GetPlayerId(p)]) == 'H000' and GetRandomInt(0, 99) <= 17 ) then

        set m = CreateUnit(p, 'n006', GetUnitX(u), GetUnitY(u), bj_UNIT_FACING )
        call RemoveUnit(u)
        call SetUnitAnimation(m, "birth")
    
    endif
    
        set u = null
        set m = null
        set p = null
    
    return false
    
    
endfunction



//===========================================================================
function InitTrig_Explosive_Mines takes nothing returns nothing
    local region r = CreateRegion()
    call RegionAddRect(r, bj_mapInitialPlayableArea)
    set gg_trg_Explosive_Mines = CreateTrigger(  )
    call TriggerRegisterEnterRegion( gg_trg_Explosive_Mines, r, null )
    call TriggerAddCondition( gg_trg_Explosive_Mines, Condition( function Trig_Explosive_Mines_Conditions ) )
    set r = null
endfunction

 

[GIMLI_2]

I have a problem with a function i made. Whenever i call the function in my trigger, the trigger wont continue from tha point. My function counts how many slots are used in the inventory. It takes a unit and returns an integer. I cannot show you the code right now, im not at home. It gives me no errors, it just stops the trigger

 

[GIMLI_2]

So, i did take a look at timers and have some questions about that. you told me to use a timer system and i did a research on that and ended up with TimerUtils or TimerTools.
From where do i get the code and which one should i use?