[vJASS] [Solved] Unexpected null owner when a unit dies

[Zoreal]

For some reason setNewOwner is called with null when two units die at the same time, possibly even the same frame.
One of the units that dies is lastDefender triggering lastDefenderDeathOrLoad
This presumeably turns the second unit into the lastDefender which is killed by a unit that has the 'Avul' ability at the same time

The result is that the new owner is colored black and hostile to everyone and the enterTrigger is disabled.

I check if the unit is null every time i call setNewOwner(GetOwningPlyer(unit)).
So my only working theory is, that GetOwningPlayer(not null unit) somehow still returns null as a player.
Is this possible?

        method enter takes unit enteringUnit returns nothing
            if(enteringUnit != null or GetUnitAbilityLevel(enteringUnit, 'Avul') == 1 or not GlobalFunctions_UnitAlive(enteringUnit)) then
                call setLastDefender(enteringUnit)
                call EnableTrigger(leaveTrigger)
                call DisableTrigger(enterTrigger)
                
                call setNewOwner(GetOwningPlayer(enteringUnit))
            endif
        endmethod
        
        method lastDefenderDeathOrLoad takes unit killer returns nothing
            local group unitsInOuterRect = null
            local boolean isProvinceEmpty
            local unit curUnit
            
            if(checkForProvinceEmptyAndSetLastDefender(false)) then
                // Check if killer is eligable for conquering a province
                if(killer == null or GetUnitAbilityLevel(killer, 'Avul') == 1 or not GlobalFunctions_UnitAlive(killer)) then
                    set unitsInOuterRect = CreateGroup()
                    call GroupEnumUnitsInRect(unitsInOuterRect, outerRect, null)
                    
                    set isProvinceEmpty = true
                    loop
                        set curUnit = FirstOfGroup(unitsInOuterRect)
                        exitwhen curUnit == null
                        call GroupRemoveUnit(unitsInOuterRect, curUnit)
                        if(GlobalFunctions_UnitAlive(curUnit) and GetUnitAbilityLevel(curUnit,'Avul') != 1) then
                            call setLastDefender(curUnit)
                            
                            call setNewOwner(GetOwningPlayer(curUnit))
                            set isProvinceEmpty = false
                            exitwhen true
                        endif
                    endloop
                    // Province is completely empty and the killer cannot conquer the province
                    if(isProvinceEmpty) then
                        call setLastDefender(null)
                        
                        call EnableTrigger(enterTrigger)
                        call DisableTrigger(leaveTrigger)
                        
                        call setNewOwner(Player(25))
                    endif
                else
                    call setLastDefender(killer)
                    call setNewOwner(GetOwningPlayer(killer))
                endif
            endif
            
            if(unitsInOuterRect != null) then
                call DestroyGroup(unitsInOuterRect)
                set unitsInOuterRect = null
            endif
            set curUnit = null
        endmethod
       
        method setNewOwner takes player newOwner returns nothing
            local boolean ownerChanged = GetOwningPlayer(barracks) != newOwner
            set callbackNewOwner = newOwner
            call ForGroup(allBuildings, function setOwner)
            if ownerChanged then
                call parentRegion.provinceOwnershipChanged()
            endif
        endmethod

Here is the full script file as a reference.

scope Provinces

    globals
        public Province array startingProvinces
        public integer startingProvincesLength = 0
        
        private hashtable provinceReferences = InitHashtable()
        private player callbackNewOwner = Player(10)
        private player paramFilterGetUnitsInRectOfNonDefenderPlayer = Player(10)
    endglobals
    
    private function setOwner takes nothing returns nothing
        call SetUnitOwner(GetEnumUnit(), callbackNewOwner, true)
    endfunction
    
    private function getProvince takes trigger triggeringTrigger returns Province
        return LoadInteger(provinceReferences, GetHandleId(triggeringTrigger),0)
    endfunction

    private function lastDefenderDeath takes nothing returns nothing
        local Province province
        set province = getProvince(GetTriggeringTrigger())
        call province.lastDefenderDeathOrLoad(GetKillingUnit())
    endfunction
    
    private function lastDefenderLoad takes nothing returns nothing
        local Province province
        set province = getProvince(GetTriggeringTrigger())
        call province.lastDefenderDeathOrLoad(null)
    endfunction
    
    private function enterRegion takes nothing returns nothing
        local Province province
        set province = getProvince(GetTriggeringTrigger())
        call province.enter(GetTriggerUnit())
    endfunction
    
    private function leaveRegion takes nothing returns nothing
        local Province province
        set province = getProvince(GetTriggeringTrigger())
        call province.leave(GetTriggerUnit())
    endfunction
    
    private function filterGetUnitsInRectOfNonDefender takes nothing returns boolean
        return GetOwningPlayer(GetEnumUnit()) != paramFilterGetUnitsInRectOfNonDefenderPlayer
    endfunction
    
    struct Province
        private boolean hasStartingUnit = false
        
        private rect innerRect
        private rect outerRect
        private unit barracks
        private unit lastDefender
        private group allBuildings
        private location circleOfPowerCenter
        private trigger enterTrigger
        private trigger deathTrigger
        private trigger loadTrigger
        private trigger leaveTrigger
        private Region parentRegion
        
        static method create takes unit circleOfPower, group allBuildings, unit startingUnit returns Province
            local Province this = Province.allocate()
            local region innerRegion
            local integer centerX
            local integer centerY
            local integer i = 0
            
            set this.circleOfPowerCenter = GetUnitLoc(circleOfPower)
            set centerX = R2I(GetLocationX(circleOfPowerCenter))
            set centerY = R2I(GetLocationY(circleOfPowerCenter))
            
            set this.innerRect = Rect(centerX - 100, centerY - 100, centerX + 100, centerY + 100)
            set this.outerRect = Rect(centerX - 150, centerY - 150, centerX + 150, centerY + 150)
            
            set this.barracks = FirstOfGroup(allBuildings)
            set this.allBuildings = allBuildings
            
            set innerRegion = CreateRegion()
            call RegionAddRect(innerRegion, innerRect)
            
            set this.enterTrigger = CreateTrigger()
            call TriggerRegisterEnterRegion(enterTrigger, innerRegion, null)
            call TriggerAddAction( enterTrigger, function enterRegion )
            
            set this.leaveTrigger = CreateTrigger()
            call TriggerRegisterLeaveRegion(leaveTrigger, innerRegion, null)
            call TriggerAddAction( leaveTrigger, function leaveRegion )
            if (startingUnit != null) then
                call this.setLastDefender(startingUnit)
                call DisableTrigger(enterTrigger)
                set this.hasStartingUnit = true
            else
                set startingProvinces[startingProvincesLength] = this
                set startingProvincesLength = startingProvincesLength + 1
                call DisableTrigger(leaveTrigger)
            endif
            
            call SaveInteger(provinceReferences, GetHandleId(leaveTrigger),0,this)
            call SaveInteger(provinceReferences, GetHandleId(enterTrigger),0,this)
            
            set innerRegion = null
            return this
        endmethod
        
        method init takes player startingPlayer returns nothing
            local unit startingUnit
            
            set startingUnit = CreateUnit(startingPlayer, 'e001', GetLocationX(circleOfPowerCenter), GetLocationY(circleOfPowerCenter), 0)
            call setLastDefender(startingUnit)
            call DisableTrigger(enterTrigger)
            call EnableTrigger(leaveTrigger)
            
            call setNewOwner(startingPlayer)
            call Power_adjustPlayerPower(startingPlayer, 1)
            
            set startingUnit = null
        endmethod
        
        method enter takes unit enteringUnit returns nothing
            if(enteringUnit != null or GetUnitAbilityLevel(enteringUnit, 'Avul') == 1 or not GlobalFunctions_UnitAlive(enteringUnit)) then
                call setLastDefender(enteringUnit)
                call EnableTrigger(leaveTrigger)
                call DisableTrigger(enterTrigger)
                
                call setNewOwner(GetOwningPlayer(enteringUnit))
            endif
        endmethod
        
        method leave takes unit triggeringUnit returns nothing
            local group enemiesInRect = null
            local unit curUnit = null
            local boolean provinceWasForfeit = false
            
            if(triggeringUnit != lastDefender) then
                return
            endif
            
            if(checkForProvinceEmptyAndSetLastDefender(true)) then
                set enemiesInRect = CreateGroup()
                set paramFilterGetUnitsInRectOfNonDefenderPlayer = GetOwningPlayer(triggeringUnit)
                call GroupEnumUnitsInRect(enemiesInRect, innerRect, function filterGetUnitsInRectOfNonDefender)
                if IsUnitGroupEmptyBJ(enemiesInRect) then
                    call SetUnitPositionLoc(triggeringUnit, circleOfPowerCenter)
                else
                    loop
                        set curUnit = FirstOfGroup(enemiesInRect)
                        exitwhen curUnit == null
                        call GroupRemoveUnit(enemiesInRect, curUnit)
                        if(GlobalFunctions_UnitAlive(curUnit) and GetUnitAbilityLevel(curUnit,'Avul') != 1) then
                            call setLastDefender(curUnit)
                            
                            call setNewOwner(GetOwningPlayer(curUnit))
                            set provinceWasForfeit = true
                            exitwhen true
                        endif
                    endloop
                    
                    if not provinceWasForfeit then
                        call SetUnitPositionLoc(triggeringUnit, circleOfPowerCenter)
                    endif
                endif
            endif
            
            if enemiesInRect != null then
                call DestroyGroup(enemiesInRect)
                set enemiesInRect = null
            endif
            set curUnit = null
        endmethod
        
        method lastDefenderDeathOrLoad takes unit killer returns nothing
            local group unitsInOuterRect = null
            local boolean isProvinceEmpty
            local unit curUnit
            
            if(checkForProvinceEmptyAndSetLastDefender(false)) then
                // Check if killer is eligable for conquering a province
                if(killer == null or GetUnitAbilityLevel(killer, 'Avul') == 1 or not GlobalFunctions_UnitAlive(killer)) then
                    set unitsInOuterRect = CreateGroup()
                    call GroupEnumUnitsInRect(unitsInOuterRect, outerRect, null)
                    
                    set isProvinceEmpty = true
                    loop
                        set curUnit = FirstOfGroup(unitsInOuterRect)
                        exitwhen curUnit == null
                        call GroupRemoveUnit(unitsInOuterRect, curUnit)
                        if(GlobalFunctions_UnitAlive(curUnit) and GetUnitAbilityLevel(curUnit,'Avul') != 1) then
                            call setLastDefender(curUnit)
                            
                            call setNewOwner(GetOwningPlayer(curUnit))
                            set isProvinceEmpty = false
                            exitwhen true
                        endif
                    endloop
                    // Province is completely empty and the killer cannot conquer the province
                    if(isProvinceEmpty) then
                        call setLastDefender(null)
                        
                        call EnableTrigger(enterTrigger)
                        call DisableTrigger(leaveTrigger)
                        
                        call setNewOwner(Player(25))
                    endif
                else
                    call setLastDefender(killer)
                    call setNewOwner(GetOwningPlayer(killer))
                endif
            endif
            
            if(unitsInOuterRect != null) then
                call DestroyGroup(unitsInOuterRect)
                set unitsInOuterRect = null
            endif
            set curUnit = null
        endmethod
        
        method setLastDefender takes unit newLastDefender returns nothing
            set lastDefender = newLastDefender
            
            if(deathTrigger != null) then
                call DestroyTrigger(deathTrigger)
            endif
            
            if(loadTrigger != null) then
                call DestroyTrigger(loadTrigger)
            endif
            
            if(lastDefender != null) then
                set deathTrigger = CreateTrigger()
                call TriggerRegisterUnitEvent(deathTrigger, lastDefender, EVENT_UNIT_DEATH)
                call TriggerAddAction( deathTrigger, function lastDefenderDeath )
                call SaveInteger(provinceReferences, GetHandleId(deathTrigger), 0, this)
                
                set loadTrigger = CreateTrigger()
                call TriggerRegisterUnitEvent( loadTrigger, lastDefender, EVENT_UNIT_LOADED )
                call TriggerAddAction( loadTrigger, function lastDefenderLoad )
                call SaveInteger(provinceReferences, GetHandleId(loadTrigger), 0, this)
                
                if(not RectContainsUnit(innerRect, lastDefender)) then
                    call SetUnitPositionLoc(lastDefender, circleOfPowerCenter)
                endif
            endif
        endmethod
        
        method checkForProvinceEmptyAndSetLastDefender takes boolean useInnerRect returns boolean
            local group defendingUnits = CreateGroup()
            local unit curUnit
            local boolean isProvinceEmpty
            
            set bj_groupEnumOwningPlayer = GetOwningPlayer(barracks)
            if (useInnerRect) then
                call GroupEnumUnitsInRect(defendingUnits, innerRect, filterGetUnitsInRectOfPlayer)
            else
                call GroupEnumUnitsInRect(defendingUnits, outerRect, filterGetUnitsInRectOfPlayer)
            endif
            
            set isProvinceEmpty = true
            loop
                set curUnit = FirstOfGroup(defendingUnits)
                exitwhen curUnit == null
                call GroupRemoveUnit(defendingUnits, curUnit)
                
                if(curUnit != lastDefender and GlobalFunctions_UnitAlive(curUnit) and GetUnitAbilityLevel(curUnit,'Avul') != 1) then
                    call setLastDefender(curUnit)
                    
                    set isProvinceEmpty = false
                    exitwhen true
                endif
            endloop
        
            call DestroyGroup(defendingUnits)
            set defendingUnits = null
            set curUnit = null
            return isProvinceEmpty
        endmethod
        
        method setNewOwner takes player newOwner returns nothing
            local boolean ownerChanged = GetOwningPlayer(barracks) != newOwner
            set callbackNewOwner = newOwner
            call ForGroup(allBuildings, function setOwner)
            if ownerChanged then
                call parentRegion.provinceOwnershipChanged()
            endif
        endmethod
        
        method isStronghold takes nothing returns boolean
            return hasStartingUnit
        endmethod
        
        method getOwner takes nothing returns player
            return GetOwningPlayer(barracks)
        endmethod
        
        method setParentRegion takes Region newParentRegion returns nothing
            set parentRegion = newParentRegion
        endmethod
        
    endstruct
    
    // no need to destroy groups, they are globally static and therefore no leaks
    public function createProvince takes nothing returns nothing
        local Province newProvince = Province.create(udg_ParamProvinceCircle, udg_ParamProvinceBuildings, udg_ParamProvinceStartingUnit)
        
        set Regions_paramRegionProvinces[Regions_paramRegionProvincesLength] = newProvince
        set Regions_paramRegionProvincesLength = Regions_paramRegionProvincesLength + 1
        
        // reset the global parameter so the next iteration can begin without destroying or clearing something in gui
        set udg_ParamProvinceCircle = null
        set udg_ParamProvinceBuildings = CreateGroup()
        set udg_ParamProvinceStartingUnit = null
    endfunction
endscope

Many thanks in advance as i know that this is a lot of stuff to go through

 

[Tsaras]

I didn't look at this thoroughly, with a quick glance I see this if statement:

// Check if killer is eligable for conquering a province
if(killer == null or GetUnitAbilityLevel(killer, 'Avul') == 1 or not GlobalFunctions_UnitAlive(killer)) then
    set unitsInOuterRect = CreateGroup()
    call GroupEnumUnitsInRect(unitsInOuterRect, outerRect, null)
   
    set isProvinceEmpty = true
    loop
        set curUnit = FirstOfGroup(unitsInOuterRect)
        exitwhen curUnit == null
        call GroupRemoveUnit(unitsInOuterRect, curUnit)
        if(GlobalFunctions_UnitAlive(curUnit) and GetUnitAbilityLevel(curUnit,'Avul') != 1) then
            call setLastDefender(curUnit)
           
            call setNewOwner(GetOwningPlayer(curUnit))
            set isProvinceEmpty = false
            exitwhen true
        endif
    endloop
    // Province is completely empty and the killer cannot conquer the province
    if(isProvinceEmpty) then
        call setLastDefender(null)
       
        call EnableTrigger(enterTrigger)
        call DisableTrigger(leaveTrigger)
       
        call setNewOwner(Player(25))
    endif
else
    call setLastDefender(killer)
    call setNewOwner(GetOwningPlayer(killer))
endif

In the else block you call setNewOwner(GetOwningPlayer(killer)) but the condition you have in the if statement does not guarantee that killer will not be null.

if(killer == null or GetUnitAbilityLevel(killer, 'Avul') == 1 or not GlobalFunctions_UnitAlive(killer)) then

Could be a possible culprit. I would fill it with debug messages to see what's going on.

 

[Zoreal]

Thanks for the answer.

The killer variable is only used in the else block. But given the if statement killer should never be null there, unless i am missing something.

                if(killer == null or GetUnitAbilityLevel(killer, 'Avul') == 1 or not GlobalFunctions_UnitAlive(killer)) then
                    ...
                else
                    call setLastDefender(killer)
                    call setNewOwner(GetOwningPlayer(killer))
                endif
            endif

Unfortunately i cannot reproduce the problem and it happens very rarely. So adding a debug message would only help if a player would make a screenshot and send it to me. Maybe i will add a big red text telling the player to screenshot some printed debug info and send the screenshot to me.

I asked ChatGPT more out of curiosty what an AI would say and he suggested that killer might be in an inconsistent state while dying causing GetOwningPlayer to return null. Is this just a AI hallucination or has someone experienced that?

 

[Tsaras]

You are right scratch my previous comment I looked at it hastily.

ChatGPT can certainly give imaginative answers. I haven't experienced an issue with the killer during an event myself.

 

[Uncle]

I haven't looked deeply into this, but note that...

1. Two or more units dying in the same frame can be very common, ie: Fan of Knives killing 2 Peasants at once.

2. The "A unit Dies" Event breaks the standard trigger queue rules. The Conditions/Actions associated with this Event will happen immediately, stepping in front of the queue as opposed to going to the back of the line. It's very easy to experience issues related to this Event, which is why it's wise to create one central "Dies" Event that's used universally so that you can order things in a way that won't cause conflicts.

3. There's also the case of units being Removed from the game which will make them null (they can remain referenceable for the remainder of the frame if I remember correctly). Sources of this include: Actions, Death Types, Abilities.

 

[Zoreal]

I am not creating the dies event myself. The units simply die by beeing attacked.

But this is all very interesting.

I create a death trigger for every lastDefender registering on EVENT_UNIT_DEATH), which can change dynamically. How would i create a central dies trigger from that?
Something like a generic deathTrigger with EVENT_PLAYER_UNIT_DEATH event and a self managed event bus system?

 

[Uncle]

Note that I can't promise that what I said has anything to do with the issue you're facing. Hopefully someone else can chime in after taking a closer look at your code.

I am not creating the dies event myself. The units simply die by beeing attacked.

If your map uses the "A unit Dies" Event in other Triggers, then the Actions from those Triggers could be the cause of the issues you're facing here.

For example:

• Events
  • Unit - A unit Dies
• Conditions
• Actions
  • Unit - Remove (Dying unit) from the game

^ This could run first and maybe null the Unit.

How would i create a central dies trigger from that?
Something like a generic deathTrigger with EVENT_PLAYER_UNIT_DEATH event and a self managed event bus system?

Yeah, that sounds correct. Here's a GUI example that could probably work:

• Events
  • Unit - A unit Dies
• Conditions
• Actions
  • Set Variable Event_Dies_Unit = (Dying unit)
  • Set Variable Event_Dies_Owner = (Owner of Event_Dies_Unit)
  • Set Variable Event_Dies_Killer = (Killing unit)
  • Set Variable Event_Dies_Killer_Owner = (Owner of Event_Dies_Killer)
  • For each (Integer X) from 1 to Event_Dies_Count do (Actions)
    • Loop - Actions
      • Trigger - Run Event_Dies_Trigger[X] (checking conditions)

You now have all of the information cached in these Variables which could easily be referenced elsewhere.

You would assign Triggers to the Event_Dies_Trigger Array in the order that you want them to execute. Event_Dies_Count is how many Triggers you have assigned in total, which you would increase by 1 at the time of assigning a new Trigger.

Note that if you Kill another unit from within a Event_Dies_Trigger it would screw everything up. Perhaps you could delay the killing by 1 frame or just make a rule that you won't do any killing/damaging from within them.

 

[Zoreal]

Thanks for the help so far.

I do not have triggers that cause damage, death or removal of a unit.
Additionally i check if the killing and dying unit is null and adjust accordingly. But for some reason that check does not grasp.

 

[Zoreal]

I did some debugging.

I forced via trigger to have a unit outside the province kill the lastDefender and then let another unit kill the new lastDefender

The resulting trigger order was as followed:
Kill trigger for the original lastDefender:

• made the second unit lastdefender
• teleported the killer into the province

Kill trigger for the new lastDefender:

• the killing unit cannot take the province, so it becomes neutral
• this trigger enables a enter region trigger for the province

Enter trigger for already dead new lastDefender:

• the dead unit passes the checks and becomes lastDefender as second time
• For some reason the owner of the dying unit has changed to Player(25) causing the owner to become Player(25)

The Problem was the check in the enter method:

        method enter takes unit enteringUnit returns nothing
            if(enteringUnit != null or GetUnitAbilityLevel(enteringUnit, 'Avul') == 1 or not GlobalFunctions_UnitAlive(enteringUnit)) then
                call setLastDefender(enteringUnit)
                call EnableTrigger(leaveTrigger)
                call DisableTrigger(enterTrigger)
                
                call setNewOwner(GetOwningPlayer(enteringUnit))
            endif
        endmethod

As long as the unit was not null the other checks did not grasp allowing a dead unit to enter and take the province. Fixing the if statement fixed the bug.

There were two things that threw me off here:

• The owner of a dying unit is apparently Player 25.
• Moving a unit into a region and then activating a trigger for entering into that region within the same frame fires the entering trigger

 

[Uncle]

I recall having issues with Regions before, perhaps related.

[Solved] - Any better solution for the detect leaving region?

Description: When unit leaves the region, triggering units orders center of the leaving region. OrmanRegen Events Unit - A unit leaves NcUstCheck6 Unit - A unit leaves NcUstCheck5 Unit - A unit leaves NcUstCheck4 Unit - A unit leaves NcUstCheck3...

www.hiveworkshop.com