Spell Contest #4 - Results!

[Romek]

Results​

Contest Thread
Submissions Thread

Introduction

It's been just about a whole month, and the results are finally here. Sorry about the delay, but I was ill for about a week, and it took longer than expected anyway. Hopefully you all survived the wait!

• Note that each good/bad point doesn't necessarily mean +/- 1 point. I made a final judgement in each category based on all my comments and observations. Some things were more significant than others, and other points were entirely insignificant in terms of effect on points.
• Tiny errors such as typos in comments in the configurables were ignored entirely.
• Note that I won't have commented on anything positive about the spell unless it really intrigued me. The negative parts are usually fewer in number, and more important (especially when learning). No positive comments doesn't mean your spell has nothing good about it.
• Note that the code was also judged based on how difficult it is to code. A 5-line spell with no bad points will get a low score. A very difficult spell to code with very few errors will get a high score.
• The eye candy was based on the highest level of the spell with the default configurable values.
• The standard score for the 'other' section is 5. If the spell is unoriginal and has nothing negative that doesn't fit into the other categories, it'll get 5 points in this section. In this contest, the 'other' section is almost entirely about originality and I therefore haven't made comments in it.
• I'm aware that I may have missed some things when looking over code. The score is based on the observations I made in the code I read and understood. It's quite easy to see how skilled a person is at coding even from a small sample of code, so don't worry about the scores being inaccurate if there are any missed comments.
• Every contestant got 10 points for theme.
• You may not agree with all the comments I make, especially in the Eye-Candy section. The whole grading process is quite subjective, so this is inevitable.
• Mistakes? Problems? Don't rage. PM me. The winner isn't going to change, though I might consider awarding some additional rep if I feel my mistake is significant.

Winners

Congratulations to:

1. Infinitegde
2. Dinowc
3. emjlr3

You all receive bragging rights and rep rewards (35, 25 and 15 rep, in order (for which you may have to wait until an admin comes online), as well as rep from anyone else who's feeling generous).

Also congratulations to everyone who submitted. The top positions were very close, and all the submissions were very good. Thanks for joining, and hopefully you'll all have learned something from this contest that'll improve your chances of winning next time.
​

Tables

Name

Total

Eye Candy

Code

Skip Comments

 

[Romek]

Comments

Accname - Split-Image Slice

• Eye Candy: 13/20
  • The ground slam effect happens before the caster actually reaches the ground.
  • You don't create any death effect on the shadows when the spell is stopped, so they disappear too suddenly.
  • The caster jumping up looks quite strange, and this is especially noticeable when there are no targets.
  • Some sort of effect or cancellation when there are no targets would've been nice.
  • The 'flaw' isn't too big of a deal. It's difficult to avoid, and it's a price for getting the exact effect and damage needed. An option to use dummy units for people who'd prefer them would've been nice, but not neccesary.
  
  
• Code: 14/20
  • The spell is easily configurable with a lot of options available. However, more configurables with relation to the spell level would've been useful. The 'FinalDamage' constant, for example, is set at a constant 100 with no option for being increased per level.
  • Don't the looks of the shadows get some configurables?
  • The use of a multiple-level spell for holding an integer is clever, though not very space efficient, as each level in a spell takes as much space as an entirely new spell with 1 level. Still clever though.
  • • Do Multiple ActionsFor each (Integer B) from 1 to CONST_DummyBonusAbilityCount, do (Actions)
      • Loop - Actions
        • Unit - Add CONST_DummyBonusAbility[((Integer B) - 1)] to (Last restored unit)
    
    Could be:
    
    • Do Multiple ActionsFor each (Integer B) from 0 to (CONST_DummyBonusAbilityCount - 1), do (Actions)
      • Loop - Actions
        • Unit - Add CONST_DummyBonusAbility[(Integer B)] to (Last restored unit)
  • The use of rally-point to attach a unit to another unit is also very clever.
  • "you might wonder about the "SetUnitX"/Y custom script together with the "Unit move instantly" action but believe me, they are needed both to function properly." Would 'Order Temp_Unit to stop" not have sufficed instead of moving the unit instantly?
  • When any caster interrupts the spell, all the shadow images on the map disappear. This is probably because of a typo you made in 'EffectFinish' where you pick every unit in SplitImageSlice_Dummies instead of Temp_Group.
• Theme: 10/10
  
• Other: 7/10
  
  
• Total: 44/60

Angel_Island - Mega Jump

• Eye Candy: 9/20
  • Slightly overkill? Perhaps.
  • The slam and ground effects look great, as do the knock-up effect on the surrounding units.
  • Pausing the caster whilst casting?
  • Strange things happen when two (or more) units cast this in the same space...
• Code: 8/20
  • In Mega Jump Casting, you should always use a set variable instead of a function call when possible. You missed this out once.
  • The two periodic triggers should've been combined into a single one.
  • [ljass]GetForLoopIndexA()[/ljass] -> [ljass]bj_forLoopAIndex[/ljass]
  • This spell has no configurable globals or anything of the sort.
  • Custom value needn't be used here, especially since you have a set index for each caster which you could've used with an integer array. Any other spells or systems that need custom values are incompatible with this spell. What you're doing is actually less efficient too. This is less easily avoidable in the second trigger.
  • What's wrong with using Integer B for nested loops?
  • You're not removing a lot of your locations after they've been used (in a periodic trigger!). You need to remove a location before overwriting the variable.
  • • Set JumpCaster2[(Integer A)] = JumpCaster[(Integer A)]
      • Set JumpCaster[(Integer A)] = No unit
    ??
  • You never actually decrement JumpInstances, so this is only MUI up to 8191 total casts throughout the game, rather than 8191 active instances at the same time.
• Theme: 10/10
  
• Other: 5/10
  
  
• Total: 32/60

Dinowc - Web Strike

• Eye Candy: 18/20
  • The spell looks brilliant. It's smooth, not overkill at all.
  • The multiple effects of the spell compliment eachother very well.
  • It'd have been nice if the webs could grab targets after the hero has moved. It seems they just go through targets they would've otherwise captured.
  • Strange things happen when two enemy spiders target eachother.
  
  
• Code: 13/20
  • Double Free error when two targets taken by different instances of the spell are pulled towards the caster, when I had 4 heroes on the map casting (I haven't tested the exact circumstances behind this).
  • I got a pretty bad bug when two heroes were holding different targets. I used drag on one of the heroes, and both heroes dragged the targets to them. The other hero remained paused and unusable too, and there was another double-free. The exact circumstances behind this are also untested.
  • Comment the configurables?
  • The BJ-like functions should just be inlined into the code.
  • You'd be better off using radians throughout rather than converting back and forth.
  • There are far better alternatives than that 200 size array within the struct. Linked lists would've worked perfectly in that situation. However, even with that array, 40 instances at any one time is still very high.
  • Local triggers in the 'init' function don't need to be nulled.
  • In actions:

    set id = GetUnitIndex(caster)

    Why? And with that gone, the 'caster' local variable is unnecessary as [ljass]GetTriggerUnit()[/ljass] could've been inlined.
  • There's no need to pause a timer if you're starting it in the next line. Just start it.
  • Empty onDestroy method in struct spider?
  • Pausing units that are targetted?
  • A lot of the variable names were single letters and completely undescriptive. The code was horrible to read.
• Theme: 10/10
  
• Other: 9/10
  
  
• Total: 50/60

emjlr3 - Omnislash

• Eye Candy: 16/20
  • There seem to be too many effects which don't really flow into eachother.
  • Besides that, it's smooth and looks good. The knockback is a nice final touch.
  
  
• Code: 17/20
  • The spell didn't cast at all when PREVENT was false (Perhaps there were some other circumstances to this too)
  • "Attempt to release null timer" when the spell is cast.
  • "Double free error" seems to happen quite often too.
  • I don't see why you nested all those if-statements in the 'Selected' function; it doesn't seem to be a preference or anything of yours.
  • You probably should've used a local in that function too.
  • In the 'Conditions' function, you could've checked the 'PREVENT' constant in a static if.
  • GroupEnum... Clears groups that aren't already cleared, so there's no need to clear groups before using GroupEnum...
  • Pausing units whilst casting?
  • You don't null [ljass]local unit T[/ljass] in GetTargets (you could've just made that a global).
  • Instead of dividing by (1./TIMEOUT), you could just multiply by TIMEOUT.
  • The code in general is clean, easy to read, and well commented.
• Theme: 10/10
  
• Other: 5/10
  
  
• Total: 48/60

HydraRancher - Spider Mines

• Eye Candy: 8/20
  • The mine and explosion are very basic
  • The mines don't actually seem to explode when they're meant to most of the time.
  
  
• Code: 8/20
  • In 'Spider Mine Cast', you could've used an ability array instead of 3 individual spells with nested ifs.
  • Instead of moving an effect to the position of a unit that's unlikely to move every 0.1 seconds, how about attaching the effect to the mine, and leaving that trigger out? Since the movement of the mine is done through triggers too, you could've alternatively just moved the effect along with the mine.
  • The removal of the effect in that trigger could've been handled by a 'Unit Dies' trigger (or even in the periodic trigger which handles the explosion).
  • Damaging circular area has problems with Macs, and should be avoided.
  • The whole Spider Mine Periodic trigger seems so bloated. I couldn't actually tell that the mine was meant to move in steps of 100 every 0.5 seconds (which would look more like a teleport than a move), and the rest of the trigger just seems to handle the mine blowing up when a unit comes in range. The whole thing should've been done better and more smoothly in general.
    
• Theme: 10/10
  • Not quite what I had in mind for physical, but I won't penalize you over a misunderstanding.
• Other: 7/10
  
  
• Total: 33/60

Infinitegde - Blademasters Big Entry

• Eye Candy: 16/20
  • The effect is quite humourus indeed. It works well though, and looks good.
  • The enemy units travel far too high.
  • The hero got stuck in the side of the cliff when I targetted the top of the mountain.
  • It's not as smooth as it could be, though that's a difficult problem to overcome.
  
  
• Code: 15/20
  • Pick a naming convention and stick to it. It's difficult to tell what's what when reading the code.
  • In all those terrain/unit Z functions, you could use [ljass]MoveLocation[/ljass] instead of creating and destroying a location every time.
  • Instead of grouping all units in range, then FoG looping with an if-condition to check if the unit is an enemy, how about using a filter which checks if the unit is an enemy instead?
  • [ljass]boolean flag[/ljass] in periodic method?
  • I fail to see why you're extending structs and using stub methods. Sure, it works; though putting the relevant methods into a single struct rather than randomly splitting them up would also work.
  • The spell seems rather difficult to code, and you did a good job with it.
• Theme: 10/10
  
• Other: 10/10
  • The idea is original and brilliant.
  
  
• Total: 51/60

Jedi - Shield Slam

• Eye Candy: 11/20
  • Looks interesting. It'd be nice if there was some sort of other animation rather than the soldier just standing and sliding around, but that's too much to hope for.
  • The hero slides too far, I think.
  • Some sort of check to see if the target is flying would've been handy. The hero gets stuck in cliffs or water if a target flying unit is above them.
  
  
• Code: 14/20
  • There's not much to say here. The code is short and quite easy to make. There are also few flaws in the code (though it could be made more efficient and cleaner through the use of vJass, I assume there was a reason for the vanilla JASS).

  • if IsUnitType(target, UNIT_TYPE_MAGIC_IMMUNE) then
        if udg_ShieldSlamPushMagicImmune then

    Could be:

    if (not udg_ShieldSlamPushMagicImmune) and IsUnitType(target, UNIT_TYPE_MAGIC_IMMUNE) then

    , then stop the spell in the 'then', and push the target in the 'else'. No duplicate code, and JASS short-circuits if-statements.
  • The last 'return' in the periodic function shouldn't be there. It doesn't do anything other than prevent leak removal.
• Theme: 10/10
  
• Other: 6/10
  
  
• Total: 41/60

Necrach - Heavy Dance

• Eye Candy: 8/20
  • It just looks like a self-target earthquake with a stomp effect.
  • The jump isn't high enough to notice without zooming in.
  
  
• Code: 11/20
  • Using rally points for attachment is quite clever.
  • As is your method of of ensuring the spell is cancelled when the caster stops channeling; although periodic triggers wouldn't be too bad with this, especially if you could avoid constantly recreating dummy units. It wouldn't be a huge difference in terms of efficiency anyway.
  • The spell is easy to code and short, so there's not much to comment on.
• Theme: 10/10
  
• Other: 5/10
  
  
• Total: 34/60

NeuroToxin - Wind Slash

• Eye Candy: 11/20
  • The caster should have some sort of animation whilst sliding.
  • The 'jump' effect isn't smooth at all.
  • The rest of the effects are nice.
  • Pausing the caster whilst casting?
  • I casted the spell twice (two heroes) at the same time (both on a single target and different targets - the effect was the same), and my hero and a random peon ended up paused in the air, floating. This would also happen if the target died whilst being casted on.
  
  
• Code: 7/20
  • Why the choice of using random BJs in random places?
  • Pick a naming convention and stick to it. The code is difficult to read otherwise.

  • set tempreal = SquareRoot(dx * dx + dy * dy)
            set dist_to_go = tempreal

    You don't use tempreal anywhere else. You should've just set dist_to_go to the value needed.
  • Instead of converting back and forth between degrees and radians, just use radians.
  • It's not as configurable as it could be. Things like how long the hero 'jumps' for are hardcoded.

  • call DestroyBoolExpr(Filter(function Units))

    ...Why?
  • You don't null tempgroup (which could be global, by the way).
• Theme: 10/10
  
• Other: 5/10
  
  
• Total: 33/60

Weep - Deep Beast

• Eye Candy: 12/20
  • The effects fit the hero quite well
  • The spell seems to have the potential to be around forever..?
  • It looks good; not overdone, and not underdone. Nothing special though.
  
  
• Code: 15/20
  • Considering you're using GUI, this was coded very well. You probably should've saved X/Y instead of location of the unit, as you're only using the location to retrieve the coordinates anyway.
  • There's not much else to say, really.
• Theme: 10/10
  
• Other: 6/10
  
  
• Total: 43/60

XeNiM666 - Blade Dance

• Eye Candy: 13/20
  • I found it almost impossible to time everything correctly to get it working, though when it worked, it looked quite spectacular.
  • It became frustrating to use because of the whole "If timed right" thing. Despite using the spell again at the times described, I couldn't progress further (besides a few flukes).
  • I would've preferred the spell if it was the full thing without any special interactions.
    
  
  
• Code: 16/20
  • Useful Functions should've just been put in the same scope as the spell.
  • You don't need to specify the size of arrays under 8191.
  • Hooray for prelaoding!
  • Instead of all those global arrays, you could've used an array struct (or better yet, an AIDS struct); it'd have kept things cleaner and more organized.

  • call IssuePointOrder( u, "move", GetUnitX( u ), GetUnitY( u ) )

    ->

    call IssueImmediateOrder(u, "stop")

  • call SetPlayerAbilityAvailable( GetOwningPlayer( d.caster ), SPELL_ID, false )

    Forcing MPI instead of MUI?
  • You don't need to null struct members.
  • The code is clean and well made.
• Theme: 10/10
  
• Other: 8/10
  
  
• Total: 47/60

​

 

[HydraRancher]

Hehe, I'm not good at spells. Congrats to the winners.

 

[Dinowc]

woah 2nd
nice

I knew the code will be a problem
I was coding that for maybe 2 hours, while fixing the bugs took 10 hours

I never tested it with multiple spiders tbh and I was afraid to, because I get really depressed and frustrated when bugs occur

thanks for taking your time looking at my junk wall of code and criticizing it, it helps me improve and avoid those mistakes next time

congratz to everybody!

EDIT: lol beaten by 1 point

 

[Accname]

nice done! a great contest.

i would like to improve the skill even more, maybe i could upload it in the turtorial & resources sub-forum.

where did the spoilers go?

The ground slam effect happens before the caster actually reaches the ground.

true, should have fixed that.

You don't create any death effect on the shadows when the spell is stopped, so they disappear too suddenly.

there is a death effect played, its: Abilities\Spells\Items\AIil\AIilTarget.mdl
by default.

The caster jumping up looks quite strange, and this is especially noticeable when there are no targets.

doesnt look that strange for me, but i cannot imagine how to improve this. could you give me some example?

Some sort of effect or cancellation when there are no targets would've been nice.

The spell is meant to be used even if no targets are there. the spell has other uses but solely dealing damage. you can hide yourself for example or avoid damage.

The 'flaw' isn't too big of a deal. It's difficult to avoid, and it's a price for getting the exact effect and damage needed. An option to use dummy units for people who'd prefer them would've been nice, but not neccesary.

I asked you whether i could upload an alternate version using dummy units, you told me i should wait for after the contest.

The spell is easily configurable with a lot of options available. However, more configurables with relation to the spell level would've been useful. The 'FinalDamage' constant, for example, is set at a constant 100 with no option for being increased per level.

yeah, got lazy at the end. i considered to put even more configurations into it but then i didnt felt like doing it.

Don't the looks of the shadows get some configurables?

they have, i gave the possibility to use abilities (like sphere) to attach special effects on the shadows. in fact i was doing this already and everybody could have changed the art effects defined in the ability.

The use of a multiple-level spell for holding an integer is clever, though not very space efficient, as each level in a spell takes as much space as an entirely new spell with 1 level. Still clever though.

not space efficient, yeah, i know. but i didnt wanted to use custom value (for obvious reasons) and i didnt wanted to use a hashtable (for less obvious reasons) and i couldnt think of something else.

• Do Multiple ActionsFor each (Integer B) from 1 to CONST_DummyBonusAbilityCount, do (Actions)
  • Loop [*]Actions
    • Unit - Add CONST_DummyBonusAbility[((Integer B) - 1)] to (Last restored unit)

Could be:

• Do Multiple ActionsFor each (Integer B) from 0 to (CONST_DummyBonusAbilityCount - 1), do (Actions)
  • Loop - Actions
    • Unit - Add CONST_DummyBonusAbility[(Integer B)] to (Last restored unit)

yes it could be. but why? there is no difference.

"you might wonder about the "SetUnitX"/Y custom script together with the "Unit move instantly" action but believe me, they are needed both to function properly." Would 'Order Temp_Unit to stop" not have sufficed instead of moving the unit instantly?

dunno, maybe. but is there this much of a difference?

When any caster interrupts the spell, all the shadow images on the map disappear. This is probably because of a typo you made in 'EffectFinish' where you pick every unit in SplitImageSlice_Dummies instead of Temp_Group.

thank you for pointing that one out! this really is a bug, misclicked that stupid GUI.

 

[Weep]

Congrats, all!

The spell seems to have the potential to be around forever..?

Just to clarify, no, the maximum number of strikes is limited.

 

[Romek]

> Hehe, I'm not good at spells. Congrats to the winners.
There's always a chance to improve. Good job anyway.

> thanks for taking your time looking at my junk wall of code and criticizing it, it helps me improve and avoid those mistakes next time


> i would like to improve the skill even more, maybe i could upload it in the turtorial & resources sub-forum.
Go ahead.

> there is a death effect played, its: Abilities\Spells\Items\AIil\AIilTarget.mdl
It's not played when the spell is stopped (interrupted).

> I asked you whether i could upload an alternate version using dummy units, you told me i should wait for after the contest.
I know I did. I felt that adding a comment seemed appropriate though (and it clearly states that it's neither a big deal nor necessary).

> yes it could be. but why? there is no difference.
Only decrements a value once. It's a very minor point.

> dunno, maybe. but is there this much of a difference?
There's no reason to move the unit, so why move it? It's comparable to 'Do Nothing' statements, just not as bad.

As for the rest of your points, Accname; I suggest you read the list in the introduction of this thread.

> Just to clarify, no, the maximum number of strikes is limited.
I did the eye-candy before I even opened the trigger editor, so that's just from what I saw.

 

[XeNiM666]

congrats to the winners!

call IssuePointOrder( u, "move", GetUnitX( u ), GetUnitY( u ) )

->

call IssueImmediateOrder(u, "stop")

i had trouble with this one. when i used "stop" it just sets the current order to "null" but able to move, making it possible to leave for the duration of the spell.

It became frustrating to use because of the whole "If timed right" thing. Despite using the spell again at the times described, I couldn't progress further (besides a few flukes).

sorry about that but thats another "physical" part of the spell, is to use the players own instincts to time it right.

The code is clean and well made.

thanks, now I'm quite content with my code now. and thanks for criticizing

also, your pics? or attachments to the first post says invalid attachment for me..

 

[Romek]

> i had trouble with this one. when i used "stop" it just sets the current order to "null" but able to move, making it possible to leave for the duration of the spell.
Oh, right. Well, that's only a minor point anyway. What bothered me is why you forced MPI...

> also, your pics? or attachments to the first post says invalid attachment for me..
Should be fixed now.

 

[XeNiM666]

What bothered me is why you forced MPI...

sorry about that. didnt really see that it made it MPI. but it was the only way to make it non spammable because if I just add remove it, it will restore its cooldown too.

 

[HydraRancher]

So Romek, you hosting the next competition?

 

[vypur85]

Didn't follow the contest much but awesome judging, Romek. Love the details and explanation.

And congratulations to the winners!

 

[Angel_Island]

Congratulations to winners. Actually made my spell in less than an hour, so didnt excpect to win, but it was fun.

 

[KaerfNomekop]

After seeing the quality of other people's work, I feel intimidated to join the next contest. Any hints about it?

 

[emjlr3]

doing this is a lot of work - not sure people realize just how much. Thats why I ended up leaving things to a poll sometimes, just because it ended up being too much.

well done Romek.

Now we just need to get another working award system in place ^^.

 

[XeNiM666]

Now we just need to get another working award system in place ^^.

I second this! Maybe some medals to go with the green bar rep system.

OT:
GJ Romek! You created, hosted, graded and commented the whole contest by yourself!

 

[Romek]

> So Romek, you hosting the next competition?
Not anytime soon. I have exams between May 18th and June 26th, so I won't have the time to. I'll host another contest at some point in time though. Any other moderator (or member - with permission) can host a contest in between though. There's no fixed schedule.

> Didn't follow the contest much but awesome judging, Romek. Love the details and explanation.
> GJ Romek! You created, hosted, graded and commented the whole contest by yourself!
Thanks

> Actually made my spell in less than an hour, so didnt excpect to win, but it was fun.
At least you enjoyed it!

> After seeing the quality of other people's work, I feel intimidated to join the next contest. Any hints about it?
I'll make it clear when I'm intending to host another contest, though it won't be anytime soon (see above). Someone else will probably host the next contest.

> doing this is a lot of work - not sure people realize just how much. Thats why I ended up leaving things to a poll sometimes, just because it ended up being too much. well done Romek.
Indeed. People only realize just how much work it is until they try it themselves . Thanks though.

> Now we just need to get another working award system in place ^^.
IIRC, we were meant to get one with the update...

Once again, congratulations to everyone who entered!

 

[Weep]

Didn't follow the contest much but awesome judging, Romek. Love the details and explanation.

Yes, it's very nice.

Any hints about it?

Do your best and have fun. :thup: