Not Detecting Dead Heroes

09-18-2008, 10:45 PM

I've coded an ability that will perform a certain action if an enemy hero matching conditions is within range of the caster's target. If there is no hero, a different action is executed. To determine whether there is an enemy hero in range I've used a FirstOfGroup loop:

JASS:

        method isHero takes nothing returns boolean
            return GetUnitState(this.group_unit, UNIT_STATE_LIFE) >= 0 and not IsUnitType(this.group_unit, UNIT_TYPE_STRUCTURE) and IsUnitVisible(this.group_unit, GetOwningPlayer(this.caster)) and IsUnitType(this.group_unit, UNIT_TYPE_HERO) and IsUnitEnemy(this.group_unit, GetOwningPlayer(this.caster))
        endmethod

        method groupCycle takes nothing returns nothing
            local boolean found=false
            call GroupEnumUnitsInRange(Filter_Group, this.allyX, this.allyY, TARGET_RADIUS, null)
            loop
                set this.group_unit=FirstOfGroup(Filter_Group)
                exitwhen this.group_unit==null or found
                if this.isHero() then
                    set this.target=this.group_unit
                    set found=true
                endif
                call GroupRemoveUnit(Filter_Group, this.group_unit)
            endloop
            call GroupClear(Filter_Group)
        endmethod

So, if the "target" member of an instance is not null, the spell will execute the first action, called "charge". If the member is null, then the second action is executed. However, it appears that dead heroes will be found by the IsHero method and the target member will be set to that unit, breaking my spell's action system. The first action will run improperly and the second will not run based on the condition:

JASS:

            call s.groupCycle()
            if s.target!=null
                call s.charge()
            else
                call s.heal()
            endif

So I would like to know two things: a) Why the heroes are being detected (probably their health at their "death" is conflicting with the condition, right?)
and b) how I can prevent the spell from detecting such heroes.

09-18-2008, 11:08 PM

Captain Griffen

- Use >= 0.405 for health; more accurate.
- Don't use null boolexpr for enuming, it causes a leak.
- You can optimise that a lot, get rid of the boolean completely, etc.
- Probably be better to just use a static group and due the action in a boolexpr.
- this.group_unit is pointless...would have been better to use a local if you were going to do it that way, rather than requiring a global and global array lookup.

09-19-2008, 12:46 AM

Zerzax

Alrighty
1. I previously changed the health to zero to see if that had any impact on detecting the right hero, changing back
2. I can definitely do a static group instead of a global. This is where I'm confused though: so when using GroupEnum I use a boolexpr (static also?) to both find the right kind of unit and execute the right action?
3. Should I still use a FirstOfGroup loop or is that taken care of in the boolexpr?

09-19-2008, 08:02 AM	#4
Captain Griffen	Do it all in the boolexpr. Your current group usage may be okay, but since we cannot actually see all the relevent code, I cannot say. If you're using one group global for all actions like this across the map, that's perfect.

09-19-2008, 07:56 PM

Zerzax

Thanks a lot Griffen, fixes are going well so far. I have a few more questions concerning efficiency. Would it be feasible to use (a a static group_unit and (b static caster members to (a reduce number of GetEnumUnit() calls and (b make sure that the boolexpr knows whether the enum unit is an enemy of the caster?An alternative to (a is of course using about 10 GetEnumUnit calls without the extra handle pointer. But I can't really think of another way to monitor the caster without globals or a static member.

09-20-2008, 01:08 AM

Captain Griffen

JASS:

        private function IsHero takes unit u, unit cast returns boolean
            return GetWidgetLife(u) >= 0.405 and not IsUnitType(u, UNIT_TYPE_STRUCTURE) and IsUnitVisible(u, GetOwningPlayer(this.caster)) and IsUnitType(u, UNIT_TYPE_HERO) and IsUnitEnemy(u, GetOwningPlayer(cast))
        endfunction

globals
    private boolean tempBool
    private unit tempUnit
    private unit tempCaster
    private group enumGroup = CreateGroup() // this should really be set in an initialization function.
endglobals

private function BoolEx takes nothing returns boolean
    if tempBool and IsHero(GetEnumUnit(), tempCaster) then
        set tempBool = false
        set tempUnit = GetEnumUnit()
    endif
    return false
endfunction

        method groupCycle takes nothing returns nothing
            set tempBool = true
            set tempUnit = null
            set tempCaster = this.caster
            call GroupEnumUnitsInRange(enumGroup, this.allyX, this.allyY, TARGET_RADIUS, Condition(function BoolEx))
            set this.target = tempUnit
        endmethod

Sorry if this doesn't compile/quite work, not tested, but that's the idea. There's also some vJASS I think to make that less verbose. Note this needs to be in a scope/library for private.

09-20-2008, 03:17 AM

Zerzax

Very sweet, I never realized that globals would help so much in finding the right unit and preventing the group from adding any units. I decided that the spell would work best (and most logically) if I made the spell branch into one of two directions and thus one of two structs. I took your globals and functions and made them struct members. It's pretty much the same thing, though somewhat more compact and more isolated than your example, but less configurable for another coder. I initialized the enum group and a boolexpr in the onInit as well as a preloading/init trigger (since order of initialized libraries doesn't matter in my init function). Since instance-related members don't matter anymore (this.target and such) I removed this.target and the groupCycle method. I haven't let JassHelper compile yet so there are probably typos, but here it is:

JASS:

    private struct surge 
        private static group enum_group
        private static boolexpr check
        private static unit temp_caster
        private static unit temp_target
        private static boolean temp_bool

        private static method isHero takes unit u, unit caster returns boolean
            return GetWidgetLife(u) >= 0.405 and not IsUnitType(u, UNIT_TYPE_STRUCTURE) and IsUnitVisible(u, GetOwningPlayer(caster)) and IsUnitType(u, UNIT_TYPE_HERO) and IsUnitEnemy(u, GetOwningPlayer(caster))
        endmethod
        
        private static method boolEx takes nothing returns boolean
            if .temp_bool and IsHero(GetEnumUnit(),.temp_caster) then
                set .temp_bool = false
                set .temp_target = GetEnumUnit()
            endif
            return false
        endfunction
        
        private static method spellExecute takes unit c, unit u returns nothing
            local surge_base s
            local real allyX=GetUnitX(a)
            local real allyY=GetUnitY(a)
            local integer castlvl=GetUnitAbilityLevel(c, ABILITY_ID)
            
            call UnitAddAbility(a, EFFECT_ATTACH_ID)
            call SetUnitState(a, UNIT_STATE_LIFE, GetUnitState(a, UNIT_STATE_LIFE) + HealAlly(castlvl))
            
            set .temp_caster=c
            set .temp_bool=true
            set .temp_target=null
            call GroupEnumUnitsInRange(.enum_group, allyX, allyY, TARGET_RADIUS, .check)
            if .temp_target!=null then
                set s=charge.create(c,a,allyX,allyY,castlvl)
                call SetTimerData(s.execute, s)
            else
                set s=heal.create(c,a,allyX,allyY,castlvl)
                call SetTimerDaata(s.execute, s)
            endif
        endmethod
        
        private static method spellEffect takes nothing returns nothing
            if GetSpellAbilityId()==ABILITY_ID then
                call surge.spellExecute(GetTriggerUnit(), GetSpellTargetUnit())
            endif
        endmethod
        
        private static method onInit takes nothing returns nothing
            local trigger t=CreateTrigger()
            local unit u=CreateUnit(Player(15), DUMMY_ID, 0, 0, 0)
            call TriggerAddAction(t, function SpellEffect)
            call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
        
            call UnitAddAbility(u, ABILITY_ID)
            call UnitAddAbility(u, STUN_ID)
            call DestroyEffect(AddSpecialEffectTarget(HEALING_WAVE_BUFF, u, "origin"))
            call DestroyEffect(AddSpecialEffectTarget(CONTACT_FX, u, "origin"))
        
        //set Filter_Group=CreateGroup()

            call RemoveUnit(u)
            set .enum_group=CreateGroup()
            set .check=Filter(function surge.boolEx)
            set u=null
        endmethod
    endstruct

09-20-2008, 01:19 PM	#8
Captain Griffen	That looks good, I think.

09-20-2008, 02:04 PM

Zerzax

At the moment it works fine except for detecting the enemy hero, maybe it's in the condition or in the boolEx method. I'll find the bug, it shouldn't be too hard. Thanks again Griffen, I really appreciate the help.

EDIT: It's not working, just for clarification here is the update code. I have 1 hour 45 mins to submit this, please help me find how to properly detect the hero. Even if the conditions are satisfied, the hero will not be detected properly and the spell will automatically run the heal action, no matter what.

JASS:

    private struct surge 
        private static group enum_group
        private static boolexpr check
        private static unit temp_caster
        private static unit temp_target
        private static boolean temp_bool

        private static method isHero takes unit u returns boolean
            call BJDebugMsg("Checking!")
            return GetWidgetLife(u) >= 0.405 and not IsUnitType(u, UNIT_TYPE_STRUCTURE) and IsUnitVisible(u, GetOwningPlayer(.temp_caster)) and IsUnitType(u, UNIT_TYPE_HERO) and IsUnitEnemy(u, GetOwningPlayer(.temp_caster))
        endmethod
        
        private static method boolEx takes nothing returns boolean
            if .temp_bool and .isHero(GetEnumUnit()) then
                call BJDebugMsg("Found enemy hero")
                set .temp_bool = false
                set .temp_target = GetEnumUnit()
            endif
            return false
        endmethod
        
        private static method spellExecute takes unit c, unit a returns nothing
            local surge_base s
            local real allyX=GetUnitX(a)
            local real allyY=GetUnitY(a)
            local integer castlvl=GetUnitAbilityLevel(c, ABILITY_ID)
            
            call UnitAddAbility(a, EFFECT_ATTACH_ID)
            call SetUnitState(a, UNIT_STATE_LIFE, GetUnitState(a, UNIT_STATE_LIFE) + HealAlly(castlvl))
            
            set .temp_caster=c
            set .temp_bool=true
            set .temp_target=null
            
            call GroupEnumUnitsInRange(.enum_group, allyX, allyY, TARGET_RADIUS, .check)
            //set .temp_caster=null
            if .temp_target==null then
                set s=heal.create(c,a,allyX,allyY,castlvl)
                call SetTimerData(s.execute,s)
            else
                call BJDebugMsg("It's working!")
                set s=charge.create(c,a,.temp_target,allyX,allyY,castlvl)
                call SetTimerData(s.execute,s)
            endif
        endmethod
        
        private static method spellEffect takes nothing returns nothing
            if GetSpellAbilityId()==ABILITY_ID then
                call surge.spellExecute(GetTriggerUnit(), GetSpellTargetUnit())
            endif
        endmethod
        
        private static method onInit takes nothing returns nothing
            local trigger t=CreateTrigger()
            local unit u=CreateUnit(Player(15), DUMMY_ID, 0, 0, 0)
            call TriggerAddAction(t, function surge.spellEffect)
            call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
        
            call UnitAddAbility(u, ABILITY_ID)
            call UnitAddAbility(u, STUN_ID)
            call UnitRemoveAbility(u, ABILITY_ID)
            call UnitRemoveAbility(u, ABILITY_ID)
            
            call DestroyEffect(AddSpecialEffectTarget(HEALING_WAVE_BUFF, u, "origin"))
            call DestroyEffect(AddSpecialEffectTarget(CONTACT_FX, u, "origin"))
        
        //set enum_group=CreateGroup()

            call RemoveUnit(u)
            set .enum_group=CreateGroup()
            set .check=Condition(function surge.boolEx)
            set u=null
        endmethod
    endstruct