• Listen to a special audio message from Bill Roper to the Hive Workshop community (Bill is a former Vice President of Blizzard Entertainment, Producer, Designer, Musician, Voice Actor) 🔗Click here to hear his message!
  • Read Evilhog's interview with Gregory Alper, the original composer of the music for WarCraft: Orcs & Humans 🔗Click here to read the full interview.

[vJASS] Changing units' height with vJass spell. Need help.

Status
Not open for further replies.
Level 11
Joined
Oct 11, 2012
Messages
711
Hi all. I am still learning vJass and what I am trying to do is to make a spell that can change the fly height of a group of units around the hero. It first elevates the group of units to a certain height, and then decrease their fly height until they hit the ground, nothing complicated. Here is my code:
JASS:
scope MoveZ
    
    struct A
        private static timer t = CreateTimer()
        private static timer t1 = CreateTimer()
        
        private thistype prev
        private thistype next
        
        private unit tri
        private unit u
        
        private group g
        
        private real x
        private real y
        
        private method destroy takes nothing returns nothing
            call this.deallocate()
            set this.next.prev = this.prev
            set this.prev.next = this.next
            call DestroyGroup(this.g)
            set this.tri = null
            set this.u = null
            set this.g = null
        endmethod
        
        private static method cb2_action takes nothing returns nothing
            if GetUnitZ(GetEnumUnit()) > 0.01 then //Somehow, the unit can never reach the height of 0.000 after elevation, but 0.010
                if UnitAlive(GetEnumUnit()) and GetEnumUnit() != gg_unit_Hpal_0000 then
                    call SetUnitFlyHeight( GetEnumUnit(), GetUnitZ(GetEnumUnit()) - 50,0 )
                endif
            endif
        endmethod
        
        private static method cb2 takes nothing returns nothing
            local thistype this = thistype(0).next
            loop
                exitwhen this == 0
                
                call ForGroup(this.g,function thistype.cb2_action)
                set this.u = FirstOfGroup(this.g) // I use this to check if the group of units has reached the ground. I couldn't figure out a better way to check it.
                if GetUnitZ(this.u) > 0.01 then
                else
                    call PauseTimer(t1)
                    call this.destroy()
                endif
                set this = this.next
            endloop
        endmethod
        
        private static method cb_action takes nothing returns nothing
            if GetUnitZ(GetEnumUnit()) < 500 then //if the unit's height is lower than 500, then increase it.
                if UnitAlive(GetEnumUnit()) then
                    call UnitAddAbility(GetEnumUnit(),'Amrf')
                    call UnitRemoveAbility(GetEnumUnit(),'Amrf')
                    call SetUnitFlyHeight( GetEnumUnit(), GetUnitZ(GetEnumUnit()) + 50,0 )
                endif
            else
                call PauseTimer(t)
                call TimerStart(t1,0.03,true,function thistype.cb2)
            endif
        endmethod
        
        private static method cb takes nothing returns nothing
            local thistype this = thistype(0).next
            loop
                exitwhen this == 0
                set this.g = CreateGroup()
                set this.x = GetUnitX(this.tri)
                set this.y = GetUnitY(this.tri)
                call GroupEnumUnitsInRange(this.g,this.x,this.y,600.,null)
                call GroupRemoveUnit(this.g,this.tri) // I don't want the casting unit in the group.
                call ForGroup(this.g,function thistype.cb_action)
                set this = this.next
            endloop
        endmethod
        
        private static method run takes nothing returns boolean
            local thistype this = thistype.allocate()
            
            set this.next = 0
            set this.prev = thistype(0).prev
            set thistype(0).prev.next = this
            set thistype(0).prev = this
            
            call TimerStart(t,0.03,true,function thistype.cb)
            set this.tri = GetTriggerUnit()
            return false
        endmethod

        //===========================================================================
        private static method onInit takes nothing returns nothing
            call RegisterSpellEffectEvent('A00U',  function thistype.run)
        endmethod
    
    endstruct
endscope

The spell worked as intended when I tested it (only cast it a few times), but I would like to know:
1. Am I deallocating things correctly? Are there any leaks? Or other errors in terms of coding? (each time I cast this spell with two heroes standing among a group of units, the memory usage increases 20 to 40 k)
2. How to optimize the code?
3. There must be a better way of doing this. Any idea or thoughts that can help me? (I don't want to use other libraries, such as CTL, because I want to learn the basic of vJass first)
Thanks a lot.

Edit: I have made some improvement to the code, it's still ugly but better than the one above, please see post #8 . :)
 
Last edited:
Level 26
Joined
Aug 18, 2009
Messages
4,097
The list does not look right and you should use a module or something. Is it kind of an aura/the elevation applier a timed effect? Because you call it periodically. Why create a group per instance in each iteration (and overwriting the previous one and not deleting it)? Why start the t1 timer multiple times in the enum loop? The amrf-fly height enabler only needs to be done once per unit. If you have a unit indexer, throw it in there.

// I use this to check if the group of units has reached the ground. I couldn't figure out a better way to check it.

Only one unit per instance is checked and the units can be on different heights. Once any unit of any instance reaches the climax, all instances make their units fall?

The modelling seems totally off. Write one struct that handles the aura, another one that handles the target buff on the unit and sees if there is interference/reappliance.
 
Level 11
Joined
Oct 11, 2012
Messages
711
The list does not look right and you should use a module or something. Is it kind of an aura/the elevation applier a timed effect? Because you call it periodically. Why create a group per instance in each iteration (and overwriting the previous one and not deleting it)? Why start the t1 timer multiple times in the enum loop? The amrf-fly height enabler only needs to be done once per unit. If you have a unit indexer, throw it in there.



Only one unit per instance is checked and the units can be on different heights. Once any unit of any instance reaches the climax, all instances make their units fall?

The modelling seems totally off. Write one struct that handles the aura, another one that handles the target buff on the unit and sees if there is interference/reappliance.

Thanks, WaterKnight! Finally someone replies, LOL.
It is not an aura, it's just an active spell. I think you are right about the issues in the code, but I am not sure how to fix it. I am still working on it based on your comment.
BTW, what do you mean by "modelling" ? Do you mean the whole concept or approach of the code?
 
Level 12
Joined
Feb 22, 2010
Messages
1,115
Instead of raising and lowering at seperate timers, use a boolean.For example when boolean "rise" is true, increase height else decrease height.
 
Level 26
Joined
Aug 18, 2009
Messages
4,097
BTW, what do you mean by "modelling" ? Do you mean the whole concept or approach of the code?

Yes, the structuring. Organize it nearly in dedicated structs, keep the tangling to a minimum, outsource tasks that can be isolated and be named well and in general make a concept beforehand about what is how dependent, how the parts interact and how the ensemble shall function.
 
Level 11
Joined
Oct 11, 2012
Messages
711
Yes, the structuring. Organize it nearly in dedicated structs, keep the tangling to a minimum, outsource tasks that can be isolated and be named well and in general make a concept beforehand about what is how dependent, how the parts interact and how the ensemble shall function.

Yea, its kind of complicated for me because I have no CS back ground and vJass is new to me. Thanks for the help though. :)
 
Level 11
Joined
Oct 11, 2012
Messages
711
Hey guys, I have made some improvement to the code, it is still ugly, but I think it's better than before:
JASS:
scope MoveZ
    
    struct A
        
        private static timer t = CreateTimer()
        private static timer t1 = CreateTimer()
        
        private thistype prev
        private thistype next
        
        private unit tri
        private unit u
        
        private group g
        
        private real x
        private real y
        
        private static integer counter1 = 0
        private static integer counter2 = 0
        
        private method destroy takes nothing returns nothing
            call this.deallocate()
            set this.next.prev = this.prev
            set this.prev.next = this.next
            call DestroyGroup(this.g)
            set counter1 = counter1 - 1
            if counter1 == 0 then
                call PauseTimer(t)
            endif
            call PauseTimer(t1)
            set this.tri = null
            set this.u = null
            set this.g = null
            set counter2 = 0
        endmethod
        
        private static method cb2_action takes nothing returns nothing
            if UnitAlive(GetEnumUnit()) then
                if GetUnitZ(GetEnumUnit()) > 0.0 then
                    call SetUnitFlyHeight( GetEnumUnit(), GetUnitZ(GetEnumUnit()) - 100,0 )
                endif
            endif
        endmethod
        
        private static method cb2_dmg takes nothing returns nothing
            if UnitAlive(GetEnumUnit()) then
                call UnitDamageTarget(gg_unit_Hpal_0000,GetEnumUnit(),133,true,false,ATTACK_TYPE_CHAOS,DAMAGE_TYPE_DEMOLITION,null)
            endif

        endmethod
        
        private static method cb2 takes nothing returns nothing
            local thistype this = thistype(0).next
            loop
                exitwhen this == 0
                call ForGroup(this.g,function thistype.cb2_action)
                loop
                    set this.u = FirstOfGroup(this.g)
                    if UnitAlive(this.u) then
                    else
                        call GroupRemoveUnit(this.g,this.u)
                        set this.u = FirstOfGroup(this.g)
                    endif
                    exitwhen UnitAlive(this.u) or CountUnitsInGroup(this.g) <= 0
                endloop
                if GetUnitZ(this.u) > 0.01 then
                else
                    call ForGroup(this.g,function thistype.cb2_dmg)
                    call this.destroy()
                endif
                set this = this.next
            endloop
        endmethod
        
        private static method cb_action takes nothing returns nothing
            if GetUnitZ(GetEnumUnit()) < 500 then
                if UnitAlive(GetEnumUnit()) then
                    call UnitAddAbility(GetEnumUnit(),'Amrf')
                    call UnitRemoveAbility(GetEnumUnit(),'Amrf')
                    call SetUnitFlyHeight( GetEnumUnit(), GetUnitZ(GetEnumUnit()) + 50,0 )
                    call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Other\\Volcano\\VolcanoDeath.mdl",GetUnitX(GetEnumUnit()),GetUnitY(GetEnumUnit())))
                endif
            else
                call DestroyEffect(AddSpecialEffectTarget("Abilities\\Spells\\Other\\Incinerate\\FireLordDeathExplode.mdl",GetEnumUnit(),"origin"))
                set counter2 = counter2 + 1
                if counter2 == 1 then
                    call TimerStart(t1,0.03,true,function thistype.cb2)
                endif
            endif
        endmethod
        
        private static method cb takes nothing returns nothing
            local thistype this = thistype(0).next
            loop
                exitwhen this == 0
                call ForGroup(this.g,function thistype.cb_action)
                set this = this.next
            endloop
        endmethod
        
        private static method filter takes nothing returns boolean
            return UnitAlive(GetFilterUnit())
        endmethod
        
        private static method run takes nothing returns boolean
            local thistype this = thistype.allocate()
            
            set this.next = 0
            set this.prev = thistype(0).prev
            set thistype(0).prev.next = this
            set thistype(0).prev = this
            set this.tri = GetTriggerUnit()
            set this.g = CreateGroup()
            set this.x = GetUnitX(this.tri)
            set this.y = GetUnitY(this.tri)
            call GroupEnumUnitsInRange(this.g,this.x,this.y,600.,function thistype.filter)
            call GroupRemoveUnit(this.g,this.tri)
            call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Orc\\WarStomp\\WarStompCaster.mdl",GetUnitX(this.tri),GetUnitY(this.tri)))
            set counter1 = counter1 + 1
            if counter1  == 1 and CountUnitsInGroup(this.g) > 0 then
                call TimerStart(t,0.03,true,function thistype.cb)
            endif
            if CountUnitsInGroup(this.g) <=0 then
                call this.destroy()
            endif
            return false
        endmethod

        //===========================================================================
        private static method onInit takes nothing returns nothing
            call RegisterSpellEffectEvent('A00U',  function thistype.run)
        endmethod
    
    endstruct
endscope
 
Status
Not open for further replies.
Top