HomeUser Control Panel (unavailable in archive)ForumsTutorialsArt GalleryResourcesMaps
Wc3C.net » Warcraft III Modding » Developer's Corner » Triggers & Scripts » [Help] Trigger's Only Running Once

[Help] Trigger's Only Running Once

01-02-2009, 03:52 PM#1
Tide-Arc Ephemera
Collapse JASS:
function Trig_GM_Units_Actions takes nothing returns nothing
    local unit u
    local group g = CreateGroup()
    local group g2 = CreateGroup()
    local rect r = GetPlayableMapRect()
    call GroupEnumUnitsInRect(g, r, null)
    loop
        set u = FirstOfGroup(g)
    exitwhen u == null
        if GetUnitTypeId(u) == 'h00A' then
            call GroupAddUnit(g2, u)
        elseif GetUnitTypeId(u) == 'h00D' then
            call GroupAddUnit(g2, u)
        endif
        call GroupRemoveUnit(g, u)
    endloop
    loop
        set u = FirstOfGroup(g2)
    exitwhen u == null
        call IssueImmediateOrder( u, "waterelemental" )
        if GetUnitState(u, UNIT_STATE_MANA) == 0 then
            call KillUnit(u)
        endif
        call GroupRemoveUnit(g2, u)
    endloop
    call DestroyGroup(g)
    call RemoveRect(r)
    set u = null
endfunction

//===========================================================================
function InitTrig_GM_Units takes nothing returns nothing
    set gg_trg_GM_Units = CreateTrigger(  )
    call TriggerRegisterTimerEventPeriodic( gg_trg_GM_Units, 0.25 )
    call TriggerAddAction( gg_trg_GM_Units, function Trig_GM_Units_Actions )
endfunction

Why?
01-02-2009, 06:54 PM#2
Veev
You remove rect r instead of setting it to null (set r = null).
01-02-2009, 08:28 PM#3
Captain Griffen
You never enum for g2. Or clean it up.
01-03-2009, 01:46 AM#4
Zerzax
Since u is null because the loop ensures that it is null, you don't need to null u. Using a null boolexpr argument for GroupEnum will also cause a leak, as Griffen once pointed out to me.

You might also want to consider ForGroups or Boolexpr enum'ing.
01-03-2009, 02:24 AM#5
chobibo
Collapse JASS:
function GetPlayableMapRect takes nothing returns rect
    return bj_mapInitialPlayableArea
endfunction

You don't need to remove the rect returned by GetPlayableMapRect since it returns a global already, you don't even need to null it. I suggest using the global variable instead of the bj function.

EDIT: I forgot to mention that destroying the rect also destroys the rect held by bj_mapInitialPlayableArea so when you call GetPlayableMapRect, it will return a null handle since you destroyed it on the first execution.

@ Captain Griffen:
Collapse JASS:
    loop
        set u = FirstOfGroup(g)
    exitwhen u == null
        if GetUnitTypeId(u) == 'h00A' then
            call GroupAddUnit(g2, u)
        elseif GetUnitTypeId(u) == 'h00D' then
            call GroupAddUnit(g2, u)
        endif
        call GroupRemoveUnit(g, u)
    endloop

I think he did add things to g2, But yes Tide's not destroying and cleaning g2.

EDIT 2: Yo Tide-Arc Ephemera, I just want to suggest an alternate code, here it is:
Expand JASS:
01-03-2009, 02:48 PM#6
Tide-Arc Ephemera
Quote:
Originally Posted by Veev
You remove rect r instead of setting it to null (set r = null).
I'm a GUI/Jass interchanging user... and yet you can remove globals but not locals?

Quote:
Originally Posted by Zerzax
Since u is null because the loop ensures that it is null, you don't need to null u. Using a null boolexpr argument for GroupEnum will also cause a leak, as Griffen once pointed out to me.

You might also want to consider ForGroups or Boolexpr enum'ing.
I use nulls in my loop because I was told that they only mattered as a leaked over 40 000 runs or so (in this case that's 10 000 seconds which is over 2 hours in my case, should my map have to run that long, everything's deserved). And yeah uh, I don't know what boolexprs look like...

Quote:
Originally Posted by Captain Griffen
You never enum for g2. Or clean it up.
g2 was a last minute solution to a previous problem, but thanks for pointing that out, didn't see that.

Quote:
Originally Posted by chobibo
Collapse JASS:
function GetPlayableMapRect takes nothing returns rect
    return bj_mapInitialPlayableArea
endfunction

You don't need to remove the rect returned by GetPlayableMapRect since it returns a global already, you don't even need to null it. I suggest using the global variable instead of the bj function.

EDIT: I forgot to mention that destroying the rect also destroys the rect held by bj_mapInitialPlayableArea so when you call GetPlayableMapRect, it will return a null handle since you destroyed it on the first execution.

@ Captain Griffen:
Collapse JASS:
    loop
        set u = FirstOfGroup(g)
    exitwhen u == null
        if GetUnitTypeId(u) == 'h00A' then
            call GroupAddUnit(g2, u)
        elseif GetUnitTypeId(u) == 'h00D' then
            call GroupAddUnit(g2, u)
        endif
        call GroupRemoveUnit(g, u)
    endloop

I think he did add things to g2, But yes Tide's not destroying and cleaning g2.

EDIT 2: Yo Tide-Arc Ephemera, I just want to suggest an alternate code, here it is:
Expand JASS:
I'm not too fond of ForGroup because I've heard it's not as malleable as looping through groups. Also, your code does exactly what I need to do.

+Rep to everyone who helped
01-03-2009, 02:57 PM#7
Captain Griffen
Collapse JASS:
scope Rawr initializer Init

globals
    private group g
endglobals

private function Expr takes nothing returns nothing
    if GetUnitTypeId(GetFilterUnit()) == 'h00A' or GetUnitTypeId(GetFilterUnit()) == 'h00D' then
        call IssueImmediateOrder(GetFilterUnit(), "waterelemental" )
        if GetUnitState(GetFilterUnit(), UNIT_STATE_MANA) == 0 then
            call KillUnit(GetFilterUnit())
        endif
    endif
    return false
endfunction

private function Trig_GM_Units_Actions takes nothing returns nothing
    call GroupEnumUnitsInRect(g, GetPlayableMapRect(), Condition(function Expr))
endfunction

//===========================================================================
private function Init takes nothing returns nothing
    set gg_trg_GM_Units = CreateTrigger(  )
    call TriggerRegisterTimerEventPeriodic( gg_trg_GM_Units, 0.25 )
    call TriggerAddAction( gg_trg_GM_Units, function Trig_GM_Units_Actions )

    set g = CreateGroup()
endfunction

endscope
01-03-2009, 03:09 PM#8
Tide-Arc Ephemera
I don't get it. Is it more efficient or faster or anything? I know it does the same thing by inserting the actions with the conditions... but is that better in any way?

EDIT!
By the way, I can't use vJass but would I be able to change the whole scope variable g thingo to...
Collapse JASS:
function Trig_GM_Units_Actions takes nothing returns nothing
    local group g = CreateGroup()
    call GroupEnumUnitsInRect(g, GetPlayableMapRect(), Condition(function Expr))
    call DestroyGroup(g)
endfunction
01-03-2009, 04:06 PM#9
Fireeye
The pure jass version would look like:
Collapse JASS:
globals
    trigger gg_trg_GM_Units = null
    group udg_CheckGroup = CreateGroup()
endglobals

function Expr takes nothing returns boolean
    if GetUnitTypeId(GetFilterUnit()) == 'h00A' or GetUnitTypeId(GetFilterUnit()) == 'h00D' then
        call IssueImmediateOrder(GetFilterUnit(), "waterelemental" )
        if GetUnitState(GetFilterUnit(), UNIT_STATE_MANA) == 0 then
            call KillUnit(GetFilterUnit())
        endif
    endif
    return false
endfunction

function Trig_GM_Units_Actions takes nothing returns nothing
    call GroupEnumUnitsInRect(udg_CheckGroup, GetPlayableMapRect(), Condition(function Expr))
endfunction

//===========================================================================
function InitTrig_GM_Units takes nothing returns nothing
    set gg_trg_GM_Units = CreateTrigger(  )
    call TriggerRegisterTimerEventPeriodic( gg_trg_GM_Units, 0.25 )
    call TriggerAddAction( gg_trg_GM_Units, function Trig_GM_Units_Actions )
endfunction

The globals are generated in the variable editor / trigger editor, therefore the nasty udg_ and gg_trg prefix
01-03-2009, 05:08 PM#10
chobibo
Quote:
Originally Posted by Captain Griffen
Collapse JASS:
scope Rawr initializer Init

globals
    private group g
endglobals

private function Expr takes nothing returns nothing
    if GetUnitTypeId(GetFilterUnit()) == 'h00A' or GetUnitTypeId(GetFilterUnit()) == 'h00D' then
        call IssueImmediateOrder(GetFilterUnit(), "waterelemental" )
        if GetUnitState(GetFilterUnit(), UNIT_STATE_MANA) == 0 then
            call KillUnit(GetFilterUnit())
        endif
    endif
    return false
endfunction

private function Trig_GM_Units_Actions takes nothing returns nothing
    call GroupEnumUnitsInRect(g, GetPlayableMapRect(), Condition(function Expr))
endfunction

//===========================================================================
private function Init takes nothing returns nothing
    set gg_trg_GM_Units = CreateTrigger(  )
    call TriggerRegisterTimerEventPeriodic( gg_trg_GM_Units, 0.25 )
    call TriggerAddAction( gg_trg_GM_Units, function Trig_GM_Units_Actions )

    set g = CreateGroup()
endfunction

endscope

That's cool, less code, same functionality.

@Tide-Arc Epherema: Yes it's more efficient since you remove 1 function call and you only iterate through the units once, my code iterated through all units in the rect and then put it onto group g, then ForGroup iterated through the contents of group g, and did some actions to the units. Captain Griffen's code on the other hand iterated through the units in the rect, and applied the actions for the selected\filtered unit.

EDIT: guys, I think it's better to replace GetPlayableMapRect() with bj_mapInitialPlayableArea since the function returns the bj global, also the function wouldn't be inlined since Tide-Arc isn't using jasshelper.
01-03-2009, 05:11 PM#11
Tide-Arc Ephemera
Alrighty, I'll use that idea.

However, a new problem has presented itself.
____

This trigger is A-100%-OK

Trigger:
TC Preset income
Collapse Events
Map initialization
Conditions
Collapse Actions
Countdown Timer - Start TmrIncome as a Repeating timer that will expire in 5.00 seconds

Collapse JASS:
function WoodFilter takes integer i returns boolean
    return GetUnitTypeId(GetFilterUnit())=='h00H' and GetOwningPlayer(GetFilterUnit())==Player(i)
endfunction

function GoldFilter takes integer i returns boolean
    return GetUnitTypeId(GetFilterUnit())=='h00I' and GetOwningPlayer(GetFilterUnit())==Player(i)
endfunction

function Trig_GM_Income_Actions takes nothing returns nothing
    local integer i = 0
    local group g = CreateGroup()
    call BJDebugMsg("Income has started")
    loop
        call GroupEnumUnitsInRect(g, bj_mapInitialPlayableArea, Condition(function WoodFilter) )
        call AdjustPlayerStateBJ( (udg_IntBasicWood + CountUnitsInGroup(g)), Player(i), PLAYER_STATE_RESOURCE_LUMBER )
        call DestroyGroup(g)
        call GroupEnumUnitsInRect(g, bj_mapInitialPlayableArea, Condition(function GoldFilter) )
        call AdjustPlayerStateBJ( (udg_IntBasicGold + CountUnitsInGroup(g)), Player(i), PLAYER_STATE_RESOURCE_GOLD )
        call DestroyGroup(g)
    set i = i + 1
    exitwhen i == 12
    endloop
    set g = null
endfunction

//===========================================================================
function InitTrig_GM_Income takes nothing returns nothing
    set gg_trg_GM_Income = CreateTrigger(  )
    call TriggerRegisterTimerExpireEventBJ( gg_trg_GM_Income, udg_TmrIncome )
    call TriggerAddAction( gg_trg_GM_Income, function Trig_GM_Income_Actions )
endfunction
Crash.
01-03-2009, 05:17 PM#12
chobibo
Collapse JASS:
function Trig_GM_Income_Actions takes nothing returns nothing
    local integer i = 0
    local group g = CreateGroup()
    call BJDebugMsg("Income has started")
    loop
        call GroupEnumUnitsInRect(g, bj_mapInitialPlayableArea, Condition(function WoodFilter) )
        call AdjustPlayerStateBJ( (udg_IntBasicWood + CountUnitsInGroup(g)), Player(i), PLAYER_STATE_RESOURCE_LUMBER )
        call DestroyGroup(g)
        call GroupEnumUnitsInRect(g, bj_mapInitialPlayableArea, Condition(function GoldFilter) )
        call AdjustPlayerStateBJ( (udg_IntBasicGold + CountUnitsInGroup(g)), Player(i), PLAYER_STATE_RESOURCE_GOLD )
        call DestroyGroup(g)
    set i = i + 1
    exitwhen i == 12
    endloop
    set g = null
endfunction

You're destroying the group twice dude, try removing the highlighted line. Oh and you're trying to use an unknown handle, since g is still pointing to the destroyed handle group.
01-03-2009, 05:20 PM#13
Tide-Arc Ephemera
But... but I thought if you set a group twice without cleaning... it... I thought GUI practices could cross over perfectly. Well, at least locations aren't used enough (as in for only 1 thing if ever, GetLocationZ) to impact THAT much else.

Even with the line removed, it crashes for some reason.

EDIT!
But yeah, still used to the "don't set a group without destroying it!" practice.

EDIT!
I've also tried this.
Collapse JASS:
function Trig_GM_Income_Actions takes nothing returns nothing
    local integer i = 0
    local group g = CreateGroup()
    call BJDebugMsg("Income has started")
    loop
        call GroupEnumUnitsInRect(g, bj_mapInitialPlayableArea, Condition(function WoodFilter) )
        call AdjustPlayerStateBJ( (udg_IntBasicWood + CountUnitsInGroup(g)), Player(i), PLAYER_STATE_RESOURCE_LUMBER )
        call GroupEnumUnitsInRect(g, bj_mapInitialPlayableArea, Condition(function GoldFilter) )
        call AdjustPlayerStateBJ( (udg_IntBasicGold + CountUnitsInGroup(g)), Player(i), PLAYER_STATE_RESOURCE_GOLD )
    set i = i + 1
    exitwhen i == 12
    endloop
    call DestroyGroup(g)
    set g = null
endfunction
01-03-2009, 05:36 PM#14
Ammorth
Your conditions are causing the problem. If you want to pass the integer I, you cannot do so like you are currently doing. Instead, make global integer variable, set it to I, and then use the condition.
01-03-2009, 05:38 PM#15
chobibo
Hmm, I didn't saw it earlier, you're destroying the group that you're still using, you got it incorrectly dude, you need to call GroupClear() before using the group again, not destroy it. When you destroy a group, you remove it's capability to store units, you also make the variable point to an unknown handle since the handle id when typecasted isn't a group handle anymore, maybe that's causing the crash. Please try this code instead:
Collapse JASS:
function Trig_GM_Income_Actions takes nothing returns nothing
    local integer i = 0
    local group g = CreateGroup()
    call BJDebugMsg("Income has started")
    loop
        call GroupEnumUnitsInRect(g, bj_mapInitialPlayableArea, Condition(function WoodFilter) )
        call AdjustPlayerStateBJ( (udg_IntBasicWood + CountUnitsInGroup(g)), Player(i), PLAYER_STATE_RESOURCE_LUMBER )
        call GroupEnumUnitsInRect(g, bj_mapInitialPlayableArea, Condition(function GoldFilter) )
        call AdjustPlayerStateBJ( (udg_IntBasicGold + CountUnitsInGroup(g)), Player(i), PLAYER_STATE_RESOURCE_GOLD )
    set i = i + 1
    exitwhen i == 12
    endloop
    call DestroyGroup(g)
    set g = null
endfunction

EDIT: yes Ammorth is right, I didn't see it either, You can't pass values to boolexpr's, his suggestion is also good. Maybe that's causing the crash lol!
Wc3C.net archive — static reconstruction. No accounts, no posting.
Hosted by www.hiveworkshop.com