• 🏆 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!

GetSpellTargetUnit() & GetSpellAbilityId() malfunction within itself

Status
Not open for further replies.
Level 19
Joined
Dec 12, 2010
Messages
2,069
Whenever you're using GetSpellTargetUnit() & GetSpellAbilityId() , you have to pre-save them into some other variable everytime you're going to cast another ability which can be detected by this trigger. Basically, if your trigger catches every spell casted, it will fuck up if you cast another spell in response to the event.

Code:
JASS:
function Earthshaker_Aftershock_Effect2 takes unit u, integer lvl returns nothing
    local unit d
    call BJDebugMsg("#"+I2S(lvl)+": starting dummy spell: target="+GetUnitName(GetSpellTargetUnit())+" id="+I2S(GetSpellAbilityId()))
    set d=CreateUnit(GetOwningPlayer(u),'ewsp',GetUnitX(u),GetUnitY(u),0)//e00E -> Spellcaster
    call UnitAddAbility(d,'AOws')
    call IssueImmediateOrder(d,"stomp")
    call BJDebugMsg("#"+I2S(lvl)+": finishing dummy spell: target="+GetUnitName(GetSpellTargetUnit())+" id="+I2S(GetSpellAbilityId()))
    set d=null
endfunction

function GenericOnCastEvent takes nothing returns nothing
    local integer i=GetSpellAbilityId()
    local unit u=GetTriggerUnit()
    local unit target=GetSpellTargetUnit()
    local integer c=GetTriggerEvalCount(GetTriggeringTrigger())
    if IsUnitType(u,UNIT_TYPE_HERO)then
        call BJDebugMsg("    ")
        call BJDebugMsg("    ")
        call BJDebugMsg("------   NEW HERO CAST -----")
    endif
    call BJDebugMsg("#"+I2S(c)+": casting spell: target="+GetUnitName(GetSpellTargetUnit())+" id="+I2S(GetSpellAbilityId()))
    if IsUnitType(u,UNIT_TYPE_HERO)then
        call Earthshaker_Aftershock_Effect2(u,c)
    endif
    call BJDebugMsg("#"+I2S(c)+": after casting: target="+GetUnitName(GetSpellTargetUnit())+" id="+I2S(GetSpellAbilityId()))
    set u=null
    set target=null
endfunction


function InitTrig_Untitled_Trigger_002 takes nothing returns nothing
    local trigger t=CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(t,EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(t,Condition(function GenericOnCastEvent))
    call BJDebugMsg("Throw a bolt onto enemy and look")
endfunction

Example:
5EK3gt1.png

Dummy cast in red, broken natives in yellow. It's over as soon as dummy cast get registered by the same trigger, no recovery.

Reason of that broken behavior - those natives were added in hurry, unlike most other getters they do not directly use a storage of GetTriggerThings, but follow absolutely different algoritm. Therefore they cannot be "stacked" as GetTriggerUnit() does.

This behavior is broken and should be fixed.
 

Attachments

  • GetSpellAbilityId.w3x
    13.5 KB · Views: 70
Level 19
Joined
Dec 12, 2010
Messages
2,069
This is old news.
It should be fixed for GUI users but it shouldn't be an issue for any jass coder as it's very bad practice to have code like that.
old news, still not fixed even tho patches went through. And no, its not a bad practice, I wanna my spells being reflected faster than it takes effect, witohut rewriting everything. In fact you're trying to defend a thing which is broken from the start.
 
Level 14
Joined
Jan 16, 2009
Messages
715
"I wanna my spells being reflected faster than it takes effect"
What do you mean by that ?

Making two functions calls, while you could only make one and store the value returned by the function into a local variable, is indeed bad practice. Any response getter should only be called once per response, calling it more than one time is a waste. As GUI users don't have direct access to local variables and must call the getter each time then want to reference that unit, this should be fixed for them.
 
Level 19
Joined
Dec 12, 2010
Messages
2,069
You can have tons of different triggers, each of them would like to obtain data, building up everything around a single trigger is only good when you're starting from scratch, not when you coming into any live project which had years of development and started with GUI.
 
Level 14
Joined
Jan 16, 2009
Messages
715
I don't understand what you are talking about. Never I mentioned building things around one trigger.
Here is a version that fix your issue, using local variables.
 

Attachments

  • GetSpellAbilityId.w3x
    14.2 KB · Views: 64
Level 14
Joined
Jan 16, 2009
Messages
715
I would love you to show me that issue taking place because when I added a few triggers to the test map, everything was working perfectly.
 

Attachments

  • GetSpellAbilityId.w3x
    15.4 KB · Views: 60
Last edited:
Level 29
Joined
Jul 29, 2007
Messages
5,174
Not sure what the surprise is, most of the getters accessible also via GUI are not per trigger-invocation. You don't need to cast another ability recursively to see this, just add a wait and cast two times. This is the standard "why you use locals in GUI" case. In pure Jass you'd generally set it to a local either way, just because caching results is standard.
 
Level 19
Joined
Dec 12, 2010
Messages
2,069
Not sure what the surprise is, most of the getters accessible also via GUI are not per trigger-invocation. You don't need to cast another ability recursively to see this, just add a wait and cast two times. This is the standard "why you use locals in GUI" case. In pure Jass you'd generally set it to a local either way, just because caching results is standard.
wrong, there are set of "default" getters which are designed to work recursively w/o any limitations. You have GetTriggerPlayer()/Unit(), GetEnumUnit() and other similar things wherever you want with no issues. While these 2 is broken by design, somehow
 
Status
Not open for further replies.
Top