[Snippet] SpellEffectEvent

[Magtheridon96]

Calling it Table was a good choice.

We shouldn't let history ruin shit for us. Even Bjarne Stroustrup regrets making C source-compatible with C++ (Hint: It's not).

local NewTable table looks silly anyway. A complete beginner would be like "OMG Why did this stupid programmer call it NewTable? How is that convenient? Why not Table? For fucks sake dude..." and he'd be mostly right.

Besides, what does Vex's Table have that this doesn't? More generated code? D)

 

[Bribe]

NewTable was a bad name, Table is the best name, but if I could go back in time I would have come up with something unique, for example Stash.

 

[Magtheridon96]

Bucket, maybe HashTable

 

[chobibo]

Stash would have been good, as you can't expect people not to make the mistake of being confused. People do recommend your Table by saying Bribe's Table, so it's partly their fault for using the other one.

 

[Troll-Brain]

Calling it Table was a good choice.

We shouldn't let history ruin shit for us. Even Bjarne Stroustrup regrets making C source-compatible with C++ (Hint: It's not).

local NewTable table looks silly anyway. A complete beginner would be like "OMG Why did this stupid programmer call it NewTable? How is that convenient? Why not Table? For fucks sake dude..." and he'd be mostly right.

Besides, what does Vex's Table have that this doesn't? More generated code? D)

That's not the problem, Vexorian's Table was "heavily" used since a "long time", and the guy is almost a legend, or something like that.
And frankly if i had to write some jass code, most of times, if not all, Vexorian's Table is enough, at least for my coding style.
So you can't come with a new library with the very same name and not the same API, even if it's better and have more features, it's just a way to confuse people.
And yeah NewTable is probably a lame name, but there are other ones, like it has been said.

 

[ZiBitheWand3r3r]

implement

Hello!
My first post in this section;]
Im new in Jass
Have problem with importing this resource to map
After import
// http://www.hiveworkshop.com/forums/jass-resources-412/snippet-registerplayerunitevent-203338/
save map and all ok.
After import SpellEffectEvent I got error from JASS helper:

Jasshelper - step 1 (textmacros, libraries, scopes)
Compile error.
Missing Requirement: optional (libraries cannot require scopes)

I got vJass version: 0.9.I.2

pls help
zibi

 

[Bribe]

you need to have JassHelper 0.A.2.B if I remember right

 

[ZiBitheWand3r3r]

Thanks Bribe!
You were right , after upgrade Jasshelper, map saved ok, I found it on wc3c.net
zibi

 

[IcemanBo]

Submission: [Snippet] SpellEffectEvent v1.1.0.0 Date: 18 October 16 Status: Graveyard Note: The resource was previously approved and therefore should work fine. If you use this script already in your codes, it's okay to keep it there. But now we have this approved [Snippet] RegisterEvent pack, and so it's time to graveyard these specializations.

 

[AGD]

I'm not sure but I dont see how this resource's functionality is included in Bannar's RegisterEvent pack.

 

[Bribe]

It doesn't. This shouldn't have been graveyarded. That one only replaces RegisterPlayerUnitEvent that Magtheridon and I wrote.

Edit: Same with OrderEvent, which no one was using.

 

[Aniki]

This kind of thing has similar approaches by other people but this is definitely the most efficient way to do it.

Maybe it was moved to the graveyard because of the above incorrect notion? The approach used is definitely NOT the most efficient way to do it.

Something like this would be a lot closer to the "most efficient way to do it":

function on_spell_effect_dispatch takes nothing returns boolean
    local integer i = GetSpellAbilityId()

    if i < 'A009' then
        if i < 'A005' then
            if i < 'A003' then
                if i == 'A001' then
                    call func_A001()

                else // if i == 'A002'
                    call func_A002()

                endif

            elsif i == 'A003' then
                call func_A003()

            else // if i == 'A004'
                call func_A004()

            endif

        elsif i < 'A007' then
            if i == 'A005' then
                call func_A005()

            else // if i == 'A006'
                call func_A006()

            endif

        elsif i == 'A007' then
            call func_A007()

        else // if i == 'A008'
            call func_A008()

        endif

    elsif i < 'A013' then
        if i < 'A011' then
            if i == 'A009' then
                call func_A009()

            else // if i == 'A010'
                call func_A010()

            endif

        elsif i == 'A011' then
            call func_A011()

        else // if i == 'A012'
            call func_A012()

        endif

    elsif i < 'A015' then
        if i == 'A013' then
            call func_A013()

        else // if i == 'A014'
            call func_A014()

        endif

    elsif i == 'A015' then
        call func_A015()

    else // if i == 'A016'
        call func_A016()

    endif

    return false
endfunction

function init takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(t, Condition(function on_spell_effect_dispatch))
endfunction

Hashtable lookup and TriggerEvaluate is much slower than if statements arranged in a binary search tree fashion (or probably even in a flat list, for fairly big flat lists even) and normal function calls.

If you don't trust me then go ahead and read this thread.

PS: even Vexorian agrees

 

[Deleted member 219079]

That's not possible in vJASS. This resource is as good as it gets.

 

[IcemanBo]

Bannar has a generic RegisterPlayerUnitEvent. It should also work fine with EVENT_PLAYER_UNIT_SPELL_EFFECT.

 

[AGD]

Yes but that would be slower if you have like hundreds of spells in your map (Each time a spell is cast, all triggerconditions with that event will be evaluated even though only one of them would meet the condition) but with this, each spell is assigned a single trigger which is evaluated if it matches the rawcode of the spell casted.

 

[IcemanBo]

Aha. Hm.

Maybe my decission was wrong. Thanks for bringing it up AGD, will come back to it again.

 

[AGD]

No problem =)

 

[Aniki]

That's not possible in vJASS.

What is that suppose to mean?

This resource is as good as it gets.

I disagree, I think something like this:

function on_spell_effect_dispatch takes nothing returns boolean
    local integer i = GetSpellAbilityId()

    if i == 'A000' then
        call func_A000()

    elseif i == 'A001' then
        call func_A001()

    elseif i == 'A002' then
        call func_A002()

    elseif i == 'A003' then
        call func_A003()

    elseif i == 'A004' then
        call func_A004()

    elseif i == 'A005' then
        call func_A005()

    elseif i == 'A006' then
        call func_A006()

    elseif i == 'A007' then
        call func_A007()

    elseif i == 'A008' then
        call func_A008()

    elseif i == 'A009' then
        call func_A009()

    elseif i == 'A00A' then
        call func_A00A()

    elseif i == 'A00B' then
        call func_A00B()

    elseif i == 'A00C' then
        call func_A00C()

    elseif i == 'A00D' then
        call func_A00D()

    elseif i == 'A00E' then
        call func_A00E()

    elseif i == 'A00F' then
        call func_A00F()

    endif

    return false
endfunction

function spell_effect_init takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(t, Condition(function on_spell_effect_dispatch))
endfunction

Is both easier to write and efficient. Now suppose the user has 1000 triggered spells in their map, I would suggest to use this flat list of if statements and when they decide to release their map and think that those if statements hurt the performance of their map, then and only then, they should rearrange them into a "binary search tree form" using a simple script/program.

Also, some abilities have "phases" that use other events like EVENT_PLAYER_UNIT_SPELL_CHANNEL, EVENT_PLAYER_UNIT_SPELL_CAST, etc. using the "flat list of if statements" approach, it would be very easy to make those, but one would have to create a verson of this library for those events as well (maybe using macros?).

 

[Deleted member 219079]

JassHelper has no functionality to generate such dispatchers. See here.

 

[Bribe]

Right. Aniki, what Jon is saying is the user has to type all those if/elses and function calls manually. Yes, if-blocks into function calls would be the fastest, but even faster would be to save the map maker a LOT of time and debugging.

Not to mention, a hashtable lookup and a trigger evaluate done only once per spell is insanely fast. It will, in fact, get faster than your proposed method at about 50 if/else blocks.

 

[Aniki]

Right. Aniki, what Jon is saying is the user has to type all those if/elses and function calls manually. Yes, if-blocks into function calls would be the fastest, but even faster would be to save the map maker a LOT of time and debugging.

Well I am pretty sure that adding the 2 lines of code in the dispatch function would be the easiest part of making a triggered spell.

Not to mention, a hashtable lookup and a trigger evaluate done only once per spell is insanely fast. It will, in fact, get faster than your proposed method at about 50 if/else blocks.

Don't know how/where you got that number 50, but it doesn't sound right to me.

The TriggerEvaluate call is ~6x times slower than the normal function call (from Alexander244's benchmark in the linked thread above), that means that according to you ~50 if statements (comparing an integer variable with a constant) would take the same amount of time as 5 DoNothing calls and a hashtable lookup, and that's really really hard to agree with.

And allow me to quote myself

and when they decide to release their map and think that those if statements hurt the performance of their map, then and only then, they should rearrange them into a "binary search tree form" using a simple script/program.

 

[Bribe]

Yeah using the hashtable lookup also means the user doesn't have to program a single thing into the function call list. They just call RegisterSpellEffectEvent with the Spell ID and the function name and they are set. Not only that, but your function call list would need to be done from the bottom of the script in case not every spell is encapsulated in a library, requiring a manual edit to war3map.j to insert it into the right place.

A map maker can choose whatever they want, but in larger maps the function call list would be several hundred lines long and difficult to micromanage. The SpellEffectEvent is easier to micromanage (all functionality rooted in one resource), much faster to install and the speed difference will NEVER EVER EVER EVER be felt (just to let you know how fast it already is).

 

[AGD]

And also this is easier for users to handle specially when making a public resource such as spells. They can't do it via single function dispatcher..

 

[Aniki]

A map maker can choose whatever they want

Agree.

Just edit out this part:

This kind of thing has similar approaches by other people but this is definitely the most efficient way to do it.

from the first post because its not true.

PS: for the speed "phreaks" here's a simple script (see attachments) that converts a "flat ifs list" to "binary search tree form", rename the .txt to .html (because .html is not a valid upload extension apparently).

 

[AGD]

This resource is already widely used upon by spell resources and was previously approved and is working properly. And since the above discussion agrees that this is a specialized library for a certain event and aims differently than Bannar's RegisterEvent Pack, I've decided to reapprove this.

Approved

 

[Bribe]

I have just updated this to include an alternative Lua script which also works seamlessly with the upcoming Lua Unit Event that will interact with the BJ.

Edit: also, to what @Aniki was saying a few years back - just want to say this is definitely the most efficient way to do it

One Lua table lookup into a function call. RIP JASS.

 

[IcemanBo]

No offense,
it's not like anyone had issues in performance, efficiency or something alike, using especially this JASS code. Reading your recent posts one could believe JASS is not useable in comparison to Lua. At same time you are a GUI defender, which is itself usually a lot slower than JASS, too.
Just my thoughts.

 

[Bribe]

No offense,
it's not like anyone had issues in performance, efficiency or something alike, using especially this JASS code. Reading your recent posts one could believe JASS is not useable in comparison to Lua. At same time you are a GUI defender, which is itself usually a lot slower than JASS, too.
Just my thoughts.

Hi Iceman, thanks for the feedback. I agree with you and Dr SuperGood qith regards to the further efficiency of this resource being trivial. However, my objective is to do the following things:

1) Support existing JASS or vJass resources as much as the syntax allows

2) Support GUI libraries as much as the syntax allows

3) Create Lua variations of these resources and utilities so that should people switch to Lua they have as big of a pool of resources as I have time to commit to building.

If efficiency can be gained from switching to Lua - which it can - then I will point it out.

If you took a deeper look into the rationale I have had with investing into Lua developmemt, it's primarily driven so that GUI users can benefit from things like accurate Polled Waits, life checks and so forth. Mostly QoL stuff.

 

[Nixaristix]

This is great decision! But how do i need to change this one? This event triggers when my unit learns a skill:

private function Init takes nothing returns nothing
        set gg_trg_Learn_Skills = CreateTrigger(  )
        call TriggerRegisterAnyUnitEventBJ( gg_trg_Learn_Skills, EVENT_PLAYER_HERO_SKILL )
        call TriggerAddCondition(gg_trg_Learn_Skills, Condition( function Conditions))
        call TriggerAddAction( gg_trg_Learn_Skills, function Actions )
    endfunction

or this one:

private function Init takes nothing returns nothing
        set gg_trg_Arrow_Thrower_after_death_Ballista = CreateTrigger(  )
        call TriggerRegisterAnyUnitEventBJ( gg_trg_Arrow_Thrower_after_death_Ballista, EVENT_PLAYER_HERO_REVIVE_FINISH )
       
        call TriggerAddCondition(gg_trg_Arrow_Thrower_after_death_Ballista, Condition( function Conditions))
        call TriggerAddAction( gg_trg_Arrow_Thrower_after_death_Ballista, function Actions )
endfunction

 

[Bribe]

You'd use the following resource:

[Snippet] RegisterEvent pack

 

[Nixaristix]

You'd use the following resource:

[Snippet] RegisterEvent pack

Wow! Thank you very much, this is exactly what I need!

 

[azoriel]

Apologies if this is not within the scope of this topic.

I'm transitioning an older map from GTrigger to use this + RegisterEvent pack. There are many spells that set up cast time effects/animations with EVENT_PLAYER_UNIT_SPELL_CAST. If I were to copy+paste this library and change instances of "effect"/"EFFECT" to "cast"/"CAST", would it work properly? (I would have the regular SpellEffectEvent for standard spell effect events, and then the edited SpellCastEvent with the "changes" for begins casting events) Or is there a better way to do it by editing/adding to the library, without the need for copy/pasting another version with 99% similar code.

Since this is swapping from a map that exclusively used GTrigger, if I were to use the RegisterEvent alone, I would have to change each Action trigger to include "if GetSpellAbilityId() == abilId then... endif". Thus I ask this, since it would mean I would only have to change one line (that is likely at the very bottom of the trigger) from "call GT_AddBeginsCastingAction(func, abilId)" to "RegisterSpellCastEvent(abilId, func)"

The same question also goes for other events such as EVENT_PLAYER_UNIT_PICKUP_ITEM, EVENT_PLAYER_UNIT_DEATH, etc (since as previously said, the map exclusively used GTrigger whenever it was possible), so this may result in the map having a dozen different versions of this library copy/pasted with minor changes if the above works.