[JASS] Problem with jass spell.

[Undead_ted]

Code fixed

function BS_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A004'
endfunction

function BS_Actions takes nothing returns nothing
    local effect e
    local string f = "Abilities\\Spells\\Other\\Stampede\\StampedeMissileDeath.mdl"
    local attacktype a = ATTACK_TYPE_HERO
    local damagetype m = DAMAGE_TYPE_NORMAL
    local weapontype w = WEAPON_TYPE_METAL_HEAVY_SLICE
    local unit c = GetTriggerUnit()
    local unit t = GetSpellTargetUnit()
    local real s = 0.3
    local real d = 30.00 * GetUnitAbilityLevel (c, 'A004')
    local real X = GetUnitX(t)
    local real Y = GetUnitY(t)
    call SetUnitPosition(c, X, Y)
    call SetUnitInvulnerable (c, true)
    call TriggerSleepAction(s)   
    if (IsUnitEnemy(t, GetOwningPlayer(c)) == true) then
    set e = AddSpecialEffectTarget (f,t,"chest") 
    call DestroyEffect (AddSpecialEffectTarget(f, t, "chest"))
    call UnitDamageTarget (c, t, d, false, false, a, m, w)
    endif
call SetUnitInvulnerable (c, false)
  set e = null
  set f = null
  set a = null
  set m = null
  set w = null
  set c = null
  set t = null
endfunction

//===========================================================================
function InitTrig_Blink_Strike takes nothing returns nothing
local trigger BlinkStrike = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ (BlinkStrike, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition (BlinkStrike, Condition( function BS_Conditions))
    call TriggerAddAction  (BlinkStrike, function BS_Actions)
endfunction

Code fixed.

 

[Titanhex]

TIP: Instead of calling another function in the if statement why not just directly place the if statement in place of the function? It will mean less lines in your overall code and allow you to use locals with those if statements if you need to.

TIP2: Instead of creating two seperate functions moving the unit to the location of the enemy or the ally, just set an if statement for the damage.

IF ENEMY
MOVE TO ENEMY
HIT ENEMY WITH SPECIAL EFFECTS

IF ALLY
MOVE TO ALLY

Instead of that up there do this

MOVE TO UNIT
IF ENEMY
HIT AND DO EFFECTS ON TARGET

TIP3:

Use X and Y (Coordinates) instead of Location. Dunno why, was just told to me by a few people.

TIP4: TriggerSleepAction doesn't go lower than 0.27 or somethin like that, no matter how low you set it. It's not a big deal unless you seriously need it to be lower than 0.27, in which case you may need timers.

TIP5: else isn't necessary after ifs unless you actually have else conditions to put in. So:

IF STUFFHAPPENS THEN
DO STUFF
ENDIF

Is fine. But in your case you would likely be better off doing:

IF TARGET IS ENEMY THEN
DO HURT
ELSEIF TARGET IS ALLY THEN
DO MOVE BUT NO HURT
ENDIF

or juse ELSE instead of ELSEIF in this case since it's either an enemy or it's not. The way you have it set up nothing happens when he targets a Nuetral.

If you'll allow me I will optomize and rearrange your code and repost it for you and see if I cant pinpoint the error that way.

Also if anyone can answer this for me, do constant declarations on globals randomize their name and confine them to the trigger they're in? I always thought that you needed scopes and libraries, a part of vJASS, to use constants properly.

Which brings me to TIP5:

While you're primarily using basic JASS, by placing globals at the top with constant declarations before them, that is vJASS, and will require people to download JNGP if they want to use your spells, incase you decide to submit any spells in the future in just JASS.

=============================

FIRST ERROR:

constant integer SpellRawCode = 'A004'

constant real StrikeBonusDamage = 30.00 * SpellRawCode

This will produce incorrect damage!

A004 does not return the level of the ability for the unit using it. For that you must use: GetUnitAbilityLevel(Caster, SpellRawCode)

SECOND ERROR: (Speculative, not 100% on this)

When you're doing an if statement that calls a function, don't you need to declare that you're calling a function? IDK, haven't seen anyone call functions using ifs before. But if so then:

if (function BSEnemy) then would seem more appropriate. Also I don't think it needs () after the function name since the function doesn't take anything.

BTW does having a space between the Function name and it's parenthesis cause issues? I'm sure it'd compile incorrectly if it did, so I doubt it.

Also a I believe on the false, true of the damage function, it should be true, false, unless you want it to be considered range by World Editor (And probably range deflectable?). In which case it would be true, true.

PS: Nullify your location too. Locations leak.

K that sums it up. Any incorrect advice or misgivings I have given you will likely be fixed by an expert who will be adding his insight soon enough >.>

 

[Undead_ted]

Yes i have Jass new gen is thats what JNGP means


And yes you optimize it.

 

[Titanhex]

Yeah JNGP is Jass Newgen Pack.

Anyways, I'm not so sure I can optomize it, everything I just told you right now will allow you to do that yourself and replicate the muscle memory needed. I know, I know, I said I would, but I ended up realizing that it'd take less time to analyze your trigger than to do the copy+paste+ seperate the lines.

However I can show you my own blnkstrike so you can compare. Mine just doesn't use global constants and instead just uses local variables.

Blink Strike

function Flurry_Actions takes nothing returns nothing
 local unit caster = GetTriggerUnit()
 local unit target = GetSpellTargetUnit()
 local real X = GetUnitX(target)
 local real Y = GetUnitY(target)
    //effects on departure
    call DestroyEffect(AddSpecialEffectLoc("Abilities\\Spells\\Orc\\MirrorImage\\MirrorImageCaster.mdl", GetUnitLoc(caster)))
    call SetUnitPosition(caster, X, Y)
    //Check if target is an enemy then do mean things to it
    if IsUnitEnemy(target, GetOwningPlayer(caster)) then
        call TriggerSleepAction(0.3)
        call IssueTargetOrder(caster, "attack", target)
        call UnitDamageTarget(caster, target, 25. * GetUnitAbilityLevel(caster, 'flur'), true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_NORMAL, WEAPON_TYPE_WHOKNOWS)
        call DestroyEffect(AddSpecialEffectTarget("Abilities\\Spells\\Other\\Stampede\\StampedeMissileDeath.mdl", target, "origin"))
    endif
 set caster = null
 set target = null
endfunction

function Flurry_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'flur'
endfunction

//===========================================================================
function InitTrig_Flurry takes nothing returns nothing
 local trigger Flurry = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( Flurry, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddAction( Flurry, function Flurry_Actions )
    call TriggerAddCondition( Flurry, function Flurry_Conditions )
 set Flurry = null
endfunction

 

[Undead_ted]

:| I guess i will use yours, at least i tried x.x

Edit* Does yours still allow the caster to blink to an ally D:

 

[Titanhex]

lol no, yours is still good. Did you make the error changes I pointed out? If you want to get better at JASS, fixing your code is a big part of it.

I mean was it not doing damage, was it not even teleporting, did you try putting BJDebugMsg("Check if it gets to here") codes in or BJDebugMsg(R2S(damage)) to check the damage, etc. etc.? Those can help you find your errors rather quickly.

 

[Undead_ted]

lol no, yours is still good. Did you make the error changes I pointed out? If you want to get better at JASS, fixing your code is a big part of it.

I mean was it not doing damage, was it not even teleporting, did you try putting BJDebugMsg("Check if it gets to here") codes in or BJDebugMsg(R2S(damage)) to check the damage, etc. etc.? Those can help you find your errors rather quickly.

It didn't teleport/damage.

Like how would I use the BJDebugMsg thing, well i mean how would i place it?

 

[Titanhex]

First place it at the begging of the Actions function, after all locals have been declared. Make the string "Actions Start"

When you test, this will tell you that the conditions are right, and the problem is in your actions. if the message doesn't show, then your conditions are the problem.

Conditions are harder to check. They fire off any time the event you've put down is registered.

Luckily your Conditions seem fine to me, but double check anyways.

Next you would put BJ after your ifs, saying something about that the if check went right. After the enemy check you would put

BJDebugMsg("Target is an enemy")

If you wanted to check the damage, BJDebugMsg(R2S(StrikeBonusDamage)) would be after the UnitDamagesTarget function. Afterwards, run the game and check.


From what you've told me, I bet your issue lies in your if statements. try making them like I told you in the tips.

 

[Undead_ted]

You said for the Rawcode: GetUnitAbilityLevel(Caster, SpellRawCode)


Where do i place that? The damage part or after the 'A004'?

Edit* Yea i'm working on the stuff you told me
like the 1 line for the code to check allys/enemys

 

[Titanhex]

To clarify, this:

constant real StrikeBonusDamage = 30.00 * SpellRawCode

should be this:

constant real StrikeBonusDamage = 30.00 * GetUnitAbilityLevel(Caster, SpellRawCode)

 

[Dr Super Good]

call UnitDamageTarget (Caster, Target, StrikeBonusDamage, false, true, AttackType, DamageType, WeaponType)

You are causing a thread crash. This function was never fully finished by blizzard so the boolean variables of it have to be both false otherwise the thread will crash and no code will be executed passed it.

try...
call UnitDamageTarget (Caster, Target, StrikeBonusDamage, false, false, AttackType, DamageType, WeaponType)[/

 

[Titanhex]

That's weird, I've done (caster, target, damage, true, false, attack_type, damage_type, null) after UnitDamageTarget and it's always executed flawlessly, such as in my posted JASS spell. Are you certain about this, Dr Supergood? Or perhaps you just haven't mentioned something else?

 

[Undead_ted]

You are causing a thread crash. This function was never fully finished by blizzard so the boolean variables of it have to be both false otherwise the thread will crash and no code will be executed passed it.

try...
call UnitDamageTarget (Caster, Target, StrikeBonusDamage, false, false, AttackType, DamageType, WeaponType)[/

I Tried that and it dealt no damage still.. :\

 

[Titanhex]

Edit your first post with the updates you've made to your spell so I can help you pinpoint any extra problems. While Doctor Supergood's claim may have some truth to it, I do not find it to be accurate.

 

[Undead_ted]

Edit your first post with the updates you've made to your spell so I can help you pinpoint any extra problems. While Doctor Supergood's claim may have some truth to it, I do not find it to be accurate.

Also i saw you said something about libraries, I think thats the problem. Because I don't have one and I have no idea what they are. lol

 

[Titanhex]

I don't really recommend learning vJASS til you're comfortable with JASS. Even then, it's better to study something like RGSS which uses methods right off the bat, if you want to get a grasp on methods.
Reason for that, vJASS is an expansion on JASS, and is considered an advancement and so you are taught like you're advanced.

RGSS is right at the beginning, and so you're taught like you're a beginner (Which you are if you've never tried using methods.)

So anyways, if you want to fix your coding by removing the constant globals you can. Simply switch to locals. It shouldn't be a problem since all your Actions can be placed in a single function.

 

[Undead_ted]

Okay

function BS_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A004'
endfunction

function BS_Actions takes nothing returns nothing
    local effect e
    local string f = "Abilities\\Spells\\Other\\Stampede\\StampedeMissileDeath.mdl"
    local attacktype a = ATTACK_TYPE_HERO
    local damagetype m = DAMAGE_TYPE_NORMAL
    local weapontype w = WEAPON_TYPE_METAL_HEAVY_SLICE
    local unit c = GetTriggerUnit()
    local unit t = GetSpellTargetUnit()
    local real s = 0.3
    local real d = 30.00 * GetUnitAbilityLevel (c, 'A004')
    local real X = GetUnitX(t)
    local real Y = GetUnitY(t)
    call SetUnitPosition(c, X, Y)
    call SetUnitInvulnerable (c, true)
    call TriggerSleepAction(s)   
    if (IsUnitEnemy(t, GetOwningPlayer(c)) == true) then
    set e = AddSpecialEffectTarget (f,t,"chest") 
    call DestroyEffect (AddSpecialEffectTarget(f, t, "chest"))
    call UnitDamageTarget (c, t, d, false, false, a, m, w)
    endif
call SetUnitInvulnerable (c, false)
  set e = null
  set f = null
  set a = null
  set m = null
  set w = null
  set c = null
  set t = null
endfunction

//===========================================================================
function InitTrig_Blink_Strike takes nothing returns nothing
local trigger BlinkStrike = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ (BlinkStrike, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition (BlinkStrike, Condition( function BS_Conditions))
    call TriggerAddAction  (BlinkStrike, function BS_Actions)
endfunction

Edit* Well it works now XD

 

[Titanhex]

Ah! Then this backs my theory that constant globals shouldn't be used without scopes (Of course regular globals are fine.)

+Rep for doing it on your own, and not succumbing to copy-pastaing the easy way out.

 

[Undead_ted]

Ah! Then this backs my theory that constant globals shouldn't be used without scopes (Of course regular globals are fine.)

+Rep for doing it on your own, and not succumbing to copy-pastaing the easy way out.

Thanks.

Is there any one to make the code cleaner or is it fine?

 

[Titanhex]

If you want to remove some of the locals that are only ever used once and just put their values in instead of their letter, that could shrink the code considerably.

If so, just do that with f, a, m, w and s.

BTW, e is being destroyed right after it's created, just so you know. In your original it looked like there was a pause before it was destroyed. Or maybe I'm wrong?

lol oh well, you can just remove e if there is no pause before it's destroyed and just do what you did with the second effect.

 

[Undead_ted]

If you want to remove some of the locals that are only ever used once and just put their values in instead of their letter, that could shrink the code considerably.

If so, just do that with f, a, m, w and s.

BTW, e is being destroyed right after it's created, just so you know. In your original it looked like there was a pause before it was destroyed. Or maybe I'm wrong?

lol oh well, you can just remove e if there is no pause before it's destroyed and just do what you did with the second effect.

It still shows e when its casted on them. Too make sure i should remove f, a, n, w and s?


Edit* Better?

function BS_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A004'
endfunction

function BS_Actions takes nothing returns nothing
    local effect e
    local string f = "Abilities\\Spells\\Other\\Stampede\\StampedeMissileDeath.mdl"
    local unit c = GetTriggerUnit()
    local unit t = GetSpellTargetUnit()
    local real d = 30.00 * GetUnitAbilityLevel (c, 'A004')
    local real X = GetUnitX(t)
    local real Y = GetUnitY(t)
    call TriggerSleepAction(0.05) 
    call SetUnitPosition(c, X, Y)
    call SetUnitInvulnerable (c, true)  
    if (IsUnitEnemy(t, GetOwningPlayer(c)) == true) then
    set e = AddSpecialEffectTarget (f,t,"chest") 
    call DestroyEffect (AddSpecialEffectTarget(f, t, "chest"))
    call UnitDamageTarget (c, t, d, false, false, ATTACK_TYPE_HERO, DAMAGE_TYPE_NORMAL, WEAPON_TYPE_METAL_HEAVY_SLICE)
    endif
call SetUnitInvulnerable (c, false)
  set e = null
  set c = null
  set t = null
  set f = null
endfunction

//===========================================================================
function InitTrig_Blink_Strike takes nothing returns nothing
local trigger BlinkStrike = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ (BlinkStrike, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition (BlinkStrike, Condition( function BS_Conditions))
    call TriggerAddAction  (BlinkStrike, function BS_Actions)
endfunction

 

[Titanhex]

Yes, better Now it's far more condensed. Blink attack spells are really simple, so there's not a lot to optomize. LOL Imagine all the work just for one spell :\ But it doesn't leak BJs and when you hit complicated targeting spells it'll be a lot faster.

 

[Undead_ted]

Yes, better Now it's far more condensed. Blink attack spells are really simple, so there's not a lot to optomize. LOL Imagine all the work just for one spell :\ But it doesn't leak BJs and when you hit complicated targeting spells it'll be a lot faster.

I've never made a Jass spell so i was pretty confused making it. It was originally GUI. :x

 

[Titanhex]

Dont be afraid to use the Function List provided by JNGP.

Just some tips using it:
Type Get into the search bar in the function list. These will show you a lot of the commands that will return a value, like what orderstring a unit used, or what spell he's casting.

GetSpellAbilityId() == 'Acur'

If the spell being cast is Curse, then this roughly is

'Acur' == 'Acur' which is true, which is what a Boolean is. Thus, it is giving a true statement.

So don't be afraid to look at the Gets, the Sets, the Unit and Item keywords in functionlist if you're looking to do a particular function or manipulate a specific object. They're all pretty self explanator.

All red text is BJs, and you don't want to use a lot of the BJs provided by blizzard. The functionlist will tell you what natives the BJ use, and you can just use the natives instead of the BJs since you know what it's trying to do.

Also again in functionlist (It's really useful) you can see what a command takes, and if it returns anything.

Hopefully you learn to use functionlist to your full advantage

 

[Undead_ted]

Dont be afraid to use the Function List provided by JNGP.

Just some tips using it:
Type Get into the search bar in the function list. These will show you a lot of the commands that will return a value, like what orderstring a unit used, or what spell he's casting.

GetSpellAbilityId() == 'Acur'

If the spell being cast is Curse, then this roughly is

'Acur' == 'Acur' which is true, which is what a Boolean is. Thus, it is giving a true statement.

So don't be afraid to look at the Gets, the Sets, the Unit and Item keywords in functionlist if you're looking to do a particular function or manipulate a specific object. They're all pretty self explanator.

All red text is BJs, and you don't want to use a lot of the BJs provided by blizzard. The functionlist will tell you what natives the BJ use, and you can just use the natives instead of the BJs since you know what it's trying to do.

Also again in functionlist (It's really useful) you can see what a command takes, and if it returns anything.

Hopefully you learn to use functionlist to your full advantage

I use functionlist alot to help me find stuff that I didn't do right