Spell Homing Missiles

[Tom Jones]

It makes absolutely no difference where your null your variables, as long as you do it after your done using them.

function...
    local unit u  //In reality local unit u = null
    local integer i = GetRandomInt(1,5)

    if i == 1 then
        set u = GetTriggerUnit()
        ...
        set u = null //When you null here, you know that the variable u contains a unit, and therefore needs to be nulled.
    endif
    set u = null //When you null here, you may not be sure that the variable u contains a unit, however if it doesn't your just setting the variable to null again.
endfunction

 

[Pyrogasm]

It makes more sense to only null them when you know they have a value, else it's an extra function call that you shouldn't need.

 

[emjlr3]

not a function call, but its something you dont need, so why do it?

 

[PurgeandFire]

Well, I meant that there is no need to null it at this area:

    if failsafe <= 0 then
        set dummy = null
        set target = null
        set caster = null
        call DestroyGroup(g)
        set g = null
        return
    endif

Because the variable is not set yet.

 

[Naminator]

Good Spell! Like it.

 

[Doomhammer]

have you had a look at collision missiles from the Caster System yet? They are homing.

 

[PurgeandFire]

Yea, but this spell is easier to implement than having to copy a spell along with a system. Or the functions...

 

[Prometheus]

Uhm, actually its only useless if there are NO units in range to begin with.
That would be useless if I nulled the dummy at the end, though.

 

[emjlr3]

let me knwo if your ever update it, graveyarded

 

[Prometheus]

Yeah, I'm bumping something from October 17th.

I was looking over my submitted stuff to approved ratio and was appalled. So, I updated this.
Uses vJass now, mucho improved coding.

 

[emjlr3]

why did you go from 2.7 to 4.0

seems 3.0 would have been more appropriate

range, total units should be lvlable

add an exitwhen firstofgroup==null, and remove that part you have, its silly

no need to null dummy before reusing it, same with target

you loop could be removed and its actions ran in the Condition(function), its better

this could benefit from use of a global group which is reused

 

[Prometheus]

why did you go from 2.7 to 4.0

seems 3.0 would have been more appropriate

When I do a massive update to any code of my, I just bump it up a ton.

range, total units should be lvlable

Range has been done, units are already doing that, it just has one level.

exitwhen loopa > BASENUM*splvl

add an exitwhen firstofgroup==null, and remove that part you have, its silly

Good and all, but that means I need to limit the number of units in the group when I actually group them.

no need to null dummy before reusing it, same with target

Fixed.

you loop could be removed and its actions ran in the Condition(function), its better

I'll change.

this could benefit from use of a global group which is reused

Wouldn't that make it not MUI?

 

[emjlr3]

Wouldn't that make it not MUI?

the groups use is instant, so a new thread would never be started before the current one is complete

Good and all, but that means I need to limit the number of units in the group when I actually group them.

leave the current exitwhen, and add the other under it
removes your redundant code, and makes it looks nicer

Range has been done, units are already doing that, it just has one level.

i dont see a

private function GetRange takes integer lvl returns real
    return lvl*400.
endfunction

for range or units

 

[Prometheus]

Yeah, I hadn't added the new code. Updated with everything.

 

[emjlr3]

just use groupenumunitsinrange

no need to count them

and never return true from your condition function - otherwise it adds units to the group, which you do not want

you seem to not use your local varaibles all the time, like caster

no need to remove units from the group if you never add them, and how you do it not does not work

 

[Prometheus]

I do need to count them, otherwise I'd have to use some other method inside the function to count it. The grouping method is a native. Added revised version.

 

[emjlr3]

use a global integer to count them

***
is is seriously not a big deal - but just so you know - again, no need to null dummy just to reuse it

you dont need to remove units from the group, because they are only added if the condition function returns true for the enumunit, which it never does

and i though the groupenum u were using was a non-native, but its native so its fine

is loopa ever used?

null dummy at the end of the if/endif, as opposed to at the end of the condition function - since its only used in there

only other way to really improve this would be to use global variables for stuff like caster, owner of caster, lvl, etc. - which you use over and over each cast - so you would just set them once and forget about it for the rest of the spell

 

[Prometheus]

If I understood correctly... see code.

 

[emjlr3]

while you are at it have a global for owner of caster

I think you should comment somewhere that in settingbaseenum and range, you are actually creating the linear function for each argument, where 400 range would mena 400 range per level

rest looks fine

also, some doc. on how to edit damage/etc. on the dummy missiles would be nice

 

[Prometheus]

I think I overdid it.