Custom script trigger problem (Not sure if it should be in JASS forum)

[I_RULE_YOU]

I'm trying to have an aura that slowly draws in enemy heroes, and when I try to enable the trigger I get a popup window highlighting:

set x = GetLocationX(udg_Grudge5)

And telling me it expected a variable name. Heres the complete trigger below:

Grudge Aura
    Events
        Time - Every 1.00 seconds of game time
    Conditions
    Actions
        Custom script:   local real x
        Custom script:   local real y
        Set GrudgeGroup = (Units in ALLREGION matching (((Level of Grudge Aura  for (Matching unit)) Greater than 0) and (((Matching unit) is alive) Equal to True)))
        Unit Group - Pick every unit in GrudgeGroup and do (Actions)
            Loop - Actions
                Set Grudge1 = (Picked unit)
                Set Grudge2 = (Position of Grudge1)
                Set Grudge3 = (Units within 900.00 of Grudge2 matching ((((Matching unit) is alive) Equal to True) and ((((Matching unit) is A structure) Equal to False) and ((((Matching unit) belongs to an enemy of (Owner of Grudge1)) Equal to True) and (((Matching unit) is A Hero) Equal
                Unit Group - Pick every unit in Grudge3 and do (Actions)
                    Loop - Actions
                        Set Grudge4 = (Position of (Picked unit))
                        Set Grudge5 = (Grudge4 offset by (10.00 x (Real((Level of Grudge Aura  for Grudge1)))) towards (Angle from Grudge4 to Grudge2) degrees)
                        Custom script:   set x = GetLocationX(udg_Grudge5)
                        Custom script:   set y = GetLocationY(udg_Grudge5)
                        Custom script:   call SetUnitX(GetEnumUnit(),x)
                        Custom script:   call SetUnitY(GetEnumUnit(),y)
                        Custom script:   call RemoveLocation(udg_Grudge4)
                        Custom script:   call RemoveLocation(udg_Grudge5)
                        Custom script:   call DestroyGroup(udg_Grudge4)
                        Custom script:   call RemoveLocation(udg_Grudge1)

Grudge AuraB
    Events
        Map initialization
    Conditions
    Actions
        Set ALLREGION = (Playable map area)

This is probably a simple problem, but it has dumbfounded me. What should I do?

 

[MasterRofl]

Go grab the NewGen Editor even if you don't use JASS; WE's checker sucks.

EDIT:

I think it's because when you use "Pick Every Unit in Group", another function is created. Therefore, "x" doesn't exist in the new function as a local.

E.g., something like this would be generated by the GUI:

function Callback takes nothing returns nothing
    set udg_Grudge4 = GetUnitLoc(GetPickedUnit())
    set udg_Grudge5 = (Grudge4 offset by (10.00 x (Real((Level of Grudge Aura  for Grudge1)))) towards (Angle from Grudge4 to Grudge2) degrees) //too lazy to convert this
    set x = GetLocationX(udg_Grudge5)
    set y = GetLocationY(udg_Grudge5)
    call SetUnitX(GetEnumUnit(),x)
    call SetUnitY(GetEnumUnit(),y)
    call RemoveLocation(udg_Grudge4)
    call RemoveLocation(udg_Grudge5)
    call DestroyGroup(udg_Grudge4)
    call RemoveLocation(udg_Grudge1)
endfunction

function Actions takes nothing returns nothing
    ...
    ForGroupBJ(udg_Grudge3, function Callback)
    ...
endfunction

and "x" isn't declared in the new function. The same applies to "y".

What you can do is change

set x = GetLocationX(udg_Grudge5)
set y = GetLocationY(udg_Grudge5)

to

local real x = GetLocationX(udg_Grudge5)
local real y = GetLocationY(udg_Grudge5)

and remove

Custom script:   local real x
Custom script:   local real y

at the top of your actions.

 

[I_RULE_YOU]

Ok, so how would I fix this?

 

[Tom_Kazansky]

Custom script:   local real x
Custom script:   local real y

These lines should be in the Loop - Actions of the Unit Group - Pick.

Grudge Aura
    Events
        Time - Every 1.00 seconds of game time
    Conditions
    Actions
        Set GrudgeGroup = (Units in ALLREGION matching (((Level of Grudge Aura  for (Matching unit)) Greater than 0) and (((Matching unit) is alive) Equal to True)))
        Unit Group - Pick every unit in GrudgeGroup and do (Actions)
            Loop - Actions
                Custom script:   local real x
                Custom script:   local real y
                Set Grudge1 = (Picked unit)
                Set Grudge2 = (Position of Grudge1)
                Set Grudge3 = (Units within 900.00 of Grudge2 matching ((((Matching unit) is alive) Equal to True) and ((((Matching unit) is A structure) Equal to False) and ((((Matching unit) belongs to an enemy of (Owner of Grudge1)) Equal to True) and (((Matching unit) is A Hero) Equal
                Unit Group - Pick every unit in Grudge3 and do (Actions)
                    Loop - Actions
                        Set Grudge4 = (Position of (Picked unit))
                        Set Grudge5 = (Grudge4 offset by (10.00 x (Real((Level of Grudge Aura  for Grudge1)))) towards (Angle from Grudge4 to Grudge2) degrees)
                        Custom script:   set x = GetLocationX(udg_Grudge5)
                        Custom script:   set y = GetLocationY(udg_Grudge5)
                        Custom script:   call SetUnitX(GetEnumUnit(),x)
                        Custom script:   call SetUnitY(GetEnumUnit(),y)
                        Custom script:   call RemoveLocation(udg_Grudge4)
                        Custom script:   call RemoveLocation(udg_Grudge5)
                        Custom script:   call DestroyGroup(udg_Grudge4)
                        Custom script:   call RemoveLocation(udg_Grudge1)

The Loop - Actions of the Unit Group - Pick... is a different function, so you should declare the local vars in it.

EDIT: a bit slower than MasterRofl

 

[MasterRofl]

Oh and by the way, I don't believe there is a point in removing/destroying global locations/groups.

 

[I_RULE_YOU]

I'm not sure what you mean, can you show what the trigger SHOULD look like?

 

[MasterRofl]

Grudge Aura
    Events
        Time - Every 1.00 seconds of game time
    Conditions
    Actions
        Set GrudgeGroup = (Units in ALLREGION matching (((Level of Grudge Aura  for (Matching unit)) Greater than 0) and (((Matching unit) is alive) Equal to True)))
        Unit Group - Pick every unit in GrudgeGroup and do (Actions)
            Loop - Actions
                Set Grudge1 = (Picked unit)
                Set Grudge2 = (Position of Grudge1)
                Set Grudge3 = (Units within 900.00 of Grudge2 matching ((((Matching unit) is alive) Equal to True) and ((((Matching unit) is A structure) Equal to False) and ((((Matching unit) belongs to an enemy of (Owner of Grudge1)) Equal to True) and (((Matching unit) is A Hero) Equal
                Unit Group - Pick every unit in Grudge3 and do (Actions)
                    Loop - Actions
                        Set Grudge4 = (Position of (Picked unit))
                        Set Grudge5 = (Grudge4 offset by (10.00 x (Real((Level of Grudge Aura  for Grudge1)))) towards (Angle from Grudge4 to Grudge2) degrees)
                        Custom script:   local real x = GetLocationX(udg_Grudge5)
                        Custom script:   local real y = GetLocationY(udg_Grudge5)
                        Custom script:   call SetUnitX(GetEnumUnit(),x)
                        Custom script:   call SetUnitY(GetEnumUnit(),y)

 

[I_RULE_YOU]

Did it just like that, and now it highlights:

local real x = GetLocationX(udg_Grudge5)

And it's telling me it expects a code statement.

 

[Tom_Kazansky]

@MasterRofl, variables must be declared at the top (of the function - top of the Loop - Actions)

@I_RULE_YOU, oh sorry, I didn't notice there is another loop

Grudge Aura
    Events
        Time - Every 1.00 seconds of game time
    Conditions
    Actions
        Set GrudgeGroup = (Units in ALLREGION matching (((Level of Grudge Aura  for (Matching unit)) Greater than 0) and (((Matching unit) is alive) Equal to True)))
        Unit Group - Pick every unit in GrudgeGroup and do (Actions)
            Loop - Actions
                Set Grudge1 = (Picked unit)
                Set Grudge2 = (Position of Grudge1)
                Set Grudge3 = (Units within 900.00 of Grudge2 matching ((((Matching unit) is alive) Equal to True) and ((((Matching unit) is A structure) Equal to False) and ((((Matching unit) belongs to an enemy of (Owner of Grudge1)) Equal to True) and (((Matching unit) is A Hero) Equal
                Unit Group - Pick every unit in Grudge3 and do (Actions)
                    Loop - Actions
                        Custom script:   local real x  //they should be here
                        Custom script:   local real y
                        Set Grudge4 = (Position of (Picked unit))
                        Set Grudge5 = (Grudge4 offset by (10.00 x (Real((Level of Grudge Aura  for Grudge1)))) towards (Angle from Grudge4 to Grudge2) degrees)
                        Custom script:   set x = GetLocationX(udg_Grudge5)
                        Custom script:   set y = GetLocationY(udg_Grudge5)
                        Custom script:   call SetUnitX(GetEnumUnit(),x)
                        Custom script:   call SetUnitY(GetEnumUnit(),y)
                        Custom script:   call RemoveLocation(udg_Grudge4)
                        Custom script:   call RemoveLocation(udg_Grudge5)
                        Custom script:   call DestroyGroup(udg_Grudge4)
                        Custom script:   call RemoveLocation(udg_Grudge1)

 

[Choppa]

Oh and by the way, I don't believe there is a point in removing/destroying global locations/groups.

Lol, memory leaks aren't just in warcraft, they're everywhere and they do exist.

Make a .bat file and make it open it's self e.g

start thisfile.bat

That's like a memory leak because you're not closing it down again, so it will keep opening untill you run out of memory.

 

[I_RULE_YOU]

@Tom_Kazansky

Did that, stops me and highlights:

call RemoveLocation(udg_Grudge1)

Telling me "Invalid argument type (location)"

 

[saw792]

Because Grudge1 is a unit.

 

[I_RULE_YOU]

So should I replace it with RemoveUnit or something, or just get rid of that line of code?

P.S: Should I try posting this in JASS help forum?

 

[I_RULE_YOU]

*bump*

 

[Choppa]

Remove these

Custom script: call DestroyGroup(udg_Grudge4)
Custom script: call RemoveLocation(udg_Grudge1

 

[Xorifelse]

> Oh and by the way, I don't believe there is a point in removing/destroying global locations/groups.

Globals that extend a handle can create leaks, aka Locations.
There is no real point of destroying a global group yes, however a call GroupClear() can sometimes avoid bugs not to mention it clears the memory a bit for the time not using the global.

Global location types do leak, since its not the global reference you destroy but the object that the global is referring to.
Giving that global a new object to refer to, without destroying the old object means it has to find a new space in the memory for a new object.
Meaning the old reference to the object is gone, and the old object still exists.

 

[I_RULE_YOU]

Awesome, it works. Thank you very much + rep to all who contributed.