#91
Here are the updated ratings, I hope you like them. Thanks for participating in the contest, with such great spells!Vexorian:
ChainBurning (84/100):
Coding (24/35):
It has, however, a single memory leak in the ChainBurning_Actions function, the local c is not set to null. This might be to solve the bug with handle's ids changing, but if that's the case, you should've written it.
The way you validate targets is good, and should be faster than using I2S everytime to check if a target is valid.
Even though the spell's code is good, a lot of it is handled by the Caster System, meaning that it is not something new.
Balance (10/10):
The spell is balanced, and has options for changing it, even though it is not needed.
Idea (20/25):
I have never seen a spell like this before, and the idea is simple but still good. It's avoidable and changes direction to find new targets. Especially the fact that it's avoidable makes it unique. But the idea becomes less exiting after several casts, it has no sudden effects, but you know what it'll do, which it does good.
Eye/ear candy (15/15):
It has all the effects needed, not too many nor too few. The effects create sound effects themself, so there no problem there.
Implementability/configuration (15/15):
Your spell has configuration functions, you give a description of what rawcodes are and the effects are changeable in object editor.
Daminon:
SplittingLightning (63/100):
Code (21/35):
When you look at the spells code, you first see a gigantic block of text, the version history. There's nothing wrong with including a version history, but you should have written it in a separate trigger/comment, as it takes up space (not much, but still it takes up space) of maps.
Debugging messages is a good thing, but normally you should remove them from the spell when it's finished, it looks messy. But in this case it's a help, so I could better see how the spell progressed.
About local variables, you should use shorter names for them, because that reduces execution time. If you want it to be easy to read, comments are better.
You can assign a value to a non-array local at it's creation, if you had done that it would remove some unneeded lines (example: local unit u = GetTriggerUnit()).
When having a loop function try to store the config functions return values in locals, just because you only need to write it once, it still has to call the function for each affected unit.
You've commented your code, that is just great.
You use normal functions for configuration, you should prefix them with "constant" when possible, that makes it faster. Also if the user then uses optimizer on a map containing the spells, it'll even faster.
Balance (9/10):
I think there is slightly overpowered in-game, but of course you can just change the values in Object Editor.
Idea (15/25):
The idea is pretty nice, I have not seen it before. It's not a stunningly impressive, but pretty good.
Eye/ear candy (8/15):
The spell has nice effects, but they aren't exeptional. Also it sucks that you don't move the dummies so the lightning will follow the main target, but pauses the units instead. As sound is attached to the effects that's fine.
Implementability/configuration (10/15):
The way you tell people to make a new spell based of the original is alot like JESP's, so why don't follow the JESP Standard (this hasn't downrated you, just a suggestion)?
You have plenty of configuration functions, just not for targets picked (which is the hardest configuration function possible to make). You use an orderstring in one of the functions, and that is bad because some spells forces you to use OrderIds.
Using Trigger Editor to determine effects is a bad idea, as there is a native that can extract that info from Object Editor.
FlashingSpiral (70/100):
Code (18/35 (again)):
Lots of the same things as on SplittingLightning, non-constant configuration functions and long local names. Also the huge read me at the spell's top is using map space again.
You either have an old version of the handle vars (like the version here, at wc3sear.ch) or you just didn't use a gamecache global as the newest script says. Not good.
You should do the polarX/Y math directly, using more functions for it makes it take extra execution time.
Don't use == false, seriously comparing a boolean with a boolean to get a boolean is kinda retarded, use a not in front of it instead.
You start setting locals at declerations in the SpiralEffect function, but in the starting function you don't.
It is more advanced than the SplittingLightning, but has more errors.
NEVER destroy a timer twice, if you're lucky you will only destroy nothing (which will do nothing at all, just use some time), but I actually think it can destroy another timer if you're unlucky and thereby cause problems.
Handle Vars is not faster than getting the values each time, they're slower, so I don't see it as a good idea to store it on initialization and then read it instead of calculating.
Balance (9/10):
Pretty balanced, maybe too underpowered at higher levels, but it's easy to change.
Idea (18/25):
Nice idea, what can I say? It's nice, but not too original, but a pretty rare concept.
Eye/ear candy (13/15):
Looks and sounds nice. I only miss some massive effect created on the caster.
Implementability/configuration (12/15):
You mentioned how to implement the spell but you forgot to tell people how to implement the Handle Vars, and if people don't have those, the spells are unuseable.
Here you use OE effects, nice. Just targeting options should be in the spell.
UnholyLightning (80/100):
Code (25/35):
Again, the read me at top looks bad, but it won't downrate you.
Local should be set at creation and have shorter names. Using I2R is unneeded, and for the radius I suggest you to calculate it every time instead. The CV_UpdateRate variable is unneeded, if you wanted it to be changeable a configuration function would have been better. Remember to make configuration functions constant.
And about spell AoE vs. script AoE, that's true, but your way is not the best, it has a change to pick unwanted targets. Check the caster system to see how to do it with no change of failing.
You don't use SquareRoot, but Pow which normally is slow aswell. But as you just use the integer 2, it won't be that slow.
Again, OrderIds are better than OrderStrings.
I didn't see it fail, which makes it extremely cool, as it so easy finds the chain spell's targets. Good job, very few people are able to make well-working chain spells!
Not very long code, but works well.
Balance (10/10):
I found it balanced, and if people don't think it is, then it's easy to change.
Idea (18/25):
COOL Idea. The whole way of making it use a normal chain spell and find that spell's targets rocks. What more can I say? Well, it only does simple things with the found targets, but it's still nice.
Eye/ear candy (15/15):
Nothing's bad here, looks good and everything can be changed/removed/added with object editor.
Implementability/configuration (12/15):
Once again you mentioned how to implement the spell but you forgot to tell people how to implement the Handle Vars, and if people don't have those, the spells are unuseable.
Effect are, agian, OE editable. Good.
Nantuko_Husk:
Holy (75/100):
Code (32/35):
Generally good, no leaks, but:
Using a single loop in the initialization functions is good. The SPELL_FINISH event happens when a unit finishes channeling due to the maximum channeling time, but the SPELL_ENDCAST event happens always when a spell is stopped, both by max channel time AND by caster interruption. This means that the SPELL_FINISH event is a complete waste, and that the spell will use double execution time when it stops.
Actions function: The commenting is great, but you should have removed the trigger you doesn't use completely (the same with the effect), not just commented it, it looks horrible, takes space (not much though) and makes reading the script hard.
Attaching the spells level to the triggering unit is ok, but DON'T use "lvl" as label. Use something specific for the spell, "lvl" could easily be used but other spells..
This won't downrate you, but it has to be said: Why the heck do you say it follows the JESP Standard? It doesn't. Look at the part of the standard about naming functions and attached stuff.
You use the same natives like GetUnitX/Y to often, better store them in locals.
In the spellfinish function: Again, the unused code should've been removed. Again, the same function calls are used to frequently. SetUnitExploded should only be used when you're killing the unit, else it's just a waste of execution time.
Pausing the timer shouldn't be needed on expiration functions, but it's not that important.
wavetimer function: Again, the same function calls (like getting the spell's level and distance) are used too often. And now when you kill the units, SetUnitExploded should be used.
You don't need to manipulate pathing of Locust units. And when the dummy is spell-specific, why didn't you just do things like changing fly height and adding abilities with object editor? Doing it with triggers is unneeded.
Pretty nice way of finding valid targets.
Make the configuration functions constant too speed it up, and better use an OrderId instead of an OrderString.
Balance (10/10):
Seems balanced to me, and if it's not, then it's at least easy to change.
Idea (16/25):
Pretty nice idea, reminds me of something... It's just that the effects are used extremely often, so even though the spell itself is different, it doesn't look new.
Eye/ear candy (12/15):
Not too many, nor too few effects, and they create sounds by themself. Nice, but not extremely original effect combination.
Implementability/configuration (5/15):
It has configuration functions, but you don't tell what a rawcode are, some of the functions doesn't take the important level parameter, and you don't tell people how to implement the Local Handle Variables functions.
Effects are changeable in Object Editor, which is good.
Result == People that don't know JASS would probably not be able to use the spell.
Volcano Bash (63/100):
Code (15/35):
Well, first EVENT_PLAYER_UNIT_ATTACKED is tricky, it executes the trigger when an attack is started. This makes you able to cheat, by issuing the orders quickly after each other. This is VERY bad.
Again, configuration functions is faster when they're constant, and OrderIds are better than strings.
The condition function is fine, but 0 is also a valid percentage (Blizzard's function starts at 0 aswell).
Main function: I don't think arrays are worth it when you only need to store two objects, it actually makes you use more lines.
You store the triggering unit in a variable, but you keeps calling it!
Again, you just commented the code you decided to don't use, making it harder to read.
x and y could be set at declaration, and it makes it look weird because your calculating is wrong. * and / are always calculated before + and - unless you have paranthesises, both by computers and in real life math. This causes graphical problems.
Again, when it's a spell-specific dummy you should've added the abilities and changed size with Object Editor. And don't use expiration timers in a situation like this, you could just have put a wait in the function.
Again, this won't downrate you, but it doesn't follow the JESP Standard, as the spell doesn't allow the stun ability to have multiple levels.
No memory leaks, but pretty simple and short code.
Balance (10/10):
Seems balanced to me, and is at least balanceable.
Idea (15/25):
Nice to see a Bash spell with new eyecandy, but that's pretty much what it is. The concept of stunning an attacked unit is not new.
Eye/ear candy (15/15):
Looks very good. What more can be said? Cool eyecandy.
Implementability/configuration (8/15):
It has configuration functions, and you tell people which objects to copy. But you don't tell them what rawcodes are, and that is a problem.
Divine Shield (40/100):
Code (7/35):
Ridicule simple code. Configuration functions should've been constant. No leaks.
Like most custom spells, it doesn't support spell steal.
Balance (10/10):
Seems balanced, or is at least balanceable.
Idea (10/25):
Ok idea to make it targetable, but the base idea is not yours.
Eye/ear candy (5/15):
Just the standard eye candy. It is good but not original at all. You could've spiced it up with something.
Implementability/configuration (8/15):
You told them to copy the spell, but they could probably have figured that out themselves. You don't tell them how to change rawcodes.
Conix
SealOfChaos (65/100):
Code (25/35):
_Actions function: No leaks. As a tip, you could've spared lines by switching the order of the locals declaration.
_Hit function: The base code is great again, I suggest you to use UnitRemoveAbility instead of the UnitRemoveBuffBJ function. Three handle derrivated locals aren't set to null (tar, cas and tim).
_Seal function: Try to look at what the bj functions does, and do it directly. No leaks, but I suggest you to use null instead of WEAPON_TYPE_UNKNOWS.
Code is simple because of the Caster System, but as you didn't mae that yourself it will make you lose several points.
You should preload the effect models you use.
Balance (4/10):
Not very balanced, and will probably never be because you totally destroy the mana of the targets.
Idea (16/25):
It's nice, I haven't seen it before, but the base idea with mana2damage has been used before.
Eye/ear candy (10/15):
Looks good, but the models you use doesn't create too good sounds (almost none).
Implementability/configuration (10/15):
It has configuration functions and the effects are changeable with OE. That is great, but you don't tell people what rawcodes are, so it would be hard for new people to use it.
ChaosOrbs (70/100):
Code (21/35):
_Actions function: Nothing really bad here, just some minor stuff could be improved like removal of unneeded bj functions, but it's very few and minor things, and the important thing is that the function isn't leaking.
_Ends/_RemoveFromGroup functions: If this is the way it's destroyed the group won't be destroyed, and the table will only be cleared, not destroyed. Bad memory leaks.
_Do: Generally nice and inspiring function, it has no locals not set to null leaks, nor too much unneeded stuff, but it has a triggeraction leak. When creating triggers on-the-go in JASS, make sure you clean the actions added to them.
_Hit: Nothing special to say, no leaks.
Again, you won't get that many points because a lot of the hard stuff is handled by the Caster System which isn't made by you. But the code is pretty nice anyway.
Balance (10/10):
It seems balanced to me, else it's easy to change.
Idea (18/25):
I like the idea, it's new and I haven't seen it before. I liked that it acted well even if the spell was instant, which makes it even better. Not legendary though.
Eye/ear candy (12/15):
Fine eye candy, not too few nor to less effects. It looks great with the orbs swirling like spheres around the caster.
Implementability/configuration (9/15):
It has configuration functions, most of those are constant. But again, you don't tell people what rawcodes are, so newbs aren't able to use it. It lacks explanation on what the functions do, for example, what is the difference between the speed and speed2 functions? That can only be seen by looking at the code. Effects are easily changeable.
ChaosStar (77/100):
Code (20/35):
_Actions function: Generally good, but it has leaks: locals not set to null, a triggeraction leak, and a location that isn't removed.
_Shoot: Again a local not set to null leak.
Again a lot of the code for the collision missiles are done by the Caster System and not by you. Creating the star isn't the easiest thing to do.
Balance (7/10):
Balance isn't too good, levelling it up does almost nothing. But it's easy to change, which makes it acceptable.
Idea (25/25):
"The first pentagram spell that didn't make me want to puke!" Seriously, this idea is extremely impressing. The spell's idea has never been seen before, and is very cool. This idea is great.
Eye/ear candy (15/15):
Looks wonderful. All effects are well placed and matches the spell, and they have sounds attached to them. And like msot spell makers you don't just have the looping effects, you have extra effects like the effect that is created when the spell ends.
Implementability/configuration (10/15):
I can repeat what I've said above: it has the needed functions, but it badly misses explanation, both on the functions and what rawcodes are. You use integer return values for functions that uses reals, which means less freedom. Effects are easily changeable.
ChaosString (56/100):
Code (18/35):
It has a few memory leaks (like the timer local not set to null in the Bouncer function).
There's a return leak in the NewTarget function, which could've used less bj functions. Even though you don't use it, other people might use it.
Again, good coding, but CS does a lot for you.
Balance (7/10):
Seems imbalanced. But it can be changed easily, so you get seven here.
Idea (15/25):
Vexorian used the idea with collision missiles for chains before (for this contest), so it's not new. But you've spiced it up, by making shadows of the chain continue.
Eye/ear candy (6/15):
Looks ok, but the effect doesn always touch the targets before it changes direrection target damaging effects would imrpove it. It's look is not very impressive, and it has almost no sounds.
Implementability/configuration (10/15):
I can repeat what I've said above: it has the needed functions, but it badly misses explanation, both on the functions and what rawcodes are. You use integer return values for functions that uses reals, which means less freedom. Effects are easily changeable.
vicky2004
Implode (86/100):
Code (32/35):
Generally a very good code. Just a few things to mention:
1. Real, integer, boolean, string, and code locals don't have to be nullified.
2. Triggeractions has to be removed, saving them in a local isn't enough.
3. Please make your configuration functions constant, that makes them a but faster, and when people uses for example the Map Optimizer on their maps it replaces the calls with the values, making your spell even faster.
Other than that, it's a very well coded spell, one of the best I've seen for a long while. Great job!
Balance (7/10):
Even though it's an ultimate it's overpowered, but it can be changed.
Idea (25/25):
Great idea. One of the most original spells I've seen for a long time. Great idea, I'm given you top character for it, even though it isn't really deserved, considering that the idea is not your's.
Eye/ear candy (15/15):
Looks/sounds nice. I couldn't see a single thing missing here.
Implementability/configuration (7/15):
Rawcodes and how to implement the Local Handle Variables should have been better explained. It has congiguration functions (even though making them constant would've been making them better, and by the way use more spaces, it's hard to read). Effects are easily changeable in the OE.