Fire Shield V1.2 [MUI]

[Moderator]

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

20:57, 2nd Mar 2014
Magtheridon96:

Review:

• Since you're storing (Key(Triggering unit)), you might as well use the variable FISH_Key instead of repeating the computation within the code blocks directly after. It saves computation time. 2 function calls become 1 variable lookup.
• Don't use magic numbers. 100.00 should probably be configurable in there. The same applies to the other magic numbers. 8 and 45.00 should probably be configurable as well. Make 8 configurable, and instead of having 45.00, you would just have use 360.00/8 where 8 is not present as a constant literal, but as a variable the user can change in your setup trigger. Do the same for the other magic numbers like 50.00. Remember: magic numbers are evil.
• Apply what I said about (Key(Triggering unit)) to (Key(Picked unit)) in the loop trigger.
• You're not removing the FISH_Caster_Location correctly in the loop trigger. It's being leaked. You need to remove it regardless of whether FISH_UnitGroup_Count is 0 or not. You need to remove it in the same scope as its allocation. That's how you ensure 0 leaks.

 

[Kazeon]

- you are not removing the leaks
- maybe you can store the FISH_Level and FISH_Owner at the cast event
- only turn on the loop when total unit in FISH_UnitGroup = 1, I mean just turn on it once
- at the Fire Shield cast trigger, change (Key (Triggering unit)) to (Key(FISH_Caster)). Put the FISH_Caster = Triggering unit at very top

just that for now

 

[Destination9747]

Oh i uploaded the wrong map and now im not at my pc. I will update it soon
Thanks for comment

 

[Andser]

You should use Every 0.04 instead of 0.03, it will still look good and also save memory, i tested it and it needs only some little upgrades (leaks, bad hash saving, everything that said ..... , btw what a "name" ) anyways, good idea nice graphic (don't know if it is yours), so 4/5


Edit: if i could i will approve it except that leaks
Edit No.2: 5/5 all properly working and that 0.03

Trigger

• Set FISH_DamageGroup = (Units within 200.00 of FISH_Caster_Location matching ((((Matching unit) is A structure) Equal to False) and ((((Matching unit) is Magic Immune) Equal to False) and ((((Matching unit) is alive) Equal to True) and (((Matching unit) belongs to an enemy of FISH_

Instead of using matching unit use another ITE (i think that the matching unit has more bugs :/ but i am not sure), i don't know why but i prefer ITE's

 

[Destination9747]

It's mine

 

[FUJI]

What is wrong in 0.03 periodic interval?

Edit:

Theres a lot of leaks of location in periodic loop trigger. You must remove it every interval because you are creating it every 0.03 seconds.

 

[Kazeon]

you removed the leaks almost totaly wrong my friend
let me make this easier for you..

In cast trigger

- move call RemoveLocation(udg_FISH_Point) from the bottom into inside of the loop below the following action

• Unit - Move FISH_Dummy instantly to FISH_Point

so it will become

• Unit - Move FISH_Dummy instantly to FISH_Point
• Custom script: call RemoveLocation(udg_FISH_Point)

- store (Key (FISH_Caster)) below

• Set FISH_Caster = (Triggering unit)

=>

• Set FISH_Caster = (Triggering unit)
• Set FISH_Key = (Key (FISH_Caster))

then change all (Key(Triggering Unit)) in that trigger to FISH_Key
- change

• Set FISH_Level = (Level of Fire Shield for FISH_Caster)

=>

• Set FISH_Level = (Level of FISH_Ability for FISH_Caster)

and also inside the loop trigger too
- may be change the fly height rate to 0.0 is better

• Animation - Change FISH_Dummy flying height to (50.00 x (Real(FISH_ForInteger))) at 0.00

In loop trigger

- just like in the cast trigger. store (Key (Picked unit)) into a variable first
- change

• Set FISH_DummyTime = (Load (Key Time) of (Key (Picked unit)) from FISH_Hashtable)
  • Set FISH_DummyTime = (FISH_DummyTime - 0.03)
  • Hashtable - Save FISH_DummyTime as (Key Time) of (Key (Picked unit)) in FISH_Hashtable

to

• Set FISH_TimeKey = ( Key Time )
  • Hashtable - Save ( (Load FISH_TimeKey of FISH_PickedKey - 0.03) as FISH_TimeKey of FISH_PickedKey in FISH_Hashtable

- move

• Custom script: call RemoveLocation(udg_FISH_Caster_Location)
• Custom script: call DestroyGroup(udg_FISH_DamageGroup)

from the ITE
=>

• Custom script: call DestroyGroup(udg_FISH_DamageGroup)
• Custom script: call RemoveLocation(udg_FISH_Caster_Location)
• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
    • FISH_UnitGroup_Count Equal to 0
  • Then - Actions
    • Trigger - Turn off (This trigger)
  • Else - Actions

- move

• Custom script: call RemoveLocation(udg_FISH_Point)

into the loop
=>

• For each (Integer FISH_ForInteger) from FISH_INT_LOOP to (FISH_INT_LOOP + ((8 x FISH_Level) - 1)), do (Actions)
  • Loop - Actions
    • Set FISH_Point = (FISH_Caster_Location offset by 100.00 towards (10.00 x ((Real(FISH_ForInteger)) + 1.00)) degrees)
    • Set FISH_Dummy = (Load (FISH_ForInteger - (FISH_INT_LOOP - 1)) of (Key (Picked unit)) in FISH_Hashtable)
    • Unit - Move FISH_Dummy instantly to FISH_Point
    • Custom script: call RemoveLocation(udg_FISH_Point)

phew, I think just that for now
overall, I love the spell, it's so eye candy. just lacking a better coding
here some additional suggestions:
- change the tooltip

Learn Fire Shield [Level 1]
Learn Fire Shield [Level 2]
...
to
Fire Shield [Level 1]
Fire Shield [Level 2]​

- give damage delay so that user can configure the damage more freely. feels like that 4 damage per 0.03 is so hurt, so make it deals 4 damage per 0.1 second or anything else where the loop still run every 0.03 seconds.
- fix the follow through time