[vJASS] Is this script correct?

[Geshishouhu]

Hey all. I am new to vJass and am trying to make a spell by using Nes's DDS. The effect is to absorb a certain amount of damage and then reflect the damage to enemies.
So, I would like to know if the following scope is correct? Do I need to use "create" and "onDestroy" methods in the struct? How about an initializer for the scope? Thanks.

scope dddd
    private function groupfilter takes nothing returns boolean
        return IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(GetTriggerUnit())) and (GetWidgetLife(GetFilterUnit()) > 0.405)
    endfunction
    struct DamageDetection extends array
        private static real dam=0
        private static method onDamage takes nothing returns nothing
            local group g=CreateGroup()
            local unit u
            local real x
            local real y
            if (archetype == Archetype.PHYSICAL) and target==gg_unit_Obla_0016 then  
                set x=GetUnitX(target)
                set y=GetUnitY(target)
                if dam<=1000 then
                    set dam=damage+dam
                    call DisplayTimedTextToPlayer(GetLocalPlayer(),0,0,60,"Damage received"+R2S(dam))
                else
                    call DisplayTimedTextToPlayer(GetLocalPlayer(),0,0,60,"Damage exceeds 1000: "+R2S(dam))
                    call GroupEnumUnitsInRange(g,x,y,800.,function groupfilter)
                    loop
                        set u=FirstOfGroup(g)
                        exitwhen u==null
                        call GroupRemoveUnit(g,u)
                        call UnitDamageTarget(target,u,dam,true,false,ATTACK_TYPE_CHAOS,DAMAGE_TYPE_DEMOLITION,null)
                    endloop
                    call DestroyGroup(g)    
                    set dam=0
                    call DisplayTimedTextToPlayer(GetLocalPlayer(),0,0,60,"Damage returns 0")
                endif
            endif
            set g=null
        endmethod
        implement DDS
    endstruct
endscope

 

[TriggerHappy]

So, I would like to know if the following scope is correct?

You can't just test it? If it works, it works..

scope DDSTest

    globals
		private group UnitGroup = CreateGroup()
		private player LocalPlayer = GetLocalPlayer() // EDIT: Place this in an initializer function
	endglobals
	
	native UnitAlive takes unit id returns boolean
	
    private function groupfilter takes nothing returns boolean
        return IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(GetTriggerUnit())) and (UnitAlive(GetFilterUnit()))
    endfunction
	
    struct DamageDetection extends array
        private static real dam=0
		
        private static method onDamage takes nothing returns nothing
            local unit u
            local real x
            local real y
			
            if (archetype == Archetype.PHYSICAL) and (target == gg_unit_Obla_0016) then  
                set x=GetUnitX(target)
                set y=GetUnitY(target)
				
                if dam<=1000 then
                    set dam=damage+dam
                    call DisplayTimedTextToPlayer(LocalPlayer,0,0,60," Damage received"+R2S(dam))
                else
                    call DisplayTimedTextToPlayer(LocalPlayer,0,0,60, "Damage exceeds 1000: "+R2S(dam))
                    call GroupEnumUnitsInRange(UnitGroup,x, y, 800., function groupfilter)
                    loop
                        set u=FirstOfGroup(UnitGroup)
                        exitwhen u==null
                        call GroupRemoveUnit(UnitGroup,u)
                        call UnitDamageTarget(target,u,dam,true,false,ATTACK_TYPE_CHAOS,DAMAGE_TYPE_DEMOLITION,null)
                    endloop  
                    set dam=0
                    call DisplayTimedTextToPlayer(LocalPlayer,0,0,60,"Damage returns 0")
                endif
				
            endif
        endmethod
		
        implement DDS
    endstruct
	
endscope

 

[Geshishouhu]

You can't just test it? If it works, it works..

scope DDSTest

    globals
		private group UnitGroup = CreateGroup()
		private player LocalPlayer = GetLocalPlayer()
	endglobals
	
	native UnitAlive takes unit id returns boolean
	
    private function groupfilter takes nothing returns boolean
        return IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(GetTriggerUnit())) and (UnitAlive(GetFilterUnit()))
    endfunction
	
    struct DamageDetection extends array
        private static real dam=0
		
        private static method onDamage takes nothing returns nothing
            local unit u
            local real x
            local real y
			
            if (archetype == Archetype.PHYSICAL) and (target == gg_unit_Obla_0016) then  
                set x=GetUnitX(target)
                set y=GetUnitY(target)
				
                if dam<=1000 then
                    set dam=damage+dam
                    call DisplayTimedTextToPlayer(LocalPlayer,0,0,60," Damage received"+R2S(dam))
                else
                    call DisplayTimedTextToPlayer(LocalPlayer,0,0,60, "Damage exceeds 1000: "+R2S(dam))
                    call GroupEnumUnitsInRange(UnitGroup,x, y, 800., function groupfilter)
                    loop
                        set u=FirstOfGroup(UnitGroup)
                        exitwhen u==null
                        call GroupRemoveUnit(UnitGroup,u)
                        call UnitDamageTarget(target,u,dam,true,false,ATTACK_TYPE_CHAOS,DAMAGE_TYPE_DEMOLITION,null)
                    endloop  
                    set dam=0
                    call DisplayTimedTextToPlayer(LocalPlayer,0,0,60,"Damage returns 0")
                endif
				
            endif
        endmethod
		
        implement DDS
    endstruct
	
endscope

It does work, but I want to know if there is any leak or efficiency issue. I just start learning vJass, so I wanna keep everything on the right direction.
Thanks for the improved script. +Rep

 

[BPower]

So far this has no leaks. Personally I wouldn't use a group filter in your case and rather just use null as boolexpr.
GetOwningPlayer(GetTriggerUnit()) I am assuming that GetTriggerUnit() will return the damage source unit,
aswell as DDS.source or just source is the Damage source unit (dunno the correct variable name), because target is the target unit.

This way you pass unit u and unit source directly into the filter function, which is useful, because you don't have to call GetFilterUnit(), GetTriggerUnit() anymore.

If GetTriggerUnit() returns the target instead of the source. You have to pass target instead of source into the Filter if (FilterUnits(u, target)) then

Also you could use bj_lastCreatedGroup instead of private group UnitGroup, but that's totally up to you.

scope DDSTest

    globals
        private group UnitGroup = CreateGroup()
        private player LocalPlayer = GetLocalPlayer()
    endglobals
   
    native UnitAlive takes unit id returns boolean
   
    private function FilterUnits takes unit u, unit n nothing returns boolean
        return (IsUnitEnemy(u, GetOwningPlayer(n)) and (UnitAlive(u))
    endfunction
   
    struct DamageDetection extends array
        private static real dam=0
       
        private static method onDamage takes nothing returns nothing
            local unit u
            local real x
            local real y
           
            if (archetype == Archetype.PHYSICAL) and (target == gg_unit_Obla_0016) then  
                set x=GetUnitX(target)
                set y=GetUnitY(target)
               
                if dam<=1000 then
                    set dam=damage+dam
                    call DisplayTimedTextToPlayer(LocalPlayer,0,0,60," Damage received"+R2S(dam))
                else
                    call DisplayTimedTextToPlayer(LocalPlayer,0,0,60, "Damage exceeds 1000: "+R2S(dam))
                    call GroupEnumUnitsInRange(UnitGroup,x, y, 800., null)
                    loop
                        set u=FirstOfGroup(UnitGroup)
                        exitwhen u==null
                        call GroupRemoveUnit(UnitGroup,u)
                        if (FilterUnits(u, source)) then//Not sure here, maybe you have to pass target instead
                            call UnitDamageTarget(target,u,dam,true,false,ATTACK_TYPE_CHAOS,DAMAGE_TYPE_DEMOLITION,null)
                        endif
                    endloop  
                    set dam=0
                    call DisplayTimedTextToPlayer(LocalPlayer,0,0,60,"Damage returns 0")
                endif
               
            endif
        endmethod
       
        implement DDS
    endstruct

 

[Geshishouhu]

So far this has no leaks. Personally I wouldn't use a group filter in your case and rather just use null as boolexpr.
GetOwningPlayer(GetTriggerUnit()) I am assuming that GetTriggerUnit() will return the damage source unit,
aswell as DDS.source or just source is the Damage source unit (dunno the correct variable name), because target is the target unit.

This way you pass unit u and unit source directly into the filter function, which is useful, because you don't have to call GetFilterUnit(), GetTriggerUnit() anymore.

If GetTriggerUnit() returns the target instead of the source. You have to pass target instead of source into the Filter if (FilterUnits(u, target)) then

Also you could use bj_lastCreatedGroup instead of private group UnitGroup, but that's totally up to you.

scope DDSTest

    globals
        private group UnitGroup = CreateGroup()
        private player LocalPlayer = GetLocalPlayer()
    endglobals
   
    native UnitAlive takes unit id returns boolean
   
    private function FilterUnits takes unit u, unit n nothing returns boolean
        return (IsUnitEnemy(u, GetOwningPlayer(n)) and (UnitAlive(u))
    endfunction
   
    struct DamageDetection extends array
        private static real dam=0
       
        private static method onDamage takes nothing returns nothing
            local unit u
            local real x
            local real y
           
            if (archetype == Archetype.PHYSICAL) and (target == gg_unit_Obla_0016) then  
                set x=GetUnitX(target)
                set y=GetUnitY(target)
               
                if dam<=1000 then
                    set dam=damage+dam
                    call DisplayTimedTextToPlayer(LocalPlayer,0,0,60," Damage received"+R2S(dam))
                else
                    call DisplayTimedTextToPlayer(LocalPlayer,0,0,60, "Damage exceeds 1000: "+R2S(dam))
                    call GroupEnumUnitsInRange(UnitGroup,x, y, 800., null)
                    loop
                        set u=FirstOfGroup(UnitGroup)
                        exitwhen u==null
                        call GroupRemoveUnit(UnitGroup,u)
                        if (FilterUnits(u, source)) then//Not sure here, maybe you have to pass target instead
                            call UnitDamageTarget(target,u,dam,true,false,ATTACK_TYPE_CHAOS,DAMAGE_TYPE_DEMOLITION,null)
                        endif
                    endloop  
                    set dam=0
                    call DisplayTimedTextToPlayer(LocalPlayer,0,0,60,"Damage returns 0")
                endif
               
            endif
        endmethod
       
        implement DDS
    endstruct

Thanks for the reply. +Rep

 

[deathismyfriend]

You should stick with private group.

 

[BPower]

You should stick with private group.

Aslong as you use FirstOfGroup iteration instead of ForGroup and only use one Group within the loop, I don't see a problem. bj_lastCreatedGroup is a global group from the blizzard.j and works totally fine. Plus you don't have to create a group for every individual resource. (Ok it's just 32bits per group).

If you need 2 groups within the loop just create one like you did above.

@deathismyfriend: Name one argument of using a private group over bj_lastCreatedGroup.

 

[deathismyfriend]

Some Functions use bj_lastCreatedGroup.
Also when using vJass you can easily make a private group.

 

[chobibo]

I agree with death, a thread interrupt might occur.

 

[BPower]

Some Functions use bj_lastCreatedGroup.

Can you specify them?

a thread interrupt might occur.

mmh, I never had problems like this. Does that mean you know it happens or you think it could happen, but you are not sure?

 

[deathismyfriend]

I don't code much on WC3 anymore so I'm not going to look for them. I know it happens and has happened before. Create 2 triggers one with dying unit and one with end cinematic. In the trigger with end cinematic pressed put create a unit group with all unit in the map. Then after that kill a unit. Then do action to kill each unit in the unit group or display number of units.
In the unit dies trigger Create a new unit group of units within 100 range. Then set all those units to invulnerable.

Make sure to place a good amount of units to see the outcome is not what you want.
The group will be overwritten and only the units in 100 range will be affected.

 

[chobibo]

Does that mean you know it happens or you think it could happen, but you are not sure?

Try testing it, much better than me telling or assuring you it does.

 

[BPower]

Tested: Result only 1 unit dies instead of all on end cinematic. 'Avul' is added to units in range 100. Both are using bj_lastCreatedGroup.
When replacing one bj_lastCreatedGroup with a private group ENU all units die, so it does if you replace both bj_lastCreatedGroups.

Thank you didn't know that.

Test

scope test

    globals
        private rect WORLD
        private group ENU = CreateGroup()
    endglobals
    
    native UnitAlive takes unit id returns boolean
    
private struct proof extends array 

    private static method kill takes nothing returns boolean
        local unit u 
        call GroupEnumUnitsInRect(bj_lastCreatedGroup, WORLD, null)
        loop 
            set u = FirstOfGroup(bj_lastCreatedGroup)
            exitwhen u == null
            call GroupRemoveUnit(bj_lastCreatedGroup, u)
            call KillUnit(u)
            exitwhen not UnitAlive(u)
        endloop
        loop 
            set u = FirstOfGroup(bj_lastCreatedGroup)
            exitwhen u == null
            call GroupRemoveUnit(bj_lastCreatedGroup, u)
            call KillUnit(u)
        endloop
        return false
    endmethod
    
    private static method invul takes nothing returns boolean
        local unit u = GetTriggerUnit()
        local real x = GetUnitX(u)
        local real y = GetUnitY(u)
        call GroupEnumUnitsInRange(ENU, x, y, 100 , null)//Replace ENU with bj_lastCreatedGroup and method kill will fail
        loop 
            set u = FirstOfGroup(ENU)//Replace ENU with bj_lastCreatedGroup and method kill will fail
            exitwhen u == null
            call GroupRemoveUnit(ENU, u)//Replace ENU with bj_lastCreatedGroup and method kill will fail
            call UnitAddAbility(u, 'Avul')
        endloop
        set u = null
        return false
    endmethod
        
    private static method spawn takes nothing returns nothing
        local integer i = 0
        loop
            exitwhen i == 100
            call CreateUnit(Player(0), 'hfoo', 0,0,0)
            set i = i + 1
        endloop
    endmethod

    private static method onInit takes nothing returns nothing
        local trigger t = CreateTrigger()
        set WORLD = GetWorldBounds()
        call TriggerRegisterPlayerEvent(t, Player(0), EVENT_PLAYER_END_CINEMATIC)
        call TriggerAddCondition(t, Condition(function thistype.kill))
        set t= CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_DEATH)
        call TriggerAddCondition(t, Condition(function thistype.invul))
        call thistype.spawn()
    endmethod
endstruct

endscope

 

[deathismyfriend]

No problem this is a helping site.