[Spell] Is it mui?

Level 25
Joined
Sep 7, 2018
Messages
539
Is it mui? thanks for checking.
  • Purification
    • Events
      • Unit - A unit Starts the effect of an ability
    • Conditions
      • (Ability being cast) Equal to Purification
    • Actions
      • -------- Config --------
      • Custom script: local location pp = GetSpellTargetLoc()
      • Custom script: local unit pc = GetTriggerUnit()
      • Custom script: local unit pt = GetSpellTargetUnit()
      • Custom script: set udg_Purification_Point = pp
      • Custom script: set udg_Purification_Caster = pc
      • Custom script: set udg_Purification_AllyTarget = pt
      • -------- Heals a friendly unit --------
      • If ((Level of Purification for Purification_Caster) Equal to 1) then do (Unit - Set life of Purification_AllyTarget to ((Life of Purification_AllyTarget) + 90.00)) else do (Do nothing)
      • If ((Level of Purification for Purification_Caster) Equal to 2) then do (Unit - Set life of Purification_AllyTarget to ((Life of Purification_AllyTarget) + 180.00)) else do (Do nothing)
      • If ((Level of Purification for Purification_Caster) Equal to 3) then do (Unit - Set life of Purification_AllyTarget to ((Life of Purification_AllyTarget) + 270.00)) else do (Do nothing)
      • -------- Damage nearby enemy units --------
      • Custom script: set bj_wantDestroyGroup = true
      • Unit Group - Pick every unit in (Units within 165.00 of Purification_Point) and do (Actions)
        • Loop - Actions
          • Set Purification_EnemyTarget = (Picked unit)
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • (Purification_EnemyTarget belongs to an enemy of (Owner of Purification_Caster)) Equal to True
            • Then - Actions
              • If ((Level of Purification for Purification_Caster) Equal to 1) then do (Unit - Cause Purification_Caster to damage Purification_EnemyTarget, dealing 90.00 damage of attack type Spells and damage type Magic) else do (Do nothing)
              • If ((Level of Purification for Purification_Caster) Equal to 2) then do (Unit - Cause Purification_Caster to damage Purification_EnemyTarget, dealing 180.00 damage of attack type Spells and damage type Magic) else do (Do nothing)
              • If ((Level of Purification for Purification_Caster) Equal to 3) then do (Unit - Cause Purification_Caster to damage Purification_EnemyTarget, dealing 270.00 damage of attack type Spells and damage type Magic) else do (Do nothing)
            • Else - Actions
              • Do nothing
      • -------- Remove leak --------
      • Custom script: set pp = null
      • Custom script: set pc = null
      • Custom script: set pt = null
      • Custom script: set udg_Purification_Point = null
      • Custom script: set udg_Purification_Caster = null
      • Custom script: set udg_Purification_AllyTarget = null
      • Custom script: set udg_Purification_EnemyTarget = null
 
Level 29
Joined
Sep 26, 2009
Messages
2,594
Yes, it is MUI, although not because of what you are doing and how you are doing it - it is safe only because you refer to Purification_EnemyTarget only once when dealing damage to that unit.
If, for example, you were to also apply a debuff via dummy spell/unit on Purification_EnemyTarget after dealing damage to it, then your trigger would be potentially unsafe.

First of all, your local variables serve no purpose. You immediately assign their values to global variables and you use the global variables throughout the trigger. I assume you wanted to use shadowed variables, but that is not how you do it.

Second, instead of having multiple single-line if/then/else actions to just deal/heal different amount based on level, you can just calculate the amount on the fly.

Third, you leak location - you null the variable, but you never call RemoveLocation, so it leaks.

The trigger below will behave the same way but is not bloated as much:
  • Purification
    • Events
      • Unit - A unit Starts the effect of an ability
    • Conditions
      • (Ability being cast) Equal to Purification
    • Actions
      • -------- Config --------
      • Set VariableSet Purification_Caster = (Triggering unit)
      • Set VariableSet Purification_AllyTarget = (Target unit of ability being cast)
      • Set VariableSet Purification_Point = (Target point of ability being cast)
      • Set VariableSet Amount = Real(Level of Purification for Purification_Caster) * 90.00
      • -------- Heals a friendly unit --------
      • Unit - Set life of Purification_AllyTarget to ((Life of Purification_AllyTarget) + Amount)
      • -------- Damage nearby enemy units --------
      • Custom script: set bj_wantDestroyGroup = true
      • Unit Group - Pick every unit in (Units within 165.00 of Purification_Point) and do (Actions)
        • Loop - Actions
          • Set Purification_EnemyTarget = (Picked unit)
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • (Purification_EnemyTarget belongs to an enemy of (Owner of Purification_Caster)) Equal to True
            • Then - Actions
              • Unit - Cause Purification_Caster to damage Purification_EnemyTarget, dealing Amount damage of attack type Spells and damage type Magic
            • Else - Actions
      • -------- Remove leak --------
      • Custom script: call RemoveLocation(udg_Purification_Point)
Edit: I believe the unit group action could also be simplified to this, as that will remove the need to have Purification_EnemyTarget variable:
  • Unit Group - Pick every unit in (Units within 165.00 of Purification_Point matching ((Matching unit) belongs to an enemy of (Triggering player))) and do (Actions)
    • Loop - Actions
      • Unit - Cause Purification_Caster to damage (Picked unit), dealing Amount damage of attack type Spells and damage type Magic
 
Level 25
Joined
Sep 7, 2018
Messages
539
Yes, it is MUI, although not because of what you are doing and how you are doing it - it is safe only because you refer to Purification_EnemyTarget only once when dealing damage to that unit.
If, for example, you were to also apply a debuff via dummy spell/unit on Purification_EnemyTarget after dealing damage to it, then your trigger would be potentially unsafe.

First of all, your local variables serve no purpose. You immediately assign their values to global variables and you use the global variables throughout the trigger. I assume you wanted to use shadowed variables, but that is not how you do it.

Second, instead of having multiple single-line if/then/else actions to just deal/heal different amount based on level, you can just calculate the amount on the fly.

Third, you leak location - you null the variable, but you never call RemoveLocation, so it leaks.

The trigger below will behave the same way but is not bloated as much:
  • Purification
    • Events
      • Unit - A unit Starts the effect of an ability
    • Conditions
      • (Ability being cast) Equal to Purification
    • Actions
      • -------- Config --------
      • Set VariableSet Purification_Caster = (Triggering unit)
      • Set VariableSet Purification_AllyTarget = (Target unit of ability being cast)
      • Set VariableSet Purification_Point = (Target point of ability being cast)
      • Set VariableSet Amount = Real(Level of Purification for Purification_Caster) * 90.00
      • -------- Heals a friendly unit --------
      • Unit - Set life of Purification_AllyTarget to ((Life of Purification_AllyTarget) + Amount)
      • -------- Damage nearby enemy units --------
      • Custom script: set bj_wantDestroyGroup = true
      • Unit Group - Pick every unit in (Units within 165.00 of Purification_Point) and do (Actions)
        • Loop - Actions
          • Set Purification_EnemyTarget = (Picked unit)
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • (Purification_EnemyTarget belongs to an enemy of (Owner of Purification_Caster)) Equal to True
            • Then - Actions
              • Unit - Cause Purification_Caster to damage Purification_EnemyTarget, dealing Amount damage of attack type Spells and damage type Magic
            • Else - Actions
      • -------- Remove leak --------
      • Custom script: call RemoveLocation(udg_Purification_Point)
Edit: I believe the unit group action could also be simplified to this, as that will remove the need to have Purification_EnemyTarget variable:
  • Unit Group - Pick every unit in (Units within 165.00 of Purification_Point matching ((Matching unit) belongs to an enemy of (Triggering player))) and do (Actions)
    • Loop - Actions
      • Unit - Cause Purification_Caster to damage (Picked unit), dealing Amount damage of attack type Spells and damage type Magic
Really thanks I learned alot today.
 
Top