Frost Wyrm v1.01

[Moderator]

11:20, 24st Jun 2015
IcemanBo: Read reply in thread.

 

[Chaosy]

Instead of using
variable[this] = x

why not use
this.variable = x

It looks a little better, not really sure if it's more efficient though.

 

[IcemanBo]

Implement native IsUnitAlive or also check for UnitTypeId == 0
in combination with your current check for unit type if a unit is dead.

You use the Tree Snippet by BPower, you can take more usage of it:

private function Tree_Kill takes nothing returns boolean
    local destructable dummy = GetFilterDestructable()
    if IsDestructableTree(dummy) then
        call KillDestructable(dummy)
    endif
    set dummy = null
    return false
endfunction

->

private function Tree_Kill takes nothing returns nothing
    call KillTree(GetFilterDestructable())
endfunction

For me the coding looks a bit weird, the use of static arrays in combination with structs like this.
Why not member of an instance?

static group ignoreGroup -> how does this work MUI? All catsers will use the same group.

After a FoG loop you don't need to null the local unit variable. It is already.
(btw players also don't need to be nulled necessarily, they are never removed/destroyed from game)

onBreath you always re-assign your static variables.
I don't quite understand it. You will leak on multiple assignment.
(onCast as well.)

 

[kawas11]

Ok, so. In: http://www.hiveworkshop.com/forums/spells-569/rain-cold-arrows-v1-04-a-266632/

BPower said that using just static unit groups, rects without arrays is enough.

Ok so now i know that i don't need to null player.

"onBreath you always re-assign your static variables.
I don't quite understand it. You will leak on multiple assignment.
( onCast as well.) "

What do you mean here? What should i do with it?

And about tree kill function:

private function Tree_Kill takes nothing returns nothing
    call KillTree(GetFilterDestructable())
endfunction

I think i can't set it to

returns nothing

it displays error because of

call EnumDestructablesInRect(treeRect[this],function Tree_Kill,null)

 

[IcemanBo]

If you need a group only for temp enumerations, then yes, one static/global group is enough.
If you need a group to keep track of "ignored" units for each instance, then you can't use one global group,
as you can't differ anymore which unit belongs to which instance.

Yes, I just wrote it by hand, you're right. The filter needs to return boolean.

What do you mean here? What should i do with it?

Well, you need to find a work around. If you overwrite an already assigned group variable, you will leak.
Try to find a solution, or also feel welcome to post in forum for questions. I hope you understand the issue with the re-assignment.

 

[IcemanBo]

Hm, there still exists this issue with groups.

Your static enum group (that is only used for temp enumeration),
can be used over and over again. No sense in re-assigning and destroying it.

Other point can be mentioned here, too:
If you can't ensure the group is destroyed before you re-assign the group variable,
then you have a potential leak, as when you overwrite the old group with a
CreateGroup(), you will have a group leak!
I advice you to http://www.hiveworkshop.com/forums/trigger-gui-editor-tutorials-279/memory-leaks-263410/#post2661133 to get a general idea, if there are still something unclear.

call FrostBreath.onBreath
^This function is called potentialy more often, so the group[this] may be also overwritten,
before it was destroyed properly, and cause malfunctions.

Your treeRect may also get overwritten, as this will run multiple times:

static if DESTROY_TREE then
    set treeRect = Rect(0, 0, 0, 0)
endif

if breathCCD[this] == breathCD[this] then
->
if breathCCD[this] >= breathCD[this] then

Your check for unit is dead still should be changed.

When working much with group creation/destruction, then it
might be a good idea to consiger to use GroupUtils to recycle groups.