Spell Contest II

[Pyrogasm]

Breaking news: I'm finished.

The raw scores for coding:

• Doom-Angel: 6/10
• kc102: 6/10
• Thanatos: 8/10 (Should've been 9 )
• jonadrian619: 8/10
• Überplayer: 5/10
• dhk_undead_lord: 6/10

Please note that I only gave 5/5 for perfection in that specific category; if there was 1 leak or 1 inefficient thing, then I gave an automatic 4/5.

Download the RTF file with the gradings formatted nicely

Gradings:

Doom-Angel — Elite Spark - 3/5 - Efficiency
3/5 - Leakage
- Let me first start by saying this: Because you made an omnislash, I'm going to be a complete ass about grading it to make sure that you did everything perfectly; an omnislash is neither a difficult nor original spell.

- "(GetUnitState(GetFilterUnit(),UNIT_STATE_LIFE) > 0)" should be "(GetWidgetLife(GetFilterUnit()) > 0.405); A unit "dies" when its life reaches 0.405 or less, and as such, the comparison should be to this number as to not get any false positives. Additionally, GetWidgetLife() is better than GetUnitState().
- Why the backwards comparison in this?: "(IsUnitEnemy(GetTriggerUnit(),GetOwningPlayer(GetFilterUnit())))". Wouldn't it be smarter to compare the Filtered unit to the owner of the triggering unit...? (Not that your way doesn't work)
- Do you really need the "Region" function? The entire function is 1 line of code...
- Shouldn't "call SetUnitTimeScale(u,4-level)" be "call SetUnitTimeScale(u,level+1)"? Why would the unit move slower at "monstrous speed" than at "elite speed"?
- "call PolledWait(0.20)" == bad. Polledwaits are bad.
- UGH. The way you go about looping is extremely stupid. If you've ALREADY started a timer callback, why not just run all the periodic group stuff inside the loop? Doing it by starting a timer with no callback and then checking its time remaining... That's abominably stupid because: A) It leaks a timer B) Is not configurable C) It leaves everything up to the polledwaits (Which, by-the-way, cause the caster to only slash ~0.09 seconds faster, whereas in the level 1-2 jump, the speed increase is 0.35 seconds), and D) Is an inefficient way to make an Omnislash!
- In "call SetUnitFacingTimed(u,AngleBetweenPoints(loc,loc2),0.01)", why are you using locations?! It wouldn't have been too hard to find a function that's widely used called "AngleBetweenPointsXY".
- "call UnitDamageTarget(u,target,100, true, false, ATTACK_TYPE_HERO,DAMAGE_TYPE_ENHANCED,WEAPON_TYPE_METAL_MEDIUM_SLICE)" should be "ATTACK_TYPE_CHAOS" and "DAMAGE_TYPE_UNIVERSAL", else the unit will not deal full damage.
- "call AddSpecialEffectTarget("Objects\\Spawnmodels\\Undead\\UndeadBlood\\UndeadBloodNecromancer.mdl",target,"chest")" and "call AddSpecialEffectTarget("Abilities\\Spells\\Orc\\MirrorImage\\MirrorImageCaster.mdl",u,"origin")" both leak special effects
- As I said earlier, the line "call PolledWait(1.06-(0.35*level))" doesn't cause the effective speed increase from levels 2-3. As the minimum wait time is roughly 0.27 seconds, the difference between 0.36 and 0.01 is not 0.35 seconds, but 0.09.
- WHY ARE YOU USING A RECT?! Additionally, the whole need for globals udg_X and udg_Y could be eliminated using the following format instead of the ForGroup() call:
loop
set U = FirstOfGroup(G) //U being a unit variable not in use and G being the group you need to loop through
exitwhen U == null
//Do your stuff with U; note that this means all variables present in the parent function are still useable, this does not create a new thread
call GroupRemoveUnit(G, U)
endloop
- There's a double-free of bj_groupAddGroupDest because you have this line twice: "call DestroyGroup(bj_groupAddGroupDest)"
- The use of that global group also makes this entire spell non-MUI, which sucks. Once again, this could've been avoided by using the above group loop format.
- "set bj_groupAddGroupDest = null" is not needed; it's a global group.
- Your function names are abominable. "filter" and "conditions"; come on.


kc102 — Blink Throw - 2/5 - Efficiency
4/5 - Leakage
- "IsUnitAliveBJ(GetFilterUnit()) == true" is bad! You should always use a life comparison: "(GetWidgetLife(GetFilterUnit()) > 0.405); A unit "dies" when its life reaches 0.405 or less, and as such, the comparison should be to this number as to not get any false positives. Additionally, GetWidgetLife() is better than GetUnitState().
- Wouldn't it be better to remove the "*4" on this line to avoid confusion: "local integer level = GetUnitAbilityLevel(GetTriggerUnit(), 'A000') * 4"? You could simply multiply 'level' by 4 when needed, or rename the variable to something like "levelfactor".
- In this line: "local group randus = GetRandomSubGroup(level, GetUnitsInRangeOfLocMatching(400.00 + (100 * level), caspos, Condition(function isally)))", there is a group leaked from the "GetUnitsInRangeOfLocMatching" part and a boolexpr leaked from the "Condition(function isally)". Would it really have been too hard to do this:
local boolexpr b = Condition(function isally)
local group g = GetUnitsInRangeOfLocMatching(400.00 + (400 * level), caspos, b)
local group randus = CreateGroup()
local integer i = 0
local unit u
loop
set i = i+1
exitwhen i > level*4
set u = GroupPickRandomUnit(g)
call GroupAddUnit(randus)
call GroupRemoveUnit(g)
endloop
call DestroyGroup(g)
call DestroyBoolExpr(b)
- Additionally, USE X AND Y COORDINATES! They're USEFUL!
- " if num == 0 then
return
else
endif " The formatting burns! Also, the "else" is not needed; only the "if" and "endif" are important.
- On a small map, "set rand = GetRandomLocInRect(GetPlayableMapRect())" seems OK... but what about on a big map? That, and you're not checking to see if it's pathable or not.
- "call DestroyEffect(AddSpecialEffectLoc("Abilities\\Spells\\NightElf\\Blink\\BlinkCaster.mdl", rand))" and "call SetUnitPositionLoc(move, rand)" should both be replaced with X/Y value-related equivalents.
- "call UnitDamageTargetBJ( caster, move, 50.00 * (level / 4), ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL )" could easily be re-written without the BJ; it's only one extra argument that never changes!
- Instead of "exitwhen loopa >= level", I would do "exitwhen loopa > level" and then declare "loopa" as 1 instead of 0.



Thanatos — Hurricane - 4/5 - Efficiency
4/5 - Leakage (Hooray for KaTTaNa's leaks!)
- First thing off is that I like the fact that everything is concise and JESP (as opposed to everything else), but I really hate the way you lay out your coding. You should never separate function names from their arguments, like so:
"local player P = Player (i)"
Even if it makes it go with the code's formatting.

- AUGH! NO! BAD! THIS IS WHY WE DON'T USE KATTANA'S SYSTEM ANYMORE:
"function LocalVars takes nothing returns gamecache
local gamecache a = InitGameCache ("jasslocalvars")
local integer i = HV_H2I (a)
set a = null
return i
return null
endfunction"
The above function LEAKS GAMECACHES! If you have to use something that uses GameCache, use CSCache instead!
- In the "Preloads" section, "local player b = Player (PLAYER_NEUTRAL_PASSIVE) " is pointless; players don't leak, and you only use the player once in the function.
- When you create a special effect like so: "AddSpecialEffectTarget (Hurricane_SpecialEffect(),c,"origin") ", and then destroy it right away, once can simply write this: "call DestroyEffect(AddSpecialEffectTarget (Hurricane_SpecialEffect(),c,"origin"))"
- Local triggers are bad. Period. Vexorian and IceFrog have shown that destroying them causes major Handle Stack screwups... but if you don't destroy them, they leak. Additionally, "call TriggerRegisterUnitEvent (castEnd,caster,EVENT_UNIT_SPELL_ENDCAST)" only registers when the unit stops casting involuntarily; you should also add this line in: "call TriggerRegisterUnitEvent (castEnd,caster,EVENT_UNIT_SPELL_FINISH)". Oh wait... now that I've read the rest of the code, you probably don't need that, do you? >_<
- "call UnitDamageTarget (caster,target,damage,false,false,ATTACK_TYPE_NORMAL,DAMAGE_TYPE_MAGIC,null)" should be "ATTACK_TYPE_CHAOS" and "DAMAGE_TYPE_UNIVERSAL".
- Holy crap. Nice coding.



jonadrian619 — Phase Swap - 3/5 - Efficiency
5/5 - Leakage
- The event "Unit - A unit is attacked" FIRES WHEN A UNIT INITIATES THE ATTACK; NOT WHEN THE ATTACK/DAMAGE ACTUALLY HAPPENS! This means that a player can constantly trigger this event by right-clicking a unit, or by ordering a unit to attack another unit. Even excluding ranged attackers does not help.
- On your Custom Script lines, you should be using the native functions, not the BJ ones. I suggest you just learn JASS if you're going to use it in Custom Script form in GUI. Really.
- Instead of "Set PointAroundattacker = (Region centered at Point with size (100.00, 100.00))" and "Set TempPoint = (Random point in PointAroundattacker)", you should learn to use the polar offset function. Using this, you would write this instead: "Set TempPoint = (Point offset by (Random real between 0.00 and 100.00) towards (Random Angle) degrees)"
- "Unit - Cause (Attacked unit) to damage (Attacking unit), dealing 50.00 damage of attack type Normal and damage type Normal" should be "attack type Chaos and damage type Universal".
- All instances of "Attacked Unit" should be replaced with "Triggering Unit".
- Why is "Custom script: call AddLightningLoc( "CHIM", udg_Point, udg_TeleportPoint )" a custom script call? One can simply use the GUI function and then set the local variable in the next line in the same way.
- Simply adding 100% evasion to the attacked unit for 1.00 seconds does work, but it has the drawback of getting screwed up if the unit is attacked twice or more during that 1.00 second window. If, for instance, the unit was attacked by 1 footman, and then 0.85 seconds later was attacked by another footman. When the first instance of the trigger ended, the "evasion" ability would be removed from the hero, which would mean that there is now NO evasion ability on it like there should be for the next 0.85 seconds (the second instance of the trigger).
- When you do this: "Unit - Move (Attacked unit) instantly to TempPoint", shouldn't you also make the hero face the target?



Überplayer — Rapid Draw - 2/5 - Efficiency
3/5 - Leakage
- AUGH AGAIN! This function leaks gamecaches!:
function LocalVars takes nothing returns gamecache
return InitGameCache("jasslocalvars.w3v")
endfunction
- Shouldn't "call Preload("Abilities\\Weapons\\ZigguratFrostMissile\\ZigguratFrostMissile.mdl")" just use the constant function for SFX?
- You didn't null "t" in the init function
- You overuse "GetTriggerUnit()" in the function "Rapid_Draw_Activate_Actions"; set it to a variable for christ's sake!
- Why the "GetUnitFacing(GetTriggerUnit())" in the dummy create function? If it's a dummy, it doesn't matter which way it's facing...
- In "call SetHandleHandle(t, "u", GetTriggerUnit())", "u" is NOT a good name; it could easily be overwritten by something else.
- Why this: "call PolledWait(8.00)"? If you've got the timer set up, do some simple math (8.00/0.11) to find out how many times the timer will loop before it's reached 8.00 seconds. Then, attach an integer counter that decreases every callback of the timer; finally, destroy the timer when its counter reaches 0. Not that hard, is it?
- GetAttacker() is overused in Rapid_Draw_Unleash_Actions.
- What the hell is with storing the unit's handle index to its custom value? Why don't you simply check to see if the attacking unit has the ability?!: "return GetUnitAbilityLevel(GetTriggerUnit(), DummySpellId()) > 0"
- Your constant functions could use prefixes.
- Instead of "call PolledWait(2.00); call RemoveUnit(d)", you can simply add a 2.00 second expiration timer to the unit: "call UnitApplyTimedLIfe(d, 'BTLF', 2.00)".
- ...any particular reason you didn't simply modify an orb of slow/frost ability and add that to the hero for the duration of the spell instead of doing complicated, unreliable attack (not damage) detection and dummy spell triggering?
-



dhk_undead_lord — Troll Speed Spells - 2/5 - Efficiency
4/5 - Leakage
- Making 2 spells for a contest and trying to pass it off as "ok" is really dumb. Point and case.

- All instances of "Unit - A unit Begins casting an ability" in all triggers should be changed to "Unit - A starts the effect of an ability"
- Why do all the triggers have "(Unit-type of (<Whatever> unit)) Equal to Troll" in them? Shouldn't the fact that they have the ability be just as good?
- The event "Unit - A unit Is attacked" registers when the unit initiates the attack, not when the damage happens; refer to above gradings for more information as to why that's bad.
- All instances of "Attacked Unit" in all triggers should be replaced with "Triggering Unit".
- "Unit - Cause (Attacking unit) to damage (Attacked unit), dealing (5.00 x (Real((Level of Troll Axe Frenzy for (Attacking unit))))) damage of attack type Hero and damage type Normal" should be "of attack type Chaos and damage type Universal".
- You should set the integer variable to a new random value before each If/Then/Else statement; the way you have it set up now makes it impossible to get bonus damage without the other two effects (for a number to be less than 5, it must also be less than 7 and 10).
- "Wait until (((Triggering unit) has buff Troll Frenzy ) Equal to False), checking every 1.00 seconds" is a bad way to do things, but in GUI you've got no option. However, since you've already knocked this down to MPI, why not just create 12 timer countdown triggers?
- "Set Temp_Point2 = (Position of (Random unit from (Units within 250.00 of Temp_Point matching ((((Matching unit) is alive) Equal to True) and (((Owner of (Matching unit)) is an enemy of (Owner of (Attacking unit))) Equal to True)))))" leaks a unit-group.
- What purpose does the "Troll Frenzy Slow Debug" trigger serve? After any sort of a wait, "Target Unit of Ability Being Cast" returns no unit, effectively making the trigger do nothing, 100% of the time.
- The sheer number of triggers is, well, daunting.​

 

[Uberplayer]

Actually..

>- You didn't null "t" in the init function
Triggers don't need nulling afaik..?

>- What the hell is with storing the unit's handle index to its custom value? Why don't you simply check to see if the attacking unit has the ability?!: "return GetUnitAbilityLevel(GetTriggerUnit(), DummySpellId()) > 0"
Wrong, I should have checked if it has rapid draw buff.

>- You overuse "GetTriggerUnit()" in the function "Rapid_Draw_Activate_Actions"; set it to a variable for christ's sake!
Doesn't affect to the trigger..

>- GetAttacker() is overused in Rapid_Draw_Unleash_Actions.
Again this doesn't affect to the code..

>- Why the "GetUnitFacing(GetTriggerUnit())" in the dummy create function? If it's a dummy, it doesn't matter which way it's facing...
Simply makes the dummy cast the spell faster as it doesn't have to move his arse a bit..

Do not blame me if KaTTaNa's Local Handle Vars leak a gamecache..

Whoa, you've done some good job here ;D.. Usually the gradings are a bit shorter

 

[Pyrogasm]

Triggers don't need nulling afaik..?

I'm talking about nulling the local variable that points to the trigger.

Wrong, I should have checked if it has rapid draw buff.

Right... that's what I meant to say.

Doesn't affect to the trigger...

Again this doesn't affect to the code..

It's a function call. Since local variables are available in JASS, you should always store such things to a variable if you're going to reference it more than once. You referenced both at least 3 times

Simply makes the dummy cast the spell faster as it doesn't have to move his arse a bit..

Again, another function call that is inefficient (no matter how small of an inefficiency it is)

Do not blame me if KaTTaNa's Local Handle Vars leak a gamecache..

I can and I will because it's been discussed before by IKilledKenny and Vexorian. KaTTaNa's is outdated in addition to being inefficient; use something better.

Whoa, you've done some good job here ;D.. Usually the gradings are a bit shorter

Well, I've always been disappointed with short/bad gradings, so I made some effort to actually go through the code and make some useful comments.

 

[Uberplayer]

Thanks for making those clear ;D, usually I make everything local if I use them more than twice, but that was more like a test using my Fade System, and it looked good so I wanted to use it in this contest.

(now this looks horribly bad for me xD)

 

[Pyrogasm]

's OK. Coding is not factored horribly well into this contest... *cough* uareanoob should've made it worth more *uncough*

If I'd had my way, there would've been sections on readability, configurability, and ease of import, too.

 

[Uberplayer]

At least my code was readable ;D! Uareanoob has the results already?

And now as I think.. Are you able to see the Orb of Slow in the hero or is it invisible?

 

[Pyrogasm]

I think it's invisible. You could always use a disabled spellbook. Do you mind explaining why you didn't just do that instead of triggering the whole effect?

Uareanoob has the results already?

I have no idea. He was, however, bitching at me to finish this .

 

[Uberplayer]

>Do you mind explaining why you didn't just do that instead of triggering the whole effect?
I have no idea.. Really, I didn't ever think of that ! I just searched for a spell that slows

>I have no idea. He was, however, bitching at me to finish this .
Then I suppose that he has the results

 

[Doom-Angel]

- Let me first start by saying this: Because you made an omnislash, I'm going to be a complete ass about grading it to make sure that you did everything perfectly; an omnislash is neither a difficult nor original spell.

i didn't make an omnislash for god sakes....................
i made the spell by my own idea and i haven't seen omnislash in my life except for a few posts with "[Spell] Omnislash" which i took a glance on it and never realy tested so don't compare it to omnislash.....

- "(GetUnitState(GetFilterUnit(),UNIT_STATE_LIFE) > 0)" should be "(GetWidgetLife(GetFilterUnit()) > 0.405); A unit "dies" when its life reaches 0.405 or less, and as such, the comparison should be to this number as to not get any false positives. Additionally, GetWidgetLife() is better than GetUnitState().

k good to know about this 0.405 but i dunno if it such a matter about the unit and widget...

- Why the backwards comparison in this?: "(IsUnitEnemy(GetTriggerUnit(),GetOwningPlayer(GetFilterUnit())))" . Wouldn't it be smarter to compare the Filtered unit to the owner of the triggering unit...? (Not that your way doesn't work)

am i realy losing points for this?

- Do you really need the "Region" function? The entire function is 1 line of code...

i dunno why i realy made a function for it.... (lol)

- Shouldn't "call SetUnitTimeScale(u,4-level)" be "call SetUnitTimeScale(u,level+1)"? Why would the unit move slower at "monstrous speed" than at "elite speed"?

actually if u wanted to make it in my calculation u would just leave "level" inside (another stupidity of mine)
and about the speed it was my own calculation since that the speed is too high anyway then i wouldn't be able to get anything from the animation so i made it slower for each increased speed.

- "call PolledWait(0.20)" == bad. Polledwaits are bad.

why is that?

- UGH. The way you go about looping is extremely stupid. If you've ALREADY started a timer callback, why not just run all the periodic group stuff inside the loop? Doing it by starting a timer with no callback and then checking its time remaining... That's abominably stupid because: A) It leaks a timer B) Is not configurable C) It leaves everything up to the polledwaits (Which, by-the-way, cause the caster to only slash ~0.09 seconds faster, whereas in the level 1-2 jump, the speed increase is 0.35 seconds), and D) Is an inefficient way to make an Omnislash!

A) no it doesn't.....
B) why not?
C) i didn't get what u were saying
D) im not making an omnislash!

In "call SetUnitFacingTimed(u,AngleBetweenPoints(loc,loc2),0.01)", why are you using locations?! It wouldn't have been too hard to find a function that's widely used called "AngleBetweenPointsXY".

yea u are right i should have used:

bj_RADTODEG * Atan2(GetLocationY(locB) - GetLocationY(locA), GetLocationX(locB) - GetLocationX(locA))

instead but i didn't notice it was a BJ....

- "call UnitDamageTarget(u,target,100, true, false, ATTACK_TYPE_HERO,DAMAGE_TYPE_ENHANCED,WEAPON_TYPE_METAL_MEDIUM_SLICE)" should be "ATTACK_TYPE_CHAOS" and "DAMAGE_TYPE_UNIVERSAL", else the unit will not deal full damage.

good to know

"call AddSpecialEffectTarget("Objects\\Spawnmodels\\Undead\\UndeadBlood\\Und eadBloodNecromancer.mdl",target,"chest")" and "call AddSpecialEffectTarget("Abilities\\Spells\\Orc\\MirrorImage\\MirrorIma geCaster.mdl",u,"origin")" both leak special effects

k i will fix now forgot about them

- As I said earlier, the line "call PolledWait(1.06-(0.35*level))" doesn't cause the effective speed increase from levels 2-3. As the minimum wait time is roughly 0.27 seconds, the difference between 0.36 and 0.01 is not 0.35 seconds, but 0.09.

the minimum is 0.1 so it's ~0.26 difference

- WHY ARE YOU USING A RECT?! Additionally, the whole need for globals udg_X and udg_Y could be eliminated using the following format instead of the ForGroup() call:
loop
set U = FirstOfGroup(G) //U being a unit variable not in use and G being the group you need to loop through
exitwhen U == null
//Do your stuff with U; note that this means all variables present in the parent function are still useable, this does not create a new thread
call GroupRemoveUnit(G, U)
endloop

what's wrong about rects?
also i know about this way but it's usefull if i need them for one time action but not if i need them to use later (as u can see).

- There's a double-free of bj_groupAddGroupDest because you have this line twice: "call DestroyGroup(bj_groupAddGroupDest)"

didn't notice fixed now

The use of that global group also makes this entire spell non-MUI, which sucks. Once again, this could've been avoided by using the above group loop format.

yea i forgot about that....
i should find another solution

- "set bj_groupAddGroupDest = null" is not needed; it's a global group.

i know that i just wanted to leave it clean...

- Your function names are abominable. "filter" and "conditions"; come on.

did i realy lose points for that?
also how do u suggest me to call them?

Edit: i fixed all my mistakes (i think)

 

[Anachron]

dhk_undead_lord — Troll Speed Spells - 2/5 - Efficiency
4/5 - Leakage
- Making 2 spells for a contest and trying to pass it off as "ok" is really dumb. Point and case.

I am sorry Ok I thought I have already said it?

- All instances of "Unit - A unit Begins casting an ability" in all triggers should be changed to "Unit - A starts the effect of an ability"

Nope then you are able to trigger spells when they are NOT finished so if you CANCEL you will already have the effects but you haven't used the mana!!!! Haven't u knew that? o.o

- Why do all the triggers have "(Unit-type of (<Whatever> unit)) Equal to Troll" in them? Shouldn't the fact that they have the ability be just as good?

Because maybe some guys wanna see how to make sure that only this one unit can trigger the spell?

- The event "Unit - A unit Is attacked" registers when the unit initiates the attack, not when the damage happens; refer to above gradings for more information as to why that's bad.

I know it, but I just said I had a few minutes to make my spell. Also I know other things how to trigger that, but I still had no time.

- All instances of "Attacked Unit" in all triggers should be replaced with "Triggering Unit".

should be but dont have to .. I thought its noobfriendly? You not!

- "Unit - Cause (Attacking unit) to damage (Attacked unit), dealing (5.00 x (Real((Level of Troll Axe Frenzy for (Attacking unit))))) damage of attack type Hero and damage type Normal" should be "of attack type Chaos and damage type Universal".

Nope thats the thing I just dont want to make!! That sucks

- You should set the integer variable to a new random value before each If/Then/Else statement; the way you have it set up now makes it impossible to get bonus damage without the other two effects (for a number to be less than 5, it must also be less than 7 and 10).

You wanna waste space to do those shit? It doesnt have to be!

- "Wait until (((Triggering unit) has buff Troll Frenzy ) Equal to False), checking every 1.00 seconds" is a bad way to do things, but in GUI you've got no option. However, since you've already knocked this down to MPI, why not just create 12 timer countdown triggers?

It was simple GUI whatever you wanna tell I just had to hurry.
Oh and btw, it would be a waste of variables.

- "Set Temp_Point2 = (Position of (Random unit from (Units within 250.00 of Temp_Point matching ((((Matching unit) is alive) Equal to True) and (((Owner of (Matching unit)) is an enemy of (Owner of (Attacking unit))) Equal to True)))))" leaks a unit-group.

Sorry I forgot that thats right

- What purpose does the "Troll Frenzy Slow Debug" trigger serve? After any sort of a wait, "Target Unit of Ability Being Cast" returns no unit, effectively making the trigger do nothing, 100% of the time.

This referes to the dummy unit spelled slow unit target which doesnt work because I paused them.

- The sheer number of triggers is, well, daunting.

Joking about other players spell isnt a good way to get reputation.

 

[Uberplayer]

>Nope then you are able to trigger spells when they are NOT finished so if you CANCEL you will already have the effects but you haven't used the mana!!!! Haven't u knew that? o.o
Vice versa

>Because maybe some guys wanna see how to make sure that only this one unit can trigger the spell?
Well.. If other units doesn't have the spell? What would they do with the spell if nothing happens when they cast it

>Nope thats the thing I just dont want to make!! That sucks
Then it won't do full damage to all armor types I guess..

 

[Pyrogasm]

No, you did not lose points for writing your function names screwey or for using backwards conditions.

Polledwait has a minmum of roughly 0.27 seconds. TriggerSleepAction is about 0.20. I set up a simple (though not nearly accurate enough) test to demonstrate. Simply call the function below during a map:

function Pyro_Timer takes nothing returns nothing
    local real PolledTotal = 0.00
    local real TSATotal = 0.00
    local integer Counter = 0
    local timer t = CreateTimer()

    call BJDebugMsg("Computing...")
    loop
        set Counter = Counter+1
        exitwhen Counter > 100
        call BJDebugMsg("Counter: "+I2S(Counter))

        call TimerStart(t, 3.00, false, null)
        call PolledWait(0.01)
        call PauseTimer(t)
        set PolledTotal = PolledTotal + (3.00-TimerGetRemaining(t))
        call DestroyTimer(t)
        set t = CreateTimer()

        call TimerStart(t, 3.00, false, null)
        call TriggerSleepAction(0.01)
        call PauseTimer(t)
        set TSATotal = TSATotal + (3.00-TimerGetRemaining(t))
        call DestroyTimer(t)
        set t = CreateTimer()
    endloop

    call BJDebugMsg("PolledWait Average: "+R2S(PolledTotal/100))
    call BJDebugMsg("TSA Average: "+R2S(TSATotal/100))

    set t = null
endfunction

My results for 3 such test were:

And yes, I did test it on your map.

 

[Doom-Angel]

ur pictures aren't working.....
and when i asked if the minimum is 0.27 this is the response i got so that's what i currently know:

Just as a notice - it's not true that waits can't go below 0.27, the actual minimum is around 0.1.
Anyway why exactly do you need to wait 0.01 seconds? Might be a better way to do whatever you're trying.

 

[Pyrogasm]

Nope then you are able to trigger spells when they are NOT finished so if you CANCEL you will already have the effects but you haven't used the mana!!!! Haven't u knew that? o.o

As Überplayer said, you've got it backwards.

Because maybe some guys wanna see how to make sure that only this one unit can trigger the spell?

This is a spell contest, not a tutorial. If "some guys" wanted to see how to do it, they would've asked.

should be but dont have to .. I thought its noobfriendly? You not!

If you want the real technical reason, it's because "Attacking Unit" is just a blizzard extension of "Triggering Unit". They both return the same thing.

Nope thats the thing I just dont want to make!! That sucks

Again, as Überplayer said, the way you've done it factors in the unit's armor and damage reduction/bonuses made in Gameplay constants. If you use Chaos attack and "Universal" damage, you can be sure that when you tell a unit to damage another unit for 50.00 damage, it will do that much and not only 37.28 damage.

You wanna waste space to do those shit? It doesnt have to be!

Yes. It does. You HAVE to generate a new integer every time you check a new ability, else it will be either A) Impossible to get 2 or 3 on the same hit B) Impossible to get anything but all 3 on one hit. Let's say that the number is 8; This will not pass the first two if's, but will satisfy the 3rd one. That's fine; now let's assume that the number is 6. This will not satisfy the first If, but it will satisfy the 2nd and third one, but because they overlap, it is IMPOSSIBLE to get the 2nd without getting the 3rd. You tell me a number that is less than or equal to 6 but also GREATER than 8. See what I'm saying?

By generating a new number, all three possibilities are now independent of each other.

This referes to the dummy unit spelled slow unit target which doesnt work because I paused them.

Then why does the trigger still exist?

Joking about other players spell isnt a good way to get reputation.

It's not a joke or a jest; it's a fact. 5 triggers for 1 spell is intimidating.

Picture download links:
http://www.mediamax.com/pyrogasm/Hosted/WC3ScrnShot_061707_040250_.tga
http://www.mediamax.com/pyrogasm/Hosted/WC3ScrnShot_061707_040501_.tga
http://www.mediamax.com/pyrogasm/Hosted/WC3ScrnShot_061707_040355_.tga

 

[Doom-Angel]

Picture download links:
http://www.mediamax.com/pyrogasm/Hos...07_040250_.tga
http://www.mediamax.com/pyrogasm/Hos...07_040501_.tga
http://www.mediamax.com/pyrogasm/Hos...07_040355_.tga

still doesn't work...

 

[--Thanatos--]

I tried to become a such nerd to edit the function LocalVars().

And now I'm disappointed. BTW thanks you show me which part I did wrong. BTW, DAMAGE_TYPE_UNIVERSAL -> Spell Damage, or what ?

 

[Prometheus]

- "IsUnitAliveBJ(GetFilterUnit()) == true" is bad! You should always use a life comparison: "(GetWidgetLife(GetFilterUnit()) > 0.405); A unit "dies" when its life reaches 0.405 or less, and as such, the comparison should be to this number as to not get any false positives. Additionally, GetWidgetLife() is better than GetUnitState().

The simple fact is, as much as Natives are better, BJs are probably about 0.000000000000000000000000000000001 seconds slower, I also didn't know that if a units life drops below 0.405 it was dead.

- Wouldn't it be better to remove the "*4" on this line to avoid confusion: "local integer level = GetUnitAbilityLevel(GetTriggerUnit(), 'A000') * 4"? You could simply multiply 'level' by 4 when needed, or rename the variable to something like "levelfactor".

I played around with that and it just ended up that way. Yea, shoulda used a local.

- In this line: "local group randus = GetRandomSubGroup(level, GetUnitsInRangeOfLocMatching(400.00 + (100 * level), caspos, Condition(function isally)))", there is a group leaked from the "GetUnitsInRangeOfLocMatching" part and a boolexpr leaked from the "Condition(function isally)". Would it really have been too hard to do this:
local boolexpr b = Condition(function isally)
local group g = GetUnitsInRangeOfLocMatching(400.00 + (400 * level), caspos, b)
local group randus = CreateGroup()
local integer i = 0
local unit u
loop
set i = i+1
exitwhen i > level*4
set u = GroupPickRandomUnit(g)
call GroupAddUnit(randus)
call GroupRemoveUnit(g)
endloop
call DestroyGroup(g)
call DestroyBoolExpr(b)
- Additionally, USE X AND Y COORDINATES! They're USEFUL!
- " if num == 0 then
return
else
endif " The formatting burns! Also, the "else" is not needed; only the "if" and "endif" are important.

I'll keep that in mind as I plan to submit this spell.
Thanks ^^

- On a small map, "set rand = GetRandomLocInRect(GetPlayableMapRect())" seems OK... but what about on a big map? That, and you're not checking to see if it's pathable or not.

This spell wasn't intended to be used in a map, its only intention was to make you laugh as your enemies were thrown about.

- "call DestroyEffect(AddSpecialEffectLoc("Abilities\\Spells\\NightElf\\Blink\ \BlinkCaster.mdl", rand))" and "call SetUnitPositionLoc(move, rand)" should both be replaced with X/Y value-related equivalents.

Do I lose points for this? I prefer to use locals over reals myself, despite locals being a bit slower.

- "call UnitDamageTargetBJ( caster, move, 50.00 * (level / 4), ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL )" could easily be re-written without the BJ; it's only one extra argument that never changes!

By not using a BJ, there are 4 more values this thing takes, 4 more!
I'm not even sure that its faster!

- Instead of "exitwhen loopa >= level", I would do "exitwhen loopa > level" and then declare "loopa" as 1 instead of 0.

Yea, I'll sometimes do 1, sometimes do 0.
I can never decide.

Edit: It seems I've come in 4th

 

[Anachron]

Then it won't do full damage to all armor types I guess..

Oh wow as wouldn't I knew that...

 

[Pyrogasm]

Do I lose points for this? I prefer to use locals over reals myself, despite locals being a bit slower.

Yes, and I hope you mean "locations", not "locals".

The simple fact is, as much as Natives are better, BJs are probably about 0.000000000000000000000000000000001 seconds slower

Yes; I, however, care about the efficiency. Same as with the X/Y coordinates.

By not using a BJ, there are 4 more values this thing takes, 4 more!

I'm sorry, I meant to say 3 other arguements, and 5 of the 8 arguements in the function call don't change. Take a look at it yourself; they will/should always be: ___, ___, ___, true, false, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL, WEAPON_TYPE_WHOKNOWS.

Thanatos, the reason you use "DAMAGE_TYPE_UNIVERSAL" is because it (and DAMAGE_TYPE_UNKNOWN) are the types used for ultimates and never cause adverse effects (DAMAGE_TYPE_COLD slows).

Yea, I'll sometimes do 1, sometimes do 0.
I can never decide.

No, you did not lose points for this.

 

[Doom-Angel]

Pyro u haven't fixed the pictures yet....
i want a proof...