• 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] Spell Problem

Status
Not open for further replies.
Level 4
Joined
Sep 12, 2008
Messages
111
Been trying to make timed abilities but can't seem to work ;_;.. Like when I use ability1 and add ability2 in I can't seem to get the trigger of ability2 to work.

JASS:
struct Punch

    private static constant integer SPELL_ID = 'A002'
    private static constant integer SPELL_ID1 = 'A001'
    private static constant integer BUFF_CODE = 'B000'
    private static constant integer UNIT_CODE = 'u000'

    private static constant real TIMEOUT = 1.5 //0.03125

    private thistype next
    private thistype prev

    private static timer iterator = CreateTimer()
    private static integer count = 0

    private unit caster
    private unit target
    private integer spell
    private real damage = 10.

    private method destroy takes nothing returns nothing
        /*
        *   Deallocate this instance.
        */
        call this.deallocate()

        /*
        *   We remove the instance from the linked list.
        */
        set this.next.prev = this.prev
        set this.prev.next = this.next

        /*
        *   We decrease the count by 1.
        *   If the count is 0, we pause the timer.
        */
        set count = count - 1
        if count == 0 then
            call PauseTimer(iterator)
        endif

        /*
        *   We null the data in the struct.
        *   This is completely optional. It doesn't really make a difference
        *   at all. (Unless you're casting the spell some hundreds of times.)
        *   If you have some real memory intense systems in your map, 
        *   you might want to do this, especially if your struct has a lot of data.
        *
        *   These are global variables, so they will be recycled eventually.
        *   It's all up to you, my friend.
        */
        set this.caster = null
        set this.target = null
        

        // Code.

    endmethod

    private static method periodic takes nothing returns nothing
        /*
        *   Starting from the first instance, we loop
        *   over all the instance in the list until we hit
        *   a dead-end.
        */
        local thistype this = thistype(0).next
        local group g = CreateGroup()
        local unit u
        local real x
        local real y
        local real face
        local real angBetween
        local real theta
        loop
            exitwhen this == 0
            debug call BJDebugMsg(I2S(this))
            if this.spell == SPELL_ID then  //if spell is punch
                //debug call BJDebugMsg("Right Punch")
                call UnitRemoveAbility(this.caster, SPELL_ID) //remove punch
                call UnitAddAbility(this.caster, SPELL_ID1)   //add combo punch
            elseif this.spell == SPELL_ID1 then
                //debug call BJDebugMsg("Left Punch")
                call UnitRemoveAbility(this.caster, SPELL_ID1) //remove combo punch
                call UnitAddAbility(this.caster, SPELL_ID)   //add punch
            endif
            set x = GetUnitX(this.caster)
            set y = GetUnitY(this.caster)
            set face = GetUnitFacing(this.caster)
            //call BJDebugMsg(I2S(this.spell) + " id")
            call GroupEnumUnitsInRange(g, x, y, 200., null)
            loop
            set u = FirstOfGroup(g)
                if u != null then
                    call GroupRemoveUnit(g, u)
                    set angBetween = Atan2(GetUnitY(u)-y,GetUnitX(u)-x)
                    set theta = Cos(face-angBetween)
                    if not IsUnitAlly(this.caster, GetOwningPlayer(u)) and theta>0. then
                        call UnitDamageTarget(this.caster, u, damage, true, false, null, null, null)
                    endif
                endif
            endloop
            call DestroyGroup(g)
            set u = null
            // Code.
            set this = this.next
            
        endloop
    endmethod

    private static method run takes nothing returns boolean
        /*
        *   We allocate an instance.
        */
        local thistype this = thistype.allocate()
        local unit u = GetTriggerUnit()
        /*
        *   We add the instance to the linked list.
        */
        set this.next = 0
        set this.prev = thistype(0).prev
        set thistype(0).prev.next = this
        set thistype(0).prev = this

        /*
        *   We increase the count by 1.
        *   If the count is 1, we start the timer to loop through 
        *   the instances. This is because recasting the spell while 
        *   an instance is already running shouldn't restart the timer.
        */
        set count = count + 1
        if count == 1 then
            call TimerStart(iterator, TIMEOUT, true, function thistype.periodic)
        endif

        /*
        *   We set our struct data.
        */
        set this.caster = u
        set this.spell = GetSpellAbilityId()
        //call SetUnitTimeScale(this.caster, 1.)
        call SetUnitAnimation(this.caster, "attack")
        // Code.

        return false
    endmethod

    private static method onInit takes nothing returns nothing
        local trigger t = CreateTrigger()
        call RegisterSpellEffectEvent(SPELL_ID, function thistype.run)
        call RegisterSpellEffectEvent(SPELL_ID1, function thistype.run)
        //call TriggerAddCondition(t, Condition(function thistype.run))
        set t = null
    endmethod
endstruct

I think that the problem lies in the remove function. Any ideas? Thanks :^):ogre_hurrhurr::ogre_hurrhurr::ogre_datass:
 
Level 7
Joined
Mar 6, 2006
Messages
282
Idk vJASS but

JASS:
       loop
            set u = FirstOfGroup(g)
                if u != null then
                    call GroupRemoveUnit(g, u)
                    set angBetween = Atan2(GetUnitY(u)-y,GetUnitX(u)-x)
                    set theta = Cos(face-angBetween)
                    if not IsUnitAlly(this.caster, GetOwningPlayer(u)) and theta>0. then
                        call UnitDamageTarget(this.caster, u, damage, true, false, null, null, null)
                    endif
                endif
        endloop

if u != null then ---> exitwhen u == null
 
Level 19
Joined
Mar 18, 2012
Messages
1,716
* We null the data in the struct.
* This is completely optional. It doesn't really make a difference
* at all.

Okay ..... That's just nonesense.

DysfunctionaI is right you have an infinite loop.

Also there is room for improvement in case this is the final code.

You could use a player variable owner.
not IsUnitAlly(this.caster, GetOwningPlayer(u)) --> IsUnitEnemy(u, this.owner)

You could exclude this line of the if then block, when you use this.spell instead of the Global:
call UnitRemoveAbility(this.caster, this.spell)

Edit:
local unit u = GetTriggerUnit() --> set this.caster = GetTriggerUnit()

You don't need the local trigger in onInit anymore.

Currently your filter allows to damage dead units.

use one global group /* private */ static group enu = CreateGroup() instead of creating a new local group each time the spell runs. The first of Group loop will null the local unit variable u, you don't have to do it on your own.

The method are all private and so should be the struct and then it could be a scope aswell.
 
Last edited:
Level 4
Joined
Sep 12, 2008
Messages
111
So I got the problems I initially had solved. Now I want this spell to get removed from the struct when the timer expires but it is also removed in the periodic timer. What should I do?

JASS:
struct Punch

    private static constant integer SPELL_ID = 'A000'
    private static constant integer SPELL_ID1 = 'A001'
    private static constant integer BUFF_CODE = 'B000'
    private static constant integer UNIT_CODE = 'u000'

    private static constant real TIMEOUT = 0.75 //0.03125
    private static constant real SPELLOUT = 1.75 //0.03125
    private static Punch array P
    
    private static group enu = CreateGroup()

    private thistype next
    private thistype prev

    private static timer iterator = CreateTimer()
    //private static timer combotimer = NewTimer()
    private static integer count = 0

    private unit caster
    private unit target
    private integer spell
    private real damage = 10.

    private method destroy takes nothing returns nothing
        /*
        *   Deallocate this instance.
        */
        call this.deallocate()

        /*
        *   We remove the instance from the linked list.
        */
        set this.next.prev = this.prev
        set this.prev.next = this.next

        /*
        *   We decrease the count by 1.
        *   If the count is 0, we pause the timer.
        */
        set count = count - 1
        if count == 0 then
            call PauseTimer(iterator)
        endif

        /*
        *   We null the data in the struct.
        *   This is completely optional. It doesn't really make a difference
        *   at all. (Unless you're casting the spell some hundreds of times.)
        *   If you have some real memory intense systems in your map, 
        *   you might want to do this, especially if your struct has a lot of data.
        *
        *   These are global variables, so they will be recycled eventually.
        *   It's all up to you, my friend.
        */
        

        // Code.

    endmethod

    private static method periodic takes nothing returns nothing
        /*
        *   Starting from the first instance, we loop
        *   over all the instance in the list until we hit
        *   a dead-end.
        */
        local thistype this = thistype(0).next
        local unit u
        local real x
        local real y
        local real face
        local real angBetween
        local real theta
        loop
            exitwhen this == 0
            //debug call BJDebugMsg(I2S(this))
            call UnitRemoveAbility(this.caster, this.spell)
            if this.spell == SPELL_ID then
                //debug call BJDebugMsg("Right Punch")
                call UnitAddAbility(this.caster, SPELL_ID1)
            elseif this.spell == SPELL_ID1 then
                //debug call BJDebugMsg("Left Punch")
                call UnitAddAbility(this.caster, SPELL_ID)
            endif
            set x = GetUnitX(this.caster)
            set y = GetUnitY(this.caster)
            set face = GetUnitFacing(this.caster)
            call GroupEnumUnitsInRange(this.enu, x, y, 200., null)
            loop
            set u = FirstOfGroup(this.enu)
            exitwhen u == null
                //debug call BJDebugMsg(GetUnitName(u))
                call GroupRemoveUnit(this.enu, u)
                set angBetween = Atan2(GetUnitY(u)-y,GetUnitX(u)-x)
                set theta = Cos(face-angBetween)
                if not IsUnitAlly(this.caster, GetOwningPlayer(u)) and theta>0. and GetUnitState(u, UNIT_STATE_LIFE) > 0.405 then
                    call UnitDamageTarget(this.caster, u, damage, true, false, null, null, null)
                endif
            endloop
            if this.spell == SPELL_ID1 then
                call destroy()
            endif
            set this = this.next
        endloop
    endmethod
    
    private static method removespell takes nothing returns nothing
        local timer t = GetExpiredTimer()
        local thistype this = P[GetTimerData(t)]
        debug call BJDebugMsg("[" + I2S(this) + "] break " + GetUnitName(this.caster))
        call UnitRemoveAbility(this.caster, SPELL_ID1)
        call UnitAddAbility(this.caster, SPELL_ID)
        call ReleaseTimer(t)
        if this.spell == SPELL_ID then
            call destroy()
        endif
    endmethod

    private static method run takes nothing returns nothing
        local thistype this = thistype.allocate()
        local timer t=NewTimer()
        /*
        *   We add the instance to the linked list.
        */
        set this.next = 0
        set this.prev = thistype(0).prev
        set thistype(0).prev.next = this
        set thistype(0).prev = this
        set count = count + 1
        if count == 1 then
            call TimerStart(iterator, TIMEOUT, true, function thistype.periodic)
        endif
        
        set P[GetTimerData(t)] = this
        set this.caster = GetTriggerUnit()
        set this.spell = GetSpellAbilityId()
        call SetUnitAnimation(this.caster, "attack")
        if this.spell == SPELL_ID then  //combo break
            call TimerStart(t, SPELLOUT, false, function thistype.removespell)
        endif
    endmethod

    private static method onInit takes nothing returns nothing
        call RegisterSpellEffectEvent(SPELL_ID, function thistype.run)
        call RegisterSpellEffectEvent(SPELL_ID1, function thistype.run)
    endmethod
endstruct

As the periodic timer and the combo breaker timer starts when the spell runs, it will remove 2 instance of the struct when they both expire..
Thanks for the advice you guys gave :)
 
Level 18
Joined
Sep 14, 2012
Messages
3,413
Longer names -> Involve slower reading/loading.

Private naming -> Adding a prefix of the scope and then a random prefix.
If you double private you'll see long var names...
I'll edit with an example soon


EDIT : I'm missing something xD
I miss thread sorry xD
It is a problem when coming to double private not like this there is no problem it is just longer to type xD
 
I really think that is not true, probably it just takes more time to type it.

Actually he is right.

The longer the function and variable names the slower it runs. So doubly privating something is longer.

Example:

normal length - onInit
length of name with private - s__Punch_onInit
length of name doubly private s__test__Punch_onInit

Now to make sense of that stuff
onInit is simply a non private function name
When the s_ Punch is added that is from the struct name. It adds the struct name to the function name with random spaces to make it private.
When s test gets added and Punch gets added it adds the library name of test then it adds the name of the struct punch. The s is saying it is in a struct.
For private functions it adds the library name along with random spaces to the function name.
 
Level 8
Joined
Feb 3, 2013
Messages
277
JASS:
        /*
        *   Deallocate this instance.
        */
        call this.deallocate()

        /*
        *   We remove the instance from the linked list.
        */
        set this.next.prev = this.prev
        set this.prev.next = this.next

EDIT:
this should be reversed
once an instance is deallocated, this will return 0.
Having said that, you need to null members before you deallocate or in an onDestroy method.

JASS:
set unit1 = null
set loc1 = null
set player1 = null

set prev[next[this]] = prev[this]
set next[prev[this]] = next[this]
call .deallocate()

next and prev should be static thistype/integer arrays

also you dont need the integer variable.. you can just do
JASS:
if next[0] == this then
call TimerStart(iterator, period, true, function yourFunc)
if next[0] == 0 then
call PauseTimer(iterator)
 
Last edited:
Level 23
Joined
Apr 16, 2012
Messages
4,041
JASS:
        /*
        *   Deallocate this instance.
        */
        call this.deallocate()

        /*
        *   We remove the instance from the linked list.
        */
        set this.next.prev = this.prev
        set this.prev.next = this.next

this should be reversed
once an instance is deallocated, this will return 0.
Having said that, you need to null members before you deallocate or in an onDestroy method.

JASS:
set unit1 = null
set loc1 = null
set player1 = null

set prev[next[this]] = prev[this]
set next[prev[this]] = next[this]
call .deallocate()

next and prev should be static thistype/integer arrays

also you dont need the integer variable.. you can just do
JASS:
if next[0] == this then
call TimerStart(iterator, period, true, function yourFunc)
if next[0] == 0 then
call PauseTimer(iterator)

dont use onDestroy, overload destroy method, onDestroy wont even get called

JASS:
        /*
        *   Deallocate this instance.
        */
        call this.deallocate()

        /*
        *   We remove the instance from the linked list.
        */
        set this.next.prev = this.prev
        set this.prev.next = this.next

this should be reversed
once an instance is deallocated, this will return 0.
Having said that, you need to null members before you deallocate or in an onDestroy method.

where you got this impression from? this is not right, because this is a local variable, and has valid struct instance until endmethod

deallocate only recycles the instance, so it is no longer valid and is open for picking(from allocate method)
 
Level 4
Joined
Sep 12, 2008
Messages
111
But the function removespell will also trigger this.destroy together with the periodic function.. When the spell starts, it will remove ability1 and add ability2 to the unit. The combo timer and the periodic timer will start together and when the periodic timer expires it will remove an instance of the struct. Then the combo timer decides to say "Hey it got removed why not remove another?" and take out one more instance. Other than this the spell works fine. How do I fix this? ^^/
 
btw this is an infinite loop since there's no exit calls

JASS:
loop
            set u = FirstOfGroup(g)
                if u != null then
                    call GroupRemoveUnit(g, u)
                    set angBetween = Atan2(GetUnitY(u)-y,GetUnitX(u)-x)
                    set theta = Cos(face-angBetween)
                    if not IsUnitAlly(this.caster, GetOwningPlayer(u)) and theta>0. then
                        call UnitDamageTarget(this.caster, u, damage, true, false, null, null, null)
                    endif
                endif
        endloop
 
Status
Not open for further replies.
Top