12th Dec 2015
IcemanBo: Too long as NeedsFix. Rejected.

oh and the triggers are not copied from http://www.hiveworkshop.com/forums/spells-569/energy-shield-gui-mui-203039/

as this was the previous spell before I uploaded it http://www.hiveworkshop.com/forums/requests-341/requesting-damage-shield-spell-199680/#post1963937

updated some fixes too group leaks
updated floating text should only be revealed to the owner of the unit.
updated Shield cast group leak
updated Shield cast group leak - leak removing was in wrong place
updated Dummy cast
Update1.2
updated Shield cast - added variable
updated Shield cast - leak code in wrong spot
updated Boolean for stacking and text display of shield life and damage spill.
Update1.3
-Stacking basically keeps the shield life of the previous cast (if it is still on the unit), by default this is off.
-Text display speaks for itself, default on.
-boolean found in Shield ini
Update 1.4
-multicast removed (causing leaks)
-theme changed (green is for squares)
-Dummy buff cast Removed.
-leaks removed from shieldblock
-Additional information added to Shieldini, including shieldlife, shielduration.
QuickFix update
- actually fixed leak in shieldblock
-import instruction in trigger comments of ShieldIni
QuickFix update 2
-revamped ShieldBlock with maker's arcane knowledge of the abysmal depths of triggering.
Update 1.5
-New effect option (default off) when turned on the effect of the skull above is replaced by three orbiting units that die off whenever a third of the target's shield is destroyed.
-Made map prettier
QuickFix
-cleared two instances of trigger turn off in shield block

not only do locations leak, groups do too, you have a few unit group leaks... you can remove with call DestroyGroup (udg_GroupName) or by not setting a variable, picking the units directly and before doing that put set bj_wantDestroyGroup = true

http://www.hiveworkshop.com/forums/trigger-gui-editor-tutorials-279/quick-tutorial-common-triggering-tips-190802/

It helped me a lot with fixing leaks, I recommend you read it.

Zaio said:

not only do locations leak, groups do too, you have a few unit group leaks... you can remove with call DestroyGroup (udg_GroupName) or by not setting a variable, picking the units directly and before doing that put set bj_wantDestroyGroup = true

Click to expand...

Is the group removed permanently when using this custom script?

Yes, if you want a group to not be removed, a thing you can do is at map init put set udg_GroupName = CreateGroup() then you can add units to it/remove units from it and pick every unit in it, just so long as you dont set it as something else, it wont leak.

EDIT: Also, your floating text is shown through fog, you need to hide it for certain players otherwise everyone can see it from anywhere.

Zaio said:

Yes, if you want a group to not be removed, a thing you can do is at map init put set udg_GroupName = CreateGroup() then you can add units to it/remove units from it and pick every unit in it, just so long as you dont set it as something else, it wont leak.

EDIT: Also, your floating text is shown through fog, you need to hide it for certain players otherwise everyone can see it from anywhere.

Click to expand...

with this I can keep the triggers the way they are without using set bj_wantDestroyGroup?

if you mean by set bj_wantDestroyGroup = true you cant use a variable, it picks units directly out of the conditions you set for the units, runs the actions then destroys it, if you mean by the constant global, at init put set udg_GroupName = CreateGroup() then add/remove/pick units in GroupName, this basically just creates a group ONE time only, it will not create it multiple times so it will not leak, it could be usefull to a spell like this to add units with the buff then remove them when the effects end.

On a side note: if you destroy this group, all units inside it will be lost, so if you are going to do this you must never destroy the group.

Alright added the changes init has the set udg_GroupName = CreateGroup()

and loop had the set bj_wantDestroyGroup removed

Any leaks?

You know that using Damage Modifier you can cut this trigger into.. lets say just couple lines?
I was doing myself absorbing mini-systems for GUI but after moving DM into GUI it's pointless since it covers block too.

Additionaly: this spell does not contain 'fatal block' checker - meaning that if unit is at 100% property trigger creates/recycles timers to add life with delay 0.00 properly. Don't say that that it does because it cotains additional trigger onExpire -> if there are multiple units around damage may be dealt many times within period 0.00 meaning this system won't heal unit by amount of all damage taken within given 0.00 period but only with the last damage value taken after such period.

Spinnaker said:

You know that using Damage Modifier cuts this trigger into.. lets say just couple lines?

Click to expand...

Example ?

Your TempGroup in ShieldCast is leaking, dont set a variable as it, pick every units in [units conditions here] but before put set bj_wantDestroyGroup = true

Devalut said:

Example ?

Click to expand...

Added few informations into last trigger.

Egzample?

• init
  • Events
    • Game - DamageModifierEvent becomes Equal to 1.00
  • Conditions
    • DamageEventOverride Equal to False
  • Actions
    • Set DamageEventAmount = 0.00

Nulls all damage taken, no mater the circumstances. And calculating damage taken goes:
MaxAbosrb = MaxAbsorb + DamageEventAmount. Where MaxAbsorb is your variable which stored absorbed dmg.

Spinnaker said:

Added few informations into last trigger.

Egzample?

• init
  • Events
    • Game - DamageModifierEvent becomes Equal to 1.00
  • Conditions
    • DamageEventOverride Equal to False
  • Actions
    • Set DamageEventAmount = 0.00

Nulls all damage taken.

Click to expand...

hmm I might need to look into using that

Zaio said:

Your TempGroup in ShieldCast is leaking, dont set a variable as it, pick every units in [units conditions here] but before put set bj_wantDestroyGroup = true

Click to expand...

added to your description, triggers updated
Appreciate the help +rep

Better don't
Your spell would take 4-5 lines what is pointless to upload.
Have you read about issue I've mentioned in my first post?

Also, some of your leak removals are in the wrong places, if you set a location within a group, you must remove it within the group, its basically setting a location per unit in the group, so you must remove it per unit, else you're just removing it for the last unit to use it.

Zaio said:

Also, some of your leak removals are in the wrong places, if you set a location within a group, you must remove it within the group, its basically setting a location per unit in the group, so you must remove it per unit, else you're just removing it for the last unit to use it.

Click to expand...

Fixed, updated triggers and map.

Im sorry but i have to agree with spinnaker, this spell is pointless because of what bribe made, but still, if you can learn from mistakes you can maybe make a better spell, another error: in ShieldCast you set T_Point, pick units in it and set T_Point again before removing it, which leaks, you cant remove it in the group because it will be = null since you only set it once, and you cant remove it before you pick units in the group cause you are picking units near it, so you must create another location and use that instead of the T_Point inside the group, remove that then after picking units in the group, remove T_Point

Zaio said:

Im sorry but i have to agree with spinnaker, this spell is pointless because of what bribe made, but still, if you can learn from mistakes you can maybe make a better spell, another error: in ShieldCast you set T_Point, pick units in it and set T_Point again before removing it, which leaks, you cant remove it in the group because it will be = null since you only set it once, and you cant remove it before you pick units in the group cause you are picking units near it, so you must create another location and use that instead of the T_Point inside the group, remove that then after picking units in the group, remove T_Point

Click to expand...

Fixed & added.

In ShieldCast at the very end, the removal should be in the code block slightly above it, not outside the if/then/else code block, also in the code where you make the floating text, at the very start you set a unit group and use bj_wantDestroyGroup = true, the point of using bj_wantDestroyGroyp = true is to NOT use group variables and pick them directly, that will still leak because you set the group var and didnt destroy it, also you are destroying T_Point at the very start for some known reason...

Zaio said:

In ShieldCast at the very end, the removal should be in the code block slightly above it, not outside the if/then/else code block, also in the code where you make the floating text, at the very start you set a unit group and use bj_wantDestroyGroup = true, the point of using bj_wantDestroyGroyp = true is to NOT use group variables and pick them directly, that will still leak because you set the group var and didnt destroy it, also you are destroying T_Point at the very start for some known reason...

Click to expand...

fixed & added

Lol I could have sworn someone made a spell just like this a few days ago...

will only remove the group created directly before it.
Check the tutorial in my signature (below) if u'd like, there's a section on that

• You are spamming (Key(Target unit of ability being cast)). Use an Integer variable and

• Custom script: set udg_T_Id = GetHandleId (GetSpellTargetUnit())

• Your variable names are too generic, rename them with a prefix, e.g. S_ (from "Spiritual").

• I really don't see why you use TempGroup:

• Set TempGroup = (Units within 10.00 of TempPoint matching ((Matching unit) Equal to GDD_DamagedUnit))

Can't you just refer to "GDD_DamagedUnit"?

•

• Custom script: set udg_Shield_Group = CreateGroup()

This is not needed; it's already created in the map initialization.

• Use (Triggering player) instead of (Owner of (Triggering unit)) -> It procs faster.

•

• Unit Group - Pick every unit in (Units within 260.00 of T_Point matching (((Matching unit) belongs to an ally of (Owner of (Triggering unit))) Equal to True)) and do (Actions)
  • Loop - Actions
    • Set r = 0.00

Why do you declare "r" in this enumeration?

Additionally,

• Unit Group - Remove (Picked unit) from TempGroup

Why do you remove it and it back, when the dummy casts the spell on the (Picked unit)? It will eventually be added anyway.

• You don't need an additional trigger to detect the dummy's ability cast. Just trigger the events within the previous enumeration.

•

• Set Shield_Target = No unit

Globals don't need to be nulled.

•

• Floating Text - Show (Last created floating text) for (All players matching ((Matching player) Equal to (Owner of (Picked unit))))

Leaks.

• Use a variable to refer the (Last created floating text), to save calls.

• Use a boolean variable, that will be declared in ShieldIni trigger, which will check whether the user wants to display the damage left in the shield.

•

• Set Shield_Target = (Picked unit)

You declare this for a single action, when there are tons of references for (Picked unit) before it (mostly in the if/then/else blocks with the floating text and before). Move it on top of the unit group enumeration and refer to picked unit with this variable.

I LOVE pharaoh's reviews

Suggestions would be nice, any options or things that are missing you think should be added lemme know.

bump for update