• 🏆 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!
  • It's time for the first HD Modeling Contest of 2024. Join the theme discussion for Hive's HD Modeling Contest #6! Click here to post your idea!

[Snippet] Triggered Spell Handler

JASS:
//RegisteredSpell_Add
//   Parameters:
//      integer Id - The spell id to bind the function to.
//      code F     - The function to be bound to a spell.
//   Returns:
//      nothing.
//
//   Adds a spell, that when cast, gets called via TriggerEvaluate by the handler trigger.
//   Note: This can be called before or after RegisteredSpell_Init.
function RegisteredSpell_Add takes integer Id,code F returns nothing
    local trigger T=CreateTrigger()
    call TriggerAddCondition(T,Filter(F))
    call SaveTriggerHandle(GLOBAL_HASHTABLE,0,-Id,T)
    set T=null
endfunction
//RegisteredSpell_AddEx
//   Parameters:
//      integer Id   - The spell id to bind the function to.
//      filterfunc F - The function(s) to be bound to a spell.
//   Returns:
//      nothing.
//
//   Same as RegisteredSpell_Add, expect that by directly passing a filterfunc you can register multiple functions to the same spell.
//      For example: call RegisteredSpell_AddEx('A000',Or(function SpellA,function SpellB))
//   Note: This can be called before or after RegisteredSpell_Init.
function RegisteredSpell_AddEx takes integer Id,boolexpr Fs returns nothing
    local trigger T=CreateTrigger()
    call TriggerAddCondition(T,Fs)
    call SaveTriggerHandle(GLOBAL_HASHTABLE,0,-Id,T)
    set T=null
endfunction
//RegisteredSpell_Cast
//   Parameters:
//      nothing.
//   Returns:
//      nothing.
//
//   This is the handler trigger. It's job is to call the registered function to the casted spell, if there is one.
function RegisteredSpell_Cast takes nothing returns nothing
    local trigger T=LoadTriggerHandle(GLOBAL_HASHTABLE,0,-GetSpellAbilityId())
    if T!=null then
        call TriggerEvaluate(T)
        set T=null
    endif
endfunction
//RegisteredSpell_Init
//   Parameters:
//      nothing.
//   Returns:
//      nothing.
//
//   This function, when called, initializes the handler trigger.
//    Note: This can be called before or after RegisteredSpell_Add or RegisteredSpell_AddEx.
function RegisteredSpell_Init takes nothing returns nothing
    local trigger T=CreateTrigger()
    call TriggerAddCondition(T,Filter(function RegisteredSpell_Cast))
    call TriggerRegisterAnyUnitEventBJ(T,EVENT_PLAYER_UNIT_SPELL_EFFECT)
    set T=null
endfunction
These three functions sets up the entry point for all triggered abilities. The purpose behind this is so only two triggers fire on spell cast; the one crated by RegisteredSpell_Init() and the trigger registered to to the spell. This saves having the massive if-then-elseif-else branches as well as removing the need for many triggers listening for the same event.
Steps for importing:
  1. Copy and paste into map header, or anywhere above your initialization trigger.
Steps for using:
  1. Call RegisteredSpell_Init() first to initialize the handler trigger (this doesn't have to be called first).
  2. Call RegisteredSpell_Add() or RegisteredSpell_AddEx() for all abilities you want triggered through this handler (again, you can call this before you initialize the handler trigger).
  3. Cast some spells.
Configurables:
  • GLOBAL_HASHTABLE needs to be replaced with a hashtable that you use.
  • Index of zero can be replaced with any constant that doesn't conflict with your hashtable.
  • EVENT_PLAYER_UNIT_SPELL_EFFECT can be replaced with the spell event that you want, incase you want to trigger it at SPELL_CAST instead or something.
Notes:
  • I use the negative value of the spell id to lessen the chances of it conflicting with index 0 and other entries to the index.
  • I use the evil, red Blizzard JASS function TriggerRegisterAnyUnitEventBJ because there is no sense to inline it. Feel free to change it/inline the entire initialization function to suit your map.
  • To have two or more spells bound to the same function (or functions) you can copy the trigger over with call SaveTriggerHandle(GLOBAL_HASHTABLE,0,-NEW_SPELL_ID,LoadTriggerHandle(GLOBAL_HASHTABLE,0,-OLD_SPELL_ID)) where OLD_SPELL_ID is the already (first) registered spell for the function(s) and NEW_SPELL_ID is the second (or later) spell to also register to the function(s).
  • Yes, I know it is not vJASS, but it doesn't need to be either. If you are so inclined to only use vJASS then just wrap all the code in library SpellHandler /* ... */ endlibrary Could even properly create the initialization function.
 
This needs more features to make it useful:
- it should assign a unique spell ID per individual spellcast that can be retrieved to be able to code channeling spells - just make an integer that gets incremented every spellcast and add it to a spell struct ... oh and convert this system to a struct based system while you're at that
- it should also save all the event responses in the struct aswell: Trigger Unit, Target Unit, Casting unit, Target coordinates, etc.
 
Zwiebelchen; Why?
  • What's the purpose of having a ref count of spellcasts?
  • This doesn't need to be a struct at all.
  • The event response function work just fine when called from inside the registered function(s).
  • Stop being a vJASS addict, you won't get any speed gain from it.
  • The point of this system is to provide a fast entry point into the triggered ability. It is not meant to give any functionality to any spell. The spell's function should keep track of its own ref count and instances and other such things as needed.
  • Lastly; I don't do anything with vJASS, you should know this by at least reading my signature.
It fulfills its purpose completely and without any other side effect. As per the JASS section rules that is what a submission should do (the exception to this being add-ons for the script).
 
There is a system which does this:
http://www.hiveworkshop.com/forums/jass-resources-412/snippet-spelleffectevent-187193/

And judging by the thread I made a while back, people were against duplicates unless there was a good reason to have both (or replace the old one). I hold that opinion too.

But as for the code itself, in case you still want some review:
  • GLOBAL_HASHTABLE isn't easily configurable. In vanilla JASS, generally it is better to have a constant function:
    JASS:
    constant function GetGlobalHashtable takes nothing returns hashtable
        return udg_SomeHashtable
    endfunction
    And then you would use GetGlobalHashtable() in place of GLOBAL_HASHTABLE. This will prevent the user from having to replace all instances of it with their hash.
  • You don't have any actions to check if the event has already been registered for that ID. You should just see if a trigger is already saved for that ID and then add a condition to it (if it exists in the hash). If not, then create the trigger and save it in the hash (see SpellEffectEvent). That way, it can also support multiple registers and you won't even need RegisteredSpell_AddEx.

But again, in the end it'll end up a clone of SpellEffectEvent translated into vanilla JASS.

Stop being a vJASS addict, you won't get any speed gain from it.

vJASS isn't used for speed (at least, that shouldn't be the main reason for it). It should be used for proper API's.
 
I don't have it check for duplicates based on two grounds:
  1. All the spell registration is assumed to be done in one place (read; function) so it's easy to check for duplicates (manually).
  2. I originally made it handle that choice of simply adding it to the trigger, however adding more conditions isn't the fastest method. The fastest method is having a single condition which has a boolenxpr that is a balanced binary tree of ORs. I don't know why but it is. The code to account for all that was too heavy and complex for it to useful. The map maker can simple use the second register function and build their OR tree then and there.
I have the 'configurables' inlined the way I do because it isn't built to cater to the constant inlining you have with JNGP (which because I don't is probably why this will die before anyone tosses out the 'duplicate library' reason).What does SpellEffectEvent turn into JASS wise? Also, a very minor thing but registering abilities is faster with mine because I don't double access the hashtable. Also there is only a single trigger used as a handler versus the 16 created with RegisterPlayerUnitEvent, again a minor issue.If you're using a library such as this, SpellEffectEvent and/or RegisterPlayerUnitEvent it is quite safe to assume you know how to do a replace function for a single word and are comfortable with using JASS.
 
Don't focus too much on micro-optimizations, it can cause liver cancer as early as age 36.

Consider the scope of the system: the function 'RegisteredSpell_Add' will generally be
used on initialization. It won't be used in periodic functions. Since it is a one-time one-deal
sort of function, micro-optimization can be excessive. After all, its biggest boon is reducing the
complexity when evaluating spell cast functions. Focus on a good API.

Speaking of which, despite Or() being slightly faster for carrying out the
evaluation, having extra trigger conditions is really not that bad considering:
  1. You can remove an entire function.
  2. You can easily support multiple registers.

Fun fact: part of In'N'Out's success can be attributed to their relatively small menu. Psychologically,
it is appealing. Nothing is more daunting than seeing a huge board of tiny text with gorillions of items,
where all-the-while some cashier is pressing you to hurry with their evil gaze.

Likewise--having one function is pretty nice. Why have the user make extra decisions on what to
use at different times? It is generally a detraction (this varies from case-to-case. In this case, consolidation is better IMO.)

Zeatherann said:
I have the 'configurables' inlined the way I do because it isn't built to cater to the constant inlining you have with JNGP

Use constant functions. You don't need jasshelper to do the inlining--Vex's optimizer will inline
those constants too. It has been tradition to use constant functions far before
vJASS even existed.
The point is to have something easily configurable. Why have the user search through the code to
change 3-4 items, when he can just change one? This is a rule throughout all programming languages,
and without find/replace, it is rather painful to edit.

Zeatherann said:
What does SpellEffectEvent turn into JASS wise?

This is what it looks like in regular JASS, essentially:

JASS:
//*************************************************
//*
//*  SpellEffectEvent
//*
//*************************************************
//*
//*  Configuration:

    constant function SEE_GetHashtable takes nothing returns hashtable
        return udg_Hash
    endfunction

//*
//*************************************************
//*
//*  RegisterSpellEffectEvent(integer abil, code onCast)
//*
//*************************************************

function RegisterSpellEffectEvent takes integer abil, code callback returns nothing
    if not HaveSavedHandle(SEE_GetHashtable(), 0, abil) then
        call SaveTriggerHandle(SEE_GetHashtable(), 0, abil, CreateTrigger())
    endif
    call TriggerAddCondition(LoadTriggerHandle(SEE_GetHashtable(), 0, abil), Filter(callback))
endfunction

function SpellEffectEventCast takes nothing returns boolean
    call TriggerEvaluate(LoadTriggerHandle(SEE_GetHashtable(), 0, GetSpellAbilityId()))
endfunction

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

Zeatherann said:
Also, a very minor thing but registering abilities is faster with mine because I don't double access the hashtable. Also there is only a single trigger used as a handler versus the 16 created with RegisterPlayerUnitEvent, again a minor issue.
If you're using a library such as this, SpellEffectEvent and/or RegisterPlayerUnitEvent it is quite safe to assume you know how to do a replace function for a single word and are comfortable with using JASS.

Refer to my explanation on the top. As for RegisterPlayerUnitEvent–that saves handles in the end.
As for replacing, it isn't that a user doesn't know how to do it–it is just inconvenient.

Zeatherann said:
(which because I don't is probably why this will die before anyone tosses out the 'duplicate library' reason).

The general consensus was that there wasn't to be duplicate libraries unless there was a sufficient
reason for one to be replaced, or for the two to coexist. Since SpellEffectEvent already does what's
needed in a solid way, I think it should stay. There may be room to cache something here or there
in Bribe's code, but it isn't any reason to reject it or replace it. But don't take it too hard, I assume you
didn't know about the library when you posted this (it is pretty old). To prevent duplicates, I think this
should be graveyarded.
 
Zwiebelchen; Why?
  • What's the purpose of having a ref count of spellcasts?
  • This doesn't need to be a struct at all.
  • The event response function work just fine when called from inside the registered function(s).
  • Stop being a vJASS addict, you won't get any speed gain from it.
  • The point of this system is to provide a fast entry point into the triggered ability. It is not meant to give any functionality to any spell. The spell's function should keep track of its own ref count and instances and other such things as needed.
  • Lastly; I don't do anything with vJASS, you should know this by at least reading my signature.
It fulfills its purpose completely and without any other side effect. As per the JASS section rules that is what a submission should do (the exception to this being add-ons for the script).
1) The purpose of having a unique spellcast ID is just to make life easier for GUI users - as you target the vanilla editor userbase, you can't neglect the GUIers (most Jassers will use vJass anyway, so GUIers will be the only people interested in this). Other than that, it still has a purpose outside of GUI use:
When making a channeling spells that does something every second, you need to check if the current order of the unit is still the same every "tick". However, you can fool spell scripts that are made this way by recasting the same ability. As the order string is still the same, even if the spell cast instance isn't, it will think you are still channeling the same spell. Having a unique spellcast ID allows you to compare if the spell got recasted during it's duration.

2) Structs make your life easier, especially when you want to retrieve more than just one value from a single function.

3) Native event responses have a lot of bugs and except for triggering unit, are not local either. A spellcast system should come with local event responses to make life easier

4) Ignorance is a bliss. Or maybe not.

5) In order to get the functionality I proposed I'll basicly have to use a second spellcasting system. And such a system will naturally come with all the functionality your system provides aswell (as it's a core mechanic needed for such a system) - which makes your system redundant in this case.

6) That's fine for me. I'm just saying that with very little extra effort, this system could have a lot more uses.
 
It doesn't matter, this submission will be graveyarded anyways because of the already existing vJASS library aforementioned. There is no point in having a core library of this purpose to provide each spellcast with a unique id/struct instance because it doesn't need one. Not all spells care about that. This purpose allows the most freedom for any spell registered to be handled by it. Your 'buggy' natives are only shown when you use them after waits (or a separate thread). Structs (really parallel arrays) are indeed useful, especially if you want a function to return more than one value. Do any of my functions return anything? Is this purpose suppose to care about orders and channeling? No. It's purpose is to turn trigger ability entry-points from O(n) to O(log n) while removing the need for checking the ability-id in the spell code itself. Yes, you'll have to use a second library for channeled spells. However you'd use this (apparently you shouldn't use this) or the already-approved vJASS spellcast library for detecting the initial spellcast and starting the spell's effect(s).I will not update or progress with this resource publicly since it is no longer worth my time because no matter what I do it'll never get approved.
 

Bannar

Code Reviewer
Level 26
Joined
Mar 19, 2008
Messages
3,140
Dude, if you wanted to update this resource or to get this approved you'd better fix your attitude first.

There are rules in jass subforum as are in any of forums. Its not an accident that "Resources" contain set of unique, effective and functional snippets. There is much to take advangate of, Search Tool would be the first one.

Everyone of us has some own resources or even modified public code for his/her own purposes. You've posted yours, and there is nothing wrong with it, in contrast, we understand that.

However, similar lib already exists, what has been already prompted.
Whats the correct action? After searching and finding an answer on question: does my resource already exists?, you can check the form of such code. Meaby your is more specific or generic in some way. If its not, its appropriate to post a replay within found thread instead of immidiately creating new thread on your own. Its a mature approach.

You could replay to, lets say Bribe's spell event with proposition of jass version, since his vJass one can be easily ported. You got a point, in such case, its desirable to have both, jass and vjass forms, especially since here there is no need for any extensive modifications to be done.

With such move you get respect, and whatmore, instead of whining you actually help out the community.
Besides, this could use some optimizations, again as mentioned.

Don't resignate, however, from posting or taking part in Jass subforum discussions as well as uploading new, usefull stuff.
 
it's quite a miserable fact that a JASS version of a library can't co-exist with a vJASS one.

That's not true.

Anyway, I had been previously confused about the usage of this. I was under the impression you had to callRegisteredSpell_Initeach time.

We should not have multiple scripts that do the same thing however I think creating a plain JASS version of SpellEffectEvent would be the best option. This way there is no API or functionality difference.
 
Level 19
Joined
Mar 18, 2012
Messages
1,716
This is what SpellEffectEvent does.
Well not excacly ... you overwrite an existing trigger handle in case you register the same ability id twice. There is no debug warning for the user.
Better would be to load the already existing trigger handle and register the expression to it.
Then you have SpellEffectEvent.

This is written in plain JASS and SpellEffectEvent is vJass, but rather then writting a copy you could contact Bribe and ask if he adds a JASS version to his thread.
He is writting JASS only stuff anyway nowadays.

The overall format should stick to the JPAG established by Bribe to keep code on THW uniform.

Graveyarded
 
Top