Panda Roll v1.01

[Moderator]

9th Dec 2011
Bribe: Approved 4/5 (Recommended).

Setting the caster's Vertex color values back to 255 could be unsafe if the caster is supposed to have different values. You should have a "static if VERTEX_COLOR" so that it is optional for the user ot have it or not.

SetUnitScale can put 0 as the second 2 parameters, they do nothing.

Your trigger fires and creates the local variables and checks if the unit has the ability before even checking if the right order was cast. There are two proposals for making this better:

if GetIssuedOrderId() == OrderId(ORDER_ON_ID) then
    //call a function to handle the actions
endif

or use OrderEvent (the best way because then the trigger doesn't even evaluate unless it's the right order):

call RegisterOrderEvent(OrderId(ORDER_ID_ON), function thistype.run)

 

[scorpion182]

and thanks to Bribe to help me fix the code

 

[Magtheridon96]

library PandaRoll requires CTL

Do you know how much I love you now? This much:

loop
    exitwhen NEVER
endloop

private constant real MAX_DISTANCE_VALUE = 10000000000.

HURRRR
You can change this to 2147483647. instead because that number causes an overflow ;o (Reals can go beyond 2^31-1 in Warcraft, but not when their initial values are greater (According to my tests a month ago))

Good Job on this spell, I love how you made your structs extend arrays and how you used static arrays :3

set dist = Pow(xCaster - GetUnitX(t), 2.) + Pow(yCaster - GetUnitY(t), 2.)
Really? ;o
You can do this instead:

set tempX = xCaster - GetUnitX(t)
set tempY = yCaster - GetUnitY(t)
set dist = tempX*tempX + tempY*tempY

Oh and you NEVER have to null t because when you exit that FirstOfGroup loop, it's going to be null anyways

Just remove that line that nulls t in the middle of CTLExpire and CTLNull and that line inside CTLNull.

                call UnitAddAbility(dummy[this], 'Amrf')
                call UnitAddAbility(dummy[this], 'Aloc')

Instead of adding these abilities, you can make them default in the object editor.

Overall, nice job. 5/5 for the quality of the code (Meets my high standards)

 

[Maker]

private constant real MAX_DISTANCE_VALUE = 10000000000.
You can change this to 2147483647

Or sqrt((480*128)^2*2) = 87000

 

[Magtheridon96]

Or sqrt((480*128)^2*2) = 87000

Didn't think of that ^.^
I think 92083 is the limit according to Nestharus :O

 

[RakEnRoL]

Rating: 4.5/5
The spell is nice
I have no problems with it
Good use of model and nice idea

 

[watermelon_1234]

For

if (GetUnitAbilityLevel(u, SPELL_ID) > 0 and GetIssuedOrderId() == OrderId(ORDER_ON_ID)) then

I'd just suggest switching the order around to

if GetIssuedOrderId() == OrderId(ORDER_ON_ID)) and (GetUnitAbilityLevel(u, SPELL_ID) > 0 then

since and short circuits.
I think you should also have configurables for the caster's vertex color.

 

[scorpion182]

UPDATED

Do you know how much I love you now? This much:

loop
    exitwhen NEVER
endloop

private constant real MAX_DISTANCE_VALUE = 10000000000.

HURRRR
You can change this to 2147483647. instead because that number causes an overflow ;o (Reals can go beyond 2^31-1 in Warcraft, but not when their initial values are greater (According to my tests a month ago))

Good Job on this spell, I love how you made your structs extend arrays and how you used static arrays :3

set dist = Pow(xCaster - GetUnitX(t), 2.) + Pow(yCaster - GetUnitY(t), 2.)
Really? ;o
You can do this instead:

set tempX = xCaster - GetUnitX(t)
set tempY = yCaster - GetUnitY(t)
set dist = tempX*tempX + tempY*tempY

Oh and you NEVER have to null t because when you exit that FirstOfGroup loop, it's going to be null anyways

Just remove that line that nulls t in the middle of CTLExpire and CTLNull and that line inside CTLNull.

                call UnitAddAbility(dummy[this], 'Amrf')
                call UnitAddAbility(dummy[this], 'Aloc')

Instead of adding these abilities, you can make them default in the object editor.

Overall, nice job. 5/5 for the quality of the code (Meets my high standards)

Or sqrt((480*128)^2*2) = 87000

- i prefer 2147483647, because 87000 or 92083 doesn't work don't know why

- i need to null t because t not nulled when meet exitwhen true

if dist < nearestDist and (t != target[this] and IsUnitEnemy(t, GetOwningPlayer(caster[this])) and not IsUnitType(t, UNIT_TYPE_STRUCTURE) and not IsUnitType(t,UNIT_TYPE_DEAD)) then
                            set nearestDist = dist
                            set target[this] = t
                            exitwhen true
                        endif

Rating: 4.5/5
The spell is nice
I have no problems with it
Good use of model and nice idea

thanks

For

if (GetUnitAbilityLevel(u, SPELL_ID) > 0 and GetIssuedOrderId() == OrderId(ORDER_ON_ID)) then

I'd just suggest switching the order around to

if GetIssuedOrderId() == OrderId(ORDER_ON_ID)) and (GetUnitAbilityLevel(u, SPELL_ID) > 0 then

since and short circuits.
I think you should also have configurables for the caster's vertex color.

added vertex color to constant

 

[Magtheridon96]

i need to null t because t not nulled when meet exitwhen true

True that :3 (Didn't notice it)

One other thing:

            local thistype this = create()
            local unit t = null
            local real nearestDist = MAX_DISTANCE_VALUE
            local real dist
            local real x
            local real y
            local real tempX
            local real tempY
        
            set target[this] = null
            set caster[this] = u
            set lvl[this] = level
            set x = GetUnitX(u)
            set y = GetUnitY(u)
            
            call DestroyEffect(AddSpecialEffect(CAST_FX, x, y))
            set dummy[this] = CreateUnit(GetOwningPlayer(u), DUMMY_ID, GetUnitX(u), GetUnitY(u), GetUnitFacing(u))
            set fx[this] = AddSpecialEffectTarget(DUMMY_FX, dummy[this], "origin")

Inefficient

->

            local thistype this = create()
            local unit t
            local real nearestDist = MAX_DISTANCE_VALUE
            local real dist
            local real x = GetUnitX(u)
            local real y = GetUnitY(u)
            local real tempX
            local real tempY
        
            set target[this] = null
            set caster[this] = u
            set lvl[this] = level
            
            call DestroyEffect(AddSpecialEffect(CAST_FX, x, y))
            set dummy[this] = CreateUnit(GetOwningPlayer(u), DUMMY_ID, x, y, GetUnitFacing(u))
            set fx[this] = AddSpecialEffectTarget(DUMMY_FX, dummy[this], "origin")

That's better :3

edit

Also, over here:

            set xCaster = GetUnitX(caster[this])
            set yCaster = GetUnitY(caster[this])
            set xTarget = GetUnitX(target[this])
            set yTarget = GetUnitY(target[this])
            set angle = Atan2(yTarget-yCaster,xTarget-xCaster)
            set dx = DISTANCE_INC * Cos(angle)
            set dy = DISTANCE_INC * Sin(angle)
            set t = null
            set nearestDist = MAX_DISTANCE_VALUE

There's this really awkward set t = null line ._.