Frozen Blast v.46

This bundle is marked as approved. It works and satisfies the submission rules.
  • Like
Reactions: Deuterium
JASS:
//////////////////////////////////////////////////////////////////////
//  Frozen Blast by 1)ark_NiTe                                      //
//  Originally made for Against the Darkness                        //
//                                                                  //
//  r = angle that wave will be fired towards                       //
//                                                                  //
//  Notes:                                                          //
//                                                                  //
//  1. Change the spells and dummy units rawcodes in the constants. //
//  2. Damage/duration data can be changed by accessing the spells  //
//     in the object editor                                         //
//     a.) Damage for each Frozen Blast Wave is modified in the     //
//         Frozen Blast (Actual Ability).                           //
//     b.) Cooldown/spell description is modified in the Frozen     //
//         Blast (Dummy Ability).                                   //
//////////////////////////////////////////////////////////////////////

//==================================================================================================================================
// Start of constants. These provide the user with easier implementing//changing of the spell's rawcodes. All of these can be changed.
//==================================================================================================================================

constant function Frozen_Blast_ID takes nothing returns integer
    return 'A000' // Frozen Blast actual ability rawcode. This is the spell that will be cast by a dummy unit.
endfunction

constant function Frozen_Blast_Dummy_ID takes nothing returns integer
    return 'A001' // Frozen Blast dummy ability rawcode. This is the spell that the Hero will use. 
endfunction
    
constant function Slow_ID takes nothing returns integer
    return 'A002' // Slow (Frozen Blast) rawcode.
endfunction
                 
constant function Dummy_ID takes nothing returns integer
    return 'h002' // Dummy unit rawcode
endfunction

//==================================================================================================================================
//End of constants. Do not touch anything below this if you are unfamiliar with JASS.
//==================================================================================================================================

//==================================================================================================================================
//Spell conditions.
//==================================================================================================================================

function Trig_Blast_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == Frozen_Blast_Dummy_ID()
endfunction

//==================================================================================================================================
//Filter function grouping enemies to be slowed.
//==================================================================================================================================

function Slow_Filter takes nothing returns boolean
    return IsPlayerEnemy(GetOwningPlayer(GetTriggerUnit()), GetOwningPlayer(GetFilterUnit())) == true
endfunction

//==================================================================================================================================
//Spell actions. Main block of coding
//==================================================================================================================================

function Trig_Blast_Actions takes nothing returns nothing    
    //Locals are declared
    local unit t = GetTriggerUnit()
    local real px = GetWidgetX(t)
    local real py = GetWidgetY(t)
    local real qx
    local real qy
    local unit array d
    local unit array u
    local unit e
    local group g = CreateGroup()
    local real r = 0.00
    local integer i = 0
    local integer z = 0
    local integer l = GetUnitAbilityLevel(t, Frozen_Blast_Dummy_ID())
    //Frozen Blasts are created. A blast occurs every 30 degrees out around the Hero 12 times (360 degrees).
    loop
        exitwhen i == 12
        set i = i + 1
        set r = r + .52359 
        set d[i] = CreateUnit(GetOwningPlayer(t), Dummy_ID(), px, py, 270.0)
        call UnitAddAbility(d[i], Frozen_Blast_ID())
        call SetUnitAbilityLevel(t, Frozen_Blast_ID(), l)
        call UnitApplyTimedLife(d[i], 'BTLF', 1.00)
        set qx = px + 300 * Cos(r)
        set qy = py + 300 * Sin(r)
        call IssuePointOrder (d[i], "carrionswarm", qx, qy)
        set d[i] = null
    endloop
    call GroupEnumUnitsInRange(g, px, py, 650.00, Condition(function Slow_Filter))
    set i = 0
    set z = CountUnitsInGroup(g)
    //Dummy units cast Slow (Frozen Blast) on the units in the area
    loop
        exitwhen i >= z
        set i = i + 1
        set u[i] = FirstOfGroup(g)
        call GroupRemoveUnit( g, u[i] )
        set e = CreateUnit(GetOwningPlayer(t), Dummy_ID(), px, py, 270.0)
        call UnitApplyTimedLife(e, 'BTLF', 1.00)
        call UnitAddAbility(e, Slow_ID())
        call IssueTargetOrder(e, "slow";, u[i])
        set u[i] = null
    endloop
    call DestroyGroup(g)
    set g = null
    set e = null
    set t = null
endfunction

function AntiLeaker takes nothing returns boolean
    return true
endfunction

//===========================================================================
function InitTrig_Frozen_Blast takes nothing returns nothing
    local trigger t = CreateTrigger( )
    local filterfunc f = Filter(function AntiLeaker)
    local integer i = 0
    loop
        call TriggerRegisterPlayerUnitEvent(t, Player(i), EVENT_PLAYER_UNIT_SPELL_EFFECT, f)
        set i = i + 1
        exitwhen i == 16
    endloop
    call TriggerAddCondition(t, Condition( function Trig_Blast_Conditions ) )
    call TriggerAddAction(t, function Trig_Blast_Actions )
    call DestroyFilter(f)
    set f = null
    set t = null
endfunction

Summons 12 powerful blasts of frost that radiate outward from the Hero. Each blast wave damages for 125/150/175 hit points and slows enemy units by 15% for 8 seconds.

This spell was originally made for my project Against the Darkness.

+Multi-Instanceable
+Leakless
+No Imports
+Easily Transferable

Updates (recent to past):

v. 46 (current):

-Cleaned up group leak (don't know why I didn't see this before!)

v. 45:

-Added multi-level support (3 levels).
-Type "-level" to level up your Hero to test each level of the spell.

v.40:

-Created constant functions to make replacing rawcodes simple during spell implementation.
-Optimized to natively use radian measure instead of using bj_DEGTORAD function to convert to radians.
-Removed parentheses from simple math
-Updated/expanded documentation
-Replaced bj_UNITFACING with 270.0

v.35:

-Changed 'Begins Casting' to 'Starts the Effect'

v.30:

-Replaced GroupPickRandomUnit() with FirstOfGroup()
-exitwhen i == z changed to i >= z in case of it bugging

v.25:

-Replaced GetUnitX/Y()'s with variables px and py.
-Replaced PolarProjectionBJ with variables qx and qy as coordinates.
-Added Changelog
*Thanks to Deuterium and Cheezeman for these suggestions.

Keywords:
Frozen, Archer, Freeze, Wave, Ice, Blast, Against the Darkness, 1)ark_NiTe, AtD
Contents

Frozen Blast (Map)

Reviews
23:03, 21th Dec 2009 Dr Super Good: The spell seems ok and usable. It is also simple enough for people to learn from and the previously mentioned problems have been fixed. Testing the spell is the only blaintently noticable fault. You should...

Moderator

M

Moderator

23:03, 21th Dec 2009
Dr Super Good:
The spell seems ok and usable. It is also simple enough for people to learn from and the previously mentioned problems have been fixed.

Testing the spell is the only blaintently noticable fault. You should make the hero level 10 so you can actually level the spell up or atleast make the creeps give EPX so it can be leveld up later otherwise its difficult to test the multi level support.
 
Level 12
Joined
Jul 27, 2008
Messages
1,181
A) Better use reals/coordinates rather than locs, it reduces lag much.
B) Why not create dummies, and have them slow the units? That'd remove the TSA too.
C) Comments seem ok.

D) Reduce the cooldown/add a button to reset it.
E) As it is the effects are stacking on units from many different liches casting it -> see B
F) Dead units are also slowed -> see B
 
Level 17
Joined
Mar 17, 2009
Messages
1,349
Deu's Spell Review:

[-]

1. Use coordinates instead of locations.


2. Uses some BJ's, but all are good BJ's except for:
JASS:
call GroupRemoveUnitSimple( u[i], g )

Replace it with:
JASS:
call GroupRemoveUnit(g, u[i])

Simple, as you see, is like the keyword Swap.


3. I think it's about time you learn to avoid this specially since it leaks 16 times at map intialization:
JASS:
call TriggerRegisterAnyUnitEventBJ(gg_trg_Frozen_Blast, EVENT_PLAYER_UNIT_SPELL_CAST)

Instead:
JASS:
function AntiLeaker takes nothing returns boolean
    return true
endfunction

//===========================================================================
function InitTrig_Frozen_Blast takes nothing returns nothing
    local filterfunc f = Filter(function AntiLeaker)
    local integer i = 0
    set gg_trg_Frozen_Blast = CreateTrigger(  )
    loop
        call TriggerRegisterPlayerUnitEvent(gg_trg_Frozen_Blast, Player(i), EVENT_PLAYER_UNIT_SPELL_CAST, f)

        set i = i + 1
        exitwhen i == 16 // or you can insert bj_MAX_PLAYER_SLOTS instead
    endloop
    call TriggerAddCondition( gg_trg_Frozen_Blast, Condition( function Trig_Blast_Conditions ) )
    call TriggerAddAction( gg_trg_Frozen_Blast, function Trig_Blast_Actions )
    set f = null
endfunction


4. Use a local trigger instead of:
JASS:
gg_trg_Frozen_Blast

Thus making our Init code with the filter I mentioned:
JASS:
function AntiLeaker takes nothing returns boolean
    return true
endfunction

//===========================================================================
function InitTrig_Frozen_Blast takes nothing returns nothing
    local trigger t = CreateTrigger(  )
    local filterfunc f = Filter(function AntiLeaker)
    local integer i = 0
    loop
        call TriggerRegisterPlayerUnitEvent(t, Player(i), EVENT_PLAYER_UNIT_SPELL_CAST, f)

        set i = i + 1
        exitwhen i == 16
    endloop
    call TriggerAddCondition(t, Condition( function Trig_Blast_Conditions ) )
    call TriggerAddAction(t, function Trig_Blast_Actions )
    set f = null
    set t = null


5. There's a point leak:
JASS:
PolarProjectionBJ(p, 300.00, r)
This function returns a location, thus you got to set it into a variable, use the variable and then destroy it.


6. Use a slow dummy spell and avoid the:
JASS:
call TriggerSleepAction(8.00)


7. You forget to set z = 0 before the loop:
JASS:
        exitwhen z == 12
        set z = ( z + 1 )
        set d[z] = null


8. Add more in-script documentation so that whoever is reading your script know what each set of actions are supposed to be doing.


9. Set things that need to be adjusted into variables (although I'd prefer using globals) at the beginning of the script.



[+]

10. Fully MUI.


11. Looks nice and function properly.


12. Scripting is efficient enough.


13. Documentation provided, and must be easy to import.



[*]

This spell is a simple, yet well-done Jass spell which function properly. The only issue would be that point leak and it's a job well-done!

Dark Nite, I always like your spells since they're simple, nice and useful. Just do some fixing considering the things I mentioned and removing that leak, and you've got a great script. Good job!
 
Level 10
Joined
Aug 19, 2008
Messages
492

Cheezeman Presents

Cheezeman's Review

Reviewing 1)ark_Nite's Frozen Blast 0.15 for fun
I'm not a mod so don't take this too serious. I'm just reflecting my ideas

Review


Not very good coding at all, apart from your leak removals.
1: Not leak free at all
1.1 Uses PolarProjectionBJ which leaks a location. See solutions below for further details.
1.2 In the second loop, z is sill 12 from the previous one, so no units are nullified
1.3 function Slow_Filter leaks 2 units and 2 players. See solutions below for details.
2: Does not have a Setup section (not mandatory but it's very useful for spell sharing)
3: Not fully customizable with - for example - number of bolts, area of effect, eyecandy
4: Damage is dealt with abilities rather than UnitDamageTarget.
Now, I'm not saying UnitDamageTarget perfect, but the computer does not know who killed the units when using dummy abilities, and thus won't give proper xp to the user. Just replace with "ForGroup(g, function DamageTheGroup)" which I'm sure you can pull off.
1: call GroupRemoveUnitSimple( u, g ) is a BJ function (evil...). Is easily replaced with call GroupRemoveUnit(g, u)
2: set m = GetUnitDefaultMoveSpeed(u) is not a good idea, since their current movement speed could've been manipulated already. So when the TSA(8.) has passed, they will revert to original and not previous.
3: In the first loop, you can use z and i for the same purpose (just use one of them)
4: I think it's a better idea to use a non-array unit local for the dummies, and nullify them the instant they cast their bolt. Gives more of a structure to it

1.1: If we have a look at polar projection in common.j (or blizzard.j, I'm not sure) we will see this:
JASS:
function PolarProjectionBJ takes location source, real dist, real angle returns location
    local real x = GetLocationX(source) + dist * Cos(angle * bj_DEGTORAD)
    local real y = GetLocationY(source) + dist * Sin(angle * bj_DEGTORAD)
    return Location(x, y)
endfunction

As you can see this returns a non-destroyed and non-nullified location at the end, and that's what we wanna avoid (since you use 12 in a loop).
I found a solution a while ago by extracting the X and Y coordinates out of it, instead of a location.
As you can see, it already uses X and Y, so instead of using IssuePointOrderLoc(), use IssuePointOrder(), something like this:
JASS:
//First loop a tad bit down
set x = GetLocationX(p) + 300.00 * Cos(r * bj_DEGTORAD) //First line of PolarProjectionBJ
set y = GetLocationY(p) + 300.00 * Sin(r * bj_DEGTORAD)
call IssuePointOrderLoc( d[z], "carrionswarm", x, y )

Now, all you need to do is remove p as you've already done




1.3 Just replace with this:
JASS:
function Slow_Filter takes nothing returns boolean
    local boolean bool = false
    local unit cast = GetTriggerUnit()
    local unit filt = GetFilterUnit()
    local player cast_own = GetOwningPlayer(cast)
    local player filt_own = GetOwningPlayer(filt)
    if IsPlayerEnemy( cast_own, filt_own ) then
        set bool = true
    endif
    set cast = null
    set filt = null
    set cast_own = null
    set filt_own = null
    return bool
endfunction

Now you may wonder why you should replace such a simple function with these complicated lines.
It's simple: You freaking leak 4 times for each unit you pass through here.
What I've done is that I have a "dummy" boolean to check the actual condition (since it returns a boolean) and at the very end when everything is worked out, I return it.
This can be done for integers, reals and something more which I can't remember right now *brainfreeze*.
And this is why we jassers hate functions returning handles, especially locations.





2: Making Setup sections are easy. For example, just replace
JASS:
function Trig_Blast_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A001'
endfunction
with
JASS:
constant function Trig_Blast_Spell_Idtakes nothing returns integer
    return 'A000'
endfunction

function Trig_Blast_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == Trig_Blast_Spell_Id()
endfunction

This is much simpler, don't you agree?
(Note: Rawcodes are integers, unless you already knew that)
This can be done with everything else aswell. Just keep all those functions on top of the code.


Originality: 2/5
Well... it's not very common but I've seen it before.

Documentation: 0/5
No documentation at all? It's 0.15 already! Do something!

Overall = (Coding * 0.50) + (Originality * 0.30) + (Documentation * 0.20)
Overall: 1.1
Rejected with a 1/5 (Change Now or Die) rating


Reviewing...
Haha! I'm faster than you!
 
Level 17
Joined
Mar 17, 2009
Messages
1,349
To add on to my review, you could have simply used i as the loop integer without even including z:
JASS:
    loop
        exitwhen i == 12
        set i = ( i + 1 )
        set z = ( z + 1 )
        set r = ( r + 30.00 )
        //+ Replace 'h000' with your Dummy unit raw code +
        set d[z] = CreateUnit(GetOwningPlayer(t), 'h002', GetUnitX(t), GetUnitY(t), bj_UNIT_FACING)
        //+ Replace 'A001' with your Frozen Blast (Actual Ability) raw code +
        call UnitAddAbility(d[z], 'A000')
        call UnitApplyTimedLife(d[z], 'BTLF', 1.00)
        set p = GetUnitLoc(t)
        call IssuePointOrderLoc( d[z], "carrionswarm", PolarProjectionBJ(p, 300.00, r) )
        call RemoveLocation(p)
    endloop

and you could've nulled the units directly in this loop instead of starting a new one just to null those locals, and you have to null p in the loop:
JASS:
    loop
        exitwhen i == 12
        set i = ( i + 1 )
        set z = ( z + 1 )
        set r = ( r + 30.00 )
        //+ Replace 'h000' with your Dummy unit raw code +
        set d[z] = CreateUnit(GetOwningPlayer(t), 'h002', GetUnitX(t), GetUnitY(t), bj_UNIT_FACING)
        //+ Replace 'A001' with your Frozen Blast (Actual Ability) raw code +
        call UnitAddAbility(d[z], 'A000')
        call UnitApplyTimedLife(d[z], 'BTLF', 1.00)
        set p = GetUnitLoc(t)
        call IssuePointOrderLoc( d[z], "carrionswarm", PolarProjectionBJ(p, 300.00, r) )
        call RemoveLocation(p)
        set d[z] = null
        set p = null
    endloop
 
Level 10
Joined
Aug 19, 2008
Messages
492
Maybe you need to know what documentation means...
Yes?
This is my first review, don't be mean :sad:

5. There's a point leak:
PolarProjectionBJ(p, 300.00, r)
This function returns a location, thus you got to set it into a variable, use the variable and then destroy it.
Setting PolarProjectionBJ into a variable will not stop PolarProjectionBJ from leaking a location

6. Use a slow dummy spell and avoid the:
I was just wondering why you (and everybody) hate TSA? I just don't get it.



aww I feel so mean now that you've given him good credits and I just rejected it completely :sad:
 
Level 17
Joined
Mar 17, 2009
Messages
1,349
Well check my review and you know what's documentation... and if you're planning on reviewing, you need to be less "mean-sounding" and more friendly :p

And why do all this?
JASS:
constant function Trig_Blast_Spell_Idtakes nothing returns integer
    return 'A000'
endfunction

Why use this, just use the normal condition function and add a line that says something needs to be changed there. Otherwise, if more "simplicity" is needed, then a global block would be the only solution...
 
Level 10
Joined
Aug 19, 2008
Messages
492
Well check my review and you know what's documentation... and if you're planning on reviewing, you need to be less "mean-sounding" and more friendly :p

And why do all this?
JASS:
constant function Trig_Blast_Spell_Idtakes nothing returns integer
    return 'A000'
endfunction

Why use this, just use the normal condition function and add a line that says something needs to be changed there. Otherwise, if more "simplicity" is needed, then a global block would be the only solution...
First, I'd recommend you to read JESP (the standard every good jasser knows and follows).
Second, I personally prefer having such functions for a better structure. I don't see why I should optimize my code for that extra nanosecond of scripting instead of having it easier to change.
When people want to implent a spell they shouldn't have a problem in the world with it. Just Copy, paste and change some stuff, not copy, paste and mess around with the spell's core.

Oh and yes, I'd prefer a global constant but this spell isn't vJass
 
Level 17
Joined
Mar 17, 2009
Messages
1,349
Cheezeman said:
Setting PolarProjectionBJ into a variable will not stop PolarProjectionBJ from leaking a location
Didn't you read that I said that the location should then be removed?

Cheezeman said:
I was just wondering why you (and everybody) hate TSA? I just don't get it.
Cause it can sometimes mess up... some issues in case of game lagging...

Cheezeman said:
aww I feel so mean now that you've given him good credits and I just rejected it completely
Well, a review looks over all aspects of whatever is being reviewed, and not only the bad things.
Here in the hive, almost all reviewers mention only negative stuff, and thus aren't really liked by the members :p
 
Level 10
Joined
Aug 19, 2008
Messages
492
Didn't you read that I said that the location should then be removed?
Yes I read it, but even removing and nullifying that variable will not remove the leak, if you know what I mean.
Maybe this is an misunderstanding aswell, cause I interpreted it as you wanted to "set loc = PolarProjectionBJ()" then "call RemoveLocation(loc)" and "set loc = null"
In that case, it won't help.
In other case, I've misunderstood you.
Enlighten me!

Cause it can sometimes mess up... some issues in case of game lagging...
Hm... alright, I'll buy it.
 
Level 18
Joined
Nov 1, 2006
Messages
1,612
Hey guys thanks for the quick response. I'm going to fix the problems with this and then update. I'll post when I do.

And Reaper2008, Frozen Archer is the hero that has this ability in Against the Darkness.

And there is plenty of documentation Cheezeman I don't know what you're talking about.

Alright beginning work on it, I'll post the update later today.
 
Level 17
Joined
Mar 17, 2009
Messages
1,349
Dark Nite here's something I didn't know until after I did the review. The filterfunc we used needs to be destroyed after being used (before nulling f & t):
JASS:
call DestroyFilter(f)

Well enjoy this:
JASS:
function Trig_Blast_Conditions takes nothing returns boolean
    //+ Replace 'A000' with your Frozen Blast (Dummy Ability) raw code +
    return GetSpellAbilityId() == 'A001'
endfunction

function Slow_Filter takes nothing returns boolean
    return IsPlayerEnemy(GetOwningPlayer(GetTriggerUnit()), GetOwningPlayer(GetFilterUnit())) == true
endfunction

function Trig_Blast_Actions takes nothing returns nothing
    //Locals are declared
    local unit t = GetTriggerUnit()
    local real px = GetWidgetX(t)
    local real py = GetWidgetY(t)
    local real qx
    local real qy
    local unit array d
    local unit array u
    local unit e
    local group g = CreateGroup()
    local real r = 0.00
    local integer i = 0
    local integer z = 0
    //Frozen Blasts are created. A blast occurs every 30 degrees out around the Hero 12 times (360 degrees).
    loop
        exitwhen i == 12
        set i = ( i + 1 )
        set r = ( r + 30.00 )
        //+ Replace 'h000' with your Dummy unit raw code +
        set d[i] = CreateUnit(GetOwningPlayer(t), 'h002', px, py, bj_UNIT_FACING)
        //+ Replace 'A001' with your Frozen Blast (Actual Ability) raw code +
        call UnitAddAbility(d[i], 'A000')
        call UnitApplyTimedLife(d[i], 'BTLF', 1.00)
        set qx = px + 300 * Cos(r * bj_DEGTORAD)
        set qy = py + 300 * Sin(r * bj_DEGTORAD)
        call IssuePointOrder (d[i], "carrionswarm", qx, qy)
        set d[i] = null
    endloop
    call GroupEnumUnitsInRange(g, GetUnitX(t), GetUnitY(t), 650.00, Condition(function Slow_Filter))
    set i = 0
    set z = CountUnitsInGroup(g)
    loop
        exitwhen i == z
        set i = ( i + 1 )
        set u[i] = GroupPickRandomUnit(g)
        call GroupRemoveUnit( g, u[i] )
        set e = CreateUnit(GetOwningPlayer(t), 'h002', GetUnitX(t), GetUnitY(t), bj_UNIT_FACING)
        call UnitApplyTimedLife(e, 'BTLF', 1.00)
        // + Replace 'A002' with your Slow (Frozen Blast) raw code +
        call UnitAddAbility(e,'A002')
        call IssueTargetOrder(e, "slow", u[i])
        set u[i] = null
    endloop
    set e = null
    set t = null
endfunction

function AntiLeaker takes nothing returns boolean
    return true
endfunction

//===========================================================================
function InitTrig_Frozen_Blast takes nothing returns nothing
    local trigger t = CreateTrigger( )
    local filterfunc f = Filter(function AntiLeaker)
    local integer i = 0
    loop
        call TriggerRegisterPlayerUnitEvent(t, Player(i), EVENT_PLAYER_UNIT_SPELL_CAST, f)
        set i = i + 1
        exitwhen i == 16
    endloop
    call TriggerAddCondition(t, Condition( function Trig_Blast_Conditions ) )
    call TriggerAddAction(t, function Trig_Blast_Actions )
    call DestroyFilter(f)
    set f = null
    set t = null
endfunction
Got rid of the degree and used radians in the first place, but couldn't find my calculator.

All you need to do, is instead of using 30 degrees (the thirty added to the r variable), use Pi/6 (convert it into decimal using a calculator) and remove the bj_DEGTORAD in the qx and qy. ;)
 
Level 10
Joined
Aug 19, 2008
Messages
492
Hey hey, you used my PolarProjection idea :thumbs_up:
Oh and, when I used them in my map I made functions out of them instead (makes more sence, though for a spell it's quite useless)
JASS:
function PolarProjectionX takes location source, real dist, real angle returns location
    local real x = GetLocationX(source) + dist * Cos(angle * bj_DEGTORAD)
    call RemoveLocation(source)
    set source = null
    return x
endfunction

function PolarProjectionY takes location source, real dist, real angle returns location
    local real y = GetLocationY(source) + dist * Sin(angle * bj_DEGTORAD)
    call RemoveLocation(source)
    set source = null
    return y
endfunction

P.S Looking on your new spell now




Cheezeman Presents

Cheezeman's Review

Reviewing 1)ark_Nite's Frozen Blast 0.20 for fun
I'm not a mod so don't take this too serious. I'm just reflecting my ideas

Review


Good job on improvements, though there are still some old, and newly discovered, buggahs'
1: In the first loop, you use d as an array. There's no need for d to be an array, since it's instantly destroyed in the loop anyway. Same goes for u in the second loop
2: You have to replace PolarProjectionBJ, because the function itself leaks a location.
3: You didn't replace Slow_Filter yet
4: Magic Immune, dead and structures are targeted in the Slow_Filter. We wouldn't wanna slow an invurnurable unit, now do we?
5: Spell does not contain preload. Easy solution, see solutions for details

4: Just add some 'and' conditions to your filter like this:
JASS:
function Slow_Filter takes nothing returns boolean
    local boolean bool = false
    local unit cast = GetTriggerUnit()
    local unit filt = GetFilterUnit()
    local player cast_own = GetOwningPlayer(cast)
    local player filt_own = GetOwningPlayer(filt)
    if IsPlayerEnemy( cast_own, filt_own ) and GetWidgetLife( filt ) > .405 and IsUnitType( filt, UNIT_TYPE_MAGIC_IMMUNE ) and IsUnitType( filt, UNIT_TYPE_STRUCTURE ) then
    //Long line, I know... :/
        set bool = true
    endif
    set cast = null
    set filt = null
    set cast_own = null
    set filt_own = null
    return bool
endfunction

There are of course more things to add if you wish, like UNIT_TYPE_HERO, but that's optional.


Originality: 2/5
Well... it's not very common but I've seen it before.

Documentation: 3/5
I saw your import guides which is enough, but I still require a changelog for a 5/5

Overall = (Coding * 0.50) + (Originality * 0.30) + (Documentation * 0.20)
Overall: 2.2
Rejected with a 2/5 (Getting closer...) rating
 
Last edited:
Level 18
Joined
Nov 1, 2006
Messages
1,612
Thanks bree for the comment. Cheeze, there is no reason to add more conditions to the filter.

4: Magic Immune, dead and structures are targeted in the Slow_Filter. We wouldn't wanna slow an invurnurable unit, now do we?

There is no point to adding any more conditions to the filter. You can't cast an ability on invulnerable, dead, or immune units. Did you not notice I no longer slow them manually but use dummy units and spells?

To your other point... I still don't see the reason to preload the filter?

Anyway, spell was updated and no longer uses PolarProjectionBJ(). Woot. Change log was also added.
 
Level 10
Joined
Aug 19, 2008
Messages
492
Thanks bree for the comment. Cheeze, there is no reason to add more conditions to the filter.



There is no point to adding any more conditions to the filter. You can't cast an ability on invulnerable, dead, or immune units. Did you not notice I no longer slow them manually but use dummy units and spells?

To your other point... I still don't see the reason to preload the filter?

Anyway, spell was updated and no longer uses PolarProjectionBJ(). Woot. Change log was also added.
You're right, they're not targeted by the spell (I missed that).
Still, you're creating a unit and attaching a spell and timed life to it by not doing this. I'm not sure which is best; create a unit and attach stuff to it, or run a unit through a long annoying filter?
That's for someone more experienced than me to say.
Anyway, spell was updated and no longer uses PolarProjectionBJ(). Woot. Change log was also added.
w00t! Best changes ever.
I'll review again tomorrow (today, duh...)
Edit: I don't have time to review today, unless it's like 23:00 (GMT+1 CEST). I will review this within 24h, I promise
 
Last edited:
Ok, constructive critism:
JASS:
set z = CountUnitsInGroup(g)
GroupPickRandomUnit(g)
  • You used 2 bjs which are unnesessary.
  • You are not able to change the spell values in the script
  • The flying units are not scripted, they are basicly just a few spells casted in a circle.
  • JASS:
    loop
            exitwhen i == z
    This could bug. If you use that in structs or so, I would suggest checking i >=, then even while being bugged and over z, it will instantly skip. Else it will run into a critical error.

Edit: Oh btw I know they are maybe good BJs, but even those bjs are not required. I wouldn't make the spell like you, so I wouldn't need that bjs. But for your script I think its ok to let them stand.
 
Level 10
Joined
Aug 19, 2008
Messages
492

Cheezeman Presents

Cheezeman's Review

Reviewing 1)ark_Nite's Frozen Blast 0.30 for fun
I'm not a mod so don't take this too serious. I'm just reflecting my ideas

Review


1: You didn't change the unit arrays in the loops as I said (=bad). Oh and you might aswell get rid of one of them, since d is nullified after the loop (inside the loop more technically) and it serves the same purpose as u
2: (Small issue) That "== true" in slow filter is unnecessary, Jass checks if it's true by default (not that it matters very much)
3: Spell still doesn't contain a preload (sorry I forgot to put it in the solutions below last time)
4: Your spell doesn't quite have much options to tweak it, unless the user knows jass very good (in which case he'd make this himself)
[+] i <= z

Preloading is easy. Either you can use xe, or you can do what I use:
JASS:
    set bj_lastCreatedUnit = CreateUnit(Player( PLAYER_NEUTRAL_PASSIVE ), DUMMY_ID, 0, 0, 0)
    call UnitAddAbility( bj_lastCreatedUnit, SPELL_ID )
    call KillUnit( bj_lastCreatedUnit )

Just put that code anywhere inside the InitTrig_ function, and your spell won't have that "first time freeze" (which I'm not quite sure it has for your small map)


Originality: 2/5
Well... it's not very common but I've seen it before.

Documentation: 5/5
Approved documentation

Overall = (Coding * 0.50) + (Originality * 0.30) + (Documentation * 0.20)
Overall: 2.6
Approved with a 3/5 (Rather useful) rating
 
Okay, I changed GroupPickRandom() to the native FirstOfGroup(). I also changed the exitwhen.

And yes, this ability creates dummy units that cast the ice waves. I don't really see anything wrong with that as long as it is leakless, efficient, and lagless. Many people create spells this way.

Did you change the i == z to i >= z? It will prevent critical crashes or bugs, while being over z. (It shouldn't take place, but you will never know what you will do later on, or blizzard (or the game itself)).
 
Top