Zephyr Contest #11 - Results

Pharaoh_ · Sep 16, 2014

Bannar:

http://www.hiveworkshop.com/forums/...ura-your-aura-254166/index22.html#post2565304

Uniqueness : 9.5/10
The concept of using dead units as supporters in form of fallen angels is very creative. Furthermore the growing and the absolute corpse limit, with a following death gives the spell a nice construction.

Coding: 17/20

function GetSpawnChance can be constant.
Also it should be noted in the comments section that the returned value is in %.
Alternatively rename the function as GetSpawnChancePercent.
You use function GetInactiveDist / function GetActiveDist and ACTIVE_SPEED / INACTIVE_SPEED.
Decide whether they should be functions or variables, one of these pairs is redundant. If you decide to make them functions, the name should have the word "speed", not "distance", so as to provide a better understanding to the user.
GetOwningPlayer(angel) --> you can add a player member to avoid this.
Attachment point should not be hardcoded.
AttackType/DamageType should not be hardcoded.
WEAPON_TYPE_WHOKNOWS --> null, it's faster, takes less space and is the same at the end.
In the callback (PudgeAngel) the set x = GetUnitX/Y(angel) should come after the check if instance is still valid, and can be put in some single elseif parts, because you don't use it in each case.
I'm not sure it's necessary to re-calculate the unit's facing instead of directly using angle in State.Fly. The difference should not be blatant, but, in case you keep it, you can use a variable to avoid two function calls.
You check if target is still indexed, that's good; but also check if target is alive to refrain the code from additional calls.
You forgot SquareRoot in order to evaluate the distance between target/angel, so the following actions might be unwanted and bring wrong results.
In callback (PudgeCaster) use tmpReals for x/y. Also GetOwningPlayer(unit) could be replaced by one player member.
In a private method you name an argument real activeOrNot.
The name sounds like a bool and not like a real.
No need to null a trigger unless you destroy it.

Code is written very clear and is nice to read. The comments are also readable and are a good guide throughout the system.

Visuals: 10/10

The floating movement of the fallen angels and the fading in-out effect is very nice and intuitive.
Icon is fitting the theme very well.
Tooltip is very informative and also its length is fine.
Blood effects are cool, and also the custom death animation before explosion looks very good.
The wings on the risen abominations is fitting the "Angel" theme well.

Everything is nice in game. I especially like the fading in/out of angel's visibility because it helps the spell not to draw too much attention.

Kyrbi0:

http://www.hiveworkshop.com/forums/...ura-your-aura-254166/index23.html#post2565468

Uniqueness : 4/10
As you can see these ability features compared to normal WarCraft 3 abilities are not very creative.
However, the combination of some abilities along with the variation of damage depending on the range of affected units towards the ability bearer are quite special.

Coding: 3/20

Combine both triggers into one.
And - All Conditions block is not needed.
Store OrderedUnit into a unit variable, to optimize performance.
Only one hardcoded UnitType may use the aura. You should have made it configurable.
Spell that is checked in condition is hardcoded. Should be configurable as well.
No configuration overall.
No import instructions.
Credits missing for used imports.

The positive aspect is that it's MUI. But even in these few lines you made relatively many coding mistakes. But that's not the biggest issue; your aura is nothing but a combination of WarCraft 3 default spells, which you add and remove to a unit via triggers.
This concept of a non-triggered aura violates the rules, and so it cannot be eligible for approval.

Rules, especially this:

Spells must be useful. Extremely simple, cinematic, or other spells which have no place in a game will not be accepted.

Visuals: 7/10

Circular aura looks good as special effect, but is a bit too small. Units outside of the visual range were also affected by its effect (which in turn makes it confusing for the player - especially the enemy ones).
The buff for affected enemies is a tornado. A tornado has not much to do with your Death Rattle aura. Thus, you should have modified the buff's icon.
The attached effect over the enemy units looks good.
Icon is fitting and it's nice how there are used two different icons used for turning the aura on and off.
Important values should be highlighted in the tooltip for the sake of consistency.

In the tooltip you accidentally use two spaces before "Also", and there is also a typo in "magicks".
Why is it that the name is "Death Rattle3" in object editor? The number makes no sense.

This entry is violating the rules - disqualified. (explained in coding part)

GunSlinger:

http://www.hiveworkshop.com/forums/...ura-your-aura-254166/index22.html#post2565398

Uniqueness : 6.5/10
The idea of draining HP/MP from enemies to load a charge burst to heal nearby allies is very cool.
What I don't find very logical is that you can also drain mana from units that have no mana at all; this might become confusing for the user.
What is strange is that the unit has to explicitly activate the aura, but has no chance anymore to deactivate it. Hence, it would make sense if it would be executed automatically after learning.

Coding: 13/20

For better performance references like PickedUnit/TriggeringUnit that are used more than twice, it would be optimal to be stored into tmpVariables.
Add a check in init trigger with Max_Index before turning on the loop trigger.
The variables Life/Mana_Loop_Start are not needed at all. You can use the Life/Mana_Max_Index.
Having two seperate variables for MP and HP effect is not needed, since the unit can have only one of them at a time. Thus, you should simply use one-general-effect variable.
For Life & Mana blast action: Put the set bj_wantDestroyGroup=true before the If/Then/Else.
The check to turn the periodic trigger off should be moved into deindex part.
Set the unitVariable[MaxIndex] = null, in your deindex part.
Also deindex if unit[index] equals null, so instances for removed units get recycled.
You could combine both periodic triggers into one. Then the check to turn it off would be a condition of MAX_LIFE- and MAX_MANA- indexes equal to 0.
The attachment point should not be hardcoded.
Damage type should not be hardcoded.
Naming of variables should be something like prefix + underscore + (useful)variableName. This way, a unique name is ensured, that is also readable and informative.
You actually use a prefix, but not for all system-relevant variables.
Import instructions are missing.

You could use two unit groups to avoid looping through all instances to check if a unit is in HP or MP array or in neither of them.
When the unit gets an instance of HP_Array, add it to your HP_Group. Same way for MP_Array. This way, the loop and the useless checks can be avoided.

Visuals: 7.5/10

The effect on Lich is interesting and looks nice. It's also informative how the color indicates the aura's state. (HP/MP)
Tooltip is readable and everything important is mentioned. (a typo: change 15 --> 20 instances for threshold)
The revenge avatar as icon is not fitting the theme very much. It's instinctively related with something more active like unit creation.
Something like mana shield as base ability would fit better for the spell, as then you also could use two different icons: one would be for mana state and one for health state.
As other effects also separate HP/MP with visuals, two different effects for blast would also be better than using solely one for both states.

Lastly, credits for used models are missing.

Tank-Commander:

http://www.hiveworkshop.com/forums/...aura-your-aura-254166/index5.html#post2550409

Uniqueness : 9/10
Multiple units getting struck within a certain AoE is similar to Priestess of the Moon's ultimate. However, the concept of Soul Tear's multiple shooting to nearby enemies as a passive aura is both sufficient and intuitively made.
The chance for additional magic damage and the AoE damage on death strengthen the creativity and potentially gameplay dominance.
The restoration of HP of affected enemy units after the bearer's death is a nice addition to the spell concept.

Coding: 15/20

In main loop:
StageId3: No need to set TempReal and TempReal2 to a new value for damage calculation. Just directly use the formula for damage.
Use one global timer that you start/pause instead of destroying and then re-create it. Even if it probably won't happen too often, it's still bad practice.
Moreover, each time you recycle a node you give GetExpiredTimer() as a parameter. This parameter will be not needed anymore, because you can just refer to the global timer in the recycling function.
In function STA_AuraEnd you loop to find the right caster. Instead you can just operate with Caster[Node], because the needed Node is already given as parameter.
Removed units aren't properly removed from the system. The problem is you can't confirm whether dg_STA_StageID[TempNode] == 3 in your RemoveAura function. Hence, nothing will happen, and it will never end.
No need to declare a local variable for only one function call. (LearnAbility function)
In function STA_NewAura you always loop through all instances to check if unit is already registered as an instance.
You can check GetUnitAbilityLevel(u, STA_AbilityID()) == 1; if unit has learned the ability, then no scan is needed at all in the NewAura function (can be applied with a boolean parameter to scan/no-scan).
Also exit the loop once you found an instance that equals to the unit, to avoid further checks.
However, alternatives for non-looping checks would be using a group for instances or by saving a boolean in index/handleId of unit. This would have some good benefits, as no scan loop would be needed at all, because even for potentially new units, the user might check periodically in function STA_EnableUnits if picked unit is in group or if boolean is true before running function STA_EnableUnits (just like you check with STA_AffectedUnits before you add a new unit).
In function STA_TargetFilter directly return the expression.
Using (GetWidgetLife(u) > 0.405) does not suffice in all cases. Using IsUnitType(u, UNIT_TYPE_DEAD) or GetUnitTypeId(u) == 0 is the recommended method in yours.
In function STA_SoulProjectileDamage you set your player variable twice.
Player(14) should be a constant. (unit creation)
If using more than one function call a tmpVariable should be used.
For if statements: Just leave the function alone instead comparing it to == true. For comparing to false it would be an additional not before.
No need to null local player, even if it's an agent. Players are constant and are never destroyed.
No need to null a trigger unless you destroy it.

The code and configuration part is documented in detail and is very helpful. Sometimes there is no need to explain every step, like "Declare locals" because it's obvious for the reader. But better to have a bit too much than not enough commented.
User has a wide range to configure the spell to his needs and that's very positive.

Visuals: 10/10

Tooltip has a very good length, is simple, and everything is explained thoroughly.
Effects in game are just excellent, there is nothing to complain about. They are related to the theme, picked very nicely, and are optimally placed.
The fact you offer a "Fire set" next to the default visuals is very nice.

Good job at visuals.

rulerofiron99:

http://www.hiveworkshop.com/forums/...aura-your-aura-254166/index8.html#post2551866

Uniqueness : 9/10
Gaining speed while running straight forward for a quick rush and consequently loading an attack in zeal is fitting very good; it would be a very cool aura for riders.
It would be an awesome feature, if the charge damage depended on the current movement speed and was not static. The more speed the unit has, the more damage would be dealt. Although it would be indeed better to make it dynamic, WarCraft III spells are used to be static in damaging, so I cannot directly argue with your choice.

Coding: 14/20

About config trigger:
Everything is explained very well.
Configuration is wide enough and can fit to the needs of any user.
TriggerRegisterTimerEventPeriodic --> TriggerRegisterTimerEvent
You forgot to remove the default periodic events for <Detect Loop> and <Level Detection Loop>. So finally, there will be two periodic events that run the trigger. It' just a small mistake, but very important to fix it.
You run this action:

If (RGMR_GroupTime Ungleich 0.00) then do (Custom script: call TriggerRegisterTimerEventPeriodic( gg_trg_RGMR_Group_Move_Loop, udg_RGMR_GroupTime )) else do (Do nothing)

... but both Group Move triggers are deactivated, so unusable in game. You should have removed both triggers and also the action posted above.

In Detection Learn trigger:
The custom script is not needed, you just can add a second GUI condition to avoid two ifs.
If you keep the custom script, then get rid off the BJ and link both conditions with and.
The Level Detect trigger can be avoided with one more If/Then/Else in Detection trigger. Just check for UnitBuffLevel[index] < LevelFor RGMR_Ability for TempUnit before you set UnitBuffLevel to a new level (then you will also have to check if unit is moving in Detect Loop).
Also store second PickedUnit into a tmpVariable in Detection Loop.
Main Loop: LastX/Y can be set only if you don't deindex and just set them equal tmpX/Y. No need to call again GetUnitX/Y. In "Check Moving" do it vice versa. Remove the Or - Conditions block, so you have two normal conditions, check for equals true not false, and then set to false in "Then - Actions" not in "Else - Actions".
You can combine the Acceleration together with LoweringSpeed after turning.
And also check if speed is over TopSpeed afterwards (now you check if the unit is turning after your check for TopSpeed).
TempLoc is not used at all.
In your SetUnitX/Y get rid of the BJs and of the GetUnitX/Y (use tmpX/Y instead that you set before).
When you check for UnitMoving stack for the unit, you have to set UnitMoving = True in "Else - Actions" part; else, the whole concept for stack has no effect.
In trigger AttackEvent you can store TriggeringUnit into a variable once your checks are all true.

The code was good to read altogether and comments helped a lot. There are small flaws in efficiency (e.g. not needed Level Detect Trigger), but the concept was fine.
Furthermore you definitely have to fix the problem with two periodic events, then it's on a good way.

Visuals: 8/10

The buff icon is not fitting much. There could be used something like boots or another sign for speed modification.
I'm certain it would look somewhat cooler if you also changed the unit's animation speed together with unit's movement speed. At the moment its animation is as slow as it is in the beginning, even if it runs 2-3x faster.
Effects of fast running but also of the charge attack look good and are fitting well.
The ability icon is good. The tooltip is written very clear and readable, functionality is mentioned and important information is highlighted well.
The aura is not too overloaded with effects and is looking quite good in game.

Credits for the icon are missing.

Mythic:

http://www.hiveworkshop.com/forums/...ura-your-aura-254166/index16.html#post2560839

Uniqueness : 6.5/10
The concept is too overloaded with HP aspects: 1) HP-Regen 2) Heal allies via taking damage 3) Every four heals 50% better healing + damage reduction 4) Active healing. So the idea is overpowered, because you just get healing for everything you do plus additional healing from time to time.
Aside from that, the healing is executed in different and creative ways.

Coding: 13/20

function HD_Setup is not needed. These operations can be executed at map init without problems. Also 'A002' --> this constant should be a variable for configuration matters.
function HD_RadarCondition:
GetFilterUnit() may be stored into a local variable.
udg_HD_Player[udg_A]) == true --> the == true part is not needed.
TriggerRegisterAnyUnitEventBJ is a good BJ, but two of them are not needed; you can combine both and loop.
function HD_LearnedCache: After you check the correct ability you apply a lot of redundant code. You can do this single check: if UnitAddAbility(udg_HD_Caster[udg_HD_Index], udg_Config_HD_SpellBook) then
It returns a boolean, so you can use it as a condition to check if the unit already has this ability or not (no loop needed here).
GetOwningPlayer(udg_HD_Caster[udg_HD_Index])-->GetTriggerPlayer()
(if TriggerRegisterPlayerUnitEvent is used to run the function you can use GetTriggerPlayer, else use GetOwningPlayer)
'B001' should not be a constant.
JASS:
```
if IsUnitInGroup(udg_target, udg_HD_Group[a]) == true then
    set udg_HD_HealCD[a] = udg_HD_HealCD[a] - 

udg_Config_HD_HealCDRed[udg_HD_Level[udg_A]]
endif
if IsUnitInGroup(udg_source, udg_HD_Group[a]) == true then
    set udg_HD_HealCD[a] = udg_HD_HealCD[a] - udg_Config_HD_HealCDRed[udg_HD_Level[udg_A]]
endif
```
^That code may not work well, because you only want to change the value once, not potentially twice. Combine the two ifs together with

or, and also remove the == true.
function HD_Detect:
(I2R(GetHeroInt(u, true))-->I2R() is notneeded. You can set reals with integers without typecasting (does not work vice versa). You only need to null the local group if GetSpellAbilityId() == udg_Config_HD_Ability is true.
Take usage of exitwhen true if correct instance was found to avoid further looping.
function Holy_Deliverance:
call GroupClear(udg_HD_Group[udg_A]) can be outside the if statement, you do it in then, but also in else.
set u2 = null --> can directly be initialized with null after declaration, not in a random line within the loop.
Look at this tutorial, it's about optimizing FoG enumerations, this chapter:

"Advanced: Retaining group contents with a swap variable".

http://www.hiveworkshop.com/forums/...st-group-loop-enumeration-223140/#post2221471
JASS:
```
if real1 < real2 then
    set real2 = real1
    set real1 = real2
    set u2 = u
endif
```
^real1/2 have the exactly same value now. You need a third real as backup.
You miss the deindexing part. If an indexed unit gets removed it will still be listed up. For this you can check if udg_HD_Caster[udg_A] == null in periodic loop.
Just use local integers for loops, no need to declare globals for it.
SetUnitState(u, UNIT_STATE_LIFE) --> SetWidgetLife, same with the getter.
To check if a unit is dead use this: not (IsUnitType(u, UNIT_TYPE_DEAD) or GetUnitTypeId(u) == 0). Just checking for life > 0.405 may be not enough in each case.
== true is in most cases not needed. Just leave the function alone or write a not in front if you compare with false.
Import instructions are missing.

All in all there are a lot of points to be fixed, in order for the code to be more efficient, but it is leakless and MUI. It's also important to store spell constant ids into a variable and put them into config.
Documentation exists but could be improved to guide one better through code, with more and better explanations; for example, more details in the configuration part for the user, but also documentation in the main code. There exists a name for most things, but functionality could be explained better (e.g. "Cooldown for heal". For which heal?, "Heal Int Scale Percent, in decimal". What's exactly the purpose?).
You should change the "Targets Allowed" for the active healing in object editor to only Self & Allies. At the moment you can also use the spell on enemies.
Furthermore, using a hashtable could prevent looping through all instances to get the correct unit, but you can see this as an optional improvement.

Visuals: 7.5/10

Spell icon is fitting good.
The tooltip contains a lot of information about the spell's functionality. It's fine, but for profit of readability the functionality could be for example listed within paragraphs.
The effect is fitting the healing theme well and looks good in game. But it was more informative if you could differentiate the automatically triggered healing from the active spell-target healing by using various special effects for different purposes.
If a unit gets healed while moving, the effect remains stationary and does not follow the unit. It looks strange that the effect shines into nothing, so it would be awesome if the effect was attached onto the unit instead.

Given the healing nature of the spell, it would fit well to a support-type of hero.

AKA.GywGod133:

http://www.hiveworkshop.com/forums/...ura-your-aura-254166/index18.html#post2562435

Uniqueness : 9/10
Execution of the Dark Circle is quite creative - especially the escalating armor reduction by standing longer in the circle gives some good individuality to the aura.
The heal on death of affected nearby enemies does not fit perfectly in the dark and mysterious energy theme. Better just stick to its "dark" characteristics.

Coding: 8.5/20

Your system is only MUI as long the duration for all levels of DarkCircle are the same constant. It could mix-up if the user sets them differently in the config part, because the deindexing method would not work anymore. You don't have to change much, but it's very important to make it fully MUI. Have a look at this tutorial: http://www.hiveworkshop.com/forums/...orials-279/visualize-dynamic-indexing-241896/
There is no need to set reals to 0.00 in the deindexing part.
In Setup trigger user can seemingly modify the number of rune dummies, but it's actually static and not dynamic. Your code works for exactly four dummy runes and is not flexible; hence, you should make it dynamic and restructure the code to fit in the configured number of runes.
No need to destroy the init trigger.
You can combine the two periodic triggers:
If maxIndex_trigger_A > 0 --> trigger_A actions
If maxIndex_Trigger_B > 0 --> trigger_B actions
Then in deindex part of trigger_A and trigger_B actions you check both, if maxIndex_A AND maxIndex_B equls 0. If true, you turn off the trigger.

Location leaks:

Set DrkC_TempPoint[2] = ((Target point of ability being cast) offset by DrkC_CCircle_Offset towards DrkC_CCircle_Sub1_Angle[DrkC_XY_TempInt] degrees)
Set DrkC_TempPoint[3] = ((Target point of ability being cast) offset by DrkC_CCircle_Offset towards DrkC_CCircle_Sub2_Angle[DrkC_XY_TempInt] degrees)
Set DrkC_TempPoint[4] = ((Target point of ability being cast) offset by DrkC_CCircle_Offset towards DrkC_CCircle_Sub3_Angle[DrkC_XY_TempInt] degrees)
Set DrkC_TempPoint[5] = ((Target point of ability being cast) offset by DrkC_CCircle_Offset towards DrkC_CCircle_Sub4_Angle[DrkC_XY_TempInt] degrees)

In "Start" trigger you redundantly store Target point of ability being cast more often into a point[array] variable and in the end you remove them all. One non array point variable would do the job as all your points are actually the same. To avoid leaks, you just need 2 single point variables.

In trigger Loop main, the following location leaks:

Set DrkC_UnitGroup[DrkC_XY_TempInt] = (Units within DrkC_AoE[DrkC_Level[DrkC_XY_TempInt]] of (Position of DrkC_CCircle[DrkC_XY_TempInt]))
...
(Distance between (Position of DrkC_CCircle[DrkC_XY_TempInt]) and (Position of DrkC_Caster[DrkC_XY_TempInt])) Kleiner gleich DrkC_AoE[DrkC_Level[DrkC_XY_TempInt]]
...
Set DrkC_TempPoint[3] = ((Position of DrkC_CCircle[DrkC_XY_TempInt]) offset by DrkC_CCircle_Offset towards DrkC_CCircle_Sub1_Angle[DrkC_XY_TempInt] degrees)
Set DrkC_TempPoint[4] = ((Position of DrkC_CCircle[DrkC_XY_TempInt]) offset by DrkC_CCircle_Offset towards DrkC_CCircle_Sub2_Angle[DrkC_XY_TempInt] degrees)
Set DrkC_TempPoint[5] = ((Position of DrkC_CCircle[DrkC_XY_TempInt]) offset by DrkC_CCircle_Offset towards DrkC_CCircle_Sub3_Angle[DrkC_XY_TempInt] degrees)
Set DrkC_TempPoint[6] = ((Position of DrkC_CCircle[DrkC_XY_TempInt]) offset by DrkC_CCircle_Offset towards DrkC_CCircle_Sub4_Angle[DrkC_XY_TempInt] degrees)

And again, you don't need an array enhancement.

In trigger Loop Sub you leak a location:
Set DrkC_HealedGroup[DrkC_XYr_TempInt] = (Units within DrkC_HealAoE[DrkC_rLevel[DrkC_XYr_TempInt]] of (Position of DrkC_rUnit[DrkC_XYr_TempInt]))
Not all variables are readable. That random "r" in some names does not make sense and you risk conflict of variable names, if your ability is imported to another map. In general, I find it very readable if you name them like following: prefix + underscore + variableName , example --> DC_Duration, DC_MaxIndex, DC_Point...
Import instructions are missing.

Comments and documention was good and very helpful. An important factor is the absence of dynamic indexing that may lead to non-MUI; the memory leaks are also something you need to improve on and have a better understanding of.

Visuals: 8.5/10

Special effects are fitting the theme very well, especially the circle on the ground that gives an authentic atmosphere.
Don't name the Death Knight himself in the Dark Circle tooltip, remember it could be used by any unit (just like the code, the tooltip should also be flexible).
The content and functionality could be explained a bit more clearly, and important values should be highlighted.
( Also don't use the word "can" here:
1) "can devise a dark circle" --> "creates/devises a dark circle"
2) "can increase the hit point regeneration" --> "increases the hit point regeneration")

"Can" denotes a possibility, which, in terms of coding pertains to chances. When an effect takes place no matter the conditions, you need to be affirmative in the description.

Dat-C3:

http://www.hiveworkshop.com/forums/...ura-your-aura-254166/index22.html#post2565330

Uniqueness : 9.5/10
The idea of angry birds surrounding your hero and waiting to attack the enemy is very cool and simple. The fact that you can avoid the bird's attack brings some flexibility into the concept.
It would be cool if they would take damage by themselves by crashing the ground and die after some attacks and explode on death.

Coding: 12.5/20

Most important aspect that affects efficiency in a negative way is the use of multiple variable unit[array] for indexing AngryBirds. Using hashtable would have so many benefits for this system. Hashtable allows you to store directly AngryBirds(units)/Index/AmountOfBirds for each specific unit. So you would not need to check each time for each of these 4 birds/levels per unit and could avoid an overhead. You would be able to make a loop to handle most of your stuff where you now need 4x more code. Also because of this you do a lot of useless checks at the moment and actions that lead nowhere if the bearer's level is not maxed out.

IF-Bedingungen
- (Level of AngryBirdAbility for (Triggering unit)) > 0
- (Level of AngryBirdAbility for (Triggering unit)) < 2

-->

(Level of AngryBirdAbility for (Triggering unit)) == 1

Use tmpVariables to avoid unneeded function calls. For example using "TriggeringUnit" more often in AngryBirdActivator can be avoided if you store it in a variable in the beginning.
There are more that need to be fixed, but I won't enumerate every single one. Just scrutinize your triggers and based on this guideline pay closer attention to it.
Basic damage of birds could be configurable, just as basic recall time.

IF-Bedingungen
- AngryBirdCaster[AngryBirdLoop] == No Unit
- Or - Any (Conditions) are true
  - Bedingungen
    - AngryBird1[AngryBirdLoop] =! No Unit
    - AngryBird2[AngryBirdLoop] =! No Unit
    - AngryBird3[AngryBirdLoop] =! No Unit
    - AngryBird4[AngryBirdLoop] =! No Unit
    - AngryBirdTarget1[AngryBirdLoop] =! No Unit
    - AngryBirdTarget2[AngryBirdLoop] =! No Unit
    - AngryBirdTarget3[AngryBirdLoop] =! No Unit
    - AngryBirdTarget4[AngryBirdLoop] =! No Unit

^I don't see a reason to have this "Or - Any" conditions block. If the first condition is be true, then there must be at least one bird.

'IF'-Bedingungen
- (AngryBirdCaster[AngryBirdLoop] is dead) Ungleich True
- AngryBirdDiedOnce[AngryBirdLoop] Gleich True

^After these checks you only reset "AmountOfHits" only for AngryBird4, not for the rest three.

The check for starting the periodic timer should be moved into index part, the check stopping into deindex part. The Trigger - Turn on/off actions are redundant because you stop/start timer.

Configuration is enough, but not all variables' role is clear to understand. Code documentation is missing from the main code, so sometimes it's not easy to follow up.
The way the spell is coded at the moment supports 4 levels and is not really changeable. By using a hashtable this problem could be solved in an acceptable way.
As mentioned in my main (first) point, the coding has some big efficiency flaws, because it's not written dynamically enough (no level flexibility) and a lot of operations are unnecessary.

Visuals: 8/10

The tooltip should be reorganized a bit. It contains unneeded information and it doesn't list the important features of the skill.
It should also describe each part of Angry Birds's functionality.
The circular movement around the bearer is acceptable, but some more intuitive behavior for the bird would make it look much more realistic. It would take more effort in triggering only for this eye-candy, so it's not a critical point, but it would definitely improve it (by doing so, it could also influence the bird's behavior in-game. For example you could use it for more autonomy regarding the bird's attacking instances).

Zeatherann:

http://www.hiveworkshop.com/forums/...ura-your-aura-254166/index20.html#post2563718

Uniqueness : 6/10
Your basic idea reminds me of the default spell "Lightning Shield", but in an advanced form. Thus, I am confident when I say that it is not unique.
The addition of toggle and active damaging are the only keys to enhance the creativity of the ability.

Coding: 13.5/20

First of all, I have to note that you were close to getting yourself disqualified because of the coding aspect.
You were asked by Pharaoh_ and myself to change it (will elaborate in my first point).

Code is imported in a War3Map.j file. This does not fit the spell rules. User can't easy import & config it, and only experienced users will be able to handle it. Furthermore, you need a third party tool to read the code which is is not preferable. Doing so in self-made maps might be acceptable, but not for sharing as public resources.
You could use a global dummy instead of creating a new one for each attack.
Only agents have to be nulled. Lightning is not an agent.
Making a trigger a constant doesnt really bring profit.
Using 0.0312500 is the recommended number for periodic timers in JASS.
Number of lightning bolts should be configurable.
In the function Bolt the local integer "Index" is not needed. Just make Bolt_Count = Bolt_Count + 1 if the condition is true and use it as the actual index.
In Bolt_Update the check to stop the timer should be in the deindex part.
You don't have to null the unit variable after a "FoG == null" group iteration.

In function AoL_Update:

JASS:

local string LightningType
if(AoL_Empowered[Aura_Index])then
    set LightningType = AoL_EmpoweredTyp
else
    set LightningType = AoL_LightningType
endif

You should follow JPAG in relation to naming:

http://www.hiveworkshop.com/forums/...80/jpag-jass-proper-application-guide-204383/
Import instructions are missing.

The major flaw here is explained in my first point. However, all config and non-config parts should have a better structure, and not be mixed up.

Visuals: 6.5/10

Lightnings everywhere, but not the icon. It mostly resembles to water/mana. Some lightning here would fit better the theme.
For damaging and marking, the range of the lightnings fit the theme very well.
Tooltip is readable and well executed. However, some light highlighting of important values would be more prominent and would draw attention to the important parts of the spells.
When I was playing with the test map I realized that if more than one units have this aura in the same area, the special effects are taking control of most part of your screen and it starts to look overloaded.
In hilly areas the lightnings go partially underground and it doesn't look good. It can actually reach a point where the effect still lasts, but the lightnings are not visible anymore. So this visual aspect can also have influence to the gameplay and bring some confusion to the user.
After the damage, the lightnings' position doesn't change. It looks weird if the bearer is moving, because then one end of lightning will link to nothing (basically where the bearer should stand).
I strongly suggest using this system by Maker, it supports moving lightnings:

http://www.hiveworkshop.com/forums/spells-569/system-timed-lightnings-v1-0-1-1-a-205105/. Alternatively, you could venture implementing something of your own creation.
Effects are supposed to be linked to units, and you also have to anticipate potential usage of multiple auras/abilities on the map. So it would also be beneficial to fix this visual flaw to prevent confusion during gameplay.

Empirean:

http://www.hiveworkshop.com/forums/...ura-your-aura-254166/index18.html#post2562290

Uniqueness : 7.5/10
Idea is very simple but it still might be useful for tank-like heroes in a Role Playing Game, for example.
Healing mates while taking damage with an additional possibility for taunt is a nice (yet predictable) combination.

Coding: 13.5/20

The healing aspect occurs directly in the beginning of the attack animation. Using a DDS would be beneficial for your case, because it's more realistic to heal after the unit gets damaged.
Also, in theory, one could abuse this, as the player can still manipulate things between the time the unit's attack starts and the moment the target gets damaged (by spamming the Stop-Attack combo).
Initializing arrays is not needed for most types. Especially when you initialize them on your own in init trigger (the size increases in the variable editor).
(Level of SA_A_ActiveAbility for (Triggering unit)) > 0
-->
(Level of SA_A_ActiveAbility for (Triggering unit)) == 1
Set SA_PL_Player = (Owner of SA_U_AtkUnit)
-->
Set SA_PL_Player = TriggeringPlayer
Taunt range could depend on level and should be configurable.
No need to clear a group before you destroy it.
Using a tmp variable is only needed to avoid more function calls. If you are to use it only once, it's pretty redundant (and potentially slower).
Using two underscores on the names of the variables has no benefits in readability. Using only one after the prefix should do the job. (prefix_variableName)

Code is written in a very simplistic way (mainly way it's really easy to understand). There are no leaks, and documentation on the operations is also helpful.
Albeit it's very brief as a code and one does not need to put a lot of effort to write it on their own.
Here is a suggestion for an extra feature:
When using custom taunt, the bearer gets an additional effect, which is temporary and only lasts for a few seconds. During this effect, the bearer absorbs (partially) damage that is dealt to nearby allies. So it would be kind of a protection possibility for the (usually) tank-unit to save his allies from potential death.

Visuals: 6/10

Main aspects of functionality are described well in tooltip.
Important values should be more highlighted, i.e. percentages.
Shield as an ability icon is not fitting the healing aspect; it mostly gives the impression of an armor-kind ability.
Blessed rain is a good special effect for healing, but it gives the intention of active and permanent healing, which is not the case here.
Only creating a special effect onHeal would be better than having a permanent effect.

Formula: (Votes / Total Votes)*30 + Judge's Results*(70/40)

Rankings:

Bannar - 6/31*30 + 36.5*70/40 = 69,68
Tank-Commander - 8/31*30 + 34*70/40 = 67,24
rulerofiron99 - 1/31*30 + 31*70/40 = 55,21
Dat-C3 - 2/31*30 + 30*70/40 = 54,43
Mythic - 3/31*30 + 27*70/40 = 50,15
AKA.GywGod133 - 4/31*30 + 26*70/40 = 49,37
Empirean - 2/31*30 + 27*70/40 = 49,18
GunSlinger - 1/31*30 + 27*70/40 = 48,21
Zeatherann - 0 + 26*70/40 = 45,5

Contest | Poll

Vinz · Sep 16, 2014

you just get healing for everything you do

You should change the "Targets Allowed" for the active healing in object editor to only Self & Allies. At the moment you can also use the spell on enemies.

shiet lol. xD

It was originally an ultimate but I made it a third with three levels. GJ on the judging IcemanBo. (Only one judge?! The other contests I've seen had at least two (this is actually a good thing in terms of time; for the previous Zephyr Contest IIRC we had to wait a while for more judges))

E: Also, a note, since my name was correctly changed; SallyAnna's name changed to Empirean.
E2: I actually was expecting TC to autowin. GJ Bannar. xD

IcemanBo · Sep 16, 2014

Yeah, thanks first for participation of you all! And of course congratulations to the winners!

As I judged the first time, I hope you are ok with the reviews. Probably it was not the last time.

Vinz · Sep 16, 2014

I suggest this be put on the notice board.

rulerofiron99 · Sep 17, 2014

IcemanBo said:
As I judged the first time, I hope you are ok with the reviews. Probably it was not the last time.

I'm quite happy with mine. You found several issues that I overlooked and you've provided good feedback. So definitely a +1 from me for you to be a judge in future contests.

Big thanks to the judge, and congratulations to the winner!

Bannar · Sep 17, 2014

Nice!

You have found most of flaws in my script, but you missed some too e.g EVENT_UNIT_INDEX (should be DEINDEX because it was refering to onDeindex method) - although I see I've also forgotten about few other things

I wanted to thank contestants, judge and host for the contest.
The only thing I'm not sure about is amount of judges - don't get me wrong, Iceman did fine job, but probably even he would feel better if he wasn't judging alone.

Kazeon · Sep 17, 2014

So there is only one judge?

Congratulations for the top-3

By the way

Formula: (Votes / Total Votes)*30 + Judge's Results*(70/40)

Should be 1,75. Needs fix

Vinz · Sep 17, 2014

wot. o.o

Judge Score is 70 percent and poll is 40 right?

GunSlinger · Sep 17, 2014

Oh coal, it's up.
Congrats to the winners and thanks for the critiques Iceman.
EDIT:
A lot of the issues in mine were because of the lack of time to fix em. (Damn me for starting a few days before the deadline!)

Vinz · Sep 17, 2014

Mine too, (though I started right after it was held but procrastinated 90% of the time and changed auras three times).

GywGod133 · Sep 17, 2014

thumbs up! for the winners

Tal'Rarity · Sep 17, 2014

Good job everyone.

Kyrbi0 · Sep 21, 2014

Eh, not excited that I was disqualified, but we're working it out. Good job to everyone who finished, & especially Bannar, Tank-Commander & RulerofIron for placing!

Zeatherann · Sep 23, 2014

Lol, woops forgot to change my icon. I fail to see how I was almost disqualified as no one said I couldn't import my war3map.j, but whatever. Destined for last seems to call harder. I guess next time I'll try to break a rule so someone else can be last.Nice job judging though. I know it can't be easy.Looking forward for the next chance to code.

IcemanBo · Sep 23, 2014

no one said I couldn't import my war3map.j

Basic question was: "Can I import my map script?"

I will quote some of our answers, you won't see us agreeing with your idea:

Pharaoh_:	Why do you need it, what does it contain?
Pharaoh_:	If it can be opened in the regular editor, however, then it's fine.
Pharaoh_:	However, I feel that you are taking too much advantage of utilities. You're already allowed to use any kind of system (unlike the first Zephyr contests), which is something I changed () and it is more than enough. It's just a spell!
Afterwards	you uploaded your WIP with imported script, and I asked you to upload it with code:
IcemanBo:	Could you upload a version with code?

^You did never answer me on that.

Zeatherann · Sep 24, 2014

My final submission had the code posted in the thread. My map could be opened in the normal editor, even saved (more than can be said for vJASS).

Vinz · Oct 5, 2014

I suggest this be removed from the notices, it's been done awhile.

Vinz · Oct 5, 2014

not a hacked editor like newgen.

I think New Gen is allowed.

Zephyr Contest #11 - Results

Similar threads