1. Updated Resource Submission Rules: All model & skin resource submissions must now include an in-game screenshot. This is to help speed up the moderation process and to show how the model and/or texture looks like from the in-game camera.
    Dismiss Notice
  2. DID YOU KNOW - That you can unlock new rank icons by posting on the forums or winning contests? Click here to customize your rank or read our User Rank Policy to see a list of ranks that you can unlock. Have you won a contest and still havn't received your rank award? Then please contact the administration.
    Dismiss Notice
  3. The Lich King demands your service! We've reached the 19th edition of the Icon Contest. Come along and make some chilling servants for the one true king.
    Dismiss Notice
  4. The 4th SFX Contest has started. Be sure to participate and have a fun factor in it.
    Dismiss Notice
  5. The poll for the 21st Terraining Contest is LIVE. Be sure to check out the entries and vote for one.
    Dismiss Notice
  6. The results are out! Check them out.
    Dismiss Notice
  7. Don’t forget to sign up for the Hive Cup. There’s a 555 EUR prize pool. Sign up now!
    Dismiss Notice
  8. The Hive Workshop Cup contest results have been announced! See the maps that'll be featured in the Hive Workshop Cup tournament!
    Dismiss Notice
  9. Check out the Staff job openings thread.
    Dismiss Notice
Dismiss Notice
60,000 passwords have been reset on July 8, 2019. If you cannot login, read this.

GetSpellTargetUnit() & GetSpellAbilityId() malfunction within itself

Discussion in 'The Lab' started by DracoL1ch, Jun 17, 2018.

  1. DracoL1ch

    DracoL1ch

    Joined:
    Dec 12, 2010
    Messages:
    1,758
    Resources:
    2
    Tutorials:
    2
    Resources:
    2
    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:
    Code (vJASS):

    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:
    [​IMG]
    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.
     

    Attached Files:

  2. Wareditor

    Wareditor

    Joined:
    Jan 16, 2009
    Messages:
    681
    Resources:
    3
    Maps:
    3
    Resources:
    3
    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.
     
  3. DracoL1ch

    DracoL1ch

    Joined:
    Dec 12, 2010
    Messages:
    1,758
    Resources:
    2
    Tutorials:
    2
    Resources:
    2
    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.
     
  4. Wareditor

    Wareditor

    Joined:
    Jan 16, 2009
    Messages:
    681
    Resources:
    3
    Maps:
    3
    Resources:
    3
    "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.
     
  5. DracoL1ch

    DracoL1ch

    Joined:
    Dec 12, 2010
    Messages:
    1,758
    Resources:
    2
    Tutorials:
    2
    Resources:
    2
    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.
     
  6. Wareditor

    Wareditor

    Joined:
    Jan 16, 2009
    Messages:
    681
    Resources:
    3
    Maps:
    3
    Resources:
    3
    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.
     

    Attached Files:

  7. DracoL1ch

    DracoL1ch

    Joined:
    Dec 12, 2010
    Messages:
    1,758
    Resources:
    2
    Tutorials:
    2
    Resources:
    2
    Sigh. If there would be 2 triggers which detects PLAYER_UNIT_SPELL_EFFECT event, the one located below would also receive null and 0.
    Why its bad? Because other getters work just like expected, staking data into a pile.
     
  8. Wareditor

    Wareditor

    Joined:
    Jan 16, 2009
    Messages:
    681
    Resources:
    3
    Maps:
    3
    Resources:
    3
    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.
     

    Attached Files:

    Last edited: Jun 17, 2018
  9. DracoL1ch

    DracoL1ch

    Joined:
    Dec 12, 2010
    Messages:
    1,758
    Resources:
    2
    Tutorials:
    2
    Resources:
    2
    So, it become broken withing single trigger thread then? Good to know, still bugged, but at least workaroundable.
     
  10. actboy168

    actboy168

    Joined:
    May 1, 2012
    Messages:
    95
    Resources:
    1
    Tools:
    1
    Resources:
    1
    Thanks for informing. I‘m thinking about fixing it in my editor.
     
  11. GhostWolf

    GhostWolf

    Joined:
    Jul 29, 2007
    Messages:
    4,836
    Resources:
    2
    Tools:
    1
    Tutorials:
    1
    Resources:
    2
    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.
     
  12. DracoL1ch

    DracoL1ch

    Joined:
    Dec 12, 2010
    Messages:
    1,758
    Resources:
    2
    Tutorials:
    2
    Resources:
    2
    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
     
  13. Kam

    Kam

    Joined:
    Aug 3, 2004
    Messages:
    2,625
    Resources:
    23
    Models:
    8
    Icons:
    2
    Maps:
    13
    Resources:
    23
    Thank you for posting this and providing a test map.
     
  14. GhostWolf

    GhostWolf

    Joined:
    Jul 29, 2007
    Messages:
    4,836
    Resources:
    2
    Tools:
    1
    Tutorials:
    1
    Resources:
    2
    Yes, most of the getters are global, didn't say all of them are. But this has been old news when I wrote my first custom script :thinking:
     
  15. DracoL1ch

    DracoL1ch

    Joined:
    Dec 12, 2010
    Messages:
    1,758
    Resources:
    2
    Tutorials:
    2
    Resources:
    2
    Sigh. As I told, that dedicated to blizzard.