1. Updated Resource Submission Rules: All model & skin resource submissions must now include an in-game screenshot. This is to help speed up the moderation process and to show how the model and/or texture looks like from the in-game camera.
    Dismiss Notice
  2. DID YOU KNOW - That you can unlock new rank icons by posting on the forums or winning contests? Click here to customize your rank or read our User Rank Policy to see a list of ranks that you can unlock. Have you won a contest and still haven't received your rank award? Then please contact the administration.
    Dismiss Notice
  3. Lead your forces to battle in the 15th Techtree Contest. The call is yours, commander!
    Dismiss Notice
  4. The reforging of the races is complete. Come see the 14th Techtree Contest Results.
    Dismiss Notice
  5. It's time to choose your horse in the race - the 32nd Modeling Contest Poll is up!
    Dismiss Notice
  6. Check out the Staff job openings thread.
    Dismiss Notice
Dismiss Notice
60,000 passwords have been reset on July 8, 2019. If you cannot login, read this.

[Solved] Loops

Discussion in 'Triggers & Scripts' started by arielavi, Feb 16, 2012.

  1. arielavi

    arielavi

    Joined:
    May 23, 2010
    Messages:
    83
    Resources:
    0
    Resources:
    0
    Hey hivers, could someone help me figure out what's wrong with this loop?

    Code (vJASS):

    library Combat initializer onInit requires ABC, PUI, TextTag, CombatState

       
        globals
            private real PERIOD = 0.3
            private real COMBAT_PERIOD = 7.
           
            private group TEMPGROUP = CreateGroup()
            private group TEMPGROUP2 = CreateGroup()
           
            private rect array DUNG_RECT
            private region array DUNG_REG
            private boolean DUNG_RESET = false
            private constant integer DUNG_COUNT = 1
           
            private constant integer MAX_ARRAY = 200
        endglobals
       
        private struct UnitData
           
            //! runtextmacro PUI()
           
            real array Threat [10]
            unit array Target [10]
            real TotalThreat
            unit TargetUnit
           
            real UnitX
            real UnitY
            real UnitFace
            integer UnitType
           
            integer Index
           
            trigger AddUnit = CreateTrigger()
           
            static method Create takes unit whichUnit returns UnitData
                local UnitData d = UnitData.allocate()
               
                set d.UnitX = GetUnitX(whichUnit)
                set d.UnitY = GetUnitY(whichUnit)
                set d.UnitFace = GetUnitFacing(whichUnit)
                set d.UnitType = GetUnitTypeId(whichUnit)
                set d.Index = 0
               
                call SetTriggerStructA(d.AddUnit, d)
               
                return d
            endmethod
           
            method onDestroy takes nothing returns nothing
                call ClearTriggerStructA(.AddUnit)
                call DestroyTrigger(.AddUnit)
            endmethod
           
        endstruct
       
        private struct DungeonData
           
            integer UnitCount = 0
           
            unit array Unit [MAX_ARRAY]
           
            trigger Trig = CreateTrigger()
           
            region Reg
           
            rect Rec
           
            static method Create takes region reg, rect r returns DungeonData
                local DungeonData d = DungeonData.allocate()
                local unit u
               
                call GroupEnumUnitsInRect(TEMPGROUP, r, null)
                set d.UnitCount = 0
                loop
                    set u = FirstOfGroup(TEMPGROUP)
                    exitwhen u == null
                    set d.Unit[d.UnitCount] = u
                    set d.UnitCount = d.UnitCount + 1
                endloop
               
                set d.Reg = reg
                set d.Rec = r
               
                call SetTriggerStructA(d.Trig, d)
               
                set u = null
                return d
            endmethod
           
            method onDestroy takes nothing returns nothing
                call ClearTriggerStructA(.Trig)
                call DestroyTrigger(.Trig)
            endmethod
           
        endstruct
       
        function StartCombat takes unit whichUnit, unit targetUnit returns nothing
            local UnitData d = UnitData[whichUnit]
            local integer i = 0
            local boolean addunit = true
           
            if d == 0 then
                set d = UnitData.Create(whichUnit)
                set UnitData[whichUnit] = d
            endif
           
            loop
                if d.Target[i] == targetUnit then
                    set addunit = false
                endif
                exitwhen i >= d.Index
                set i = i + 1
            endloop
           
            if addunit then
                set d.Target[d.Index] = targetUnit
                set d.Threat[d.Index] = 0
                set d.Index = d.Index + 1
                debug call BJDebugMsg("Unit added to combat system via StartCombat() function")
                debug call BJDebugMsg("Total units in combat with current unit: " + I2S(d.Index - 1))
            endif
           
            call SetUnitInCombat(whichUnit)
            call SetUnitInCombat(targetUnit)
           
        endfunction
       
        function AddThreat takes unit whichUnit, unit targetUnit, real Threat returns nothing
            local UnitData d = UnitData[whichUnit]
            local integer i = 0
            local boolean addunit = true
           
            if d == 0 then
                call StartCombat(whichUnit, targetUnit)
                debug call BJDebugMsg("Unit data not found. Creating data via AddThreat() function.")
            else
                loop
                    if d.Target[i] == targetUnit then
                        set addunit = false
                    endif
                    exitwhen i >= d.Index
                    set i = i + 1
                endloop
                if addunit then
                    set d.Target[d.Index] = targetUnit
                    set d.Threat[d.Index] = 0
                    set d.Index = d.Index + 1
                    debug call BJDebugMsg("Unit added to combat system via AddThreat() function.")
                endif
                loop
                    if d.Target[i] == targetUnit then
                        set d.Threat[i] = d.Threat[i] + Threat
                        debug call BJDebugMsg("Threat added succesfully.")
                    endif
                    exitwhen i >= d.Index
                    set i = i + 1
                endloop
            endif
        endfunction
       
        private function CombatInit takes nothing returns nothing
       
            local unit whichUnit = GetTriggerUnit()
            local unit attackingUnit = GetAttacker()
            local player whichPlayer = GetOwningPlayer(whichUnit)
           
            local UnitData d
            local integer i = 0
            local boolean addUnit = true
           
            call SetUnitInCombat(whichUnit)
            call SetUnitInCombat(attackingUnit)
           
            if whichPlayer == Player(PLAYER_NEUTRAL_AGGRESSIVE) then
                set d = UnitData[whichUnit]
                if d == 0 then
                    set d = UnitData.Create(whichUnit)
                    set UnitData[whichUnit] = d
                    debug call BJDebugMsg("New unit data created via CombatInit() function.")
                    set i = 0
                    loop
                        if d.Target[i] == attackingUnit then
                            set addUnit = false
                        endif
                        set i = i + 1
                        exitwhen i >= d.Index
                    endloop
                else
                    set i = 0
                    loop
                        if d.Target[i] == attackingUnit then
                            set addUnit = false
                        endif
                        set i = i + 1
                        exitwhen i >= d.Index
                    endloop
                endif
                if addUnit then
                    set d.Target[d.Index] = attackingUnit
                    set d.Threat[d.Index] = 0.
                    set d.Index = d.Index + 1
                    debug call BJDebugMsg("Hero added to combat system via CombatInit() function.")
                endif
            else
                set d = UnitData[attackingUnit]
                if d == 0 then
                    set d = UnitData.Create(attackingUnit)
                    set UnitData[attackingUnit] = d
                    debug call BJDebugMsg("New unit data created via CombatInit() function.")
                    set i = 0
                    loop
                        if d.Target[i] == whichUnit then
                            set addUnit = false
                        endif
                        set i = i + 1
                        exitwhen i >= d.Index
                    endloop
                else
                    set i = 0
                    loop
                        if d.Target[i] == whichUnit then
                            set addUnit = false
                        endif
                        set i = i + 1
                        exitwhen i >= d.Index
                    endloop
                endif
                if addUnit then
                    set d.Target[d.Index] = whichUnit
                    set d.Threat[d.Index] = 0.
                    set d.Index = d.Index + 1
                    debug call BJDebugMsg("Hero added to combat system via CombatInit() function.")
                endif
            endif
           
            if d == 0 then
                debug call BJDebugMsg("c|ffff0000Unit data not found. Check CombatInit() function.")
            endif
           
        endfunction
       
        private function Reset takes nothing returns nothing
            local trigger t = GetTriggeringTrigger()
            local integer i = 0
            local DungeonData d = GetTriggerStructA(t)
            local UnitData d1
            local unit u
           
            set DUNG_RESET = true
            call GroupEnumUnitsInRect(TEMPGROUP, d.Rec, null)
            debug call BJDebugMsg("Checking if there are units in dungeons.")
            loop
                set u = FirstOfGroup(TEMPGROUP)
                exitwhen u == null
                call GroupRemoveUnit(TEMPGROUP, u)
                if u != null and GetPlayerController(GetOwningPlayer(u)) == MAP_CONTROL_USER then
                    set DUNG_RESET = false
                endif
            endloop
           
            if DUNG_RESET then
                debug call BJDebugMsg("No unit was found in dungeons. Reset initiated.")
                loop
                    if d.Unit[i] == null or IsUnitType(d.Unit[i], UNIT_TYPE_DEAD) then
                        set d1 = UnitData[d.Unit[i]]
                        call RemoveUnit(d.Unit[i])
                        set d.Unit[i] = CreateUnit(Player(PLAYER_NEUTRAL_AGGRESSIVE), d1.UnitType, d1.UnitX, d1.UnitY, d1.UnitFace)
                        call IssueImmediateOrder(d.Unit[i], "stop")
                        debug call BJDebugMsg("Unit created.")
                    else
                        call SetUnitFacing(d.Unit[i], d1.UnitFace)
                        call SetUnitX(d.Unit[i], d1.UnitX)
                        call SetUnitY(d.Unit[i], d1.UnitY)
                        debug call BJDebugMsg("Unit repositioned.")
                    endif
                    exitwhen i >= d.UnitCount - 1
                    set i = i + 1
                endloop
                debug call BJDebugMsg("Dungeon reset.")
            else
                debug call BJDebugMsg("There are player units in dungeons. Leave dungeons to reset them.")
            endif
            set DUNG_RESET = false
        endfunction
           
       
        private function CheckCombatState takes nothing returns nothing
            local player p = GetTriggerPlayer()
            local integer i = GetPlayerId(p)
            local unit u = Hero[i]
            if GetUnitCombatState(u) then
                debug call BJDebugMsg("Unit is in combat.")
            else
                debug call BJDebugMsg("Unit is not in combat")
            endif
        endfunction
       
        private function AddGroup takes nothing returns nothing
            call GroupAddUnit(TEMPGROUP, GetEnumUnit())
        endfunction
       
        private function ReturnCheck takes nothing returns nothing
       
        endfunction
       
        private function onInit takes nothing returns nothing
       
            local unit u
            local real r
            local integer ut
            local integer i
            local trigger reset = CreateTrigger()
            local trigger combat = CreateTrigger()
           
            local DungeonData d
            local UnitData d1
           
            local trigger test = CreateTrigger()
           
            call TriggerRegisterPlayerChatEvent(test, Player(0), "-combat", true)
            call TriggerAddAction(test, function CheckCombatState)
           
            call TriggerRegisterAnyUnitEventBJ(combat, EVENT_PLAYER_UNIT_ATTACKED)
            call TriggerAddAction(combat, function CombatInit)
           
            set DUNG_RECT[0] = gg_rct_dung01
           
            set i = 0
            loop
                set DUNG_REG[i] = CreateRegion()
                call RegionAddRect(DUNG_REG[i], DUNG_RECT[i])
                set d = DungeonData.Create(DUNG_REG[i], DUNG_RECT[i])
                call TriggerRegisterPlayerChatEvent(d.Trig, Player(0), "-reset", true)
                call TriggerAddAction(d.Trig, function Reset)
                call GroupEnumUnitsInRect(TEMPGROUP, DUNG_RECT[i], null)
               
                loop
                    set u = FirstOfGroup(TEMPGROUP)
                    exitwhen u == null
                    call GroupRemoveUnit(TEMPGROUP, u)
                    set d1 = UnitData.Create(u)
                    set UnitData[u] = d1
                endloop
               
                set i = i + 1
                exitwhen i >= DUNG_COUNT
            endloop
                   
           
        endfunction

    endlibrary
     


    Could it possibly be the
    exitwhen u == null
    or something?

    What actually happens is: The structs inside the loop are not created (I made another function to check that) and if I put another call AFTER the
    endloop
    , it just doesn't run. And also, the trigger inside the loop isn't called either (I typed "-reset" in-game and it doens't even call the function)

    EDIT: everything is ok (the vars, globals, functions, structs: I triple-checked that) and the jasshelper doesn't point any error there, I just don't paste the whole library because it has about 400 lines :/
    EDIT: posted the whole library now.

    There are "useless" functions on the library yet, I pretend to use them when I code them (those under development duh)
     
    Last edited: Feb 16, 2012
  2. WaterKnight

    WaterKnight

    Joined:
    Aug 18, 2009
    Messages:
    4,034
    Resources:
    5
    Maps:
    1
    Tutorials:
    4
    Resources:
    5
    Does not matter, post it. You can determine where the code halts by inserting debug messages. Maybe TEMPGROUP is not initialized, maybe it does not pick units, maybe the create method of UnitData crashes. To what does UnitData= convert? Why do you even need to attach the data to the unit there, why isn't that done in the create method already?
     
  3. arielavi

    arielavi

    Joined:
    May 23, 2010
    Messages:
    83
    Resources:
    0
    Resources:
    0
    I pasted the whole code now. I use Cohadar's ABC, perfect unit indexing (PUI) and the other libraries are my systems.
     
  4. Tirlititi

    Tirlititi

    Joined:
    Jul 11, 2010
    Messages:
    396
    Resources:
    12
    Models:
    6
    Maps:
    2
    Spells:
    3
    JASS:
    1
    Resources:
    12
    Code (vJASS):
            static method Create takes region reg, rect r returns DungeonData
                local DungeonData d = DungeonData.allocate()
                local unit u
               
                call GroupEnumUnitsInRect(TEMPGROUP, r, null)
                set d.UnitCount = 0
                loop
                    set u = FirstOfGroup(TEMPGROUP)
                    exitwhen u == null
                    set d.Unit[d.UnitCount] = u
                    set d.UnitCount = d.UnitCount + 1
                endloop

    You are not removing units from the group, thus making an infinite loop.

    You should also be carefull with your variable TEMPGROUP and not use it with functions that may call each other. I think, there is no problem for now but if
    UnitData.Create
    used it, for instance, it'd bug and that wouldn't be so easy to locate the error.
    Just saying.
     
  5. arielavi

    arielavi

    Joined:
    May 23, 2010
    Messages:
    83
    Resources:
    0
    Resources:
    0
    Ok, I'll take a carefull look.. But it isn't better to use always global groups? Because of memory leaks and stuff?
     
  6. Cokemonkey11

    Cokemonkey11

    Wurst Reviewer

    Joined:
    May 9, 2006
    Messages:
    3,234
    Resources:
    18
    Tools:
    1
    Maps:
    5
    Spells:
    3
    Tutorials:
    2
    JASS:
    7
    Resources:
    18
    It's better to have 1 global group which is never destroyed, and only created on initialization, because that's faster.

    But doing that only works if you do it properly.

    Here's an example, which kills all units in the map every 60 seconds.

    Code (vJASS):



    scope example initializer i
        globals
            private group grp=CreateGroup()
        endglobals
       
        private function f takes nothing returns boolean
            call KillUnit(GetFilterUnit())
            return false
        endfunction
       
        private function c takes nothing returns boolean
            call GroupEnumUnitsInRect(grp,bj_mapInitialPlayableArea,Filter(function f))
            return false
        endfunction
       
        private function i takes nothing returns nothing
            local trigger t=CreateTrigger()
            call TriggerRegisterTimerEvent(t,60.,true)
            call TriggerAddCondition(t,Condition(function c))
        endfunction
    endscope
     
     
  7. Troll-Brain

    Troll-Brain

    Joined:
    Apr 27, 2008
    Messages:
    2,372
    Resources:
    1
    JASS:
    1
    Resources:
    1
    It has be proven that a GroupEnum (null filter) + FirstOfGroup/GroupRemoveUnit loop is faster than a GroupEnum(not null filter).
    It's simply because each time the filter is evaluated a new thread is opened.

    Also beyond the speed, it's always nice to don't split the code for a such thing, sure in your example it's not highlighted, but if you have more arguments you will have to use globals.
    Now the con is the verbosity of the loop, but for that you can use the last jasshelper by cohadar :

    Code (vJASS):
    globals
        group whichGroup = CreateGroup()
    endglobals

    local unit u

    call GroupEnum...(whichGroup,null)
    for u in whichGroup
        // here you are in FirsOfGroup/GroupRemoveUnit loop
    endfor


    And no, this time i will not submit a test code or a link, you can test it by yourself.
     
  8. Cokemonkey11

    Cokemonkey11

    Wurst Reviewer

    Joined:
    May 9, 2006
    Messages:
    3,234
    Resources:
    18
    Tools:
    1
    Maps:
    5
    Spells:
    3
    Tutorials:
    2
    JASS:
    7
    Resources:
    18
    Sorry troll-brain, but I did that for over a month before people I trust a lot more than you corrected me on it.

    You're not going to change something I learned from PurplePoot nearly two years ago without proving that it:

    Is faster (using timer calculations) on a small scale - 1-10 units
    Is faster (the same way) on a large scale - 100-1000 units
    Doesn't leak (null filter)

    Next you're going to tell me that something in GUI is faster than my script.
     
  9. cohadar

    cohadar

    Joined:
    Jun 16, 2007
    Messages:
    234
    Resources:
    0
    Resources:
    0
    I think you need to set i to zero in front of some of your loops
    Code (vJASS):

                set i = 0   // <-----<< you are missing this in lots of places
                loop
                    if d.Target[i] == targetUnit then
                        set addunit = false
                    endif
                    exitwhen i >= d.Index
                    set i = i + 1
                endloop
     


    or you could simply use new for loops
    Code (vJASS):

        for i = 0 to d.Index
            if d.Target[i] == targetUnit then
                set addunit = false
            endif
        endfor
     


    http://www.hiveworkshop.com/forums/warcraft-editing-tools-277/jasshelper-2011-12-19-a-208802/
     
  10. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,198
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    You're working with incredibly outdated information. There are hundreds of posts explaining that the null filter leak was corrected in 1.24, and there are dozens of posts explaining that FirstOfGroup loops are significantly faster in every scenario.
     
  11. Troll-Brain

    Troll-Brain

    Joined:
    Apr 27, 2008
    Messages:
    2,372
    Resources:
    1
    JASS:
    1
    Resources:
    1
    It's also obvious that if it is not done on an heavy loop, or/and inside a quick periodic timer the user will hardly see any difference even with a wood pc.
    But for me the speed is not the main feature here, it's just a plus.
    The best pro is to keep the code in the scope where GroupEnum is called.

    Also, i've gave a reason why it's faster i'm not saying random senseless shit such as "GUI is faster than your script".
    Now, you don't have to believe me, you just can test it by yourself, it's your problem if you don't believe in me that much, not mine.
     
  12. arielavi

    arielavi

    Joined:
    May 23, 2010
    Messages:
    83
    Resources:
    0
    Resources:
    0
    I just realized that nothing works if its not done properly...

    I'm reviewing the whole library, recoding some loops, added globals and if it works I'll post it here.

    EDIT: basically, it worked, I'm dealing with minor bugs now (with the combat system, probably the main problem was the loop inside the struct (which had an infinite loop and double-usage of the sabe global).

    BTW, thanks for all the help. If I get in trouble again I'll post here :)

    EDITEDIT: Solved.
     
    Last edited: Feb 17, 2012
  13. cohadar

    cohadar

    Joined:
    Jun 16, 2007
    Messages:
    234
    Resources:
    0
    Resources:
    0
    Btw you really need to learn to write comments in your code.
     
  14. Cokemonkey11

    Cokemonkey11

    Wurst Reviewer

    Joined:
    May 9, 2006
    Messages:
    3,234
    Resources:
    18
    Tools:
    1
    Maps:
    5
    Spells:
    3
    Tutorials:
    2
    JASS:
    7
    Resources:
    18
    My implication was that it's only worth doing that if you do it a certain way.

    I hope you won't do it the way I said to, because if bribe and troll-brain are right (and I assume they are) then this method is basically obsolete.

    Anyway, glad you solved the issue.