Help with learning JASS

[izaiphovias]

So, I was reading a bunch of tutorials for JASS scripting here and also Vexorian's and am trying to get the hang of things.

In my new project I have a simple GUI spell based on Entangling Roots that hides the caster, creates 4 visible dummies around the target and damage the area, then it shows the caster again. So I decided to script this spell in JASS, as close as I could get to a JESP format, from scratch (without simply converting the trigger to JASS using WE).

This is the code i end up with:

//==================================================================================================//
//---------------------------------------------OVERPOWER--------------------------------------------//
//==================================================================================================//


//===============================================CONF===============================================//

//OVERPOWER ID
private function OP_AbilityID takes nothing returns integer
    return = 'a000'
endfunction

//PROJECTION EFFECT
private function OP_FXProjection takes nothing returns string
    return = "ProjFX" //PRETEND THIS IS THE INVISIBILITY EFFECT PATH
endfunction

//HIDE UNIT EFFECT
private function OP_FXHide takes nothing returns string
    return = "HideFX" //PRETEND THIS IS THE MASS TELEPORT CASTER EFFECT PATH
endfunction

//AOE EFFECT 1
private function OP_FXBlast1 takes nothing returns string
    return = "BlastFX1" //PRETEND THIS IS THE THUNDER CLAP EFFECT PATH
endfunction

//AOE EFFECT 2
private function OP_FXBlast2 takes nothing returns string
    return = "BlastFX2" //PRETEND THIS IS THE MANA FLARE EFFECT PATH
endfunction

//PROJECTION DUMMY UNIT ID
private function OP_ProjectionDummy takes nothing returns integer
    return = 'u000'
endfunction

//AOE DAMAGE TYPE
private function OP_DamageType takes nothing returns damagetype
    return = DAMAGE_TYPE_MAGIC
endfunction

//AOE WEAPON TYPE
private function OP_WeaponType takes nothing returns weapontype
    return = WEAPON_TYPE_WHOKNOWS
endfunction

//AOE ATTACK TYPE
private function OP_AttackType takes nothing returns attacktype
    return = ATTACK_TYPE_MAGIC
endfunction

//RADIUS OF THE AOE
private function OP_Range takes nothing returns real
    return = 300.
endfunction

//DAMAGE OF THE AOE
private function OP_DamageAOE takes nothing returns real
    return = 100. + 100. * GetUnitAbilityLevel(GetTriggerUnit(), OP_AbilityID())
endfunction

//===============================================COAC===============================================//

//VARIABLES
local unit op_Caster = GetTriggerUnit()
local player op_Owner = GetOwningPlayer(op_Caster)
local integer op_OwnerID = GetPlayerId(op_Caster)
local unit array op_Dummy
local unit op_Target = GetSpellTargetUnit()
local location op_PointTarget = GetUnitLoc(op_Target())
local location op_PointCaster = GetUnitLoc(op_Caster)
local integer i = 1
local real tx
local real ty
local effect array effectOP

//CONDITION
private function Trig_OP_Condition takes nothing returns boolean
    call GetSpellAbilityId() == OP_AbilityID
endfunction

//ACTIONS
private function Trig_OP_Actions takes nothing returns nothing
    //CREATE FX ON CASTER LOCATION, PAUSE AND HIDE THE CASTER
    set effectOP[1] = AddSpecialEffectLoc(OP_FXHide(), op_PointCaster)
    call DestroyEffect(effectOP[1])
    call PauseUnit(op_Caster, true)
    call UnitAddAbility(op_Caster, 'a001') //PRETEND THIS IS INVULNERABLE
    call ShowUnit(op_Caster, false)

    //CREATE FX ARROUND THE TARGET FOR THE DUMMY UNITS AND CREATE THE DUMMY UNITS
    loop
        exitwhen i > 4
        set i = i + 1
        if i == 1
            set tx = GetUnitX(op_Target)
            set ty = GetUnitY(op_Target) + 100
        elseif i == 2
            set tx = GetUnitX(op_Target) + 100
            set ty = GetUnitY(op_Target)
        elseif i == 3
            set tx = GetUnitX(op_Target)
            set ty = GetUnitY(op_Target) - 100
        elseif i == 4
            set tx = GetUnitX(op_Target) - 100
            set ty = GetUnitY(op_Target)
        endif
        set effectOP[i+1] = AddSpecialEffect(OP_FXProjection(), tx, ty)
        call DestroyEffect(effectOP[i+1])
        set op_Dummy[i] = CreateUnit(op_OwnerID, OP_ProjectionDummy(), tx, ty, op_PointTarget)
        call SetUnitAnimation(op_Dummy[i], "spell")
    endloop

    //CREATE THE BLAST FX ON TARGET LOCATION
    set effectOP[6] = AddSpellEffectLoc(OP_FXBlast1(), op_PointTarget)
    call DestroyEffect(effectOP[6])
    set effectOP[7] = AddSpellEffectLoc(OP_FXBlast2(), op_PointTarget)
    call DestroyEffect(effectOP[7])

    //DAMAGE THE AREA AROUND THE TARGET
    set tx = GetUnitX(op_Target)
    set ty = GetUnitY(op_Target)
    call UnitDamagePoint(op_Caster, OP_Range(), tx, ty, OP_DamageAOE, false, false, OP_AttackType(), OP_DamageType(), OP_WeaponType())

    //CREATE FX FOR THE DUMMY UNITS AND REMOVE THEM
    loop
        exitwhen i > 4
        set i = i + 1
        if i == 1
            set tx = GetUnitX(op_Target)
            set ty = GetUnitY(op_Target) + 100
        elseif i == 2
            set tx = GetUnitX(op_Target) + 100
            set ty = GetUnitY(op_Target)
        elseif i == 3
            set tx = GetUnitX(op_Target)
            set ty = GetUnitY(op_Target) - 100
        elseif i == 4
            set tx = GetUnitX(op_Target) - 100
            set ty = GetUnitY(op_Target)
        endif
        set effectOP[i+1] = AddSpecialEffect(OP_FXProjection(), tx, ty)
        call DestroyEffect(effectOP[i+1])
        call RemoveUnit(op_Dummy[i])
    endloop
    
    //CREATE FX ON CASTER LOCATION, UNPAUSE AND UNHIDE THE CASTER
    set effectOP[1] = AddSpecialEffectLoc(OP_FXHide(), op_PointCaster)
    call DestroyEffect(effectOP[1])
    call ShowUnit(op_Caster, true)
    call UnitRemoveAbility(op_Caster, 'a001') //PRETEND THIS IS INVULNERABLE
    call PauseUnit(op_Caster, false)
    endloop

endfunction

//===============================================INIT===============================================//

private function InitTrig_OP takes nothing returns nothing
    set gg_trg_OP = CreateTrigger()
    call TriggerRegisterPlayerUnitEvent(gg_trg_OP, Player(0), EVENT_UNIT_SPELL_EFFECT, Trig_OP_Condition)
    call TriggerAddAction(gg_trg_OP, function Trig_OP_Actions)
endfunction

I could not test this in WC as I don't have my gaming computer here, but I'm interested in some help identifying if this would work, could be improved and where it leaks, as I am not yet familiar with the JASS scripting good practices.

Would appreciate any feedback.

Thanks!

 

[Deleted member 219079]

What is JESP?

 

[izaiphovias]

As follows:

JESP stands for “JASS Enhanced Spell Pseudotemplate”. Both spells made in JASS and in vJASS can (and in my opinion should) follow this JESP standard and one of the reasons why vJASS is so good is because it is easier to follow this standard using it. However, I never saw any GUI spell following it, and I don’t even think such thing is possible.

Here is the JESP official document that all spells following this standard should have:
JESP document
This is the JESP standard document, if a map contains this document it means that there are
spells that follow this standard.

Spells of this map that follow the standard:
¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯
- “Spell Name 1”
- "Spell Name 2"
- "Spell Name N"

Advantages of the Standard
¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯
- Implementing spells that follow the standard is relatively easier than implementing JASS
spells that don't follow the standard.

- Configuring/Balancing spells that follow the standard is relatively easier than
implementing JASS spells that don't follow the standard.

- Users may do the following procedure to make a new ability that uses the spell's script :

* Create a new Trigger with a name (case sensitive)
* Convert that trigger to custom text.
* Copy the spell's script to a text editor like Notepad or your OS equivalent.
* Replace the spell's Code name with the name you used on the trigger.
* Copy the new text to the new trigger
* Duplicate the Spell's original objects to have new ones for the new spell script.

You are now able to use that new version of the spell.

- In case two guys give the same name to 2 different spells, there are no conflict problems
because you can easily change the name of one of them

What is the JESP Standard?
¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯
The JESP standard was designed to make spell sharing much better. And to make sure JASS
enhanced spells follow a rule, to prevent chaos.

What does JESP Standard stands for?
¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯
JASS
Enhanced
Spell
Pseudotemplate

Requirements for a spell to follow the JESP Standard
¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯
- The spell is written in JASS
- The spell is 100% multi instanceable.
- The spell script is ready to support spells of any number of levels.
(default config header is not required to support all of them)

- The Spell has an specific code name.

- The Spell's trigger must have the spell's codename as name

- The Spell's InitTrig function must be named: InitTrig_<CodeName>

- The spell has a configuration header.

- It is mandatory that rawcodes of objects are configurable in the header.

- All the spell's specific code is inside the spell's "Trigger" (Trigger== that custom text
slot that world editor calls Trigger, the spell may use as many 'trigger' OBJECTS as needed)

- Every spell-specific single identifier or key works in such a way that reproducing the
spell's trigger but after performing a text-replace of codename with another name (and thus
renaming the cloned trigger to the new code name) it won't cause compile errors / conflicts
when playing the map.

- There is no code inside the spell's "Trigger" that is not specific to the spell.

- There are no requirements for GUI variables that are specific to the spell. If a system
used by the spell requires GUI variables the code for the system must be outside the "Trigger"

- Eyecandy and spell's balance have to be easy to configure

- The name of the author should be included in the spell's script.

- The reason to exist of this standard is spell sharing. This document should be included within the map. And it should specify which spell follows the standard, in the top list.

I've edited the main post, correcting some syntax errors I've found.

 

[Flux]

This is the code i end up with:

I skimmed through it and saw lots of errors.
Returning a value does not need an equal sign.
return = 'a000'
->
return 'a000'

You can't declare locals outside a function

//VARIABLES
local unit op_Caster = GetTriggerUnit() //ERROR
...

Use the return keyword when a function returns something

private function Trig_OP_Condition takes nothing returns boolean
    call GetSpellAbilityId() == OP_AbilityID
endfunction

->

private function Trig_OP_Condition takes nothing returns boolean
    return GetSpellAbilityId() == OP_AbilityID
endfunction

There could more be less obvious errors I did not bother to look at.

as I am not yet familiar with the JASS scripting good practices.

Here's the coding convention

What is JESP?

[code=jass] - The JESP standard, JASS spell makers read.
I don't know if anyone still follows it.

 

[izaiphovias]

Thank you Flux, I've corrected these errors.
Thank you for the convention link too, I'll read through it!

 

[Elprede]

If you going to destroy effects immediately after creating them, you do not need the effect array. You can put the two functions in one line.

call DestroyEffect(AddSpecialEffectLoc(OP_FXHide(), op_PointCaster))

The above works because AddSpecialEffectLoc or any similar function returns an effect. A fair warning: destroying certain effects immediately after creation can cause them to not appear because these effects have death animations that last longer than a moment.

If you still want to use a variable for the effect for whatever reason, you still will not need an array. A non-array variable will do since you only use the array within one function. In this case, keep reusing the variable.


The following works only for the units of Player Red. Is that intentional?

call TriggerRegisterPlayerUnitEvent(gg_trg_OP, Player(0), EVENT_UNIT_SPELL_EFFECT, Trig_OP_Condition)

If not, then feel free to use this instead

 call TriggerRegisterAnyUnitEventBJ(gg_trg_OP, EVENT_UNIT_SPELL_EFFECT)

Now, that function does not have a parameter for the condition function, so you will have to either combine the condition and action functions or do

 call TriggerAddCondition(gg_trg_OP, Trig_OP_Condition)

Functions cannot be privatized outside of libraries, scopes, etc., so you will need to enclose them in such:

library YourLibrary
//Code here
endlibrary

 

[izaiphovias]

Thanks GhostHunter, the effects tip is quite handy, I did not realize I could do that

The Trigger is supposed to work only for that player, yes, but thank you anyway.

I think they are all private due to the Sublime auto-complete, it adds private automatically upon auto-completing a function, so I thought it was correct, guess I'll have to change the setup to constant functions and the others to open (right?).

 

[izaiphovias]

Thanks GhostHunter, the effects tip is quite handy, I did not realize I could do that

The Trigger is supposed to work only for that player, yes, but thank you anyway.

I think they are all private due to the Sublime auto-complete, it adds private automatically upon auto-completing a function, so I thought it was correct, guess I'll have to change the setup to constant functions and the others to open (right?).

Edit____________________________________________________________________________________

I've updated the code taking into consideration the feedback so far and added some stuff, the main thing is the use of call PolleWait to give the spell some time to finish all the required animations and some preloading in the init function:

//==================================================================================================//
//---------------------------------------------OVERPOWER--------------------------------------------//
//==================================================================================================//


//===============================================CONF===============================================//

//OVERPOWER ID
constant function OP_AbilityID takes nothing returns integer
    return 'a000'
endfunction

//PROJECTION EFFECT
constant function OP_FXProjection takes nothing returns string
    return "ProjFX" //PRETEND THIS IS THE INVISIBILITY EFFECT PATH
endfunction

//HIDE UNIT EFFECT
constant function OP_FXHide takes nothing returns string
    return "HideFX" //PRETEND THIS IS THE MASS TELEPORT CASTER EFFECT PATH
endfunction

//AOE EFFECT 1
constant function OP_FXBlast1 takes nothing returns string
    return "BlastFX1" //PRETEND THIS IS THE THUNDER CLAP EFFECT PATH
endfunction

//AOE EFFECT 2
constant function OP_FXBlast2 takes nothing returns string
    return "BlastFX2" //PRETEND THIS IS THE MANA FLARE EFFECT PATH
endfunction

//PROJECTION DUMMY UNIT ID
constant function OP_ProjectionDummy takes nothing returns integer
    return 'u000'
endfunction

//AOE DAMAGE TYPE
constant function OP_DamageType takes nothing returns damagetype
    return DAMAGE_TYPE_MAGIC
endfunction

//AOE WEAPON TYPE
constant function OP_WeaponType takes nothing returns weapontype
    return WEAPON_TYPE_WHOKNOWS
endfunction

//AOE ATTACK TYPE
constant function OP_AttackType takes nothing returns attacktype
    return ATTACK_TYPE_MAGIC
endfunction

//RADIUS OF THE AOE
constant function OP_Range takes nothing returns real
    return 300.
endfunction

//DAMAGE OF THE AOE
constant function OP_DamageAOE takes nothing returns real
    return 100. + 100. * GetUnitAbilityLevel(GetTriggerUnit(), OP_AbilityID())
endfunction

//===============================================COAC===============================================//

//CONDITION
function Trig_OP_Condition takes nothing returns boolean
    return GetSpellAbilityId() == OP_AbilityID()
endfunction

//ACTIONS
function Trig_OP_Actions takes nothing returns nothing

    //VARIABLES
    local unit op_Caster = GetTriggerUnit()
    local player op_Owner = GetOwningPlayer(op_Caster)
    local integer op_OwnerID = GetPlayerId(op_Caster)
    local unit array op_Dummy
    local unit op_Target = GetSpellTargetUnit()
    local location op_PointTarget = GetUnitLoc(op_Target())
    local location op_PointCaster = GetUnitLoc(op_Caster)
    local integer i = 1
    local real tx
    local real ty
 
    //CREATE FX ON CASTER LOCATION, PAUSE AND HIDE THE CASTER
    call DestroyEffect(AddSpecialEffectLoc(OP_FXHide(), op_PointCaster))
    call PauseUnit(op_Caster, true)
    call UnitAddAbility(op_Caster, 'a001') //PRETEND THIS IS INVULNERABLE
    call ShowUnit(op_Caster, false)

    //CREATE FX ARROUND THE TARGET FOR THE DUMMY UNITS AND CREATE THE DUMMY UNITS
    loop
        exitwhen i > 4
        set i = i + 1
        if i == 1
            set tx = GetUnitX(op_Target)
            set ty = GetUnitY(op_Target) + 100
        elseif i == 2
            set tx = GetUnitX(op_Target) + 100
            set ty = GetUnitY(op_Target)
        elseif i == 3
            set tx = GetUnitX(op_Target)
            set ty = GetUnitY(op_Target) - 100
        elseif i == 4
            set tx = GetUnitX(op_Target) - 100
            set ty = GetUnitY(op_Target)
        endif
        call DestroyEffect(AddSpecialEffect(OP_FXProjection(), tx, ty))
        set op_Dummy[i] = CreateUnit(op_OwnerID, OP_ProjectionDummy(), tx, ty, op_PointTarget)
        call SetUnitAnimation(op_Dummy[i], "spell")
    endloop

    //CREATE THE BLAST FX ON TARGET LOCATION
    call DestroyEffect(AddSpellEffectLoc(OP_FXBlast1(), op_PointTarget))
    call DestroyEffect(AddSpellEffectLoc(OP_FXBlast2(), op_PointTarget))

    //DAMAGE THE AREA AROUND THE TARGET
    set tx = GetUnitX(op_Target)
    set ty = GetUnitY(op_Target)
    call UnitDamagePoint(op_Caster, OP_Range(), tx, ty, OP_DamageAOE, false, false, OP_AttackType(), OP_DamageType(), OP_WeaponType())

    //CREATE FX FOR THE DUMMY UNITS AND REMOVE THEM
    call PolledWait(1.) //not sure if I should be using this to give the dummies time to play their animations
    loop
        exitwhen i > 4
        set i = i + 1
        if i == 1
            set tx = GetUnitX(op_Target)
            set ty = GetUnitY(op_Target) + 100
        elseif i == 2
            set tx = GetUnitX(op_Target) + 100
            set ty = GetUnitY(op_Target)
        elseif i == 3
            set tx = GetUnitX(op_Target)
            set ty = GetUnitY(op_Target) - 100
        elseif i == 4
            set tx = GetUnitX(op_Target) - 100
            set ty = GetUnitY(op_Target)
        endif
        call DestroyEffect(AddSpecialEffect(OP_FXProjection(), tx, ty))
        call RemoveUnit(op_Dummy[i])
    endloop
     
    //CREATE FX ON CASTER LOCATION, UNPAUSE AND UNHIDE THE CASTER
    call PolledWait(1.) //not sure if I should be using this to give for the dummies to disappear before showing the hero again
    call DestroyEffect(AddSpecialEffectLoc(OP_FXHide(), op_PointCaster))
    call ShowUnit(op_Caster, true)
    call UnitRemoveAbility(op_Caster, 'a001') //PRETEND THIS IS INVULNERABLE
    call PauseUnit(op_Caster, false)
    endloop

    //CLEARING LEAKS
    local unit op_Caster = null
    local player op_Owner = null
    local unit op_Dummy = null
    local unit op_Target = null
    call RemoveLocation(op_PointTarget)
    call RemoveLocation(op_PointCaster)
    local location op_PointTarget = null
    local location op_PointCaster = null

endfunction

//===============================================INIT===============================================//

function InitTrig_OP takes nothing returns nothing
    set gg_trg_OP = CreateTrigger()
    call TriggerRegisterPlayerUnitEvent(gg_trg_OP, Player(0), EVENT_UNIT_SPELL_EFFECT, Trig_OP_Condition)
    call TriggerAddAction(gg_trg_OP, function Trig_OP_Actions)

    //PRELOADING
    call Preload(OP_FXBlast2)
    call Preload(OP_FXBlast1)
    call Preload(OP_FXHide)
    call Preload(OP_FXProjection)
endfunction

 

[SAUS]

I haven't used PolledWait() before so I don't really know how it works exactly, but I think timers are pretty standard for controlling the timing of things. I personally have found timers to be extremely necessary for lots of abilities.

 

[Elprede]

You can drop the "local this" once you already done so at the top. Replace them with set as you already have done with tx and ty variables. Anyway, you can only declare locals at top of function.

PolledWait() leaks a reference to the timer that it creates. You will need a custom function that fixes the leak. Besides that, I would not know whether that function or a timer is necessary or not unless I know which model you are using.

You will need to determine for yourself whether or not you want the dummy's animation to finish before the effects are created.

Calling the RemoveUnit function on the dummy will remove it instantaneously as well as its attachments.

 

[SAUS]

This is more of a question for me, but are those functions that are basically used as constants a standard thing for JASS code? You get to skip the "udg_variableName" nonsense and don't need to clutter the variables list so much, but I assume it hurts performance a bit (not sure how much though). Does anyone have more knowledge on this that they could share?

 

[izaiphovias]

You can drop the "local this" once you already done so at the top. Replace them with set as you already have done with tx and ty variables. Anyway, you can only declare locals at top of function.

PolledWait() leaks a reference to the timer that it creates. You will need a custom function that fixes the leak. Besides that, I would not know whether that function or a timer is necessary or not unless I know which model you are using.

You will need to determine for yourself whether or not you want the dummy's animation to finish before the effects are created.

Calling the RemoveUnit function on the dummy will remove it instantaneously as well as its attachments.

Oh, the locals declarations at the bottom was a big lack of attention, thanks for pointing it out, I've replaced them with set.

Hmm, I used PolledWait because somewhere I saw that TriggerSleepAction may not work during forced "pauses" like when someone disconnects or when the game is paused via triggers. As soon as I get back home I'll paste this script in a map and attach it here for better understanding, I'll try to explain anyway:

What it does is hiding the hero (Jaina model) at the same time a mass teleport effect is created on her, right after she is not visible anymore, 4 dummies of the same model as the hero will appear around the target and start their spell animation, which lasts for about 1.1 seconds, this is when the damage is applied and I don't want these dummies to disappear before their spell animation finishes, thus I've added a PolledWait(1.), so the remaining actions that removes the dummies and shows back the hero won't run before they are intended.

This is more of a question for me, but are those functions that are basically used as constants a standard thing for JASS code? You get to skip the "udg_variableName" nonsense and don't need to clutter the variables list so much, but I assume it hurts performance a bit (not sure how much though). Does anyone have more knowledge on this that they could share?

I don't know much, but after seeing a bunch of JASS spells that were posted here in the Hive, I've noticed quite a few using constant functions and avoiding a lot of udg_, I quite like the idea so decided to use it, but it indeed will hurt the performance considering that you are calling a bunch of functions whenever you need to use a constant (I guess).

 

[Pyrogasm]

This is more of a question for me, but are those functions that are basically used as constants a standard thing for JASS code? You get to skip the "udg_variableName" nonsense and don't need to clutter the variables list so much, but I assume it hurts performance a bit (not sure how much though). Does anyone have more knowledge on this that they could share?

AFAIK a constant function will get inlined but a non-constant one won't. If you really want to stick with JESP (which is definitely totally dead and useful only as a guideline for how to make user-friendly spells in vanilla jass) you should have any function that could conceivably depend on level (damage, range, targets, etc.) take the level of the ability as an argument. See below for both:

constant function DamageGood takes integer lvl returns nothing
    return 100.00*lvl
    //Inlined: call UnitDamageTarget(..., DamageGood(spellLevel), ...) becomes call UnitDamageTarget(..., 100.00*spellLevel, ...)
endfunction

function DamageBad takes integer lvl returns nothing
    return 50.00 + 50.00*lvl
    //Not inlined: call UnitDamageTarget(..., DamageBad(spellLevel), ...) stays as call UnitDamageTarget(..., DamageBad(spellLevel), ...)
endfunction

Hmm, I used PolledWait because somewhere I saw that TriggerSleepAction may not work during forced "pauses" like when someone disconnects or when the game is paused via triggers.

You are correct, TSA keeps going during DCs and game pauses. PolledWait isn't any more accurate than TSA at low durations anyway and it leaks a timer so just generally not a good idea to use it. Always use TSA and I guarantee you'll never even notice it being a little off because of some lag. Honestly most of the time you should be using timers and structs anyway.

4 dummies of the same model as the hero will appear around the target and start their spell animation, which lasts for about 1.1 seconds

Instead of TSA-ing to figure out when to remove them, you can just give them timed lives (like summons): call UnitApplyTimedLife(op_Dummy[i], 'btlf', 1.1) That way you don't have to remove them yourself after the requisite amount of time. Generally this is a better option for dummy units, but now that I think about it it'll probably play their death animations too so maybe you don't want that. Food for thought.

_______

At this age of the wc3 modding scene there just really isn't a reason to just use vanilla JASS anymore when we have JassNewGen; in my opinion you would be best served taking a dive into the JASSHelper Manual to take full advantage of a more user-friendly version of JASS. You seem to know what you're doing with this stuff and all of that knowledge will be directly applicable with vJASS. It'll open yourself up to the world of globals blocks, scopes, libraries, and most importantly structs! All of that will make making spells like this much easier/more compact.

 

[izaiphovias]

Well, I am using JASS as part of the learning curve as I felt quite comfortable with the fact that I can easily convert GUI into JASS scripts with WE to study them. As soon as I get the hang of it I'll move to vJASS and learn it's added features.

I tried to open a map with JNGP, create a simple AOE firebolt spell in GUI, convert it to JASS and then test, but I got a lua error when saving the map, not sure what happened though, so couldn't go right into vJASS anyway, since JNPG doesn't seem to be working for me.

 

[Pyrogasm]

Well, I am using JASS as part of the learning curve as I felt quite comfortable with the fact that I can easily convert GUI into JASS scripts with WE to study them. As soon as I get the hang of it I'll move to vJASS and learn it's added features

Doing that is a great way to learn the content of JASS. vJASS is additional structure around that content. You're using the same regular ol' JASS functions and the same regular ol' JASS tricks/workarounds just with different structure around it that honestly simplifies your code.

 

[EdgeOfChaos]

JESP is for writing spells to post to the THW Spells section btw, and is mostly outdated. If you're writing this for your own map, you do not need to follow that (though there are a few useful points in there to consider). And currently, initializers are almost always called "onInit". If you want a better standard, use this: JPAG - JASS Proper Application Guide there are still some stupid things in it (like having different format for functions and methods and a silly math operator format) but its mostly ok.

To be honest, you have chosen a difficult spell to code correctly for your first JASS spell - because there are waits in there. Use of PolledWait and TriggerSleepAction have serious issues around them. I used to not believe this until I made a map with extensive use of waits, and failed with them, then had to re-write it all with timers. Using waits in the World Editor is a kind of complex issue.

-----

constant function OP_AbilityID takes nothing returns integer
    return 'a000'
endfunction

I hate constant functions like this.
I honestly don't know if "regular" JASS provides this functionality, but with vJASS (which I highly encourage you to download and use), you can create a "globals" block in code that could contain variables like this - and you could even make them private to skip the whole "OP_" prefix.

-----

call DestroyEffect(AddSpecialEffectLoc(OP_FXHide(), op_PointCaster))

Locations in jass is bad
I see you're using a mix of locations and coordinates. It's time to switch to all coordinates, for example

local location op_PointTarget = GetUnitLoc(op_Target())

->

local real targetX = GetUnitX(op_Target)
local real targetY = GetUnitY(op_Target)

This may seem like more code at first, but there are performance advantages to using coordinates, as well as actually being more convenient in most cases since you can operate them separately. Believe me, if you switch now you'll be thankful for it later. There's only one time when I think locations are necessary, and that's when you need to get the Z coordinate of a point (which you must use a location for).

-----

call UnitAddAbility(op_Caster, 'a001') //PRETEND THIS IS INVULNERABLE

Why do you need to pretend? Just change it to 'Avul'

-----

local integer i = 1
..
    loop
        exitwhen i > 4
        set i = i + 1

Issue here. Imagine what will happen the first time your code runs.. i is initialized to one, and then you immediately add one more to it before you do anything. Now i==2 on your first iteration. Now one of your if statements is if(i==1). This will never happen.

-----

call UnitDamagePoint(op_Caster, OP_Range(), tx, ty, OP_DamageAOE, false, false, OP_AttackType(), OP_DamageType(), OP_WeaponType())

Beware. Using UnitDamagePoint hurts allies and enemies in the area. I don't know if this is your intended effect.

-----

    loop
        exitwhen i > 4

This is your second loop. You never reset i to its initial value, so it's still 5 because of your first loop. So, this enters the loop and instantly exits.

-----

endloop

Your last line before //CLEARING LEAKS.
I see and endloop here, but I don't see the loop beginning. Was this code supposed to be in a loop?


-----

    call Preload(OP_FXBlast2)
    call Preload(OP_FXBlast1)
    call Preload(OP_FXHide)
    call Preload(OP_FXProjection)

I think these should be function calls, since they are constant functions and not variables.

 

[izaiphovias]

I hate constant functions like this.
I honestly don't know if "regular" JASS provides this functionality, but with vJASS (which I highly encourage you to download and use), you can create a "globals" block in code that could contain variables like this - and you could even make them private to skip the whole "OP_" prefix.

Not sure either if JASS support the globals block.

Locations in jass is bad
I see you're using a mix of locations and coordinates. It's time to switch to all coordinates, for example

Thanks for the tip!

Why do you need to pretend? Just change it to 'Avul'

I wrote all of this while at work, so I didn't have WE with me to check the rawcodes and no time to look for them elsewhere, lol

Issue here. Imagine what will happen the first time your code runs.. i is initialized to one, and then you immediately add one more to it before you do anything. Now i==2 on your first iteration. Now one of your if statements is if(i==1). This will never happen.

Guess I didn't put enough thought into that, thanks for pointing it out.

Beware. Using UnitDamagePoint hurts allies and enemies in the area. I don't know if this is your intended effect.

The spell is quite powerful, it does a lot of damage in it's GUI version, it is supposed to hurt allies as well so the player would need to be more careful when using it.

This is your second loop. You never reset i to its initial value, so it's still 5 because of your first loop. So, this enters the loop and instantly exits.

Forgot to set i back to 1 there, thanks for pointing it out.

Your last line before //CLEARING LEAKS.
I see and endloop here, but I don't see the loop beginning. Was this code supposed to be in a loop?

I really don't know how that happened, probably while copying and pasting some lines from somewhere else in the script

I think these should be function calls, since they are constant functions and not variables.

And again I forgot stuff, the problem of trying to study and develop while at work xD

Thanks to all you guys who spared some of your time to help me with this, really appreciated.

 

[DracoL1ch]

if you want to LEARN jass, forget vjass or cjass (assuming u have no idea how to code on java/C), coding standarts and stuff. Get any PURE jass maps and learn on practice with them. you can't learn if you have no idea what you doing.

 

[izaiphovias]

So, now that I'm at home I decided to try JNGP again, see if works for me so I can start vJASS. I've opened my campaign file (this editor is quite slow, isn't it?), opened a map from it and tried to save, with Scorpio's compiler on, but I got three errors, resulting in "Campaign Export Error".
It only works if I leave the Campaign Compiler off, but that would not compile vJASS when I insert it, since it is a campaign map.

 

[Pyrogasm]

Not sure either if JASS support the globals block.

No it does not.

So, now that I'm at home I decided to try JNGP again, see if works for me so I can start vJASS. I've opened my campaign file (this editor is quite slow, isn't it?), opened a map from it and tried to save, with Scorpio's compiler on, but I got three errors, resulting in "Campaign Export Error".
It only works if I leave the Campaign Compiler off, but that would not compile vJASS when I insert it, since it is a campaign map.

But you can save the map individually just fine? (Not through the campaign editor). The slowness of the JNGP is due to a problem with the release of TESH included in it. Replace the TESH folder (inside the JNGP folder) with the one I've attached below and it should be faster.

 

[izaiphovias]

Thanks, Pyrogasm

The editor slow performance has indeed been fixed after replacing the TESH plugin folder with the one you provided. As for the campaign, I guess I'll really have to do it the annoying way, by saving the maps outside the campaign and manually adding them back after editing.
__________________________________________________________________________________________________________________________________________________________________________________________

Edit:

So, since there is no native way to register a damage event that's not reffering to a specific unit I was trying to get creative in order to make a spell that damages a target unit whenever the caster of a debuff gets damaged, thing is, there might be multiple casters of that spell in the map during a match, so I must find means to register whenever any of them get damaged. I came up with:

Spoiler: Spell

scope CarcinogenicBind initializer Init

    globals
        private hashtable hash = InitaHashtable()
        private group casters = CreateGroup()
    endglobals
  
    private function Damage takes nothing returns nothing
        local trigger trg
        local unit c = GetTriggerUnit()
        local unit t = LoadUnitHandle(hash,0,GetHandleId(c))
        local real d = GetEventDamage()
      
        call BJDebugMsg(GetUnitName(c) + " damaged by " + R2S(d))
      
        if (GetUnitAbilityLevel(t,'B01I') > 0) and (GetWidgetLife(t) > 0.405) then
            call DestroyEffect(AddSpecialEffectTarget("Abilities\\Spells\\Orc\\Devour\\DevourEffectArt.mdl",t,"chest"))
            call UnitDamageTarget(c,t,d,true,false,ATTACK_TYPE_MAGIC,DAMAGE_TYPE_DEATH,WEAPON_TYPE_WHOKNOWS)
            call BJDebugMsg(GetUnitName(t) + " damaged!")
        else
            call SaveUnitHandle(hash,0,GetHandleId(c),null)
            call GroupRemoveUnit(casters,c)
            call SaveBoolean(hash,1,GetHandleId(c),false)
            set trg = LoadTriggerHandle(hash,2,GetHandleId(c))
            call DestroyTrigger(trg)
            set trg = null
        endif
      
        set c = null
        set t = null
    endfunction
        
    private function UpdateTriggers takes nothing returns nothing
        local trigger trg
        local unit c = GetTriggerUnit()
        local unit t = GetSpellTargetUnit()
        local unit u
        local group g
        local boolean b
      
        if GetSpellAbilityId() == 'A04M' then
            call BJDebugMsg(GetUnitName(c) + " --> " + GetUnitName(t))
            call GroupAddUnit(casters, c)
            call SaveUnitHandle(hash,0,GetHandleId(c),t)
            set g = casters
      
            loop
                set u = FirstOfGroup(g)
                exitwhen u == null
                call GroupRemoveUnit(g, u)
                set b = LoadBoolean(hash,1,GetHandleId(u))
                if b == false then
                    set trg = CreateTrigger()
                    call TriggerRegisterUnitEvent(trg,u,EVENT_UNIT_DAMAGED)
                    call TriggerAddAction(trg,function Damage)
                    call SaveBoolean(hash,1,GetHandleId(u),true)
                    call SaveTriggerHandle(hash,2,GetHandleId(u),trg)
              
                    set trg = null
                endif
            endloop
          
            set c = null
            set t = null
            set g = null
            set u = null
        endif
    endfunction
  
    private function Init takes nothing returns nothing
        local trigger trg = CreateTrigger()
            
        call TriggerRegisterAnyUnitEventBJ(trg,EVENT_PLAYER_UNIT_SPELL_EFFECT)
        call TriggerAddAction(trg, function UpdateTriggers)
      
        set trg = null
    endfunction

endscope

That obviously didn't work... I don't know if it is even possible to add trigger events outside the Initializer function, but I tried. Is there any means for me to properly register a damage event that check all casters of that spell?