Koleshy Spell Pack By ArOn [KE]

[Moderator]

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

Koleshy Spell Pack | Reviewed by Maker | 29th May 2013

NEEDS FIX

Add importing instructions into the map file and mention all the things one needs to set like the abilities and dummy unit types Spell buff is not MUI, it won't work correctly if you have more than one of the same hero for one player. This is because you enable/disable the abilities You don' need to use an array variable for something you remove right away, SA_TargetPoint in Spirit Attack This makes unit not selectable, it can still be targeted and damaged You can try use it instead of deselecting with a trigger Actions Set u = some unit Custom script: call UnitAddAbility(udg_u, 'Aloc') Custom script: call UnitRemoveAbility(udg_u, 'Aloc') Unit - Add Metamorphosis to u Unit - Order u to Night Elf Demon Hunter - Metamorphosis Unit - Remove Metamorphosis from u [/stable] [*]You don't need any kind of indexing in Group Of Snake or Lucent Beam [*]Locust ability makes the dummy invulnerable, it doesn't need Invulnerability ability [*]Set Art - Animation - Cast point to 0 for the dummy for instant casting [*]Spell buff hould have active type learn icon [*]You could improve the tooltips [*]Mega illusion buff has no description [*]Lucent Beam has wrong hotkeys [tr]

[tr][td]

Old review 22:28, 18th Feb 2013 Magtheridon96: Spirit Attack The "165" value should not be hardcoded. It should be configurable in the form of a duration of some sort. The configuration would be done with a trigger that runs on map initialization. The user would be able to configure your spell using that trigger without having to touch any of the spell code directly. The special effect should also be configurable. None of the points need to be arrays since you're using them as Temporary containers. You can use 1 point variable and you'd still be fine here. (A non-array variable) The abilities should also be configurable so that the user can customize the spell and set it up correctly without having to mess too much with the code. Group of Snake Spell Since this is an instant spell, you don't need to do indexing for it. In addition to point 1, you don't need to use arrays here at all. The distance and the number of snakes should be configurable from a trigger that runs on map init to set some variables. You would determine the angle (The 50.00) by doing (360.0 / SnakeCount). Make sure this supports multiple levels as well. All spells on this site must support multiple levels in order to be approved. The level of the ability should be stored outside of the loop because it doesn't change while you're iterating. The same thing can be said about the owner of the caster. Instead of (Owner of Caster), do (Triggering player). The expiration time should be configurable. The Snake unit type should be configurable. Pretty much every constant that a user can change should be configurable :v Don't use (Integer A), use your own custom integer for the loop. Extreme Cut Animation speed and pretty much every other constant including the special effects and the damage per level should all be configurable from a trigger that runs on map initialization. You're never removing the locations you're creating, so you're leaking them in the process. Don't use (Owner of caster), use (Triggering player) The tick count should be configurable in the form of a duration (11 should be configurable as 0.33 seconds by default). Users should be able to change these kinds of things. Lucent Beam You don't need to use indexing here, and you don't need the arrays at all because the spell runs instantly. Warcraft III is single threaded, so you will have no problems with Instanceability in this case. The duration should be configurable. So should the unit type, the order id and the abilities. You should be able to share the variables you're using in the Group of Snake Spell with the ones you're using in this one. All instant spells can share the same variables to store temporary data and you will face no problems. Mega Illusion The damage per level should be configurable from an initialization trigger. Whether or not collision is turned off should be configurable as well. This can apply to your above triggers too. Some people don't want anyone messing with their units' collision flags no matter what. Since you're only using the periodic trigger for the sake of having a 0.03 second delay between the actions you're doing (and they happen to be consecutive), I see no reason why they couldn't all just be in one trigger. This could all be 1 instant spell. In that case, you wouldn't need to use indexing, arrays or even a loop trigger. The duration (0.75) should be configurable and so should the effects. Remember, if it's a constant, it should be configurable from an initialization trigger so users don't have to mess with your code and break something just to customize the spell a bit. Make all the abilities and order ids in all your trigger configurable too, and even the ability that is supposed to be casted, because when importing from one map to the other, the raw codes will get messed up. In the Move trigger, instead of (Entering unit), use (Triggering unit) General tips In some spells, you're not caching important data that should be cached. For example, the level of the ability for a caster should be stored in the beginning of a spell instance, either in an array for timed spells or in a non-array variable for instant spells because the level will not/rarely change. Even if it does change, users would want to retain the same behavior for the level in which this spell was casted. The same can apply to the owner of the caster. I've repeated this above many times, but I'll repeat it again anyways: It's important that you have triggers that run on map initialization in which you set constant values and have lots of comments so users can customize your spell without having to mess with your code because they can mess something up and they'll come running over to the thread and they'll blame you for it because users will be users. It's always important to cache things into variable if you're going to repeat them, especially if they're inside parentheses. When something other than an arithmetic operation is inside parantheses, it's a function call. Function calls should be minimized because they have a lot of overhead relative to other operations. Even Math in Warcraft III is a slow behemoth :V I hope this helps!

[/TD][/TR]

 

[Elprede]

Entering Unit --> Triggering Unit in Mega IIusion Move. You should also cache the triggering unit because caching will reduce function calls, increasing speed. What is wrong with some help? I do not understand why you mention there is 1 extra when your pack contains exactly 5 triggered spells. Perhaps, you considered Lucent Beam as an extra because the spell has only 1 trigger as compared to the other spells. Oh yeah, forgot to mention this, could you post triggers for those who do not have a copy of Warcraft 3?

That's all for now.

 

[ArOn]

not know like putting here triggers :/

 

[Elprede]

Here is a tutorial: http://www.hiveworkshop.com/forums/miscellaneous-tutorials-456/how-easily-post-triggers-163285/. Copy your code as text and paste it inbetween

• 
• 

.

 

[ArOn]

What is the code to place the triggers? []?
i said an extra skill because that is not in the pack koleshy (Lucent beam is dota skill [Luna Hero])

I did it on the same map because a friend of mine needed this Lucent Beam

 

[Elprede]

What is the code to place the triggers? []?
i said an extra skill because that is not in the pack koleshy (Lucent beam is dota skill [Luna Hero])

I did it on the same map because a friend of mine needed this Lucent Beam

Post your code in between trigger and /trigger with [] around trigger and /trigger.

 

[ArOn]

I will download WE Unlimited (default english)

EDIT: DONE, triggers are all posted

 

[super_nova]

What is the code to place the triggers? []?
i said an extra skill because that is not in the pack koleshy (Lucent beam is dota skill [Luna Hero])

I did it on the same map because a friend of mine needed this Lucent Beam

What the?
You don't even need a trigger for that skill man

 

[Almia]

The only thing this spell pack must have is Configuration Triggers.
Also,Using 0.50,0.25 or greater than 0.03125 or less will make a looping spell ugly.
Please make a custom timer instead so that units wont have same effects per loop.
Like:

set timer = timer + 0.03
if timer == maxtime then
//Do things
endif

 

[ArOn]

What the?
You don't even need a trigger for that skill man

Here I am adding a dummy to the target evade attacks

The only thing this spell pack must have is Configuration Triggers.
Also,Using 0.50,0.25 or greater than 0.03125 or less will make a looping spell ugly.
Please make a custom timer instead so that units wont have same effects per loop.
Like:

set timer = timer + 0.03
if timer == maxtime then
//Do things
endif

already change the time of detonators (loops)
but this:

set timer = timer + 0.03
if timer == maxtime then
//Do things
endif

try creating a real variable to a timer but the performance got worse...
or maybe the wrong place idk

 

[Almia]

note,timer is just a name for the real variable

 

[ArOn]

I know it's a variable
but I do not understand what else you want to add
i use variable C like timer...

 

[xorkatoss]

very good spells man! althought you forgot to add R hotkey for Lucent Beam.

optional:
add this to map init:

• Visibility - Disable fog of war
• Visibility - Disable black mask

Rating: 5/5
Award: (+2 rep)

 

[ArOn]

very good spells man! althought you forgot to add R hotkey for Lucent Beam.

optional:
add this to map init:

• Visibility - Disable fog of war
• Visibility - Disable black mask

Rating: 5/5
Award: (+2 rep)

thank you very much.

and I wonder how would be the state of my spell (?)
I'm waiting for an answer xd

 

[ArOn]

sorry for the double comment but would like to know the status of the map if you are approved or?

 

[ArOn]

spells fixed now:
Spirit Attack, Extreme Cut, Mega Illusion and new spell i added.
i think now is MUI...