2011 Spring Spellpack 0.4 by Manasurge

[Moderator]

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

Required changes

• Point2 is removed in a wrong place in Static Cast
• In Static Lightning Clear, don't do number of units in group checks when any unit dies
• Shatter messes up with the classification of the unit. Store the original classification
• Shatter -> Proper use of Crow Form
• In Shatter Loop, do the turn off check only after you remove a unit from the group
• In BoL Cast, don't use a group enumeration
• There's no point in saving the handle of the caster for the caster
• In BoL Loop you load stuff that you don't necessarily need
• TC Cast seems to leak the effect group. I don't see a point in picking a group only to add the target into it and then instantly removing it from the group

Also don't create player groups if the floating text booleans are false.


Suggested changes

• You could use tree detection in Static Cast
• Share variables between spells. The spell pack has so many variables. For example you have 20 point variables, you only need about 4 or 5. You could use only one hashtable instead of 7
• Improve tooltips, make them more like standard Blizzard tooltips

Is the spell pack really v0.4? That alone would have been a reason for me not to review this, since going from v0.4 to v1.0 implies major changes, and it is not worth it to revie incomplete resources.

Old review

11:26, 3rd Jun 2011
Maker:
Discharge:
Offest by 1.0 is quite useless:

• Set TC_MovePoint = (TC_CasterPoint offset by 1.00 towards TC_Angle degrees)

Leaks:

• Set TC_MovePoint = (TC_MovePoint offset by TC_Speed towards TC_Angle degrees)

Good job otherwise, fix those and it can be approved.

 

[x3GlikE]

Please fix the code.

 

[Manasurge]

Had issues with the posting the code since it's so much code i missed a few things. Fixed now.

 

[Maker]

Comments

Static Lightning Cleat shouldn't be on all the time.
Why are these custom scripts:

• Custom script: set udg_x1 = GetLocationX(udg_St_Point1)

Remove the or

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
    • Or - Any (Conditions) are true
      • Conditions
        • ((Triggering unit) is in St_DurationGroup) Equal to True
  • Then - Actions

Learn tooltip has no hotkey, and you could list the damage.

Use ...at 0:

• Animation - Change (Picked unit) flying height to (Default flying height of (Picked unit)) at 9999999.00

Somehow when units rise and come back down, it doesn't look good. I think you could try to change it. Like make the drop speed have acceleration.
Increase the follow through time so the caster has time to complete the casting animation.

I like the moving effects
Increase the follow through time.

This is in wrong place, causing leaks:

• Custom script: call RemoveLocation(udg_S_Point2)

Increase the follow through time.
Can be cast even if target's health is full.

Special effect leaks:

• Unit Group - Pick every unit in TC_EffectGroup and do (Special Effect - Create a special effect attached to the origin of (Picked unit) using TC_EffectTarget)

TC Dummy Clear could be turned on/off.
This is in a wrong place:

• Custom script: call RemoveLocation(udg_TC_Return)

You could load the 0 into a variable in the Dummy Clear trigger.

General:

Improve the tooltips.
Only do this after you remove a unit from the group:

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
    • (Number of units in *Group*) Equal to 0
  • Then - Actions
    • Trigger - Turn off (This trigger)
  • Else - Actions

Remove the and, it adds an unneeded function:

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
    • And - All (Conditions) are true
      • Conditions
        • Condition 1
        • Condition 2

You have way too many variables. You should share variables between spells.

Good job overall.

 

[Adiktuz]

the screenshot says, "I am THOR!!!"

Anyway good job, just follow the comments above...

 

[Manasurge]

Comments

Static Lightning Cleat shouldn't be on all the time.
Why are these custom scripts:

• Custom script: set udg_x1 = GetLocationX(udg_St_Point1)

Remove the or

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
    • Or - Any (Conditions) are true
      • Conditions
        • ((Triggering unit) is in St_DurationGroup) Equal to True
  • Then - Actions

Learn tooltip has no hotkey, and you could list the damage.

Use ...at 0:

• Animation - Change (Picked unit) flying height to (Default flying height of (Picked unit)) at 9999999.00

Somehow when units rise and come back down, it doesn't look good. I think you could try to change it. Like make the drop speed have acceleration.
Increase the follow through time so the caster has time to complete the casting animation.

I like the moving effects
Increase the follow through time.

This is in wrong place, causing leaks:

• Custom script: call RemoveLocation(udg_S_Point2)

Increase the follow through time.
Can be cast even if target's health is full.

Special effect leaks:

• Unit Group - Pick every unit in TC_EffectGroup and do (Special Effect - Create a special effect attached to the origin of (Picked unit) using TC_EffectTarget)

TC Dummy Clear could be turned on/off.
This is in a wrong place:

• Custom script: call RemoveLocation(udg_TC_Return)

You could load the 0 into a variable in the Dummy Clear trigger.

General:

Improve the tooltips.
Only do this after you remove a unit from the group:

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
    • (Number of units in *Group*) Equal to 0
  • Then - Actions
    • Trigger - Turn off (This trigger)
  • Else - Actions

Remove the and, it adds an unneeded function:

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
    • And - All (Conditions) are true
      • Conditions
        • Condition 1
        • Condition 2

You have way too many variables. You should share variables between spells.

Good job overall.

Tyvm, will take a look to my triggers when i get home round 3, 3.30
And I will share the ActualXXX variables, such as AOE and DMG, but Specific dmg sets and range set should be specific for each spell.

the screenshot says, "I am THOR!!!"

indeed

 

[x3GlikE]

Yeh its a mad pack also added the heal into my new map kepp up da good work

 

[88WaRCraFT3]

ty for sharing this, really epic 5/5

 

[Abitcent01]

Looks cool enough for me