For some reason my ability doesn't add buff, but deals dmg to my hero

[meleander]

Hi, so this is the code:

scope DivineRally initializer InitTrig_Divine_Rally
  
    globals
        private constant integer ID = 'A04O'
      
        private real array spellHealingArmorPercentIncrease
        private real array spellDamageDamagePercentIncrease
        private real array intAttackSpeedPercentIncrease
      
        private integer Buff
    endglobals
  
    private function GroupFilter takes nothing returns boolean
        call DisplayTimedTextToForce(Players, 5., "group filter")
        return GetWidgetLife(GetFilterUnit()) > .405 and not IsUnitDummy(GetFilterUnit()) and not IsUnitEnemy(GetFilterUnit(),GetOwningPlayer(SpellEvent.CastingUnit))
    endfunction
      
    private function BuffonAdd takes nothing returns nothing
        local dbuff db = GetEventBuff()
        local integer ablev = GetUnitAbilityLevel(db.source, ID)
        local integer armorChange = R2I(spellHealingArmorPercentIncrease[ablev] * GetSpellBonus(db.source, 2))
        local integer damageChange = R2I(spellDamageDamagePercentIncrease[ablev] * GetSpellBonus(db.source, 1))
        local integer attackSpeedChange = R2I(intAttackSpeedPercentIncrease[ablev] * GetHeroInt(db.source, true))
        call AddUnitBonus(db.target, BONUS_ARMOR, armorChange)
        call AddUnitBonus(db.target, BONUS_DAMAGE, damageChange)
        call AddUnitBonus(db.target, BONUS_ATTACK_SPEED, damageChange)
    endfunction
  
    private function BuffonEnd takes nothing returns nothing
        local dbuff db = GetEventBuff()
        local integer ablev = GetUnitAbilityLevel(db.source, ID)
        local integer armorChange = -R2I(spellHealingArmorPercentIncrease[ablev] * GetSpellBonus(db.source, 2))
        local integer damageChange = -R2I(spellDamageDamagePercentIncrease[ablev] * GetSpellBonus(db.source, 1))
        local integer attackSpeedChange = -R2I(intAttackSpeedPercentIncrease[ablev] * GetHeroInt(db.source, true))
        call AddUnitBonus(db.target, BONUS_ARMOR, armorChange)
        call AddUnitBonus(db.target, BONUS_DAMAGE, damageChange)
        call AddUnitBonus(db.target, BONUS_ATTACK_SPEED, damageChange)
    endfunction
    private function Enum takes nothing returns nothing
        local unit caster = SpellEvent.CastingUnit
        local unit target = GetEnumUnit()
      
        call DisplayTimedTextToForce(Players, 5., "enum")
        call UnitAddBuff(caster, target, Buff, 4., GetUnitAbilityLevel(caster, ID))
      
        set caster = null
        set target = null
    endfunction
  
    private function act takes nothing returns nothing
        local unit caster = GetTriggerUnit() 
        local integer pid = GetPlayerId(GetOwningPlayer(caster))
        local group g = NewGroup()
        call DisplayTimedTextToForce(Players, 5., "000000000000001")
      
        call GroupEnumUnitsInArea(g, GetSpellTargetX(), GetSpellTargetY(), 400.*abb2[pid], Condition(function GroupFilter))
        call DisplayTimedTextToForce(Players, 5., "000000000000002")
        call ForGroup(g, function Enum)
        call ReleaseGroup(g)
        call DisplayTimedTextToForce(Players, 5., "000000000000003")
        set caster = null
        set g = null
    endfunction
  
    private function Conditions takes nothing returns boolean
        return GetSpellAbilityId() == ID
    endfunction
  
    //===========================================================================
    private function InitTrig_Divine_Rally takes nothing returns nothing
        //call RegisterSpellEffectResponse(ID,act)
        set gg_trg_Divine_Rally = CreateTrigger(  )
        call TriggerRegisterAnyUnitEventBJ( gg_trg_Divine_Rally, EVENT_PLAYER_UNIT_SPELL_EFFECT )
        call TriggerAddCondition( gg_trg_Divine_Rally, Condition( function Conditions ) )
        call TriggerAddAction( gg_trg_Divine_Rally, function act )
      
        set spellHealingArmorPercentIncrease[1] = 0.09
        set spellHealingArmorPercentIncrease[2] = 0.10
        set spellHealingArmorPercentIncrease[3] = 0.12
        set spellHealingArmorPercentIncrease[4] = 0.14
        set spellHealingArmorPercentIncrease[5] = 0.16
        set spellHealingArmorPercentIncrease[6] = 0.19
      
        set spellDamageDamagePercentIncrease[1] = 0.3
        set spellDamageDamagePercentIncrease[2] = 0.31
        set spellDamageDamagePercentIncrease[3] = 0.32
        set spellDamageDamagePercentIncrease[4] = 0.33
        set spellDamageDamagePercentIncrease[5] = 0.34
        set spellDamageDamagePercentIncrease[6] = 0.35
      
        set intAttackSpeedPercentIncrease[1] = 0.71
        set intAttackSpeedPercentIncrease[2] = 0.82
        set intAttackSpeedPercentIncrease[3] = 0.93
        set intAttackSpeedPercentIncrease[4] = 1.04
        set intAttackSpeedPercentIncrease[5] = 1.15
        set intAttackSpeedPercentIncrease[6] = 1.26
        set Buff = DefineBuffType('A0AC', 'B03H', 0., false, true, BuffonAdd, 0, BuffonEnd)
    endfunction
endscope

For some reason instead of going into the Enum function nothing happens and at the end I am even being dealt damage (but only the first time this ability is use on me or friendly unit). I have no idea why this is happening...

To add more information, I get text:

000000000000001
group filter
000000000000002
000000000000003

Please help

 

[Pyrogasm]

IMO it's not good practice to define things like this as scopes (which are AFAIK automatically inserted into the compilation process after all libraries so effectively 'require' all libraries by default) because you aren't making it explicitly clear what libraries this spells needs in order to function. Looks like there's a buff library and a spell event library that this spell needs, but I (as someone looking at your code) have no idea what those libraries are because they're not explicitly listed.

Best guess is that the problem is that SpellEvent.CastingUnit is never set before the GroupEnumUnitsInRange call. Thus all units in range fail the IsUnitEnemy() check. If this global is being set by some other library you are using for spell events... you probably need to register the spell event via that library instead of using TriggerRegisterAnyUnitEventBJ(). If you were also seeing the "enum" message then I wouldn't think this is the case but I don't see it in your example debug text; thus the group that you're using ForGroup() on is likely empty.

---

This won't matter unless you are enum-ing over a large number of units repeatedly, but it would be best practice to declare local unit f = GetFilterUnit() in your filter function instead of calling it 3 times for every unit (also define a boolean for the return value, then null the local unit before returning the boolean). Same logic applies to GetOwningPlayer(SpellEvent.CastingUnit): you can save the player and caster in scope-private global variables before the GroupEnumUnitsInRange to avoid computing it each time (instead of the local variables you use in the act function).

BJDebugMsg("TEXT HERE") is perfectly reasonable to display debug text, btw. BJs are only bad when efficiency matters; otherwise they can be helpful convenience functions like TriggerRegisterAnyUnitEventBJ()

 

[meleander]

IMO it's not good practice to define things like this as scopes (which are AFAIK automatically inserted into the compilation process after all libraries so effectively 'require' all libraries by default) because you aren't making it explicitly clear what libraries this spells needs in order to function. Looks like there's a buff library and a spell event library that this spell needs, but I (as someone looking at your code) have no idea what those libraries are because they're not explicitly listed.

Best guess is that the problem is that SpellEvent.CastingUnit is never set before the GroupEnumUnitsInRange call. Thus all units in range fail the IsUnitEnemy() check. If this global is being set by some other library you are using for spell events... you probably need to register the spell event via that library instead of using TriggerRegisterAnyUnitEventBJ(). If you were also seeing the "enum" message then I wouldn't think this is the case but I don't see it in your example debug text; thus the group that you're using ForGroup() on is likely empty.

---

This won't matter unless you are enum-ing over a large number of units repeatedly, but it would be best practice to declare local unit f = GetFilterUnit() in your filter function instead of calling it 3 times for every unit (also define a boolean for the return value, then null the local unit before returning the boolean). Same logic applies to GetOwningPlayer(SpellEvent.CastingUnit): you can save the player and caster in scope-private global variables before the GroupEnumUnitsInRange to avoid computing it each time (instead of the local variables you use in the act function).

BJDebugMsg("TEXT HERE") is perfectly reasonable to display debug text, btw. BJs are only bad when efficiency matters; otherwise they can be helpful convenience functions like TriggerRegisterAnyUnitEventBJ()

Thanks, I was able to find out, that the problem was mainly due to Base Order Id being the same (it seems that it's impossible to change that for the battleroar based abilities...). Now I have another problem which is that after I finish the buff, my attack speed is negative (around -284) which shocks me. What is the best way to not get less attack speed than it was at the start? Get a struct to remember each unit?

Also my code changed:

scope DivineRally initializer init

    private keyword divinerally
    
    globals
        constant integer DIVINE_RALLY_ID = 'A04O'
        
        private real array spellHealingArmorPercentIncrease
        private real array spellDamageDamagePercentIncrease
        private real array intAttackSpeedPercentIncrease
        
        private integer Divine_Rally_Buff
    endglobals
        
    private function BuffonAdd takes nothing returns nothing
        local dbuff db = GetEventBuff()
        local integer ablev = GetUnitAbilityLevel(db.source, DIVINE_RALLY_ID)
        local integer armorChange = R2I(spellHealingArmorPercentIncrease[ablev] * GetSpellBonus(db.source, 2))
        local integer damageChange = R2I(spellDamageDamagePercentIncrease[ablev] * GetSpellBonus(db.source, 1))
        local integer attackSpeedChange = R2I(intAttackSpeedPercentIncrease[ablev] * GetHeroInt(db.source, true))

        //call DisplayTimedTextToForce(Players, 15., "000000000000002c1")
        //call DisplayTimedTextToForce(Players, 15., "000000000000002c1: " + R2S(armorChange))
        //call DisplayTimedTextToForce(Players, 15., "000000000000002c1: " + R2S(damageChange))
        //call DisplayTimedTextToForce(Players, 15., "000000000000002c1: " + R2S(attackSpeedChange))
        call AddUnitBonus(db.target, BONUS_ARMOR, armorChange)
        call AddUnitBonus(db.target, BONUS_DAMAGE, damageChange)
        call AddUnitBonus(db.target, BONUS_ATTACK_SPEED, damageChange)
    endfunction
    
    private function BuffonEnd takes nothing returns nothing
        local dbuff db = GetEventBuff()
        local integer ablev = GetUnitAbilityLevel(db.source, DIVINE_RALLY_ID)
        local integer armorChange = -R2I(spellHealingArmorPercentIncrease[ablev] * GetSpellBonus(db.source, 2))
        local integer damageChange = -R2I(spellDamageDamagePercentIncrease[ablev] * GetSpellBonus(db.source, 1))
        local integer attackSpeedChange = -R2I(intAttackSpeedPercentIncrease[ablev] * GetHeroInt(db.source, true))

        //call DisplayTimedTextToForce(Players, 15., "000000000000002c2")
        call AddUnitBonus(db.target, BONUS_ARMOR, armorChange)
        call AddUnitBonus(db.target, BONUS_DAMAGE, damageChange)
        call AddUnitBonus(db.target, BONUS_ATTACK_SPEED, damageChange)
    endfunction
  
    private function GroupFilter takes nothing returns boolean
        return GetWidgetLife(GetFilterUnit()) > .405 and not IsUnitDummy(GetFilterUnit())
    endfunction

    private function Enum takes nothing returns nothing
        local unit caster = GetTriggerUnit() //SpellEvent.CastingUnit
        local unit target = GetEnumUnit()
        local integer ablev = GetUnitAbilityLevel(caster, DIVINE_RALLY_ID)
        local real dmg
        local real heal
        //call DisplayTimedTextToForce(Players, 15., "000000000000002b")
        
        if not IsUnitEnemy(caster, GetOwningPlayer(target)) then
            //call DisplayTimedTextToForce(Players, 15., "000000000000002c")
            call UnitAddBuff(caster, target, Divine_Rally_Buff, 4., GetUnitAbilityLevel(caster, DIVINE_RALLY_ID))
        endif
        //call DisplayTimedTextToForce(Players, 15., "000000000000002d")

        set caster = null
        set target = null
    endfunction
    
    private function act takes nothing returns nothing  
        local unit caster = GetTriggerUnit()     
        local group g = NewGroup()
        //call DisplayTimedTextToForce(Players, 15., "000000000000001")
        
        call GroupEnumUnitsInArea(g, GetUnitX(caster), GetUnitY(caster), 600., Condition(function GroupFilter))
        //call DisplayTimedTextToForce(Players, 15., "000000000000002")
        call ForGroup(g, function Enum)
        //call DisplayTimedTextToForce(Players, 15., "000000000000003")
        call ReleaseGroup(g)
        //call DisplayTimedTextToForce(Players, 15., "000000000000004")

        set caster = null
        set g = null
    endfunction
    
    private function init takes nothing returns nothing
        call RegisterSpellEffectResponse(DIVINE_RALLY_ID, act)

        set spellHealingArmorPercentIncrease[1] = 0.09
        set spellHealingArmorPercentIncrease[2] = 0.10
        set spellHealingArmorPercentIncrease[3] = 0.12
        set spellHealingArmorPercentIncrease[4] = 0.14
        set spellHealingArmorPercentIncrease[5] = 0.16
        set spellHealingArmorPercentIncrease[6] = 0.19
        
        set spellDamageDamagePercentIncrease[1] = 0.3
        set spellDamageDamagePercentIncrease[2] = 0.31
        set spellDamageDamagePercentIncrease[3] = 0.32
        set spellDamageDamagePercentIncrease[4] = 0.33
        set spellDamageDamagePercentIncrease[5] = 0.34
        set spellDamageDamagePercentIncrease[6] = 0.35
        
        set intAttackSpeedPercentIncrease[1] = 0.71
        set intAttackSpeedPercentIncrease[2] = 0.82
        set intAttackSpeedPercentIncrease[3] = 0.93
        set intAttackSpeedPercentIncrease[4] = 1.04
        set intAttackSpeedPercentIncrease[5] = 1.15
        set intAttackSpeedPercentIncrease[6] = 1.26
        
        set Divine_Rally_Buff = DefineBuffType('A0AC', 'B03H', 0., false, true, BuffonAdd, 0, BuffonEnd)

    endfunction
    
endscope

And what do you propose instead of creating a scope? A library?

 

[Pyrogasm]

And what do you propose instead of creating a scope? A library?

Yeah sorry I forgot to say. They're almost identical, except that scopes don't have explicit dependencies and can't be depended on.

it seems that it's impossible to change that for the battleroar based abilities...

Almost no abilities can have their base order altered, despite that field being editable. Channel, conveniently, can be altered.

Now I have another problem which is that after I finish the buff, my attack speed is negative (around -284) which shocks me.

You're using damageChange instead of attackSpeedChange in the two set lines.

However, there is another consideration: because you are computing the armor/damage/attackspeed change twice (in the BuffOnAdd and BuffOnEnd functions) it's possible that the computed number to 'undo' the change is different from the amount you added before. Imagine that the casting unit's Intelligence is increased after the spell is cast but before the buff expires (perhaps it leveled up, acquired an item, or used a tome). Now the value of attackSpeedChange computed when the buff ends is larger than when the buff was applied, and when the buff expires it will have permanently lost attack speed because more was subtracted than was initially added by the buff.

The way around this is to save the computed value somewhere and re-use it when the buff ends instead of recomputing it. I see you have some sort of buff system already in place but without that another option is to supply the bonus damage/armor/attack speed buffs via a custom dummy spell cast on the affected units. It's now possible to alter the data fields of the dummy spell with a trigger now, so you can just set the appropriate damage/armor/attackspeed bonuses before dummy-casting the spell.