[vJASS] suggestions

[Chaosy]

Hello. Today I made a vjass spell with a struct. (omg!)

The spell works fine, but I want tips/suggestions on how improve my spell code wise.

There are two potential leaks that I am aware of, the special effect and the unitgroups are not cleared. Mainly because I am unsure if they still leak since I don't use the BJ version of them.

Note: a few config variables are not used yet, no need to point that out.

The ability is supposed to make the caster travel to a certain location. Enemies who gets hit while traveling gets knocked backwards and at the end of the path every unit gets knocked up.

globals
    List l
    unit temp
    
    real knockbackDamage                = 50
    real knockupDamage                  = 75
    constant real strModifier           = 0.3
    constant real knockbackTime         = .75
    constant real knockbackDistance     = 150
    constant real knockupHeight         = 300
    constant real knockupSpeed          = 5
    constant real travelSpeed           = 6
    constant real travelSpeedModifier   = 0.3
    constant trigger knockupTrigger     = gg_trg_KU_prepare 
    constant trigger knockbackTrigger   = gg_trg_Knockback_2D
    constant damagetype damageType      = DAMAGE_TYPE_FORCE
    constant string knockupEffect       = "Abilities\\Spells\\Human\\ThunderClap\\ThunderClapCaster.mdl"
    constant string travelEffect        = "Objects\\Spawnmodels\\Undead\\ImpaleTargetDust\\ImpaleTargetDust.mdl"
    constant string knockupAnimation    = "Attack"
endglobals

function Knockup takes nothing returns nothing
    if IsUnitInGroup(GetEnumUnit(), udg_KUS_group) == false then
        set udg_KUS_target    = GetEnumUnit()
        set udg_KUS_effect    = knockupEffect
        set udg_KUS_height    = knockupHeight
        set udg_KUS_speed     = knockupSpeed
        set udg_KUS_animation = knockupAnimation
        call ConditionalTriggerExecute(knockupTrigger)
    endif
endfunction

function Knockback takes nothing returns nothing
    if GetUnitAbilityLevelSwapped('Aloc', GetEnumUnit()) == 0  then
        set udg_CenterPoint         = GetUnitLoc(temp)
        set udg_TargetPoint         = GetUnitLoc(GetEnumUnit())
        set udg_Knockback2DAngle    = AngleBetweenPoints(udg_CenterPoint, udg_TargetPoint)
        set udg_Knockback2DTime     = knockbackTime
        set udg_Knockback2DDistance = knockbackDistance
        set udg_Knockback2DUnit     = GetEnumUnit()
        call RemoveLocation(udg_CenterPoint)
        call RemoveLocation(udg_TargetPoint)
        call TriggerExecute(knockbackTrigger)
    endif
endfunction


function EnemyFilter takes nothing returns boolean
    return IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(temp)) == true
endfunction

struct Underground

    unit caster
    location targetPos
    real speed = travelSpeed    
    
    public static method create takes unit u, location targetLoc returns thistype
        local thistype this = thistype.allocate()
        set caster = u
        set targetPos = targetLoc
        call l.Add(this)
        call SetUnitInvulnerable(u, true)
        call SetUnitVertexColor(u, 100, 100, 100, 0)
        call SetUnitPathing(u, false)
        return this
    endmethod
    
    public method Update takes nothing returns nothing
        local location loc = GetUnitLoc(caster)
        local real angle = AngleBetweenPoints(loc, targetPos)
        local real xpos = GetLocationX(loc) + speed * Cos(angle * bj_DEGTORAD)
        local real ypos = GetLocationY(loc) + speed * Sin(angle * bj_DEGTORAD)
        
        set speed = speed + travelSpeedModifier
        call SetUnitPosition(caster, xpos, ypos)
        set loc = Location(xpos, ypos)
        set temp = caster
        call ForGroupBJ(GetUnitsInRangeOfLocMatching(100, loc, Condition(function EnemyFilter)), function Knockback)
        call AddSpecialEffectLoc(travelEffect, loc)
        
        if DistanceBetweenPoints(loc, targetPos) < 50 then
            set temp = caster
            call ForGroup(GetUnitsInRangeOfLocMatching(200, loc, Condition(function EnemyFilter)), function Knockup)
            call l.Remove(this)
            call SetUnitVertexColor(caster, 255, 255, 255, 255)
            call SetUnitPathing(caster, true)
            call this.deallocate()
        endif
        call RemoveLocation(loc)
    endmethod
endstruct

 

[chobibo]

And a Lazy guy replied:

A. In method Update, how do you reference the struct instance(s) to work on? It's currently missing that at this point, that is if I still remember correctly how to use structs.

B. I suggest using coordinates (x,y) in place of locations since locations have overhead (constructors and destructors) and this is a periodic (probably high frequency?) function

location targetPos
->
struct Underground
...
real PosX
real PosY
...
endstruct

local location loc = GetUnitLoc(caster)
local real angle = AngleBetweenPoints(loc, targetPos)
->
local real x=GetUnitX(caster)
local real y=GetUnitY(caster)
local real angle=Atan2(this.PosY-y,this.PosX-x)

local real xpos = GetLocationX(loc) + speed * Cos(angle * bj_DEGTORAD)
local real ypos = GetLocationY(loc) + speed * Sin(angle * bj_DEGTORAD)
->
set x=x+speed*Cos(angle)
set y=y+speed*Sin(angle)

if DistanceBetweenPoints(loc, targetPos) < 50 then
->
local real distance=SquareRoot((this.PosX-x)*(this.PosX-x)+(this.PosY-y)*(this.PosY-y))
...
if distance < 50 then

Or just use an approximate value to save processing time

if DistanceBetweenPoints(loc, targetPos) < 50 then
->
local real distance=(this.PosX-x)*(this.PosX-x)+(this.PosY-y)*(this.PosY-y)
...
if distance < 50*50 then

Here's how it looks after editing:

    public method Update takes nothing returns nothing
        //local location loc = GetUnitLoc(caster)
        //local real angle = AngleBetweenPoints(loc, targetPos)
        local real x=GetUnitX(caster)
        local real y=GetUnitY(caster)
        
        // Assuming that the moving angle is constant,
        // you can save processing time by trading off
        // memory to store the computed value in the
        // struct:
        // this.Angle or this.Direction
        // IIRC, Trigonometric functions are heavy (requires
        // a good deal of processing time)
        local real angle=Atan2(this.PosY-y,this.PosX-x)
        
        // We're gonna use an additional variable for
        // the distance check. This is the approximate
        // value method.
    
        local real distance=(this.PosX-x)*(this.PosX-x)+(this.PosY-y)*(this.PosY-y)
        
        //local real xpos = GetLocationX(loc) + speed * Cos(angle * bj_DEGTORAD)
        //local real ypos = GetLocationY(loc) + speed * Sin(angle * bj_DEGTORAD)
        
        // Again, if this is a constant angle, then you
        // can save the cosine and sine values to the 
        // struct too:
        // this.Cos
        // this.Sin
        // IIRC, Trigonometric functions are heavy (requires
        // a good deal of processing time)
        set x=x+speed*Cos(angle)
        set y=y+speed*Sin(angle)
       
        set speed = speed + travelSpeedModifier
        
        //call SetUnitPosition(caster, xpos, ypos)
        
        call SetUnitPosition(caster,x,y)
        
        //set loc = Location(xpos, ypos)
        set temp = caster
        
        //call ForGroupBJ(GetUnitsInRangeOfLocMatching(100, loc, Condition(function EnemyFilter)), function Knockback)
        
        // TargetGroup is a global unit group, this is
        // done so as to save more processing time.
        call GroupEnumUnitsInRange(TempGroup,x,y,100,Condition(function EnemyFilter))
        call ForGroup(TempGroup, function Knockback)
        
        //call AddSpecialEffectLoc(travelEffect, loc)
        
        call AddSpecialEffect(travelEffect,x,y) //this leaks
        
        // you can however choose a model with a desired/appropriate
        // death animation for the knockback dust effect (or whatever
        // you need) and use this call method to display the effects
        // without leaking the handle (or using a timed cleanup method):
        // call DestroyEffect(AddSpecialEffect(travelEffect,x,y))
        
        //if DistanceBetweenPoints(loc, targetPos) < 50 then
        
        if distance < (50*50) then // using the approximate method
            set temp = caster
            
            //call ForGroup(GetUnitsInRangeOfLocMatching(200, loc, Condition(function EnemyFilter)), function Knockup)
            
            call GroupEnumUnitsInRange(TempGroup,x,y,200,Condition(function EnemyFilter))
            call ForGroup(TempGroup, function Knockup)
        
            call l.Remove(this)
            call SetUnitVertexColor(caster, 255, 255, 255, 255)
            call SetUnitPathing(caster, true)
            call this.deallocate()
        endif
        // This call is no longer required.
        //call RemoveLocation(loc)
    endmethod

I think it's better to store the moving direction rather than the target point coordinates, that is if it's constant.

Note: I haven't compiled this, so expect errors (I hope not too much lol)

 

[Chaosy]

@a
I call them from another trigger. Note how I use l.add(This), that is a dynamic array of mine which I loop through to keep track of the instances.

• update
  • Events
    • Time - Every 0.03 seconds of game time
  • Conditions
  • Actions
    • Custom script: local integer i = 0
    • Custom script: if l.size != 0 then
    • Custom script: loop
    • Custom script: exitwhen i >= l.size
    • Custom script: call Underground(l.data[i]).Update()
    • Custom script: set i = i + 1
    • Custom script: endloop
    • Custom script: endif

@b
Thanks, so overall my faults were:
1. replace the BJs' with the 'hidden' code and code it manually do the math even though those BJs' aren't exactly bad.

2. replace locations with x/y reals

3. do something odd with the unit groups that I fail to understand. Care to explain? also I get error for not find the group variables. Should I just declare them in the struct?

 

[chobibo]

1. replace the BJs' with the 'hidden' code and code it manually do the math even though those BJs' aren't exactly bad.

Not really, I did the required code change since the BJs you were using used locations as parameters and not real (I'm talking data type here, not real that is synonymous to fact) coordinates, not because I'm trying to be a dick or something.

3. do something odd with the unit groups that I fail to understand. Care to explain? also I get error for not find the group variables. Should I just declare them in the struct?

Ah, sorry about that, I messed it up. I mistakenly used two group variables, in any case, declare a global group variable TempGroup and immediately initialize it with a group object:

globals
 ...
 constant group TempGroup=CreateGroup()
 ...
endglobals

Alternatively:

struct
 ... 
 static group TempGroup=CreateGroup()
 ...
endstruct

But if you do that, be sure to reference it by (if memory serves me right) StructName.TempGroup, in this case Underground: Underground.TempGroup

I edited the code above to fix the 'TempGroup' and 'TargetGroup' variable issue

 

[Chaosy]

I was forced to make another change to the code *gasp*

You didn't set PosY and PosX to values, so they were both 0. But I fixed that so no worries.

This is the code that I am running now: seems to work fine just as before ;D

globals
    List l
    unit temp
	
    constant group tempGroup            = CreateGroup()
	
    real knockbackDamage                = 50
    real knockupDamage                  = 75
    constant real strModifier           = 0.3
    constant real knockbackTime         = .75
    constant real knockbackDistance     = 150
    constant real knockupHeight         = 300
    constant real knockupSpeed          = 5
    constant real travelSpeed           = 6
    constant real travelSpeedModifier   = 0.3
    constant damagetype damageType      = DAMAGE_TYPE_FORCE
    constant string knockupEffect       = "Abilities\\Spells\\Human\\ThunderClap\\ThunderClapCaster.mdl"
    constant string travelEffect        = "Objects\\Spawnmodels\\Undead\\ImpaleTargetDust\\ImpaleTargetDust.mdl"
    constant string knockupAnimation    = "Attack"
endglobals

function Knockup takes nothing returns nothing
    if IsUnitInGroup(GetEnumUnit(), udg_KUS_group) == false then
        set udg_KUS_target    = GetEnumUnit()
        set udg_KUS_effect    = knockupEffect
        set udg_KUS_height    = knockupHeight
        set udg_KUS_speed     = knockupSpeed
        set udg_KUS_animation = knockupAnimation
        call ConditionalTriggerExecute(gg_trg_KU_prepare )
    endif
endfunction

function Knockback takes nothing returns nothing
    if GetUnitAbilityLevelSwapped('Aloc', GetEnumUnit()) == 0  then
        set udg_CenterPoint         = GetUnitLoc(temp)
        set udg_TargetPoint         = GetUnitLoc(GetEnumUnit())
        set udg_Knockback2DAngle    = AngleBetweenPoints(udg_CenterPoint, udg_TargetPoint)
        set udg_Knockback2DTime     = knockbackTime
        set udg_Knockback2DDistance = knockbackDistance
        set udg_Knockback2DUnit     = GetEnumUnit()
        call RemoveLocation(udg_CenterPoint)
        call RemoveLocation(udg_TargetPoint)
        call TriggerExecute(gg_trg_Knockback_2D)
    endif
endfunction

function EnemyFilter takes nothing returns boolean
    return IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(temp)) == true
endfunction

struct Underground

    unit caster
    location targetPos
    real speed = travelSpeed    
    real posX
    real posY
	real angle
	effect tempEffect
	
    public static method create takes unit u, location targetLoc returns thistype
        local thistype this = thistype.allocate()
        set caster = u
        set targetPos = targetLoc
		set posX = GetLocationX(targetLoc)
		set posY = GetLocationY(targetLoc)
		set angle=Atan2(this.posY-GetUnitY(u),this.posX-GetUnitX(u))
		call SetUnitInvulnerable(u, true)
        call SetUnitVertexColor(u, 100, 100, 100, 0)
        call SetUnitPathing(u, false)
        call l.Add(this)
        return this
    endmethod
    
    public method Update takes nothing returns nothing
        local real x=GetUnitX(caster)
        local real y=GetUnitY(caster)
        local real distance=(this.posX-x)*(this.posX-x)+(this.posY-y)*(this.posY-y)
        
        set x=x+speed*Cos(angle)
        set y=y+speed*Sin(angle)
        set speed = speed + travelSpeedModifier
        call SetUnitPosition(caster,x,y)
        
        set temp = caster
        
        call GroupEnumUnitsInRange(tempGroup,x,y,100,Condition(function EnemyFilter))
        call ForGroup(tempGroup, function Knockback)
        
        
        set tempEffect = AddSpecialEffect(travelEffect,x,y)
		call DestroyEffect(tempEffect)
        
        
        if distance < (50*50) then
            set temp = caster
            
            call GroupEnumUnitsInRange(tempGroup,x,y,200,Condition(function EnemyFilter))
            call ForGroup(tempGroup, function Knockup)
        
            call l.Remove(this)
            call SetUnitVertexColor(caster, 255, 255, 255, 255)
            call SetUnitPathing(caster, true)
            call this.deallocate()
        endif
    endmethod
endstruct

 

[chobibo]

Oh yeah, forgot to edit the create method lol, sorry.

EDIT:

    unit caster
    location targetPos
    real speed = travelSpeed    
    real posX
    real posY
    public static method create takes unit u, location targetLoc returns thistype
        local thistype this = thistype.allocate()
        set caster = u
        set targetPos = targetLoc
        call l.Add(this)
        call SetUnitInvulnerable(u, true)
        call SetUnitVertexColor(u, 100, 100, 100, 0)
        call SetUnitPathing(u, false)
        set posX = GetLocationX(targetLoc)
        set posY = GetLocationY(targetLoc)
        return this
    endmethod

You can remove the location struct member already

    unit caster
    real speed = travelSpeed    
    real posX
    real posY

    public static method create takes unit u, location targetLoc returns thistype
        local thistype this = thistype.allocate()
        set caster = u
        call l.Add(this)
        call SetUnitInvulnerable(u, true)
        call SetUnitVertexColor(u, 100, 100, 100, 0)
        call SetUnitPathing(u, false)
        set posX = GetLocationX(targetLoc)
        set posY = GetLocationY(targetLoc)
        return this
    endmethod