[Solved] Loops

[arielavi]

Hey hivers, could someone help me figure out what's wrong with this loop?

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)

 

[WaterKnight]

I just don't paste the whole library because it has about 400 lines :/

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?

 

[arielavi]

I pasted the whole code now. I use Cohadar's ABC, perfect unit indexing (PUI) and the other libraries are my systems.

 

[Tirlititi]

        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.

 

[arielavi]

Ok, I'll take a carefull look.. But it isn't better to use always global groups? Because of memory leaks and stuff?

 

[Cokemonkey11]

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.

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

 

[Troll-Brain]

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 :

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.

 

[Cokemonkey11]

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.

 

[cohadar]

I think you need to set i to zero in front of some of your loops

            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

    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/

 

[Bribe]

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.

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.

 

[Troll-Brain]

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.

 

[arielavi]

But doing that only works if you do it properly.

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.

 

[cohadar]

Btw you really need to learn to write comments in your code.

 

[Cokemonkey11]

I just realized that nothing works if its not done properly...

My implication was that it's only worth doing that if you do it a certain way.

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

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.