Storm Spirit Spellpack v1.3

[Moderator]

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

• Add importing instructions into the map file
• Harvest and Overload Dummy are the same except the abilities. Delete one or the other unit type
• The death type for dummies should be Can't raise, does not decay
• Static Remnant Effect can leak a unit group and a location
• Static Remnant Effect isn't turned off if the caster dies while the remnant exists
• In Electric Vortex it makes no sense to save the handle id of the caster for the caster
• In Electric Vortex Pull you load the caster into a variable from the hashtable but you don't do anything with it, and you don't need to
• In Electric Vortex Pull you load the pull and sec before if/then/else, but you don't need it in the else
• In Overload Missile, take call RemoveLocation(point1) and call RemoveLocation(point2) out of the then and else
• You should create the harvester in the hastable trigger as it belongs to your system

Old reviews

Required changes:

• Use only one hashtable
• You have mixed map spesific actions with spell related actions in Hashtable trigger
• Add importing instructions into the map file
• Static Remnant Damage should be stored as a real
• You load Static Remnant Damage before you know if you need it or not
• You're not flushing child hashtable at the bottom of Static Remnant Effect
• In Electric Vortex Pull, you load stuff that you might not need
• Electric Vortex Pull can leak TempPoint1
• Use less dummy units in object editor
• Overload should have passive icon
• Overload buff description is lacking
• Overload Effect should not be on all the time
• Ball Lightning leaks point1 and point2
• In Ball Lightning Move, you load stuff that you might not need

Suggested changes:

• Try to use less variables, share them between abilities
• A unit can overload by using an ability, and hitting stop immediately. The ability isn't actually cast, but the unit gets the overload buff. Is that intended?
• Use AddLightningEx and MoveLightningEx as custom scripts to move create/move the lighning to get some z-offset

Always put two or more casters in spell maps.

 

[Magtheridon96]

For Static Remnant, I wouldn't recommend 0.01, it's way too low as an interval.
0.03 is much better.

Also, you should store things like (Triggering unit) and (Attacking unit) in temporary variables to increase efficiency.

Another tip:

• Set Temp_Key = (Key (Picked unit))
• Set Temp_Unit = (Picked unit)

->

• Set Temp_Unit = (Picked unit)
• Custom script: set udg_Temp_Key = GetHandleId(udg_Temp_Unit)

 

[maddeem]

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
  • (Ball Lightning Move <gen> is on) Equal to False
  • Then - Actions
  • Trigger - Turn on Ball Lightning Move <gen>
  • Else - Actions

It doesn't matter if you turn on a trigger that's already on.
So remove the If/Then/Else
Also, you stored the casting unit, but you barely used him. Instead you used triggering unit. (On ball lightning)

• Lightning - Create a Forked Lightning lightning effect from source Temp_Point1 to target Temp_Point1

Tell me if I am wrong, but isn't that suppose to go to temppoint1 to temppoint2?

Not to be a downer but the overall triggering here is kinda disappointing. :s
Though I have not tested it ingame I will give this a 3/5. Improve the code n get a 5/5

 

[x50x20]

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
  • (Ball Lightning Move <gen> is on) Equal to False
  • Then - Actions
  • Trigger - Turn on Ball Lightning Move <gen>
  • Else - Actions

It doesn't matter if you turn on a trigger that's already on.
So remove the If/Then/Else
Also, you stored the casting unit, but you barely used him. Instead you used triggering unit. (On ball lightning)

• Lightning - Create a Forked Lightning lightning effect from source Temp_Point1 to target Temp_Point1

Tell me if I am wrong, but isn't that suppose to go to temppoint1 to temppoint2?

Not to be a downer but the overall triggering here is kinda disappointing. :s
Though I have not tested it ingame I will give this a 3/5. Improve the code n get a 5/5

it's good how it is ,believe me
and IF/Then/Else is good for some pc-s , cuz if there is no if/then/else my pc goes lagging...(btw my pc is not the best :S)(sorry for my fail english xD!)

For Static Remnant, I wouldn't recommend 0.01, it's way too low as an interval.
0.03 is much better.

Also, you should store things like (Triggering unit) and (Attacking unit) in temporary variables to increase efficiency.

Another tip:

• Set Temp_Key = (Key (Picked unit))
• Set Temp_Unit = (Picked unit)

->

• Set Temp_Unit = (Picked unit)
• Custom script: set udg_Temp_Key = GetHandleId(udg_Temp_Unit)

thx . and i updated !