[JASS] efficiency ?

[deathismyfriend]

hello i was just wondering if this is efficient as can be or if theres something i can change to make it better basically its a unit group tht calls all units and changes there custom value to 2001, 2002, or 2003 with tht it adds or removes it or does nothing its for a minigame ik it works good the way it is now just wanted to know if there is something better and thx to all tht help

function GroupCounter_Life_or_Death_Random takes nothing returns nothing
    local unit u = GetEnumUnit()
    local integer i = GetRandomInt(1, 3)
    if ( i == 1) then
        call SetUnitColor( u, PLAYER_COLOR_RED)
        call UnitRemoveType(u, UNIT_TYPE_UNDEAD)
        call UnitAddType(u, UNIT_TYPE_MECHANICAL)
        call PauseUnit(u, true)
    elseif ( i == 2) then
        call SetUnitColor( u, ConvertPlayerColor(12))
        call PauseUnit(u, false)
        call UnitAddType(u, UNIT_TYPE_UNDEAD)
        call UnitRemoveType(u, UNIT_TYPE_MECHANICAL)
    elseif ( i == 3) then
        call SetUnitColor( u, PLAYER_COLOR_GREEN)
        call UnitAddType(u, UNIT_TYPE_UNDEAD)
        call UnitRemoveType(u, UNIT_TYPE_MECHANICAL)
        call PauseUnit(u, true)
    endif
    set u = null
endfunction    // changing the random units to kill heal or do nothing

function Trig_Life_or_Death_Changing_Actions takes nothing returns nothing
    call ForGroup(Life_or_Death_Random_Group, function GroupCounter_Life_or_Death_Random)
endfunction

//===========================================================================
function InitTrig_Life_or_Death_Changing takes nothing returns nothing
    set gg_trg_Life_or_Death_Changing = CreateTrigger(  )
    call DisableTrigger( gg_trg_Life_or_Death_Changing )
    call TriggerRegisterTimerEvent( gg_trg_Life_or_Death_Changing, 2.00, true )
    call TriggerAddAction( gg_trg_Life_or_Death_Changing, function Trig_Life_or_Death_Changing_Actions )
endfunction

 

[Ender]

call TriggerRegisterTimerEventPeriodic( gg_trg_Life_or_Death_Changing, 2.00 ) could be replaced with this call TimerStart(CreateTimer(),2,true, function Trig_Life_or_Death_Changing_Actions)

maybe set this GetRandomInt(2001,2003) to a var.

efficiency shouldn't be that big of an issue with what you are doing.

 

[ap0calypse]

I wonder why you set the custom value of the unit. Couldn't you just use an integer variable?
I also find the indenting a bit awkward, but that has nothing to do with efficiency .
The last elseif is unnecessary: it will always be 2003 if not the previous ones, so you can just turn that into an "else".

The event can be rewritten to TriggerRegisterTimerEvent( gg_trg_Life_or_Death_Changing, 2.00, true )

function GroupActions takes nothing returns nothing
    local unit u = GetEnumUnit()
    local integer i = GetRandomInt( 1, 3 )
    if i == 1 then
        call SetUnitColor( u, PLAYER_COLOR_RED)
        call UnitRemoveType(u, UNIT_TYPE_UNDEAD)
        call UnitAddType(u, UNIT_TYPE_MECHANICAL)
        call PauseUnit(u, true)
    elseif i == 2 then
        call SetUnitColor( u, ConvertPlayerColor(12))
        call PauseUnit(u, false)
        call UnitAddType(u, UNIT_TYPE_UNDEAD)
        call UnitRemoveType(u, UNIT_TYPE_MECHANICAL)
    else
        call SetUnitColor( u, PLAYER_COLOR_GREEN)
        call UnitAddType(u, UNIT_TYPE_UNDEAD)
        call UnitRemoveType(u, UNIT_TYPE_MECHANICAL)
        call PauseUnit(u, true)
    endif
    
    set u = null
endfunction


function LifeOrDeath takes nothing returns nothing
    call ForGroup(Life_or_Death_Random_Group, function GroupActions)
endfunction

function InitTrig_Life_or_Death_Changing takes nothing returns nothing
    set gg_trg_Life_or_Death_Changing = CreateTrigger(  )
    call DisableTrigger( gg_trg_Life_or_Death_Changing )
    call TriggerRegisterTimerEvent( gg_trg_Life_or_Death_Changing, 2.00, true )
    call TriggerAddAction( gg_trg_Life_or_Death_Changing, function LifeOrDeath )
endfunction

Well, if you do need the custom values you can leave that in of course.
There's nothing really bad in your trigger, mostly aesthetic things as far as I can see.

 

[deathismyfriend]

actually ya makes sense not sure y i used custom value of the unit lol cant remember y i did something like tht haha im making my map from gui to jass it was originally done in gui but figured id learn jass and it was rather simple just dont know all the best ones to use for efficiency yet lol what u mean aesthetic things ? and thx ender ik efficiency isnt really needed but i dont like making things halfway gonna say a curse if u know which one goes w tht lol ill keep it for kids tho but doing things halfway just isnt me id rather everything be as efficient as possible u know wat i mean ? thx for ur advice tho
apocalypse thx for ur advice to it helped a lot if there is anything more to make it better plz say so lol

 

[ap0calypse]

In that case, don't forget to join the JASS Class!
I think it's a brilliant initiative . You do have to put some time in it, but you can't expect to learn a language without doing so.

Anyway. I don't think there's much to make it better. It's a pretty short and simple trigger . If you ever need advice, feel free to ask (posting it on these forums is better since I'm don't know everything there is to know).

 

[deathismyfriend]

cool cool thx and not sure if i have the time haha

 

[Almia]

If this uses Unit Indexer,it will be a disaster

 

[deathismyfriend]

i dont use unit indexer lol and oops i never updated the top post

top post now updated lol

 

[mateuspv]

You could do this to make it more readable : )

function InitTrig_Life_or_Death_Changing takes nothing returns nothing
    set gg_trg_Life_or_Death_Changing = CreateTrigger(  )
    call DisableTrigger( gg_trg_Life_or_Death_Changing )
    call TriggerRegisterTimerEvent( gg_trg_Life_or_Death_Changing, 2.00, true )
    call TriggerAddAction( gg_trg_Life_or_Death_Changing, function Trig_Life_or_Death_Changing_Actions )
endfunction

>>

function InitTrig_Life_or_Death_Changing takes nothing returns nothing
    local trigger t = CreateTrigger(  )
    call DisableTrigger( t )
    call TriggerRegisterTimerEvent( t, 2., true )
    call TriggerAddAction( t, function Trig_Life_or_Death_Changing_Actions )
    set t = null
endfunction

 

[deathismyfriend]

ooo so thts how u null the trigger tht makes sense but how about for this its multiple regions and im not sure how to null it or to make it so it doesnt leak ? and how to change it over to triggerregistereventregion ? i couldnt get it to work says i need boolexpr and not really sure how to do them ? thx

function InitTrig_Hero_Spawns_All takes nothing returns nothing
    set gg_trg_Hero_Spawns_All = CreateTrigger(  )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Blood_Mage )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Far_Seer )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Dreadlord )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Naga_Sea_Witch )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Beastmaster )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Firelord )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Priestess_of_the_Moon )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Warden )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Archmage )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Mountain_King )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Blademaster )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Death_Knight )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Crypt_Lord )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Illidan_evil )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Dark_Ranger )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Pit_Lord )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Demon_Hunter )
    call TriggerRegisterEnterRectSimple( gg_trg_Hero_Spawns_All, gg_rct_Nefarius_Paladin )
    call TriggerAddAction( gg_trg_Hero_Spawns_All, function Trig_Hero_Spawns_All_Actions )
endfunction

 

[Almia]

btw,all of that pink words that is colored are bjs,w/c means ugly

 

[deathismyfriend]

ik thts y i asked for help lol read the thing above it

 

[ap0calypse]

You can just use "null" for boolexpr.
Create a local region, add all those rects to the region (e.g.: RegionAddRect( r, gg_rct_Nefarius_Paladin ) ).
The event will then be call TriggerRegisterEnterRegion( t, r, null ).

 

[deathismyfriend]

oooo ok pretty sure i understand wat u mean lol when i get a chance ill do tht thx a lot for ur help

umm only one problem how do i turn off the trigger from another trigger if i use local trigger t = createtrigger() ?

 

[themerion]

how do i turn off the trigger from another trigger if i use local trigger t = createtrigger() ?

If you want to be able to disable it from outside you should probably keep the old name.

OR, if you're using vJass/jasshelper you can declare the trigger global in the script.

globals
    trigger lifeOrDeath
endglobals

function onInit takes nothing returns nothing
    lifeOrDeath=CreateTrigger();
// etc....
endfunction

 

[deathismyfriend]

it doesnt cause leaks doing it tht way ?

 

[themerion]

You pretty much never need to be concerned with leaks on GLOBAL objects. If you initialize a trigger on map init, it will hold 1 "slot" in memory during the game, period. Of course, if you destroy the trigger before game ends it will stop using memory (but then it will not work anymore, because you destroyed it )

The problem with leaks arise when the same code can be run multiple times:

function FunctionSetUpToRunWhenYouCastASpell takes nothing returns nothing
    call AddSpecialEffectLoc("SomeEffect.mdx", GetSpellTargetLoc())

    // Creating an effect and a location EVERY time the spell is cast...
    // GetSpellTargetLoc() creates a new location-object
    // AddSpecialEffect creates a new effect-object
endfunction

// This is how you deal with location leaks.
// The effect will still leak, you could clean that up with a timer.
// OR, use the method Blizzard uses in Extreme Candy Wars, if you want a really easy solution.

function FunctionSetUpToRunWhenYouCastASpell takes nothing returns nothing
    local location loc = GetSpellTargetLoc()
 // loc is now pointing to our newly created location

    call AddSpecialEffectLoc("SomeEffect.mdx", loc)

    call RemoveLocation(loc)  // Remove the location which loc is pointing to
    set loc = null  // Make loc point to nothing
endfunction

// just for the sake of it, an example that doesn't leak at all.

function FunctionSetUpToRunWhenYouCastASpell takes nothing returns nothing
    // This will only show the effect's death animation
    // Sadly, this is the quickest and simplest way to clean up effects

    // Notice how the location leak is circumvented by not actually using a location.

    call DestroyEffect( AddSpecialEffectLoc("SomeEffect.mdx", GetSpellTargetX(), GetSpellTargetY()) )

endfunction

 

[deathismyfriend]

thts what i thought i just wanted to make sure tho lol thx