[Solved] Can we fix issue with my trigger?

[JFAMAP]

During the last tests i was realized there was a problems with this trigger and decided to recreate it.
Let me tell you what this ability does.

Point target ability that deals damage to enemies within 300 range. But there are conditions.
1) Deals 100/150/200/250 damage to TempGroup
2) If TempGroup includes your hero and enemy units, your intelligence value adding as damage and you will heal based on Number of Units in TempGroup
3) If TempGroup includes your hero and 1 enemy hero at least your intelligence value adding as damage and you will heal based on Number of Units in TempGroup
AND permanent life gain for every hero within range

• Events
  • Time - BloodStrikeTimer expires
• Conditions
  • (Hero[17] is alive) Equal to True
  • (Number of units in TempGroup) Greater than 1
• Actions
  • Set TempGroup = (Units within 300.00 of TempPointBloodstrike)
  • Special Effect - Create a special effect at TempPointBloodstrike using war3mapImported\Flamestrike Blood I.mdx
  • Special Effect - Destroy (Last created special effect)
  • Special Effect - Destroy SidaraEfekt[1]
  • Set BloodStrikeBool = False
  • Unit Group - Pick every unit in TempGroup and do (Actions)
    • Loop - Actions
      • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        • If - Conditions
          • (Hero[17] is in TempGroup) Equal to True
          • ((Picked unit) belongs to an enemy of (Owner of Hero[17])) Equal to True
          • ((Picked unit) is in NNN) Equal to True
        • Then - Actions
          • Unit - Set life of Hero[17] to ((Life of Hero[17]) + (((Real((Level of Blood Strike // for Hero[17]))) x 30.00) + 0.00))
        • Else - Actions
  • Unit Group - Pick every unit in TempGroup and do (Actions)
    • Loop - Actions
      • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        • If - Conditions
          • ((Picked unit) belongs to an enemy of (Owner of Hero[17])) Equal to True
          • ((Picked unit) is in NNN) Equal to True
        • Then - Actions
          • Set TempInt = ((50 + (50 x (Level of Blood Strike // for Hero[17]))) + 0)
          • Unit - Cause Hero[17] to damage (Picked unit), dealing (Real(TempInt)) damage of attack type Spells and damage type Magic
          • Game - Display to (All players) the text: (String(TempInt))
          • Game - Display to (All players) the text: Sadece düsman var
          • Skip remaining actions
        • Else - Actions
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • (Hero[17] is in TempGroup) Equal to True
            • Then - Actions
              • Set TempInt2 = (Intelligence of Hero[17] (Include bonuses))
              • Set TempInt = ((50 + (50 x (Level of Blood Strike // for Hero[17]))) + TempInt2)
              • Unit Group - Pick every unit in TempGroup and do (Actions)
                • Loop - Actions
                  • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                    • If - Conditions
                      • ((Picked unit) belongs to an enemy of (Owner of Hero[17])) Equal to True
                      • ((Picked unit) is in NNN) Equal to True
                    • Then - Actions
                      • Unit - Cause Hero[17] to damage (Picked unit), dealing ((Real(TempInt)) + 0.00) damage of attack type Spells and damage type Magic
                      • Unit Group - Pick every unit in TempGroup and do (Actions)
                        • Loop - Actions
                          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                            • If - Conditions
                              • ((Picked unit) belongs to an enemy of (Owner of Hero[17])) Equal to True
                              • ((Picked unit) is in NNN) Equal to True
                              • ((Picked unit) is A Hero) Equal to True
                            • Then - Actions
                              • Hero - Create Manual of Health JFA and give it to Hero[17]
                            • Else - Actions
                    • Else - Actions
            • Else - Actions
  • Custom script: call RemoveLocation(udg_TempPointBloodstrike)
  • Custom script: call DestroyGroup(udg_TempGroup)

 

[Uncle]

1) You're going to be leaking TempPointBloodstrike if your first 2 Conditions fail.
2) You should never reference a Temp variable after a Wait/Timer. Use unique variables made specifically for this trigger.
3) A lot of Item abilities stack, at least on later versions, so you shouldn't need to create an Item. Just add the Life Bonus ability to the caster.
4) This Condition looks very wrong and unnecessary:

• (Number of units in TempGroup) Greater than 1

Here's a "fixed" version, although I couldn't test it:

• Blood Strike Timer
  • Events
    • Time - BloodStrike_Timer expires
  • Conditions
  • Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (BloodStrike_Caster is alive) Equal to True
      • Then - Actions
        • Set BloodStrike_Target_Group = (Units within 300.00 of BloodStrike_Point.)
        • -------- --------
        • -------- Reset variables: --------
        • Set BloodStrike_Use_Caster_Bonus = False
        • Set BloodStrike_Unit_Count = 0
        • Set BloodStrike_Enemy_Hero_Count = 0
        • -------- --------
        • -------- Track caster + enemy units and other variables: --------
        • Unit Group - Pick every unit in BloodStrike_Target_Group and do (Actions)
          • Loop - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • Or - Any (Conditions) are true
                  • Conditions
                    • ((Picked unit) belongs to an ally of (Owner of BloodStrike_Caster).) Equal to True
                    • ((Picked unit) is alive) Equal to False
                    • ((Picked unit) is A structure) Equal to True
                    • ((Picked unit) is Magic Immune) Equal to True
              • Then - Actions
                • -------- Remove unwanted units (allies, dead, structures, etc) --------
                • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                  • If - Conditions
                    • (Picked unit) Not equal to BloodStrike_Caster
                  • Then - Actions
                    • Unit Group - Remove (Picked unit) from BloodStrike_Target_Group.
                  • Else - Actions
              • Else - Actions
                • Set BloodStrike_Unit_Count = (BloodStrike_Unit_Count + 1)
                • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                  • If - Conditions
                    • (Picked unit) Equal to BloodStrike_Caster
                  • Then - Actions
                    • Set BloodStrike_Use_Caster_Bonus = True
                  • Else - Actions
                    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                      • If - Conditions
                        • ((Picked unit) is A Hero) Equal to True
                      • Then - Actions
                        • Set BloodStrike_Enemy_Hero_Count = (BloodStrike_Enemy_Hero_Count + 1)
                      • Else - Actions
        • -------- --------
        • -------- Calculate damage: --------
        • Set BloodStrike_Damage = (50.00 + (50.00 x (Real((Level of Blood Strike for BloodStrike_Caster)))))
        • -------- --------
        • -------- Calculate heal: --------
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • BloodStrike_Use_Caster_Bonus Equal to True
          • Then - Actions
            • Set BloodStrike_Heal = (30.00 x (Real((Level of Blood Strike for BloodStrike_Caster))))
            • Set BloodStrike_Heal = (BloodStrike_Heal x (Real(BloodStrike_Unit_Count)))
          • Else - Actions
        • -------- --------
        • -------- Apply heal: --------
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • BloodStrike_Heal Greater than 0.00
          • Then - Actions
            • Unit - Set life of BloodStrike_Caster to ((Life of BloodStrike_Caster) + BloodStrike_Heal)
          • Else - Actions
        • -------- --------
        • -------- Apply permanent Life Bonus: --------
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • BloodStrike_Use_Caster_Bonus Equal to True
            • BloodStrike_Enemy_Hero_Count Greater than 0
          • Then - Actions
            • For each (Integer A) from 1 to BloodStrike_Enemy_Hero_Count, do (Actions)
              • Loop - Actions
                • Unit - Add Item Life Bonus (Greater) to BloodStrike_Caster
          • Else - Actions
        • -------- --------
        • -------- Apply damage: --------
        • Unit Group - Pick every unit in BloodStrike_Target_Group and do (Actions)
          • Loop - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (Picked unit) Not equal to BloodStrike_Caster
              • Then - Actions
                • Unit - Cause BloodStrike_Caster to damage (Picked unit), dealing BloodStrike_Damage damage of attack type Spells and damage type Normal
              • Else - Actions
        • -------- --------
        • Custom script: call DestroyGroup( udg_BloodStrike_Target_Group )
      • Else - Actions
    • -------- --------
    • Custom script: call RemoveLocation( udg_BloodStrike_Point )

Note how I always remove BloodStrike_Point even if the caster died and the spell failed. That way it doesn't leak when that happens.

 

[JFAMAP]

I already have a trigger about cancellation.
Your system seems advanced and bit of long when compared with mine. Do we really need more variables like this? I was already deleted and removed triggers because i was on limit. There was a problem with Warcraft III maps, if you have too much triggers, game runs with many issues. This is why time to time i am optimizing and combining triggers. I even combined many variables as one with arrays.
I will try to fix it with my way. If i cannot do that, i will change something with trigger that makes detect things difficult. Otherwise i will implement your trigger.
Side note: I really enjoy to lower triggers.

 

[Uncle]

Combining variables isn't safe, though. You simply cannot use TempGroup here, it's not going to work and if it does work it'll only be sometimes.

The other Temp variables are dangerous as well since you're potentially killing units, running the "A unit Dies" Event, which could then be using those very same Temp variables. This would be a conflict where the variables get overwritten with unwanted values.

But it's possible to continue using the Temp variables with careful consideration and an understanding of how things run.

Lastly, if you're having organization issues then perhaps you could start using Jass code, that'll be much more organized. The variables are contained within the script and don't bloat the Trigger Editor.

 

[JFAMAP]

I have managed to fix by my own. I do not think there is a problem with there.
By the way i am already removing many things if the user dies or spell cancels somehow.

Here is my fixed trigger. I am 95% sure but feel free to check. It works during the game but maybe there is a problem with my trigger.

• BloodStrike Finish Copy Copy
  • Events
    • Time - BloodStrikeTimer expires
  • Conditions
    • (Hero[17] is alive) Equal to True
  • Actions
    • Set BloodStrikeBool = False
    • Set TempGroup = (Units within 300.00 of TempPointBloodstrike)
    • Special Effect - Create a special effect at TempPointBloodstrike using war3mapImported\Flamestrike Blood I.mdx
    • Special Effect - Destroy (Last created special effect)
    • Special Effect - Destroy SidaraEfekt[1]
    • Unit Group - Pick every unit in TempGroup and do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • (Hero[17] is in TempGroup) Equal to True
            • ((Picked unit) belongs to an enemy of (Owner of Hero[17])) Equal to True
            • ((Picked unit) is in NNN) Equal to True
          • Then - Actions
            • Unit - Set life of Hero[17] to ((Life of Hero[17]) + (((Real((Level of Blood Strike // for Hero[17]))) x 30.00) + 0.00))
          • Else - Actions
    • Unit Group - Pick every unit in TempGroup and do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • ((Picked unit) belongs to an enemy of (Owner of Hero[17])) Equal to True
            • ((Picked unit) is in NNN) Equal to True
          • Then - Actions
            • Set TempInt = ((50 + (50 x (Level of Blood Strike // for Hero[17]))) + 0)
            • Unit - Cause Hero[17] to damage (Picked unit), dealing (Real(TempInt)) damage of attack type Spells and damage type Magic
          • Else - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Hero[17] is in TempGroup) Equal to True
      • Then - Actions
        • Set TempInt2 = (Intelligence of Hero[17] (Include bonuses))
        • Unit Group - Pick every unit in TempGroup and do (Actions)
          • Loop - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • ((Picked unit) belongs to an enemy of (Owner of Hero[17])) Equal to True
                • ((Picked unit) is in NNN) Equal to True
              • Then - Actions
                • Unit - Cause Hero[17] to damage (Picked unit), dealing ((Real(TempInt2)) + 0.00) damage of attack type Spells and damage type Magic
                • Unit Group - Pick every unit in TempGroup and do (Actions)
                  • Loop - Actions
                    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                      • If - Conditions
                        • ((Picked unit) belongs to an enemy of (Owner of Hero[17])) Equal to True
                        • ((Picked unit) is in NNN) Equal to True
                        • ((Picked unit) is A Hero) Equal to True
                      • Then - Actions
                        • Hero - Create Manual of Health JFA and give it to Hero[17]
                      • Else - Actions
              • Else - Actions
      • Else - Actions
    • Custom script: call RemoveLocation(udg_TempPointBloodstrike)
    • Custom script: call DestroyGroup(udg_TempGroup)

 

[Uncle]

Do whatever you think is best.

Last thing I'll mention is that anytime Hero[17] is Dead when this Timer expires you're going to leak the Point:

• Custom script: call RemoveLocation(udg_TempPointBloodstrike)

This will never get Removed, so if you cast the ability again you'll Set TempPointBloodstrike again, leaking the previous one.

That's not the end of the world but it's the reason why I check if the Caster is Alive within the Actions section of the trigger.

 

[JFAMAP]

I have this already. Thank you for the reminder.

• BloodStrike Cancel
  • Events
    • Unit - A unit Is issued an order targeting an object
    • Unit - A unit Is issued an order targeting a point
    • Unit - A unit Is issued an order with no target
  • Conditions
    • (Ordered unit) Equal to Hero[17]
    • BloodStrikeBool Equal to True
    • Or - Any (Conditions) are true
      • Conditions
        • (Issued order) Equal to (Order(smart))
        • (Issued order) Equal to (Order(stop))
        • (Issued order) Equal to (Order(holdposition))
        • (Issued order) Equal to (Order(attack))
  • Actions
    • Custom script: call RemoveLocation(udg_TempPointBloodstrike)
    • Game - Display to (Player group((Owner of (Ordered unit)))) for 2.00 seconds the text: Blood Strike yetene...
    • Set BloodStrikeBool = False
    • Special Effect - Destroy SidaraEfekt[1]

Have a nice day to everyone.