Criticism of Spell-casting Responder

12-16-2008, 07:01 AM

From the purport of this thread:
http://www.hiveworkshop.com/forums/f...ps-lag-110473/

I decided to make this script. It 'centralizes' all spell casting triggers into one 'main'. Peculiar spell executions are compartmentalized in functions.
These functions are ran on spell-casting-event if the event ability Id matches that of the function's spell. Event variables (e.g. GetTriggerUnit(), GetSpellTargetUnit(), etc.) are stored as global variables which must be stored into locals variables in the functions, at their first lines, if they are to be used.

Criticism? Anything I should fix or change?

JASS:

library SpellCastResponder initializer Init

globals
    unit SpellCaster = null
    real SpellTargetLocX = 0.00
    real SpellTargetLocY = 0.00
    unit SpellTargetUnit = null
    item SpellTargetItem = null
    destructable SpellTargetDest = null
endglobals

    function interface SpellCastFunc takes nothing returns nothing

struct Responder

    private integer intId //Ability Id that coheres the spell event with the spell function.
    private SpellCastFunc func
    private boolean boolCaster //Indicates if spell casters are a factor for the spell (function).
    private boolean boolUnit //Indicates if spell target units are a factor for the spell (function).
    private boolean boolPoint //Indicates if spell target points (coordinates) are a factor for the spell (function).
    private boolean boolItem //Indicates if spell target items are a factor for the spell (function).
    private boolean boolDest //Indicates if spell target destructables are a factor for the spell (function).
    
    public static method Create takes integer abilityId, SpellCastFunc func, boolean c, boolean u, boolean p, boolean i, boolean d returns Responder
        local Responder New = Responder.allocate()
        
        //Input.
        set New.intId = id
        set New.func = func
        set New.boolCaster = c
        set New.boolUnit = u
        set New.boolPoint = p
        set New.boolItem = i
        set New.boolDest = d

        //Manage indices.
        set New.intInstIndex = Responder.intCountInst
        set Responder.Inst[Responder.intCountInst] = New
        set Responder.intCountInst = Responder.intCountInst + 1

        return New
    endmethod

    public static method Destroy takes nothing returns nothing
        //(Nothing to flush.)
        //Manage indices.
        set Responder.intCountInst = Responder.intCountInst - 1
        set Responder.Inst[Responder.intCountInst].intInstIndex = .intInstIndex
        set Responder.Inst[.intInstIndex] = Responder.Inst[Responder.intCountInst]
        set Responder.Inst[Responder.intCountInst] = 0
    endmethod

    private static Responder array Inst[8190]
    private static integer intCountInst = 0
    private integer intInstIndex

    private static method Respond takes nothing returns boolean
        local integer INT_Index = 0
        local integer INT_Id = GetSpellAbilityId()
        local location LOC_Target = null

        //Find a matching responder.
        loop
            exitwhen INT_Index == Responder.intCountInst
            if Responder.Inst[INT_Index].intId == INT_Id then
                //Prepare event variables.
                //---Caster.
                if Responder.Inst[INT_Index].boolCaster then
                    set SpellCaster = GetTriggerUnit()
                endif

                //---Target unit.
                if Responder.Inst[INT_Index].boolUnit then
                    set SpellTargetUnit = GetSpellTargetUnit()
                endif

                //---Target point (coordinates).
                if Responder.Inst[INT_Index].boolPoint then
                    set LOC_Target = GetSpellTargetLoc()

                    set SpellTargetLocX = GetLocationX(LOC_Target)
                    set SpellTargetLocY = GetLocationY(LOC_Target)

                    call RemoveLocation(LOC_Target)
                    set LOC_Target = null
                endif

                //---Target item.
                if Responder.Inst[INT_Index].boolItem then
                    set SpellTargetItem = GetSpellTargetItem()
                endif

                //---Target destructable.
                if Responder.Inst[INT_Index].boolDest then
                    set SpellTargetDest = GetSpellTargetDestructable()
                endif

                //---Run the function.
                call Responder.Inst[INT_Index].func.execute()
            endif
            set INT_Index = INT_Index + 1
        endloop
    endmethod

    private static trigger trig = CreateTrigger() //Responds to spell CAST/EFFECT/FINISH events.

    //Initialize above trigger.
    public static method Initialize takes nothing returns nothing
        call TriggerRegisterAnyUnitEventBJ(Responder.trig, EVENT_PLAYER_UNIT_SPELL_CAST)
        call TriggerRegisterAnyUnitEventBJ(Responder.trig, EVENT_PLAYER_UNIT_SPELL_EFFECT)
        call TriggerRegisterAnyUnitEventBJ(Responder.trig, EVENT_PLAYER_UNIT_SPELL_FINISH)
        call TriggerAddAction(Responder.trig, function Responder.Respond)
    endmethod

endstruct

    private function Init takes nothing returns nothing
        call Responder.Initialize()
    endfunction
    
endlibrary

12-16-2008, 02:39 PM

rain9441

I use a similar system in my map. But instead of an array I use gamecache as a hash table of ints to a linked-list of function interfaces+conditional variables. I also have seperate categories for global spell event handlers that have conditions which are evaluated first, but there should be a lot less of these.

May also want to differentiate all the events into different categories of responders. The same thing is being called when the spell casts, starts effect, or finishes casting.

Any reason you don't rename the public static Initialize method to onInit?

EDIT: Oh yeah forgot one thing. Theres no reason you can't use this for more than just spells. Pretty much every generic trigger that fires can do this. I like to do it with a generic unit-takes-damage trigger that uses a hash table of the damaging unit's unittype =]. This way I can have 50 different unit-takes-damage events linked to my units and the resolution of the trigger is O(1), but 1 is a gamecache call =]