Confinement v1.5

This bundle is marked as approved. It works and satisfies the submission rules.
Confinement v1.5:

Short Description:
Some sort of a Prison for enemy units.

Full Description:
Creates two magical fence that loops around a targeted point. Enemy units that is inside or caught within the fence's AoE cannot get out. If an enemy unit touches the fence it will deal damage as well.

|cffffcc00Level 1|r - Spell lasts 10 seconds, vortex damages 8 per touch.
|cffffcc00Level 2|r - Spell lasts 15 seconds, vortex damages 13 per touch.
|cffffcc00Level 3|r - Spell lasts 20 seconds, vortex damages 18 per touch.
|cffffcc00Level 4|r - Spell lasts 25 seconds, vortex damages 23 per touch.
|cffffcc00Level 5|r - Spell lasts 30 seconds, vortex damages 28 per touch.

JASS:
/*
===Confinement v1.5
===Made by: Mckill2009

REQUIRES and CREDITS:
- T32 by Jesus4Lyf
- BoundSentinel by Vexorian

HOW TO USE:
- Make a new trigger and convert to custom text via EDIT >>> CONVERT CUSTOM TEXT
- Copy ALL that is written here (overwrite the existing texts in the trigger)
- Copy the Dummy unit and the custom ability OR make your own
- You MUST input or change the correct raw ID's (see below)
- To view raw ID, press CTRL+D in the object editor
*/

library Confinement uses T32, BoundSentinel

globals
    //the SPELL_ID and ORDER_ID must match to each other because of channeling reasons
    private constant integer        SPELL_ID = 'A000' //blizzard
    private constant integer       FENCE1_ID = 'h002'
    private constant integer       FENCE2_ID = 'h003'
    private constant integer         AURA_ID = 'h000' 
    private constant integer        ORDER_ID = 852089 //blizzard
    private constant attacktype          ATT = ATTACK_TYPE_PIERCE
    private constant damagetype          DAM = DAMAGE_TYPE_DEATH
    private constant real         PRISON_AOE = 400 //this is the prison range
    private constant real            AOE_DAM = 80 //this is the damage range of the fence
    private constant real     ROTATION_SPEED = 0.1 //rotation in radians, so this should be low
    private real Pull
endglobals

private function UnitAlive takes unit u returns boolean
    return not IsUnitType(u, UNIT_TYPE_DEAD) and u!=null
endfunction

private function GetDuration takes integer i returns real
    return 5 + i * 5.
endfunction

private function GetDamage takes integer i returns real
    return 3 + i * 5.
endfunction

private struct Confinement
    unit caster
    unit aura
    unit fence1
    unit fence2
    real x
    real y
    real hurt
    real angle1
    real angle2
    real duration
    boolean isChanneling
    
    private method filterunits takes unit u returns boolean
        return IsUnitEnemy(u, GetOwningPlayer(.aura)) and UnitAlive(u) and not /* 
        */ IsUnitType(u, UNIT_TYPE_STRUCTURE) and not IsUnitType(u, UNIT_TYPE_MECHANICAL)
    endmethod
    
    private method damageThem takes unit u returns nothing
        local unit first
        call GroupEnumUnitsInRange(bj_lastCreatedGroup, GetUnitX(u), GetUnitY(u), AOE_DAM, null)
        loop
            set first = FirstOfGroup(bj_lastCreatedGroup)
            exitwhen first==null
            if .filterunits(first) then 
                call UnitDamageTarget(.aura, first, .hurt , false, false, ATT, DAM, null) 
            endif
            call GroupRemoveUnit(bj_lastCreatedGroup, first)
        endloop
    endmethod
    
    method periodic takes nothing returns nothing
        local real angle
        local real dist
        local real x1
        local real y1
        local real x2
        local real y2
        local unit u
        if UnitAlive(.caster) and .isChanneling then
            if .duration > 0 and GetUnitCurrentOrder(.caster)==ORDER_ID then
                set .angle1 = .angle1 + ROTATION_SPEED
                set .angle2 = .angle2 - ROTATION_SPEED
                set .duration = .duration - T32_PERIOD
                call SetUnitX(.fence1, .x + PRISON_AOE * Cos(.angle1))
                call SetUnitY(.fence1, .y + PRISON_AOE * Sin(.angle1))
                call SetUnitX(.fence2, .x + PRISON_AOE * Cos(.angle2))
                call SetUnitY(.fence2, .y + PRISON_AOE * Sin(.angle2))
                //===ConfineThem
                call GroupEnumUnitsInRange(bj_lastCreatedGroup, .x, .y, PRISON_AOE, null)
                loop
                    set u = FirstOfGroup(bj_lastCreatedGroup)
                    exitwhen u==null
                    if filterunits(u) then 
                        set x1 = GetUnitX(u)
                        set y1 = GetUnitY(u)
                        set x2 = x1-.x
                        set y2 = y1-.y
                        set angle = Atan2(y-y1, x-x1) 
                        set dist = (x2*x2) + (y2*y2)
                        if dist > Pull then
                            call SetUnitX(u, x1 + 30 * Cos(angle))
                            call SetUnitY(u, y1 + 30 * Sin(angle))
                        endif
                    endif
                    call GroupRemoveUnit(bj_lastCreatedGroup, u)
                endloop
                //===Fence1, DamageThem
                call damageThem(.fence1)
                //===Fence2, DamageThem
                call damageThem(.fence2)
            else
                set .isChanneling = false
                call IssueImmediateOrder(.caster, "stop")
            endif
        else
            call KillUnit(.aura)
            call KillUnit(.fence1)
            call KillUnit(.fence2)
            set .aura = null
            set .fence1 = null
            set .fence2 = null
            call .stopPeriodic()
            call .destroy()
        endif
    endmethod
    
    implement T32x
    
    static method cast takes nothing returns boolean
        local thistype this
        local integer level
        local player p
        if GetSpellAbilityId()==SPELL_ID then
            set this = create()
            set p = GetTriggerPlayer()
            set .caster = GetTriggerUnit()
            set level = GetUnitAbilityLevel(caster, SPELL_ID)
            set .x = GetSpellTargetX()
            set .y = GetSpellTargetY()
            set .aura = CreateUnit(p, AURA_ID, .x, .y, 0)
            set .fence1 = CreateUnit(p, FENCE1_ID, .x, .y, 0)
            set .fence2 = CreateUnit(p, FENCE2_ID, .x, .y, 0)
            set .duration = GetDuration(level)
            set .hurt = GetDamage(level)
            set .angle1 = 0
            set .angle2 = 0
            set .isChanneling = true
            call .startPeriodic()
            set p = null
        endif
        return false
    endmethod

    private static method onInit takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
        call TriggerAddCondition(t, function thistype.cast)
        set Pull = (PRISON_AOE-50)*(PRISON_AOE-50)
        set t = null
    endmethod
endstruct

endlibrary


v1.5
- Fixed the caster will automatically stop when duration is done

v1.4
- Code optimized and shortened

v1.3
- Scope converted to library
- Added BoundSentinel library
- Dummy removed as it's useless
- Degrees converted to radians
- Code fully optimized

v1.2
- StructRecycler removed.
- Some constants & text adjustments.

v1.1
- TimerUtils replaced by T32.
- Struct recyling applied.
- Codes fully optimized and efficient.


Keywords:
prison, black, hole, jngp, vjass, jass, ultimate, fire, ice, pull, mckill2009, awesome, magic, dota, diablo
Contents

Confinement (Map)

Reviews
15 Dec 2011 Bribe: Approved 3.5/5 (good spell). Philisophically, I think this should be a channeling spell that just stops working as soon as the caster moves. I don't care for the effects that you chose, plus the way you have imported them means...

Moderator

M

Moderator

15 Dec 2011
Bribe: Approved 3.5/5 (good spell).

Philisophically, I think this should be a channeling spell that just stops working as soon as the caster moves. I don't care for the effects that you chose, plus the way you have imported them means that you need to have 3 different types of dummy units. I recommend instead to use the dummy.mdx model so that you can just attach things to those dummies.

There may run the risk of crashing for out-of-bounds casting. It doesn't work on your map but it could work on someone else's. I recommend when the spell is first cast to check if the dummy units will run the risk of exiting the world bounds.

The group loop with Fence2 still has a pointless "set u = null" at the bottom of the loop.
 
Level 22
Joined
Nov 14, 2008
Messages
3,256
Seems cool, some sort of originality although I dislike that no units are stuck "within" the fence at the screenshots, does this work or? :)

You really should learn how to use Timer32, so much easier and also much better.

-player p = GetOwningPlayer(GetTriggerUnit())
Does not have to be initialized like that, better to do that in the onCreate method. Creates alot of unnecessary stuff if I record it right from Bribe.

-local real dist = SquareRoot(x1*x1 + y1*y1) and dist > AoERange-50 should be

x1*x1 + y1*y1 > (aoerange-50)^2(aoerange-50)*(aoerange-50)*

AS sqr is slower.

-bj_lastCreatedGroup, I'm not a fan using this in an enumeration. As GroupUtils both provides with GroupEnumUnitsInArea and a enumgroup I see this unnecessary and also

what happen if a GUIer used set wantToDestroy = true? Then it will destroy this group variable if I remember the ForGroupBJ correct. So either way, use a static group or a global group variable or even use GroupUtils.

-The applied life can be 0.00 instead of 0.01 :)

Else it seems fine to me. Good job!
 
Here's what I would've done:

:3

JASS:
library Confinement requires T32
    // ==== CONFIGURABLES ====
    globals
        private constant integer    ABIL_CODE   = 'A000'
        private constant integer    DUMMYID     = 'h001'
        private constant integer    FENCE1ID    = 'h002'
        private constant integer    FENCE2ID    = 'h003'
        private constant integer    AURAID      = 'h000' 
        private constant attacktype ATT         = ATTACK_TYPE_PIERCE
        private constant damagetype DAM         = DAMAGE_TYPE_DEATH
        private constant real       AOERANGE    = 400.0
        private constant real       AOEDAMAGE   = 80.0
        private constant group      GROUP       = CreateGroup()
    endglobals
    
    private function GetDuration takes integer i returns real
        return 5 + i * 5.
    endfunction
    
    private function GetDamage takes integer i returns real
        return 3 + i * 5.
    endfunction
    
    // ==== END CONFIGURABLES ====
    
    private module Init
        private static method onInit takes nothing returns nothing
            local trigger t = CreateTrigger()
            call TriggerRegisterAnyUnitEventBJ(t,EVENT_PLAYER_UNIT_SPELL_EFFECT)
            call TriggerAddCondition(t,Condition(function thistype.run))
            set t = null
        endmethod
    endmodule
    
    private struct Data
        unit caster
        unit aura
        unit dummy
        unit fenceA
        unit fenceB
        real xLoc
        real yLoc
        real hurt
        real angleA
        real angleB
        real duration
        player owner
        integer level
        static thistype TEMPDATA
        method destroy takes nothing returns nothing
            set .caster = null
            set .aura = null
            set .dummy = null
            set .fenceA = null
            set .fenceB = null
            set .owner = null
            call .deallocate()
        endmethod
        static method onDamage takes nothing returns boolean
            local thistype this = TEMPDATA
            local unit u = GetFilterUnit()
            if IsUnitEnemy(.dummy,GetOwningPlayer(u)) and GetWidgetLife(u)>0.405 and not IsUnitType(u,UNIT_TYPE_STRUCTURE) and not IsUnitType(u,UNIT_TYPE_MECHANICAL) then 
                call UnitDamageTarget(.dummy,u,.hurt,false,false,ATT,DAM,null)  
            endif
            set u = null
            return false          
        endmethod
        static method onRange takes nothing returns boolean
            local thistype this = TEMPDATA
            local unit u = GetFilterUnit()
            local real x0 = GetUnitX(u)
            local real y0 = GetUnitY(u)
            local real x = GetUnitX(.dummy)
            local real y = GetUnitY(.dummy)
            local real xd = x0 - x
            local real yd = y0 - y
            local real angle = Atan2(y-y0,x-x0)
            if IsUnitEnemy(.dummy,GetOwningPlayer(u)) and GetWidgetLife(u)>0.405 and xd*xd+yd*yd>(AOERANGE-50.0)*(AOERANGE-50.0) and not IsUnitType(u,UNIT_TYPE_STRUCTURE) and not IsUnitType(u,UNIT_TYPE_MECHANICAL) then 
                call SetUnitX(u,x0+30*Cos(angle))
                call SetUnitY(u,y0+30*Sin(angle))
            endif
            set u = null
            return false          
        endmethod
        method periodic takes nothing returns nothing
            local real a = .angleA * bj_DEGTORAD
            local real a2 = .angleB * bj_DEGTORAD
            local real xd = GetUnitX(.dummy)
            local real yd = GetUnitY(.dummy)
            if .duration > 0 then
                set .angleA = .angleA + 5.0
                set .angleB = .angleB - 5.0
                set .duration = .duration - FENCESPEED
                call SetUnitX(.fenceA,xd + AOERANGE * Cos(a))
                call SetUnitY(.fenceA,yd + AOERANGE * Sin(a))
                call SetUnitX(.fenceB,xd + AOERANGE * Cos(a2))
                call SetUnitY(.fenceB,yd + AOERANGE * Sin(a2))
                set TEMPDATA = this
                call GroupEnumUnitsInRange(GROUP,GetUnitX(.fenceB),GetUnitY(.fenceA),AOEDAMAGE,function thistype.onDamage)
                call GroupEnumUnitsInRange(GROUP,xd,yd,AOERANGE,function thistype.onRange)
            else
                call UnitApplyTimedLife(.aura,'BTLF',0.01)
                call UnitApplyTimedLife(.dummy,'BTLF',0.01)
                call UnitApplyTimedLife(.fenceA,'BTLF',0.01)
                call UnitApplyTimedLife(.fenceB,'BTLF',0.01)
                call .stopPeriodic()
                call .destroy()
            endif
        endmethod
        implement T32x
        static method create takes unit u returns thistype
            local thistype this = thistype.allocate()
            set .caster = u
            set .xLoc = GetSpellTargetX()
            set .yLoc = GetSpellTargetY()
            set .level = GetUnitAbilityLevel(.caster,ABIL_CODE)
            set .owner = GetOwningPlayer(.caster)
            set .aura = CreateUnit(.owner,AURAID,.xLoc,.yLoc,0)
            set .dummy = CreateUnit(.owner,DUMMYID,.xLoc,.yLoc,0)
            set .fenceA = CreateUnit(.owner,FENCE1ID,.xLoc,.yLoc,0)
            set .fenceB = CreateUnit(.owner,FENCE2ID,.xLoc,.yLoc,0)
            set .duration = GetDuration(.level)
            set .hurt = GetDamage(.level)
            set .angleA = 0
            set .angleB = 0
            call .startPeriodic()
            return this
        endmethod
        static method run takes nothing returns boolean
            if GetSpellAbilityId()==ABIL_CODE then
                call thistype.create(GetTriggerUnit())
            endif 
            return false
        endmethod
        implement Init
    endstruct
endlibrary
 
Last edited:
Level 22
Joined
Nov 14, 2008
Messages
3,256
Updated the one in my post ;)
That SquareRoot is actually impossible to avoid cause we're adding xd*xd and yd*yd :p

You dare to challenge me?

JASS:
            local real dist = SquareRoot(xd*xd+yd*yd)
            local real angle = Atan2(y-y0,x-x0)
            if IsUnitEnemy(.dummy,GetOwningPlayer(u)) and GetWidgetLife(u)>0.405 and dist>AOERANGE-50.0 and not IsUnitType(u,UNIT_TYPE_STRUCTURE) and not IsUnitType(u,UNIT_TYPE_MECHANICAL) then 
                call SetUnitX(u,x0+30*Cos(angle))
                call SetUnitY(u,y0+30*Sin(angle))
            endif

-->

JASS:
            local real angle = Atan2(y-y0,x-x0)
            if IsUnitEnemy(.dummy,GetOwningPlayer(u)) and GetWidgetLife(u)>0.405 and (xd*xd+yd*yd)>((AOERANGE-50.0)*(AOERANGE-50.)) and not IsUnitType(u,UNIT_TYPE_STRUCTURE) and not IsUnitType(u,UNIT_TYPE_MECHANICAL) then 
                call SetUnitX(u,x0+30*Cos(angle))
                call SetUnitY(u,y0+30*Sin(angle))
            endif
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
- I know how to use T32 but TimerUtils is my favorite at the moment so I dont think I should change to T32 even if it's faster than hashtables...

- bj_lastCreatedGroup = Bribe said before that with this, you need not to create nor destroy a group...

- I didnt know about your formula, let me change that

Not bad at all the one above. Although implementing t32 module should be below periodic method or else it will create unnecessary evaluations (quoted from Bribe? =) ).

I've put like this, and still it worked...

JASS:
  static method create takes unit u returns thistype
            local thistype this = thistype.allocate()
            set .caster = u
            set .xLoc = GetSpellTargetX()
            set .yLoc = GetSpellTargetY()
            set .level = GetUnitAbilityLevel(.caster,ABIL_CODE)
            set .owner = GetOwningPlayer(.caster)
            set .aura = CreateUnit(.owner,AURAID,.xLoc,.yLoc,0)
            set .dummy = CreateUnit(.owner,DUMMYID,.xLoc,.yLoc,0)
            set .fenceA = CreateUnit(.owner,FENCE1ID,.xLoc,.yLoc,0)
            set .fenceB = CreateUnit(.owner,FENCE2ID,.xLoc,.yLoc,0)
            set .duration = GetDuration(.level)
            set .hurt = GetDamage(.level)
            set .angleA = 0
            set .angleB = 0
            call .startPeriodic()
            return this
        endmethod
        static method run takes nothing returns boolean
            if GetSpellAbilityId()==ABIL_CODE then
                call thistype.create(GetTriggerUnit())
            endif 
            return false
        endmethod
        implement T32xs
        implement Init


@MAgtheridon96
- You lack "T32_PERIOD"...
- Why are you using T32x?, Isnt it that T32xs is safe?...
 
- Why are you using T32x?, Isnt it that T32xs is safe?...

T32x is faster ;)

I've put like this, and still it worked...

I know it works, but the issue is actually the order of the methods.
The T32x module should be implemented right above where you call startPeriodic (the create method) and right below your periodic method.
If not, it will create a bunch of triggers :p


edit
mckill, can we make a deal? Never use scopes. Alright? :p
Libraries are much better because then you can make the requirements explicit =)

edit
Oh and by the way, sorry for hijacking your thread xD
 
Level 22
Joined
Nov 14, 2008
Messages
3,256
- I know how to use T32 but TimerUtils is my favorite at the moment so I dont think I should change to T32 even if it's faster than hashtables...

- bj_lastCreatedGroup = Bribe said before that with this, you need not to create nor destroy a group...

@MAgtheridon96
- You lack "T32_PERIOD"...
- Why are you using T32x?, Isnt it that T32xs is safe?...

Use it. And btw TU shouldn't be used as blue version, it's bugged :D (yet works I believe, I always use orange or red, orange is easier though)

My bad, the BJ returned ANOTHER group variable.

T32x is faster and you won't call .startPeriodic() within the periodic method anyway so :D
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
mckill, can we make a deal? Never use scopes. Alright? :p
Libraries are much better because then you can make the requirements explicit =)

maybe not all, it depends upon the taste of the coder :)...as you have noticed in my spells, I do not use other libraries such as GroupUtils and a-like coz Im really not a fan of using another code even though it is much efficient/faster and easier...well, except for TU...

anyway thanks for the advice guys, next spell Ima use T32 but not this one, as for me I dont have any problems using TU...also Im a very big fan of hashtables...
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
1) Bribe: StructRecycler is not an approved resource. However, Alloc is an approved resource and it does the same thing. I recommend using that one, inlining the utility or simply using regular vJass structs with their built-in allocate/deallocate.

2) You are adding and subtracting ROTATION_SPEED which is whacky, it looks like it should be in degrees based on the value of "5" yet you are treating that value as a radian.

3) You don't need "set u = null" in your FirstOfGroup loops. Think about it, you only exit the loop when "u == null" ;)

4) Is there a point why you do this: "set this = srRecycle(this)" at the end of the function? It doesn't do anything more than "call this.deallocate()".

1) Excuse me but you said 'it does the same thing', so idk why it shouldnt be used, i think we should consider the situation sometimes...it doesnt mean that if it's not in the resource section, then reject the spell?, well, the Spell Rules doesnt require that the resource must be approved in the snippet section...

2) To make it rotate clockwise/counter...

3) Maker told me last time that you dont need to null the variables if it doesnt point anywhere, in this case the "FirstOfGroup" points to something, the "exitwhen==null" in my understanding is when the Group in range has none to filter...

4) I dont wanna argue that deallocate()/destroy() and srRecycle() does the same thing...

Anyway, I already did what you say, but I think we shouldt be very strict in things coz we are not perfect no matter how good we are...
 
Level 38
Joined
Sep 26, 2009
Messages
8,477
FirstOfGroup returns null for an empty group so by setting the variable to the FirstOfGroup you are nulling it.

If you want to use a library, the library should at least be published as its own resource. If it's not, then it's basically a waste of a library because only yours is using it, which is why I said it would be a good idea to either inline the library into your own to eliminate a requirement, or to use Alloc or to use normal structs.

I get that it makes it rotate in a certain direction, however why the value of 5 when it's in radians? A full circle will go from -pi to pi in radians, and the number 5 is nowhere to be found there. I think you should use 5 * bj_DEGTORAD there.
 
Level 10
Joined
Sep 19, 2011
Messages
527
JASS:
/*====================Confinement v1.2======================
============================================================
====================Made by: Mckill2009=====================
============================================================
============================================================

REQUIRES:
- JassNewGenPack by Vexorian
- T32 by Jesus4Lyf

HOW TO USE:
- Make a new trigger and convert to custom text via EDIT >>> CONVERT CUSTOM TEXT
- Copy ALL that is written here (overwrite the existing texts in the trigger)
- Copy the Dummy unit and the custom ability OR make your own
- You MUST input or change the correct raw ID's (see below)
- To view raw ID, press CTRL+D in the object editor

FULL DESCRIPTION:
Creates two magical fence that loops around a targeted point. Enemy units that is inside or caught within the fence's AoE cannot get out. If an enemy unit touches the fence it will deal damage as well. 

|cffffcc00Level 1|r - Spell lasts 10 seconds, vortex damages 8 per touch.
|cffffcc00Level 2|r - Spell lasts 15 seconds, vortex damages 13 per touch.
|cffffcc00Level 3|r - Spell lasts 20 seconds, vortex damages 18 per touch.
|cffffcc00Level 4|r - Spell lasts 25 seconds, vortex damages 23 per touch.
|cffffcc00Level 5|r - Spell lasts 30 seconds, vortex damages 28 per touch.

*/

library Confinement requires T32

    globals
        private constant integer        SPELL_ID = 'A000'
        private constant integer        DUMMY_ID = 'h001'
        private constant integer       FENCE1_ID = 'h002'
        private constant integer       FENCE2_ID = 'h003'
        private constant integer         AURA_ID = 'h000' 
        private constant attacktype          ATT = ATTACK_TYPE_PIERCE
        private constant damagetype          DAM = DAMAGE_TYPE_DEATH
        private constant real          AOE_RANGE = 400 //this is the prison range
        private constant real            AOE_DAM = 80 //this is the damage range of the fence
        private constant real     ROTATION_SPEED = 5
        private constant group             GROUP = bj_lastCreatedGroup
    endglobals

    private function GetDuration takes integer i returns real
        return 5 + i * 5.
    endfunction

    private function GetDamage takes integer i returns real
        return 3 + i * 5.
    endfunction

    private function FilterUnits takes unit u1, unit u2 returns boolean
        return IsUnitEnemy(u1, GetOwningPlayer(u2)) and not IsUnitType(u1, UNIT_TYPE_DEAD) and not IsUnitType(u1, UNIT_TYPE_STRUCTURE) and not IsUnitType(u1, UNIT_TYPE_MECHANICAL)
    endfunction

    private struct Confinement extends array
        private static integer instanceCount = 0
        private static thistype recycle = 0
        private thistype recycleNext
        
        private unit aura
        private unit dum
        private unit fence1
        private unit fence2
        private real x
        private real y
        private real hurt
        private real angle1
        private real angle2
        private real duration
        
        private method destroy takes nothing returns nothing
            set this.aura = null
            set this.dum = null
            set this.fence1 = null
            set this.fence2 = null
            
            set recycleNext = recycle
            set recycle = this
        endmethod
        
        private method periodic takes nothing returns nothing
            local real angle
            local real dist
            local real angle01
            local real angle02
            local real x1
            local real y1
            local real x2
            local real y2
            local unit u
            
            if .duration > 0 then
                set angle01 = .angle1 * bj_DEGTORAD
                set angle02 = .angle2 * bj_DEGTORAD
                set .angle1 = .angle1 + ROTATION_SPEED
                set .angle2 = .angle2 - ROTATION_SPEED
                set .duration = .duration - T32_PERIOD
                call SetUnitX(.fence1, .x + AOE_RANGE * Cos(angle01))
                call SetUnitY(.fence1, .y + AOE_RANGE * Sin(angle01))
                call SetUnitX(.fence2, .x + AOE_RANGE * Cos(angle02))
                call SetUnitY(.fence2, .y + AOE_RANGE * Sin(angle02))
                
                call GroupEnumUnitsInRange(GROUP, .x, .y, AOE_RANGE, null)
                
                loop
                    set u = FirstOfGroup(GROUP)
                    exitwhen u==null
                    if FilterUnits(u, .dum) then 
                        set x1 = GetUnitX(u)
                        set y1 = GetUnitY(u)
                        set x2 = x1-.x
                        set y2 = y1-.y
                        set angle = Atan2(y-y1, x-x1) 
                        set dist = (x2*x2) + (y2*y2)
                        if dist > (AOE_RANGE-50)*(AOE_RANGE-50) then
                            call SetUnitX(u, x1 + 30 * Cos(angle))
                            call SetUnitY(u, y1 + 30 * Sin(angle))
                        endif
                    endif
                    call GroupRemoveUnit(GROUP, u)
                endloop
                
                call GroupEnumUnitsInRange(GROUP, GetUnitX(.fence1), GetUnitY(.fence1), AOE_DAM, null)
                loop
                    set u = FirstOfGroup(GROUP)
                    exitwhen u==null
                    if FilterUnits(u, .dum) then 
                        call UnitDamageTarget(.dum, u, .hurt , false, false, ATT, DAM, null) 
                    endif
                    call GroupRemoveUnit(GROUP, u)
                endloop
                
                call GroupEnumUnitsInRange(GROUP, GetUnitX(.fence2), GetUnitY(.fence2), AOE_DAM, null)
                loop
                    set u = FirstOfGroup(GROUP)
                    exitwhen u==null
                    if FilterUnits(u, .dum) then 
                        call UnitDamageTarget(.dum, u, .hurt , false, false, ATT, DAM, null) 
                    endif
                    call GroupRemoveUnit(GROUP, u)
                endloop
            else
                call KillUnit(.aura)
                call KillUnit(.fence1)
                call KillUnit(.fence2)
                call KillUnit(.dum)
                call this.stopPeriodic()
                call this.destroy()
            endif
        endmethod
        
        implement T32x
        
        private static method create takes nothing returns thistype
            local thistype this
            local unit caster = GetTriggerUnit()
            local integer level = GetUnitAbilityLevel(caster, SPELL_ID)
            local player p = GetTriggerPlayer()
            
            if (recycle == 0) then
                set instanceCount = instanceCount + 1
                set this = instanceCount
            else
                set this = recycle
                set recycle = recycle.recycleNext
            endif
            
            set this.x = GetSpellTargetX()
            set this.y = GetSpellTargetY()
            set this.aura = CreateUnit(p, AURA_ID, .x, .y, 0)
            set this.dum = CreateUnit(p, DUMMY_ID, .x, .y, 0)
            set this.fence1 = CreateUnit(p, FENCE1_ID, .x, .y, 0)
            set this.fence2 = CreateUnit(p, FENCE2_ID, .x, .y, 0)
            set this.duration = GetDuration(level)
            set this.hurt = GetDamage(level)
            set this.angle1 = 0
            set this.angle2 = 0
            call this.startPeriodic()
            
            set p = null
            set caster = null
            
            return this
        endmethod
        
        private static method Cond takes nothing returns boolean
            if GetSpellAbilityId()==SPELL_ID then
                call thistype.create()        
            endif 
            return false
        endmethod

        private static method onInit takes nothing returns nothing
            local trigger t = CreateTrigger()
            call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
            call TriggerAddCondition(t, function thistype.Cond)
        endmethod
    endstruct
endlibrary

Don't use scope, instead use library and make it that requires T32.
When you create variables, put descriptive names to them.
Don't use bj_lastCreatedGroup like this.
Use destroy method to null the components of the struct.
Use this recycle technique when you use structs.

Nice effect and spell :)

Greetings.
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
- well, nothing wrong with the scopes & non-descriptive variables anyway, it's my taste...
- I already implemented my StructRecycler but Bribe says Alloc is better, but
completely removed it and use standard structs which is nothing wrong...
- I dont want to add another line just for GROUP 'which' btw does the same thing...

Tnx anyway :)...
 
Level 10
Joined
Sep 19, 2011
Messages
527
Is much better add 4 or 5 lines before add an entire library (Alloc) in this case.
"allocate" and "deallocate" creates a lot of trash code.

I didn't see the "destroy" method in your code, add it, there you can null all of your variables.

You don't need pass arguments to the "create" method, you can use GetTriggerUnit and GetTriggerPlayer in it.

You don't need to null the trigger (only null it if you will destroy it).

set u = null

You don't need null the "u" variable, because the loop exits when the "u" variable is equals to null.

I insist, don't use bj_lastCreatedGroup like you're doing on this.

Greetings :)

EDIT: - I dont want to add another line just for GROUP 'which' btw does the same thing...

No, it's not the same, because bj_lastCreatedGroup can take another values.
 
Last edited:
Level 38
Joined
Sep 26, 2009
Messages
8,477
Ruke I completely disagree on almost every point ;)

- Null all handles is a backed philosophy with good arguments why, but to argue that here is off topic.

- Then you argue he should not have to null the trigger but he should null the arrays? Pick a side please.

- bj_lastCreatedGroup is always the same group! I argued this with Troll-Brain endlessly on some other subject and it kind of came to the conclusion that it was pretty much preference and not forgoing functionality. Btw "GroupEnum.." clears the group first so it doesn't even matter if someone used it before you (that's what she said).

vJass built-in allocate and deallocate are fine. They add a couple safety things and some BJ debug messages if you are doing some unsafe stuff. I have seen "pro users" who were doing double-frees on structs without knowing why their map was getting errors. Yeah you think "oh I am gonna do it right" but seriously everyone makes mistakes no joke. And it's not about making the mistake or not. In fact it's not even about the length of code, because you could make an optimizer that trims all the vJass safeties out of compiled code (easy enough to do actually). And last I checked Frotty's optimizer when finished will inline functions which are only called once.
 
Level 10
Joined
Sep 19, 2011
Messages
527
- Then you argue he should not have to null the trigger but he should null the arrays? Pick a side please.

?

As far as I know, a handle only need to be nulled if it's destroyed.

- bj_lastCreatedGroup is always the same group! I argued this with Troll-Brain endlessly on some other subject and it kind of came to the conclusion that it was pretty much preference and not forgoing functionality. Btw "GroupEnum.." clears the group first so it doesn't even matter if someone used it before you (that's what she said).

O_O, the same group?. That doesn't make any sense. When you create a new group, bj_lastCreatedGroup is equals to that, last-created-group, the name tells everything.

Btw "GroupEnum.." clears the group first so it doesn't even matter if someone used it before you (that's what she said).

I didn't know that.

vJass built-in allocate and deallocate are fine. They add a couple safety things and some BJ debug messages if you are doing some unsafe stuff. I have seen "pro users" who were doing double-frees on structs without knowing why their map was getting errors. Yeah you think "oh I am gonna do it right" but seriously everyone makes mistakes no joke. And it's not about making the mistake or not. In fact it's not even about the length of code, because you could make an optimizer that trims all the vJass safeties out of compiled code (easy enough to do actually). And last I checked Frotty's optimizer when finished will inline functions which are only called once.

I read a nice tutorial who talks about "structs extends array" and for what is not recommended use allocate/deallocate.
The guy that wrote the tutorial was right, de/allocate creates a lot of "trash" code (and you can see it in the compile code).

Greetings and sry for my english.

EDIT: Here you have the tutorial: http://www.hiveworkshop.com/forums/...ls-280/coding-efficient-vjass-structs-187477/
 
Level 38
Joined
Sep 26, 2009
Messages
8,477
(Last created unit group) is not equal to bj_lastCreatedGroup:

JASS:
function GetLastCreatedGroup takes nothing returns group
    set bj_groupLastCreatedDest = CreateGroup()
    call ForGroup(bj_lastCreatedGroup, function GetLastCreatedGroupEnum)
    return bj_groupLastCreatedDest
endfunction

In fact, the only GUI function which fills bj_lastCreatedGroup is CreateNUnitsAtLoc. It is a perpetual group that never gets destroyed.

I know that tutorial and if you know Nestharus' approach as much as I do the thing he cares about more than any other aspect of coding is efficiency. I value other virtues: ease of use, readability and prioritizing which areas of code need the most work. Improving the allocate/deallocate speed is one of the most useless things you can do. One CreateUnit call is about the same efficiency as about 1,000 built-in allocates.

Nulling arrays is good, something that I practice, as well as inlining allocate/deallocate. However, to require my enormously and unreasonably high standards for other users is absurd. Nestharus didn't write the "efficient structs" as a rule - he is not a mod, and no mod should make such stuff a rule.
 
Level 38
Joined
Sep 26, 2009
Messages
8,477
I don't use Alloc personally, and I did say I inline my allocators as well because it is faster and saves on generated code and I have way too high standards to throw something like that away. What I meant was that if someone gets their allocators from struct/endstruct instead of a module or something it is definitely not the end of the world because efficiency is not everything.
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
1) Philisophically, I think this should be a channeling spell that just stops working as soon as the caster moves. I don't care for the effects that you chose, plus the way you have imported them means that you need to have 3 different types of dummy units. I recommend instead to use the dummy.mdx model so that you can just attach things to those dummies.

2) There may run the risk of crashing for out-of-bounds casting. It doesn't work on your map but it could work on someone else's. I recommend when the spell is first cast to check if the dummy units will run the risk of exiting the world bounds.

3) The group loop with Fence2 still has a pointless "set u = null" at the bottom of the loop.

1) I think that'd good if it's going to be attachments instead of 3 models, but what about the size of the aura?, can it be adjusted as well?...

2) Maybe BoundSentinel can fix that...

3) I forgot to remove it, nxt update Ima do it...

EDIT: - I dont want to add another line just for GROUP 'which' btw does the same thing...

No, it's not the same, because bj_lastCreatedGroup can take another values.

and GROUP cant take another value?, I dont think so coz GROUP equals to bj_lastCreatedGroup and a global variable so oviously it can take another value as well...
 
Level 38
Joined
Sep 26, 2009
Messages
8,477
Structs are not true OOP and are therefore recycled and not truly "destroyed". Because vJass has all the limits of JASS and more, a struct instances is just an integer and a struct member is just an array. Therefore you will eventually re-write the array in 99% of cases making nulling of those variables not a requirement.
 
Level 10
Joined
Sep 19, 2011
Messages
527
Structs are not true OOP and are therefore recycled and not truly "destroyed". Because vJass has all the limits of JASS and more, a struct instances is just an integer and a struct member is just an array. Therefore you will eventually re-write the array in 99% of cases making nulling of those variables not a requirement.

You're saying that is not required null all variables (only handle type) when you're not using it?

Structs are like classes but more basics (that's what I read from wikipedia :D).
 
Level 38
Joined
Sep 26, 2009
Messages
8,477
Ruke, what's the point of "not nulling" anyway? What's the extent of the tests you have done? Because I have gotten feedback from some people that not nulling "player" handles has caused the game to lag.

However, arrays (struct variables) will be overwritten in 99.99% of cases therefore I don't "require" it but I do highly recommend it.
 
Level 38
Joined
Sep 26, 2009
Messages
8,477
That's a myth that was spawned during a strange time (many patches ago) where nulling some types of handles would cause bugs.

While the handle ID will not be recycled if a destroyed handle is still pointed to in memory, there is apparently some RAM that is collected for each pointer (variable pointing to a handle) and nulling the pointer will clear the RAM.
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
it may leak but very minimal, I've created a test with nulling a unit and not...

here's the result: creates unit every 0.03/sec, units created 1500...

Ram 101,432 = nulls the unit with your method

Ram 101,498 = w/o nulling them but with .destroy()

maybe 1500 isnt that much but I believe even if 10000, results is not a big deal...

EDIT: New test 3000 units...

Ram 134396 = no nulling
Ram 134664 = nulls with your method
 
Top