• 🏆 Texturing Contest #33 is OPEN! Contestants must re-texture a SD unit model found in-game (Warcraft 3 Classic), recreating the unit into a peaceful NPC version. 🔗Click here to enter!
  • It's time for the first HD Modeling Contest of 2024. Join the theme discussion for Hive's HD Modeling Contest #6! Click here to post your idea!

[JASS] Issue hero unit to cast custom spell

Status
Not open for further replies.
Level 2
Joined
Feb 26, 2012
Messages
8
Hi guys,
I am trying to create custom Jass spell. It should looks like this:

1) Hero casts custom spell, which has a trigger connected to it.
2) In trigger, I obtain a spell target location
3) hero unit "jumps" to the location
4) After "landing" hero should automatically cast another spell which stuns all surrounding enemy units.

I am having problems while integrating the last step. I have another custom spell based on Warstomp and following Jass code:

JASS:
// Get ID of "Jump" spell
local integer Level = GetUnitAbilityLevel( Attacker, Jump_ID )

// Add custom warstomp based spell to hero
call UnitAddAbility( Attacker, Warstomp_Based_ID )

// Set level of warstomp based spell only when "Jump" spell level differs from 1
if Level != 1 then
    call SetUnitAbilityLevel( Attacker, Warstomp_Based_ID, Level )
endif
        
// Cast spell
call IssueImmediateOrder( Attacker, "stomp")

// Remove warstomp from hero
call UnitRemoveAbility( Attacker, Warstomp_Based_ID )

The warstomp spell is not casted. Unit has enought mana, level etc. But if I remove the last call, it works. At least the first time the Jump spell is called. I suppose it has to do something with cast time or Issue function is asynchronous or whatever.

Could you please recomend me a appropriate solution for this? Thank you!
 

Bannar

Code Reviewer
Level 26
Joined
Mar 19, 2008
Messages
3,140
Warstomp takes some time to cast, thus removing this ability within such short period of time might prevent actuall effect from happening. You can a) remove this ability after short delay instead of immidiately or b) create a dummy unit which will cast this for you.

Also make sure this ability is truely based on Warstomp or Channel (with id: stomp) before doing anything else.

Btw: use call SetUnitAbilityLevel( Attacker, Warstomp_Based_ID, Level ) solely, the if statement is not really needed.
 
Level 2
Joined
Feb 26, 2012
Messages
8
Warstomp takes some time to cast
How is that? It has 0s casting time.

You can a) remove this ability after short delay instead of immidiately
How? Wait is not MUI, is there another way how to delay function from being called?

b) create a dummy unit which will cast this for you.
This would require to create another hero unit. It will show as icon in top left corner, would not it?

Also make sure this ability is truely based on Warstomp or Channel (with id: stomp) before doing anything else.
Are you sure the order parameter for IssueImmediateOrder is an spell ID not spell command?

Btw: use call SetUnitAbilityLevel( Attacker, Warstomp_Based_ID, Level ) solely, the if statement is not really needed.
Ok.
 

Bannar

Code Reviewer
Level 26
Joined
Mar 19, 2008
Messages
3,140
How is that? It has 0s casting time.
0 cast time doesn't mean ability has 0 casting point. Those "instant" abilities are usually limited to berserk, windwalk, avatar and such.
How? Wait is not MUI, is there another way how to delay function from being called?
Waits? Who talks about waits? TimerUtils or constantly looping timer with list structure is the answer. If you choose TimerUtils, an Unit-Indexer library might help you retrieve actual unit.
This would require to create another hero unit. It will show as icon in top left corner, would not it?
Nope, it would not require hero unit creation. Just create a dummy unit, add to it ability (not hero-ability) which will act as an "effector" for your warstomp.
Are you sure the order parameter for IssueImmediateOrder is an spell ID not spell command?
Channel is unique ability which can be "base" for any spell (id wise). Plus, you are not really calling orders via id here. If you were, you would write: call IssueImmediateOrderById(unit, 852127)
 
Level 2
Joined
Feb 26, 2012
Messages
8
Channel is unique ability which can be "base" for any spell (id wise)
I know that. I meant that you have a spell ID (for my custom spell, ID = A006) and spell command (derivated from warstomp is just "stomp" without quotes)

Waits? Who talks about waits? TimerUtils or constantly looping timer with list structure is the answer.
I will check out these.

Nope, it would not require hero unit creation. Just create a dummy unit, add to it ability (not hero-ability) which will be in fact act as "effector" for your warstomp.
That is the trouble. It is hero ability - I require different ability stats (dmg, stun time) for each level. I suppose it is not allowed to set more than one level for non hero ability.
 

Bannar

Code Reviewer
Level 26
Joined
Mar 19, 2008
Messages
3,140
That is the trouble. It is hero ability - I require different ability stats (dmg, stun time) for each level. I suppose it is not allowed to set more than one level for non hero ability.
Not only it is allowed, it is frequently used by mapmakers throughout warcraft3 custom maps universe.

Adding/Removing/Setting lvl works in such case exactly the same as if you would be dealing with heroes.

If you would have trouble finding mentioned libraries / further spell development - don't be afraid to ask here.
 
Level 2
Joined
Feb 26, 2012
Messages
8
Now I realize, that the spell ignores the cooldown. I saw some links which say it could be caused by moving/issuing unit. It is necessary to set cooldown by script?
 

Bannar

Code Reviewer
Level 26
Joined
Mar 19, 2008
Messages
3,140
Script doesnt have "cooldown". "Triggers" basically allow you to program connection described in most simple way as: event (some action) -> execution code (some reaction).

Considering those facts, you have to set cooldown in object editor for given spell. Script takes care of it's effect, not the object data related to that spell since those have no real connection whatsoever (except for spells id and lvl) :)
 
Level 2
Joined
Feb 26, 2012
Messages
8
Script doesnt have "cooldown". "Triggers" basically allow you to program connection described in most simple way as: event (some action) -> execution code (some reaction).
I know that. Maybe, I expressed it poorly.

I have a custom ability with defined cooldown in Object Editor. There is also a trigger connected to cast event of this ability. After unit casts this ability, it "jumps" to the location of the ability target. The problem is, the spell does not get on cooldown. I can reuse it right after "jumping" again. Even the cooldown is around 15s.

I heard it could be caused by using "Issue" or "SetLocation" functions of unit. That is why I asked you if there is some workaround for this.

On the other hand, it is possible that just wrong base spell was chosen. I will try to use another one if you assure me I am wrong with "Issue" and "SetLocation".
 
Level 2
Joined
Feb 26, 2012
Messages
8
Using SetUnitX/Y solved the problem. However, I still get small delay after ability casting before the unit starts "jumping" to the target area.

There is my script:

JASS:
////////////////////////////////////////////////////////////////////
// Spell:   Rozbeh
// Postava: Dawek
// Popis:   Caster se rozebehne na cilovou lokaci a vsechny nepratele
//          v dosahu po dobehu stunne.
//
// Verze: 1.0
//
////////////////////////////////////////////////////////////////////

scope Rozbeh initializer Init

//==================================================================
// NASTAVENI SPELLU - TUTO CAST JE NUTNE PRI PREVODU MAPY ZMENIT 
//==================================================================

    globals

        // ID spellu, na ktery bude trigger navazan
        private constant integer ID = 'A00C'
    
        // ID spellu, ktery bude pouzit po pristani
        private constant integer STUN_ID = 'A006'
        
        // Prikaz u base spellu od ktereho je stun odvozen
        private constant string STUN_BC = "stomp"
        
        // ID jednotky pro pouziti jako dummy
        private constant integer DUMMY_ID = 'n000'
    
        // Rychlost skoku. Pro vetsi range spellu je dobre
        // i zvysit rychlost.
        private constant real SPEED =   50.00
        
        // Rozsah stunu po dopadu na cil
        private constant real SPLASH =  100.00
        
        // Efekt prehrany po doskoku do cile
        // Pozor: Kazde zpetne lomeno musi byt v adrese uvedeno dvakrat!
        private constant string EFFECT_LANDING = "Abilities\\Spells\\Human\\ThunderClap\\ThunderClapCaster.mdl"
        
        // Efekt pridany postave behem skoku
        // Pozor: Kazde zpetne lomeno musi byt v adrese uvedeno dvakrat!
        private constant string EFFECT_FLYING = "Abilities\\Weapons\\WaterElementalMissile\\WaterElementalMissile.mdl"
    
    endglobals

//==================================================================
// ZACATEK SAMOTNEHO SPELLU - NEMENIT!
//==================================================================

// ===PRIVATNI DATA SPELLU==========================================
    private struct Info
        // Startovni pozice, ze ktere se vyskakuje
        location    Start
        real        Distance = 0
        
        // Cilova pozice, kam chceme skocit
        location    Target
        
        // Urcuje, jak daleko uz jme skocili. 
        // Pokud je Position rovne vzdalenosti Start - Target jsme v cili
        real        Position = 0
        
        // Promenne obsahujici efekty, ktere po sobe musime uklidit
        effect      FlyingEffect
        effect      LandingEffect
        
        // Jednotka ktera utoci
        unit        Caster
        
        timer       Timer
        
        // Funkce vytvori instanci Infa a vrati ji
        static method create takes unit caster, location target returns Info
        
            // Vytvorime novou instanci Infa
            local Info data = Info.allocate()
            
            // Nastavime data
            set data.Start = GetUnitLoc( caster )
            set data.Target = target
            set data.Caster = caster
            set data.Distance = DistanceBetweenPoints( data.Start, data.Target )
            set data.Position = 0
            set data.FlyingEffect = AddSpecialEffectTarget( EFFECT_FLYING, caster, "origin" )
            set data.LandingEffect = null // Bude naplneno pozdeji
            set data.Timer = NewTimer()
            
            return data
       
        endmethod
        
        // Funkce uklidi po ukonceni
        method onDestroy takes nothing returns nothing
            
            call DestroyEffect( .FlyingEffect )
            set .FlyingEffect = null
            
            call RemoveLocation( .Start )
            
            call DestroyEffect( .LandingEffect )
            set .LandingEffect = null
            
            call RemoveLocation( .Target )
            set .Target = null
            
            call RemoveUnit( .Caster )
            set .Caster = null
            
            call PauseTimer( .Timer )
            set .Timer = null
             
        endmethod
    endstruct
    
// ===FUNKCE SPELLU=================================================        

    // Znici dummy
    private function RemoveDummy takes nothing returns nothing
        
        call KillUnit( GetTriggerUnit() )
        
    endfunction

// ======================================================================================
    
    private function OnLanding takes Info data returns nothing
    
        // Jednotka vytvorena po doskoku pro vyvolani spellu stun
        local unit Dummy
        
        // Trigger navazany na dummy
        local trigger DummyTrg = CreateTrigger()
        
        call BJDebugMsg( "Na miste" )
        
        // Pridame k triggeru akci
        call TriggerAddAction( DummyTrg, function RemoveDummy )
        
        // Smazeme efekt letu
        call DestroyEffect( data.FlyingEffect )
        
        // Vytvorime dummy
        set Dummy = CreateUnitAtLoc( GetOwningPlayer( data.Caster ), DUMMY_ID, data.Target, 0.00 )
        
        // Naucime ji spravny spell
        call UnitAddAbility( Dummy, STUN_ID )
        
        // Dame ji odpovidajici uroven
        call SetUnitAbilityLevel( Dummy, STUN_ID, GetUnitAbilityLevel( data.Caster, ID ) )
        
        // Vycastime spell
        call IssueImmediateOrder( Dummy, STUN_BC )
        
        // Na vycasteni navazeme trigger, ktery jednotku po dokonceni smaze
        call TriggerRegisterUnitEvent( DummyTrg, Dummy, EVENT_UNIT_SPELL_FINISH )
        set DummyTrg = null
        
        // Pridame efekt dopadu
        set data.LandingEffect = AddSpecialEffectLoc( EFFECT_LANDING, data.Target )
        call DestroyEffect( data.LandingEffect )
        
        // Uklid
        set DummyTrg = null
        
        call RemoveUnit( Dummy )
        set Dummy = null
    
    endfunction
    
    private function OnJumping takes nothing returns nothing
    
        local Info data = Info( GetTimerData( GetExpiredTimer() ) )
        
        // Nova pozice
        local location NewPosition

        // Nastavime novou pozici na pozici plus posun o rychlost
        set data.Position = data.Position + SPEED
        
        // Pokud bychom pri pristim posunu preskocili cil
        if data.Position > data.Distance then
            // Skocime rovnou na cil
            set data.Position = data.Distance
        endif
            
        // Zjistime novou pozici jednotky po posunu
        set NewPosition = PolarProjectionBJ( data.Start, data.Position, GetUnitFacing( data.Caster ) )
            
        // Nastavime novou pozici. Pouziti setUnitPositionLoc by bylo rychlejsi, ale blbne s cooldownem
        call SetUnitX( data.Caster, GetLocationX( NewPosition ) )
        call SetUnitY( data.Caster, GetLocationY( NewPosition ) )
        
        // Doleteli jsme, konec
        if data.Position == data.Distance then
        
            //call BJDebugMsg( "Pripravujeme se na dopad" )
            call PauseTimer( data.Timer )
            call ReleaseTimer( data.Timer )
            
            call RemoveLocation( NewPosition )
            set NewPosition = null
                
            call OnLanding( data )
        endif
    
    endfunction
 

// ======================================================================================
    // Funkce kontrolujici, zda jde opravdu o spell, na ktery chceme reagovat
    private function Conditions takes nothing returns boolean
    
        local Info data
        if GetSpellAbilityId() == ID then
            set data = Info.create( GetTriggerUnit(), GetSpellTargetLoc() )
            
            // Zamerime jednotku utocnika celem k misto skoku
            call SetUnitFacingTarget( data.Caster, data.Target )
        
            // Nacteme do timeru data
            call SetTimerData( data.Timer, integer( data ) )
        
            ////////////////////////////////////////////////////////
            // LOOP
            ////////////////////////////////////////////////////////
        
            // Aktivujeme timer
            call TimerStart( data.Timer, 0.03, true, function OnJumping )
            
        endif
        
        return false
    
    endfunction
   
// ======================================================================================   
    // Tato funkce vytvori novou instanci teto scope
    private function Init takes nothing returns nothing
    
        local trigger RozbehTrg = CreateTrigger()
        
        call TriggerRegisterAnyUnitEventBJ( RozbehTrg, EVENT_PLAYER_UNIT_SPELL_EFFECT )
        call TriggerAddCondition( RozbehTrg, Condition( function Conditions ) )
        
        set RozbehTrg = null
    
    endfunction
    
endscope

Comments are written in Czech language, but I do not think it is necessary to translate them to English. The code should be self explanatory.

Do not blame me too much about the script. I am new to vJass. :)
Thank you.
 
Some feedback on your script:

*You don't need to null struct members when you destroy the struct. Structs are actually a set of global arrays where the struct instance is just the index (an integer).

*The difference between static and regular methods is that regular methods have a "this" variable specified. You can make a static function act like a regular one by defining "this", which can make the "create" function look a lot cleaner:

JASS:
static method create takes unit caster location target returns nothing
    local thistype this = thistype.allocate() //"thistype" is automatically replaced by the name of the struct. You can use the name of your struct instead if you want, if it makes it more readable for you.
    set .caster = caster //You can now just use the dot operator without any prefix, just like in regular methods!
    set .target = target
    //etc....
    return this
endmethod

*Instead of doing PolarProjection every loop, instead save a "vel_x" and "vel_y" variable, and set them on struct creation, like this:
JASS:
set .vel_x = SPEED*Cos(GetUnitFacing(caster)*bj_DEGTORAD)
set .vel_y = SPEED*Sin(GetUnitFacing(caster)*bj_DEGTORAD)

In the update function, just add .vel_x and .vel_y to the x and y position of the unit to move it. This way you don't have to call the trigonometric functions over and over again, which is more effective since these functions are quite heavy. You also don't have to use locations.

*You can save yourself a couple of effect variables in the struct by just doing this:

JASS:
call DestroyEffect(AddSpecialEffectLoc(....))

*Instead of using timer data (which is still an OK approach), you can learn how to use LinkedLists. Use ListModule by Grim001 and implement it into your struct. Then, in the OnJumping function, you can just iterate through all instances of the struct. Read the documentation thoroughly. If .count == 0 as you remove an instance from the list, you can pause the timer. If count == 1 after you create an instance, you can start the timer. You will also just need one timer. Let me know if you want samples on how to make this - once you learn how to use linked lists, it will revolutionise how you write your code, i promise.

Other than that, you do really nice coding for a newbie! It is very clean and readable.
 
Last edited:
Level 2
Joined
Feb 26, 2012
Messages
8
This is a new version using ListModule you suggested:

JASS:
////////////////////////////////////////////////////////////////////
// Spell:   Rozbeh
// Postava: Dawek
// Popis:   Caster se rozebehne na cilovou lokaci a vsechny nepratele
//          v dosahu po dobehu stunne.
//
// Pozadavky: ListModule, TimerUtils
//
// Verze: 1.1
//
////////////////////////////////////////////////////////////////////

scope Rozbeh initializer Init

//==================================================================
// NASTAVENI SPELLU - TUTO CAST JE NUTNE PRI PREVODU MAPY ZMENIT 
//==================================================================

    globals

        // ID spellu, na ktery bude trigger navazan
        private constant integer ID = 'A00C'
    
        // ID spellu, ktery bude pouzit po pristani
        private constant integer STUN_ID = 'A006'
        
        // Prikaz u base spellu od ktereho je stun odvozen
        private constant string STUN_BC = "stomp"
        
        // ID jednotky pro pouziti jako dummy
        private constant integer DUMMY_ID = 'n000'
    
        // Rychlost skoku. Pro vetsi range spellu je dobre
        // i zvysit rychlost.
        private constant real SPEED =   50.00
        
        // Rozsah stunu po dopadu na cil
        private constant real SPLASH =  100.00
        
        // Efekt prehrany po doskoku do cile
        // Pozor: Kazde zpetne lomeno musi byt v adrese uvedeno dvakrat!
        private constant string EFFECT_LANDING = "Abilities\\Spells\\Human\\ThunderClap\\ThunderClapCaster.mdl"
        
        // Efekt pridany postave behem skoku
        // Pozor: Kazde zpetne lomeno musi byt v adrese uvedeno dvakrat!
        private constant string EFFECT_FLYING = "Abilities\\Weapons\\WaterElementalMissile\\WaterElementalMissile.mdl"
    
    endglobals

//==================================================================
// ZACATEK SAMOTNEHO SPELLU - NEMENIT!
//==================================================================

    globals
        private timer g_Timer
    endglobals

// =================================================================
//  STRUKTURA INFO A PRIDRUZENE FUNKCE
// =================================================================
    private struct Info
        implement List
    
        ////////////////////////////////////////////////////////////
        // Privatni data
    
        // Startovni pozice, ze ktere se vyskakuje
        location    Start
        real        Distance = 0
        
        // Cilova pozice, kam chceme skocit
        location    Target
        
        // Urcuje, jak daleko uz jme skocili. 
        // Pokud je Position rovne vzdalenosti Start - Target jsme v cili
        real        Position = 0
        
        // Promenne obsahujici efekty, ktere po sobe musime uklidit
        effect      FlyingEffect
        
        // Jednotka ktera utoci
        unit        Caster
        
        real        Velocity_X
        real        Velocity_Y
        
        ////////////////////////////////////////////////////////////
        // Funkce

        // Funkce vytvori novou instanci teto struktury
        static method create takes unit caster, location target returns Info
        
            // Pro cisci kod si zajistime dostupnost this
            local thistype this = thistype.allocate()

            // Pridame instanci na seznam
            call listAdd()
            
            // Nastavime data
            set .Start = GetUnitLoc( caster )
            set .Target = target
            set .Caster = caster
            set .Distance = DistanceBetweenPoints( .Start, .Target )
            set .Position = 0
            set .FlyingEffect = AddSpecialEffectTarget( EFFECT_FLYING, caster, "origin" )
            
            // Posun na ose X a Y 
            set .Velocity_X = SPEED*Cos(GetUnitFacing(.Caster)*bj_DEGTORAD) 
            set .Velocity_Y = SPEED*Sin(GetUnitFacing(.Caster)*bj_DEGTORAD)

            return this
       
        endmethod
        
        // Funkce uklidi po ukonceni
        method onDestroy takes nothing returns nothing
            
            call DestroyEffect( .FlyingEffect )
            
            call RemoveLocation( .Start )

            call RemoveLocation( .Target )
             
            call listRemove() 
        endmethod
    endstruct

// =================================================================
// FUNKCE SPELLU
// =================================================================

    // Znici dummy
    private function RemoveDummy takes nothing returns nothing
        
        call KillUnit( GetTriggerUnit() )
        
    endfunction

// =================================================================
    
    // Funkce zpracuje jednotku po pristani a provede potrebne efekty
    private function OnLanding takes Info data returns nothing
    
        // Jednotka vytvorena po doskoku pro vyvolani spellu stun
        local unit Dummy
        
        // Trigger navazany na dummy
        local trigger DummyTrg = CreateTrigger()
        
        // Pridame k triggeru akci
        call TriggerAddAction( DummyTrg, function RemoveDummy )
        
        // Smazeme efekt letu
        call DestroyEffect( data.FlyingEffect )
        
        // Vytvorime dummy
        set Dummy = CreateUnitAtLoc( GetOwningPlayer( data.Caster ), DUMMY_ID, data.Target, 0.00 )
        
        // Naucime ji spravny spell
        call UnitAddAbility( Dummy, STUN_ID )
        
        // Dame ji odpovidajici uroven
        call SetUnitAbilityLevel( Dummy, STUN_ID, GetUnitAbilityLevel( data.Caster, ID ) )
        
        // Vycastime spell
        call IssueImmediateOrder( Dummy, STUN_BC )
        
        // Na vycasteni navazeme trigger, ktery jednotku po dokonceni smaze
        call TriggerRegisterUnitEvent( DummyTrg, Dummy, EVENT_UNIT_SPELL_FINISH )
        set DummyTrg = null
        
        // Pridame efekt dopadu a ihned ho odstranime. Tim zamezime, aby se prehral vicekrat
        call DestroyEffect( AddSpecialEffectLoc( EFFECT_LANDING, data.Target ) )
        
        // Vycistime instanci tohoto spellu
        call data.destroy()
        
        // Pokud jiz nemame zadnou instanci, muzeme ukoncit timer
        if ( Info.count == 0 ) then
            // Pozastavi Timer
            call PauseTimer( g_Timer )
            // Uvolni timer
            call ReleaseTimer( g_Timer )
        endif
        
        // Uklid
        set DummyTrg = null
        
        call RemoveUnit( Dummy )
        set Dummy = null
    
    endfunction

// =================================================================    
    
    // Funkce projde vsechny prave jednotky pouzivajici tento spell
    // a postara se o jejich pohyb.
    private function OnJumping takes nothing returns nothing
    
        // Projdeme vsechny instance teto struktury
        local Info data = Info.first

        loop
            exitwhen data == 0
            
            // Nastavime novou pozici na pozici plus posun o rychlost
            set data.Position = data.Position + SPEED
        
            // Pokud bychom pri pristim posunu preskocili cil
            if data.Position > data.Distance then
                // Skocime rovnou na cil
                set data.Position = data.Distance
            endif
            
            // Nastavime pozici utocnika. 
            call SetUnitX( data.Caster, GetUnitX( data.Caster ) + data.Velocity_X )
            call SetUnitY( data.Caster, GetUnitY( data.Caster ) + data.Velocity_Y )
        
            // Doleteli jsme, konec
            if data.Position == data.Distance then
                call OnLanding( data )
            endif
            
            // iterace
            set data = data.next
        endloop
    
    endfunction

// =================================================================

    // Funkce kontrolujici, zda jde opravdu o spell, na ktery chceme reagovat
    private function Conditions takes nothing returns boolean
    
        local Info data
        
        // Pokud pouzita abilita souhlasi s ID spellu
        if GetSpellAbilityId() == ID then
            
            // Vytvorime novou instanci 
            set data = Info.create( GetTriggerUnit(), GetSpellTargetLoc() )
            
            // Zamerime jednotku utocnika celem k misto skoku
            call SetUnitFacingTarget( data.Caster, data.Target )
        
            // Pokud jde o prvni intanci, vytvorime i timer
            if ( Info.count == 1 ) then
                
                // Vytvorime timer
                set g_Timer = NewTimer()
                
            endif
        
            // Aktivujeme timer
            call TimerStart( g_Timer, 0.03, true, function OnJumping )
            
        endif
        
        return false
    
    endfunction
   
// =================================================================
   
    // Tato funkce vytvori novou instanci teto scope
    private function Init takes nothing returns nothing
    
        local trigger RozbehTrg = CreateTrigger()
        
        call TriggerRegisterAnyUnitEventBJ( RozbehTrg, EVENT_PLAYER_UNIT_SPELL_EFFECT )
        call TriggerAddCondition( RozbehTrg, Condition( function Conditions ) )
        
        set RozbehTrg = null
    
    endfunction
    
endscope

I am not sure about destroying and recreating Timers:
JASS:
private function OnLanding takes Info data returns nothing
    
    // ...

    if ( Info.count == 0 ) then
        // Pause timer
        call PauseTimer( g_Timer )
        // Release timer
        call ReleaseTimer( g_Timer )
    endif

    // ...
        
endfunction

and

JASS:
private function Conditions takes nothing returns boolean
        
    // ...

    if ( Info.count == 1 ) then
        set g_Timer = NewTimer()
    endif
    
    // ...

endfunction

I tried to avoid timer from leaking, but I am not sure this is the right way to do so.

In additional, I still get small delay between the spell is used and moving the caster unit. It is the same it was with the last version.
 

Bannar

Code Reviewer
Level 26
Joined
Mar 19, 2008
Messages
3,140
ListModule is outdated, this is much better.

onDestroy is big nonono, besides, you dont need to create trigger and initialize it with all of other stuff just to kill dummy after short delay. Increase expiration timer (UnitApplyTimedLife function) if need be.

Below you find kind of a template of how you could efficiently code any spell that involves periodic operations.

Ask any questions you want if you dont understand something. And as you see, since you have already implemented struct, you can merge functions and add them as struct members.
Note: I'm not sure if this actually works :) You need to check by yourself, code compiles. This could also be improved to reduce variable count, however, its not the case now.
JASS:
scope Rozbeh

    globals
        private constant integer ID                = 'A00C'
        private constant integer STUN_ID           = 'A006'
        private constant integer STUN_BC           = 852127
        private constant integer DUMMY_ID          = 'n000'
        private constant real    SPEED             = 50.00
        private constant real    SPLASH            = 100.00
        private constant string  EFFECT_LANDING    = "Abilities\\Spells\\Human\\ThunderClap\\ThunderClapCaster.mdl"
        private constant string  EFFECT_FLYING     = "Abilities\\Weapons\\WaterElementalMissile\\WaterElementalMissile.mdl"
    endglobals

/**
    Internal
*/

    globals
        private timer looper = CreateTimer()
    endglobals

    private struct Info extends array
		static integer count = 0
		thistype recycle
		thistype next
		thistype prev

        unit caster
        real casterX
        real casterY
        real targetX
		real targetY

		real distance
        real position
		real velocityX
        real velocityY
        effect flyingEffect

        method deallocate takes nothing returns nothing
            set this.recycle = thistype(0).recycle
            set thistype(0).recycle = this
            set this.next.prev = this.prev
            set this.prev.next = this.next

            if ( thistype(0).next == 0 ) then
                call PauseTimer(looper)
            endif
        endmethod

        method destroy takes nothing returns nothing            
            call DestroyEffect(flyingEffect)
			set flyingEffect = null
            set caster = null

			call deallocate()
        endmethod

		private static method onJumping takes nothing returns nothing
			local Info data = thistype(0).next
			local unit dummy

			loop
				exitwhen data == 0
				set data.position = data.position + SPEED
				
				call SetUnitX( data.caster, GetUnitX( data.caster ) + data.velocityX )
				call SetUnitY( data.caster, GetUnitY( data.caster ) + data.velocityY )

				if data.position > data.distance then
					set dummy = CreateUnit( GetOwningPlayer(data.caster), DUMMY_ID, data.targetX, data.targetY, 0.00 )
					call UnitAddAbility( dummy, STUN_ID )
					call SetUnitAbilityLevel( dummy, STUN_ID, GetUnitAbilityLevel(data.caster, ID) )
					call IssueImmediateOrderById( dummy, STUN_BC )
					call UnitApplyTimedLife(dummy, 'BTLF', 1)

					call DestroyEffect( AddSpecialEffect( EFFECT_LANDING, data.targetX, data.targetY ) )
					call data.destroy()
				endif

				set data = data.next
			endloop

			set dummy = null
		endmethod

        static method allocate takes nothing returns thistype
            local thistype this = thistype(0).recycle
            if ( thistype(0).next == 0 ) then
                call TimerStart(looper, 0.031250000, true, function thistype.onJumping)
            endif

            if ( this == 0 ) then
                set count = count + 1
                set this = count
            else
                set thistype(0).recycle = this.recycle
            endif

            set this.next = 0
            set this.prev = thistype(0).prev
            set thistype(0).prev.next = this
            set thistype(0).prev = this

            return this
        endmethod

		private static method onCast takes nothing returns boolean
			local Info data
			local real dx
			local real dy

			if ( GetSpellAbilityId() == ID ) then
				set data = Info.allocate()
				set data.caster = GetTriggerUnit()
				set data.casterX = GetUnitX(data.caster)
				set data.casterY = GetUnitY(data.caster)
				set data.targetX = GetSpellTargetX()
				set data.targetY = GetSpellTargetY()

				set dx = data.targetX - data.casterX
				set dy = data.targetY - data.casterY
				set data.distance = SquareRoot(dx*dx + dy*dy)
				set data.position = 0

				set data.flyingEffect = AddSpecialEffectTarget( EFFECT_FLYING, data.caster, "origin" )
				set data.velocityX = SPEED * Cos(GetUnitFacing(data.caster)*bj_DEGTORAD) 
				set data.velocityY = SPEED * Sin(GetUnitFacing(data.caster)*bj_DEGTORAD)

				// call SetUnitFacingTarget( data.Caster, data.Target ) ???
			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.onCast )

			set t = null
		endmethod
    endstruct

endscope
 
There is nothing wrong about ListModule, it works perfectly if you only want to iterate through instances of a struct, and its API is more readable than Diracs. Struct extends array is also just..... ugh. Don't. I choose readability before miniscule speed gains anytime. And like i said, there is also no need to null struct members since they are globals. You only null them if you ever want to compare their value to null, but even then i am not sure it is neccesary. I also don't see why you feel the need to merge all your functions. Sure, functions in Jass use string lookups which are kinda slow, but unless performance is an issue, it is often better to use informative function names to help yourself and others keep track of your code.

I made my own code for the struct which i personally think is a lot neater. I did not include the library and its globals, but you can add that yourself:

JASS:
struct jump

    implement List

    unit caster

    real x = 0 //I perfer to have my members start with lower case letters.
    real y = 0

    real tar_x = 0 //We don't use locations since they are lame.
    real tar_y = 0

    real vel_x = 0
    real vel_y = 0

    real distance = 0
    real position = 0 //Since we initialise this here, we don't need to do so in the create method.

    effect flyingEffect

    static timer t = null //Static members are the same as global variables, except private for the struct.

    method onDestroy takes nothing returns nothing
        call DestroyEffect(.flyingEffect)
        call .listRemove()
        if .count == 0 then
            call PauseTimer(.t) //We can stop the timer if there are no units jumping.
        endif
    endmethod

    method onFinish takes nothing returns nothing
        local unit dummy = CreateUnit(GetOwningPlayer(.caster), DUMMY_ID, .tar_x, .tar_y, 0)

        call UnitAddAbility(dummy, STUN_ID)
        call IssueImmediateOrder(dummy, STUN_BC)
        call UnitApplyTimedLife(dummy, 'BTLF', 2) //This will cause the dummy to dissapear after 2 seconds, also it doesn't leak a trigger like your solution.
        call DestroyEffect(AddSpecialEffect(EFFECT_LANDING, .tar_x, .tar_y))
        call .destroy()

        set dummy = null
    endmethod

    static method onUpdate takes nothing returns nothing
        local thistype this = .first
        local thistype temp

        loop
            exitwhen this == 0
            set temp = .next //Since we might be destroying "this" inside the loop, it is safe to save the next member of the list - otherwise the loop will crash when it gets an invalid instance.
	
            set .position = .position+SPEED

            set .x = .x+.vel_x
            set .y = .y+.vel_y

            call SetUnitPosition(.caster, .x, .y) //This is safer since it checks if the position is valid. Otherwise you might end up on the bottom of the ocean, or inside a tree!
            //Best would be to first check if the point is pathable before you move the unit. There are many ways of doing this which all have their pros and cons.

            if .position > .distance then //Finish if we have reached our destination.
                call .onFinish()
            endif

            set this = temp //If we ran onFinish this loop, we will still know the next instance of the list, since we saved it.
        endloop
    endmethod

    static method create takes unit caster, real tx, real ty returns thistype
        local thistype this = thistype.allocate()
        local real dx = tx-GetUnitX(caster)
        local real dy = ty-GetUnitY(caster)

        call .listAdd()

        set .caster = caster

        set .tar_x = tx
        set .tar_y = ty

        set .vel_x = SPEED*Cos(GetUnitFacing(caster)*bj_DEGTORAD)
        set .vel_y = SPEED*Sin(GetUnitFacing(caster)*bj_DEGTORAD)

        set .distance = SquareRoot(dx*dx+dy*dy) //This is the same as DistanceBetweenPoints

        set .flyingEffect = AddSpecialEffectTarget(EFFECT_FLYING, caster, "origin")

        call SetUnitFacing(caster, Atan2(dy, dx)*bj_RADTODEG) //Some simple trigonometry will get you the angle

        if .t == null then
            set .t = NewTimer() //You could do this in a static onInit method instead, so it doesn't have to run all the time, but meh
        endif

        if .count == 1 then //We start the timer if this is the only instance, i.e. if there used to be none.
            call TimerStart(.t, 0.03, true, function thistype.onUpdate)
        endif


        return this
    endmethod

endstruct

To make a unit jump, just put this inside your trigger:
JASS:
call jump.create(GetTriggerUnit(), GetSpellTargetX(), GetSpellTargetY())

The rest will handle itself.
 

Bannar

Code Reviewer
Level 26
Joined
Mar 19, 2008
Messages
3,140
@Fingolfin - have you ever read spells/jass resources submission rules?
If so, you wouldn't write multiple times about nulling the variables. Here, we actually do null variables (that store handles) when instance gets deallocated.

onDestroy is big no. Don't teach someone bad habbits from the begining. There are plenty of Jass/vJass tutorials in Tutorials section and as you probably know, none of them advices to use either onDestroy or "structs" without "exteds array". Although basic structs are not as crappy, still it's better to actually implement your own alloc/dealloc or just use Alloc.

And no, you are wrong about readability. I doubt that your outdated script is any more readable than mine version.
I haven't even used Diracs linked list here, in case, I prefer showing EVERYTHING to guys new in vJass. Afterall, it's prefered for them to know that methods such as allocate/deallocate do exist. On top of that, coding thier own create/destroy method also improves their struct knowledge.

Btw, when "jumping" its better to use SetUnitX/Y than SetUnitPosition. When you need safety (pathing/is terrain walkable) you use certain resources for that, rather than trusting this native.
"0.03" periodic is also bad of you - reals are very inacrurate, plus 0.03 is kinda unspecified. We either use 0.05 or 0.03125000 to recieve 20 or 32 calls per second respectively.
About the outdated list.
 
onDestroy uses TriggerEvaluate which indeed is bad (although not a big deal in this case). Last time i read the jasshelper manual though it stated that overwriting the destroy method is not legal. Correct me if i am wrong. I also missed the fact that you nulled the variables in your own destroy method which i guess is proper. Still kinda useless though since they will be overwritten again on allocation, so you will never have to deal with them in the first place. Also, i really don't see the problem with the automatically generated allocator:

JASS:
function s__a__allocate takes nothing returns integer
 local integer this=si__a_F
    if (this!=0) then
        set si__a_F=si__a_V[this]
    else
        set si__a_I=si__a_I+1
        set this=si__a_I
    endif
    if (this>8190) then 
        return 0
    endif

    set si__a_V[this]=-1
 return this
endfunction

This is virtually the same as what you're doing. You are just writing a bunch of code which you really don't need to see as a user. All this system does is to move a unit forward, and it doesn't even run all the time. And do you really think warcraft cares if you run your function an even number of times per second?? I feel like you've misunderstood, well, math in general. It doesn't matter what the interval is, you can just multiply your values with the interval in the create method if it is important for them to be inputted with a per-second unit.

It is good coding practice to use dot operators to signify that you are referencing members and not globals or locals. Defining a "this" variable in your static methods is also a neat way of removing repetative references to the instance.

About the "outdated list", funny that the problem you linked was already fixed in the module. You also never encounter the issue with calling from invalid structs in my implementation. Diracs system forces you to use structs extending array.

Array structs are great if you already know the desired index of a struct, like if you want to use a player or units index as the struct index (when you have a unit indexer), or if you need them to be allocated in an otherwise specific way. However, they do not support default values and cannot contain arrays (you have to define arrays as a type and then include them, which is kinda lame). You are replacing all the things that make structs usable whith what we used to have before vJass, a set of arrays and indexes.

You are right about SetUnitPosition, since in this particular case we want to fly. Not that the unit actually flies, it will just kinda slide across the ground. I would have suggested an item-based approach for checking pathability, but i didn't want to overwhelm the OP with stuff which are kinda beyond the scope of his question.

About that question, i think the reason for your delay is the base ability. Try making a dummy ability using Channel instead, set "art - duration" and "follow through time" to 0, make sure "visible" and "targeting image" are checked.
 
Last edited:

Bannar

Code Reviewer
Level 26
Joined
Mar 19, 2008
Messages
3,140
You are being stubborn a bit.

I'm not forcing your or anyone else to use what "I think is better". It's up to you what you use afterall.

You do not overwrite destroy method since when you extends an array, entire allocation/deallocation is up to user.
You use "." before your struct/class members within object's behaviour functions (c, c++, perl whateva)? I don't think so ^)^
However, if you really like that dot, go ahead =)

By extending an array you also gain few additional advantages:
- reduce variable count
- removal of double free protection
- possiblity to merge create/allocate with destroy/deallocate which is impossible with standard structs.

About periodic issue, honestly it's kind of standard that we use 1/32 or 1/20 (second) here.

Afterall, there is a reason that, 99% vJass resources follow trends which I've been describing within last few posts.
 
Well, from my perspective you are the one being stubborn, lol.

Saying that 99% of the vJass resources do this is a rather big, not to mention incorrect claim. You are trying to force completely reduntant rules onto people for no other reason than bigotry.

Yes, in many real programming languages you don't mark any difference between members and locals, but you also don't see no classes extending arrays in C#. ;)
On a serious note, i just think it is good courtesy to print out these things, but ofcourse there are no musts. The note i added about defining a "this" variable in static methods is a good way of keeping your code more laconic though.
 

Bannar

Code Reviewer
Level 26
Joined
Mar 19, 2008
Messages
3,140
Then head to Jass resources section, here on hive =)

You are trying to force completely reduntant rules onto people for no other reason than bigotry.
I've already stated once or twice that I'm not forcing anything, I'm just enlisting cons in regard to your approach while you basically write no constructive pros in return.

I haven't replaced "data" with "this" in case it will be easier for him to find out what is where and whats goin on. "this" is preffered, as you pointed out.
And yes, you can not deny that trends I've described are very popular and are desired both in Spell section and Jass resources section.

I'll not continue this, since its rather pointless, and thread should be more concerned about helping new comer to vJass.
 
And yes, you can not deny that trends I've described are very popular and are desired both in Spell section and Jass resources section.

The thing is, i feel like writing your own allocator is just that - a trend. You say that you are trying to give people a choice, but what you actually said was that my code was outdated and that i was teaching him wrong.

The pros of using normal structs are:

*You hide some low-level code which you really don't need to see
*Easier for new users since there is less for you to screw up
*You can use default values for your variables
*You can use arrays as members in an intuitive way
*Your code becomes much cleaner and more readable

And the cons are:

*onDestroy generates a trigger and a TriggerEvaluate call, which is not really a con at all since you can avoid it by simply not naming your destrcutor onDestroy and using it as a hook method instead.
*You are technically using two methods (create and allcoate) instead of just one, which is also not a performance issue either unless you are creating hundreds of instances per minute.
*Deallocation has an if-check for double free protection which some people find useless. Wait, what? One single if-check? I think i can live with that.

Array structs are great in cases such as:

*When you need to have control of your struct indexes
*When your struct has no instanced methods
*When you want to make your code look more complicated than it is so that you seem like a smartass

Finally, some systems which don't use array structs:
http://www.hiveworkshop.com/forums/jass-resources-412/system-shield-229158/
http://www.hiveworkshop.com/forums/jass-resources-412/system-knockback3d-217056/
http://www.hiveworkshop.com/forums/jass-resources-412/system-trackable2-124888/
http://www.hiveworkshop.com/forums/jass-resources-412/system-texttagutils-140954/
 
Status
Not open for further replies.
Top