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.

[Snippet] SpellEffectEvent

Discussion in 'JASS Resources' started by Bribe, Jan 13, 2011.

  1. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,051
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    A handy approach to using spell effect responses. You can assign an event to fire for only a specific ability code, or when any ability is cast. API is intuitive and avoids creating tons and tons of handles to get the job done. This kind of thing has similar approaches by other people but this is definitely the most efficient way to do it.

    Why this is important

    Effectively, this is supposed to turn:

    Code (vJASS):

    private function onCast takes nothing returns boolean
        if (GetSpellAbilityId() == 'A000') then
            // Do actions
        endif
        return false
    endfunction
     
    private function onInit takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
        call TriggerAddCondition(t, Condition(function onCast))
        set t = null
    endfunction
     


    Into:

    Code (vJASS):

    private function onCast takes nothing returns nothing
        // Do actions
    endfunction
     
    private function onInit takes nothing returns nothing
        call RegisterSpellEffectEvent('A000', function onCast)
    endfunction
     


    As you can see, a lot fewer lines of code. It saves TONS of efficiency on maps with a lot of spells in them. The two main reasons why are:

    1. Does not generate 16 events per spell.
    2. This uses one trigger evaluation instead of one for each individual spell (some maps have quite a few). This helps keep framerate high and fluid when a spell is cast.


    Code (vJASS):
    //============================================================================
    // SpellEffectEvent
    // - Version 1.1.0.0
    //
    // API
    // ---
    //     RegisterSpellEffectEvent(integer abil, code onCast)
    //
    // Requires
    // --------
    //     RegisterPlayerUnitEvent: hiveworkshop.com/forums/showthread.php?t=203338
    //
    // Optional
    // --------
    //     Table: hiveworkshop.com/forums/showthread.php?t=188084
    //
    library SpellEffectEvent requires RegisterPlayerUnitEvent, optional Table
     
    //============================================================================
    private module M
       
        static if LIBRARY_Table then
            static Table tb
        else
            static hashtable ht = InitHashtable()
        endif
       
        static method onCast takes nothing returns nothing
            static if LIBRARY_Table then
                call TriggerEvaluate(.tb.trigger[GetSpellAbilityId()])
            else
                call TriggerEvaluate(LoadTriggerHandle(.ht, 0, GetSpellAbilityId()))
            endif
        endmethod
     
        private static method onInit takes nothing returns nothing
            static if LIBRARY_Table then
                set .tb = Table.create()
            endif
            call RegisterPlayerUnitEvent(EVENT_PLAYER_UNIT_SPELL_EFFECT, function thistype.onCast)
        endmethod
    endmodule
     
    //============================================================================
    private struct S extends array
        implement M
    endstruct
     
    //============================================================================
    function RegisterSpellEffectEvent takes integer abil, code onCast returns nothing
        static if LIBRARY_Table then
            if not S.tb.handle.has(abil) then
                set S.tb.trigger[abil] = CreateTrigger()
            endif
            call TriggerAddCondition(S.tb.trigger[abil], Filter(onCast))
        else
            if not HaveSavedHandle(S.ht, 0, abil) then
                call SaveTriggerHandle(S.ht, 0, abil, CreateTrigger())
            endif
            call TriggerAddCondition(LoadTriggerHandle(S.ht, 0, abil), Filter(onCast))
        endif
    endfunction
     
    endlibrary


    Code (Lua):

    --Lua RegisterSpellEffectEvent
    do
        local tb =  {}
        local trig = nil
       
        function RegisterSpellEffectEvent(abil, onCast)
            if not trig then
                trig = CreateTrigger()
                TriggerRegisterPlayerUnitEventBJ(trig, EVENT_PLAYER_UNIT_SPELL_EFFECT)
                TriggerAddCondition(trig, Condition(function()
                    local i = tb[GetSpellAbilityId()]
                    if i then i() end
                end))
            end
            tb[abil] = onCast
        end
    end
       
    --syntax is:
    --RegisterSpellEffectEvent(FourCC('Abil'), function() print(GetUnitName(GetTriggerUnit())) end)
     
     
    Last edited: Jul 15, 2019
  2. Nestharus

    Nestharus

    Joined:
    Jul 10, 2007
    Messages:
    6,146
    Resources:
    8
    Spells:
    3
    Tutorials:
    4
    JASS:
    1
    Resources:
    8
    At this point
    Code (vJASS):

            local player p = casterowner
            local integer c = casterid
            local integer t = targetid
            local integer a = abilcode
            local real cx = casterx
            local real cy = castery
            local real tx = targetx
            local real ty = targety
     


    You want to probably use an array. Incrementing and decrementing an array would probably be a lot faster than declaring all of those locals ; P.

    No . needed on these. Also please follow JASS convention (camelcase)
    Code (vJASS):

        readonly static real casterX = 0
        readonly static real casterY = 0
        readonly static real targetX = 0
        readonly static real targetY = 0
     


    And why are you storing the x,y positions anyways? The user may not need that data at all. You should remove the storage of the coordinates.

    private function getEvent takes integer abilcode returns Event


    Should be

    private function GetEvent takes integer abilcode returns Event


    Get rid of this
    function OnSpellEffect takes boolexpr c returns nothing


    Fire off via an Event instead within your onCast method. You realize that your recursion will fail with how you have it right now right? For any, the direct event can be used. A function wrapper name would probably be

    function RegisterAnySpellEffectEvent takes boolexpr c returns nothing



    This hurts my eyes to read
    Code (vJASS):

    // RegisterSpellEffectEvent(boolexpr, abilcode)
    // TriggerRegisterSpellEffectEvent(trigger, abilcode)
    // OnSpellEffect(boolexpr)
    // readonly integer SpellEffect.casterid
    // readonly integer SpellEffect.targetid
    // readonly integer SpellEffect.abilcode
    // readonly player SpellEffect.casterowner
    // readonly unit SpellEffect.caster
    // readonly unit SpellEffect.target
    // readonly real SpellEffect.casterx
    // readonly real SpellEffect.castery
    // readonly real SpellEffect.targetx
    // readonly real SpellEffect.targety
     


    Make it prettier ; (, like

    Code (vJASS):

    //Functions:
    //---------------------------------------------------
        // function RegisterSpellEffectEvent takes boolexpr, abilityId returns nothing
        // function TriggerRegisterSpellEffectEvent takes trigger, abilityId returns nothing
        // function RegisterAnySpellEffectEvent takes boolexpr returns nothing

    //SpellEffect:
    //---------------------------------------------------
        // readonly integer casterId
        // readonly integer targetId
        // readonly integer ability
        // readonly player casterOwner
        // readonly unit caster
        // readonly unit target
     
     
  3. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,051
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    >>You want to probably use an array. Incrementing and decrementing an array would probably be a lot faster than declaring all of those locals ; P.

    You're definitely right here.

    >>No . needed on these.

    Definitely not needed, but it is good programming practice, when you use a real, to use at least a . at the end so someone flipping through a script doesn't have to constantly consult the API to know if it's a real or integer when you omit the .

    >>Also please follow JASS convention (camelcase)

    It's more annoying to uppercase obviously separated words, but for the case of matching JASS' already-poor syntax, fine.

    >>You should remove the storage of the coordinates.

    Doing so would make GetSpellTargetX/Y do nothing (remember it's a variable event), so that would be foolish. Although getting rid of caster x/y would be good in case the caster's position changed.

    >>This hurts my eyes to read

    Hence why I could never be an artist. First 6 or 7 months of my JASS career I was just copying Berb, now I don't know what I'm doing.

    Can't fault the recursion as that's not going to screw anything as the spell responses are stored into variables, negating the need for calling the natives. And remember you were the one who told me to use a simple boolexpr evaluation for "any spell event", and now you think I should change it back.

    Editing to factor in the changes.
     
    Last edited: Jan 14, 2011
  4. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,051
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    Updated. Removed the "casterOwner" variable for the extremely unlikely event that the owner changes during that time, and removed casterX and casterY.

    RegisterAnySpellEffectEvent now exists and can actually use the native event responses OR the struct variables. RegisterSpellEffectEvent and the trigger-version can only use the struct variables because they are triggered by a variable event and not by a spell event, but it's more optimal to do it like that anyway.

    This is the first release of mine which uses a shorter scope-prefix to privatise names instead of the longer library prefix (because Vex's map optimiser is so broken). There are other libraries planned to be released like this.
     
  5. Nestharus

    Nestharus

    Joined:
    Jul 10, 2007
    Messages:
    6,146
    Resources:
    8
    Spells:
    3
    Tutorials:
    4
    JASS:
    1
    Resources:
    8
    This now breaks if it's done within the trigger
    Code (vJASS):

    function RegisterAnySpellEffectEvent takes boolexpr c returns nothing
        call TriggerRemoveCondition(trig, func)
        call TriggerAddCondition(trig, c)
        set func = TriggerAddCondition(trig, cond)
    endfunction
     


    You do know that removing a triggercondition from within the trigger stops the trigger in its tracks right? : )

    Just run it thru an Event man >.>.
     
  6. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,051
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    I still don't follow what you mean by that.
     
  7. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,051
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    Updated. I think I know what you mean by that and I've adjusted for it as well.
     
  8. Nestharus

    Nestharus

    Joined:
    Jul 10, 2007
    Messages:
    6,146
    Resources:
    8
    Spells:
    3
    Tutorials:
    4
    JASS:
    1
    Resources:
    8
    Now it's actually slower than just firing off an event... >.<

    You have an extra trigger evaluation now ; |

    2 trigger evals on the firing trigger and 1 trigger evaluation in one of tem

    If you did it right, u'd have 1 trigger eval on firing trigger and 1 trigger evaluation on it

    Plus you can't register triggers to fire whenever a spell fires.


    Just run it through an Event =O, I've said it before and I'll say it as many times as I have to until you get it :p
     
    Last edited: Jan 16, 2011
  9. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,051
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    Updated to fix a JASSHelper bug that automatically prefixes nested scopes even without the public keyword.
     
  10. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,051
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    Updated to 0.1.1.0 in Zinc syntax. This version should be a bit sped up for normal events but with a drawback.

    RegisterAnySpellEffectEvent works just fine, but you can't reference the global variables (SpellEffect.caster, SpellEffect.target, ...) you have to use GetTriggerUnit(), GetSpellTargetUnit(), ...
     
  11. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,051
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    Totally restructured for 0.2.0.0, stripped the library requirements entirely and stripped a lot of the API for raw functionality.

    The only public products are two functions available for users: RegisterSpellEffectEvent(integer whichAbility, boolexpr onCast) and RegisterAnySpellEffectEvent(boolexpr onCast).
     
    Last edited: Jan 21, 2011
  12. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,051
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    Updated to 0.3.0.0 with re-added functionality with the Event library, but this time without the requirement of Vexorian's Table because it breaks with module initializers.

    The speed is almost the same but now you have the TriggerRegisterSpellEffectEvent(integer whichAbility, trigger whichTrigger) functionality back.
     
  13. Nestharus

    Nestharus

    Joined:
    Jul 10, 2007
    Messages:
    6,146
    Resources:
    8
    Spells:
    3
    Tutorials:
    4
    JASS:
    1
    Resources:
    8
    ->TriggerRegisterSpellEffectEvent(integer whichAbility, trigger whichTrigger)

    Trigger first, lol.
     
  14. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,051
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    Useage of the world 'lol' is forthwith prohibited from this forum.
     
  15. Nestharus

    Nestharus

    Joined:
    Jul 10, 2007
    Messages:
    6,146
    Resources:
    8
    Spells:
    3
    Tutorials:
    4
    JASS:
    1
    Resources:
    8
    What's wrong with this JASS : P
    Code (vJASS):

    //boolexpr second? : O
    function RegisterSpellEffectEvent takes integer whichAbility, boolexpr c returns nothing
        call GetEvent(whichAbility).register(c)
    endfunction

    //trigger first
    //============================================================================
    function TriggerRegisterSpellEffectEvent takes trigger whichTrigger, integer whichAbility returns nothing
        call GetEvent(whichAbility).registerTrigger(whichTrigger)
    endfunction

    //boolexpr first
    //============================================================================
    function RegisterAnySpellEffectEvent takes boolexpr c returns nothing
        call TriggerAddCondition(trig, c)
    endfunction
     
     
  16. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,051
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    The effect is ugly.

    RegisterSpellEffectEvent('A000', Condition(function blarg))

    That's more readable than:

    RegisterSpellEffectEvent(Condition(function blarg), 'A000')
     
  17. Nestharus

    Nestharus

    Joined:
    Jul 10, 2007
    Messages:
    6,146
    Resources:
    8
    Spells:
    3
    Tutorials:
    4
    JASS:
    1
    Resources:
    8
    Ugly or not, that's JASS convention ; \...
     
  18. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,051
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    Updated to 0.4.0.0 to use NewTable.

    Updated to be compatible with Table version 1.2.0.0 and higher.
     
    Last edited: Aug 2, 2011
  19. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    8,051
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    I'd like a second opinion on this system to find out if it's approve worthy.
     
  20. Nestharus

    Nestharus

    Joined:
    Jul 10, 2007
    Messages:
    6,146
    Resources:
    8
    Spells:
    3
    Tutorials:
    4
    JASS:
    1
    Resources:
    8
    This should be private

    static method onInit takes nothing returns nothing


    Otherwise looks good



    Remember that a module initializer must be private, otherwise it will turn into a struct initializer.


    This is the second time you've done a public module initializer : P.