Ravage

[Moderator]

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

17:43, 14th Jan 2013
Magtheridon96:

• The description needs to be improved. You need to have importing instructions, a changelog, the triggers, and any other elements that would seem fit to be in a description.
• There are location leaks. I would recommend reading this thread to learn about how to clear memory leaks.
• After you create the floating text, it would be better if you were to store the last created floating text into a variable so that you don't repeat the function call over and over again (quite inefficient)
• Before you change the flying height of the caster, you should assure that he has the crowform ability. Here is a tutorial I made to teach the proper way of giving units the ability to fly. (No object editor work is needed)
• Don't use (Integer A), use a custom integer variable. You will face bugs while using (Integer A) because all spells share this same integer, so using (Integer A) in a loop that fires another the trigger that also happens to use (Integer A) will cause bugs because the value of (Integer A) will be tampered with.
• You should make the spell more configurable. There should be a configuration trigger that runs on map initialization that allows the user to directly configure the floating text properties, the special effect, the damage, the range and other things for multiple levels. All spells on THW need to support multiple levels for enhanced usefulness.
• The spell does not function properly. You're ordering a group of units to do a certain order, but this group is null.
• You should only be creating one unit and ordering that unit to cast impale in all directions in a loop. Currently, you're looping from 1 to 10, meaning you're creating 10 dummies. You only need 1.
• The dummy should have the following properties:
  • Death-type: Can't raise, Does not decay
  • Cast Backswing: 0.00 seconds
  • Collision: 0
  • Model: ""
  • Death time: 0.00 seconds
  • All animation times: 0.00 seconds
  • Selection scale: 0.00 (Hold shift to set it accordingly)
  • Abilities: Locust
  This is essential for any dummy. Dummies need to have a 0 death time so that they would be removed upon death. Dummy casters need 0-second animation backswings and times so that you could make multiple orders consecutively and not have any problems.
• Since you cached the triggering unit into a variable at the top of the trigger, you don't need to repeat it again at the end of the trigger.
• You shouldn't be turning off the collision of the dummy. The dummy should have no pathing or collision in the first place.

This needs a ton of work.
It's far too simple to be approved in its current state.
You need to focus on efficiency and configuration. Configuration, efficiency, readability and documentation/comments are what I look at most of all.

This spell is also highly unoriginal, so it's going to be very hard for it to be approved without it having flawless configuration and code. :/

 

[GreeN!X]

too simple, I must say..

 

[Magtheridon96]

I forgot to stress on the importance of a proper screenshot for the spell as well and the lack of impale effects popping up :|

 

[zv27]

Ravage

• Ravage
  • Events
    • Unit - A unit Starts the effect of an ability
  • Conditions
    • (Ability being cast) Equal to Ravage [Q]
  • Actions
    • Set RS_Unit = (Triggering unit)
    • Set RS_Point = (Target unit of ability being cast)
    • Animation - Change RS_Unit flying height to 255.00 at 255.00
    • Special Effect - Create a special effect attached to the origin of RS_Unit using Abilities\Spells\Orc\WarStomp\WarStompCaster.mdl
    • Unit - Move RS_Unit instantly to (Target point of ability being cast), facing Default building facing degrees
    • For each (Integer A) from 1 to 10, do (Actions)
      • Loop - Actions
        • Unit - Create 1 Dummy (Ravage) for (Triggering player) at (Position of RS_Unit) facing Default building facing degrees
        • Unit Group - Order (Last created unit group) to Undead Crypt Lord - Impale ((Position of RS_Unit) offset by 200.00 towards ((Real((Integer A))) x 36.00) degrees)
        • Unit - Turn collision for (Last created unit) Off
        • Unit - Add a 1.00 second Generic expiration timer to (Last created unit)
    • Special Effect - Destroy (Last created special effect)
    • Floating Text - Create floating text that reads (|cff32cd32 + ((Name of (Ability being cast)) + |r)) at (Position of RS_Unit) with Z offset 0.00, using font size 10.00, color (100.00%, 100.00%, 100.00%), and 0.00% transparency
    • Floating Text - Change (Last created floating text): Disable permanence
    • Floating Text - Set the velocity of (Last created floating text) to 150.00 towards 150.00 degrees
    • Floating Text - Change the lifespan of (Last created floating text) to 2.00 seconds
    • Floating Text - Change the fading age of (Last created floating text) to 1.50 seconds
    • Animation - Reset (Triggering unit)'s animation

You forgot to remove the location

• call RemoveLocation( udg_RS_Point )

Maybe I'm blind but I see no reason why this exists

• Set RS_Point = (Target unit of ability being cast)

This can be very problematic

• For each (Integer A) from 1 to 10, do (Actions)

use this

• For each (Integer your_integer) from 1 to 10, do (Actions)

And one more thing. You miss the group

• (Last created unit group)

 

[rulerofiron99]

effort detected

magtheridon96:

The description needs to be improved. You need to have importing instructions, a changelog, the triggers, and any other elements that would seem fit to be in a description.
There are location leaks. I would recommend reading this thread to learn about how to clear memory leaks.
After you create the floating text, it would be better if you were to store the last created floating text into a variable so that you don't repeat the function call over and over again (quite inefficient)
before you change the flying height of the caster, you should assure that he has the crowform ability. Here is a tutorial i made to teach the proper way of giving units the ability to fly. (no object editor work is needed)
don't use (integer a), use a custom integer variable. You will face bugs while using (integer a) because all spells share this same integer, so using (integer a) in a loop that fires another the trigger that also happens to use (integer a) will cause bugs because the value of (integer a) will be tampered with.
You should make the spell more configurable. There should be a configuration trigger that runs on map initialization that allows the user to directly configure the floating text properties, the special effect, the damage, the range and other things for multiple levels. All spells on thw need to support multiple levels for enhanced usefulness.
The spell does not function properly. You're ordering a group of units to do a certain order, but this group is null.
You should only be creating one unit and ordering that unit to cast impale in all directions in a loop. Currently, you're looping from 1 to 10, meaning you're creating 10 dummies. You only need 1.
The dummy should have the following properties:
Death-type: Can't raise, does not decay
cast backswing: 0.00 seconds
collision: 0
model: ""
death time: 0.00 seconds
all animation times: 0.00 seconds
selection scale: 0.00 (hold shift to set it accordingly)
abilities: Locust
this is essential for any dummy. Dummies need to have a 0 death time so that they would be removed upon death. Dummy casters need 0-second animation backswings and times so that you could make multiple orders consecutively and not have any problems.
Since you cached the triggering unit into a variable at the top of the trigger, you don't need to repeat it again at the end of the trigger.
You shouldn't be turning off the collision of the dummy. The dummy should have no pathing or collision in the first place.


This needs a ton of work.
It's far too simple to be approved in its current state.
You need to focus on efficiency and configuration. Configuration, efficiency, readability and documentation/comments are what i look at most of all.

This spell is also highly unoriginal, so it's going to be very hard for it to be approved without it having flawless configuration and code. :/

you put so much effort into these d: (all caps btw)