wc3campaigns
WC3C Homepage - www.wc3c.netUser Control Panel (Requires Log-In)Engage in discussions with other users and join contests in the WC3C forums!Read one of our many tutorials, ranging in difficulty from beginner to advanced!Show off your artistic talents in the WC3C Gallery!Download quality models, textures, spells (vJASS/JASS), systems, and scripts!Download maps that have passed through our rigorous approval process!

Go Back   Wc3C.net > Resources > Code Resources > Scripts
User Name
Password
Register Rules Get Hosted! Chat Pastebin FAQ and Rules Members List Calendar



Reply
 
Thread Tools Search this Thread
Old 04-07-2009, 11:24 PM   #1
Anitarf
Procrastination Incarnate


Development Director
 
Join Date: Feb 2004
Posts: 8,028

Submissions (19)

Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)

2008 Spell olympics - Fire - SilverApproved Map: Old School Alliance TacticsHero Contest #2 - 3rd PlaceSpell making session 2 winner

Default SpellEvent

I feel the documentation in the library explains it well enough, so there's not much I can add here.
It requires Table, by the way.
Collapse JASS:
library SpellEvent initializer Init requires Table

//*****************************************************************
//*  SPELL EVENT LIBRARY 1.2
//*
//*  written by: Anitarf
//*  requires: -Table
//*
//*  Maps with many triggered spells require many triggers that run
//*  on spell events. Whenever a spell is cast, all those triggers
//*  need to be evaluated by the game even though only one actually
//*  needs to run. This library has been written to reduce the
//*  number of triggers in such maps; instead of having a trigger
//*  per spell, this library contains a single trigger which then
//*  runs only the code associated with the spell that's actually
//*  being cast.
//*
//*  Perhaps more significant than the marginal speed gain is the
//*  feature that allows you to access all the spell event
//*  responses from all spell events, something that the native
//*  functions senselessly do not support. With this system you can
//*  for example easily get the target unit of the spell on the
//*  casting finish event.
//*
//*  All functions following the Response function interface that
//*  is defined at the start of this library can be used to respond
//*  to spell events. You can register a response with one of the
//*  following functions, each for a different spell event:
//*
//*    function RegisterSpellChannelResponse takes integer spellId, Response r returns nothing
//*    function RegisterSpellCastResponse takes integer spellId, Response r returns nothing
//*    function RegisterSpellEffectResponse takes integer spellId, Response r returns nothing
//*    function RegisterSpellFinishResponse takes integer spellId, Response r returns nothing
//*    function RegisterSpellEndCastResponse takes integer spellId, Response r returns nothing
//*
//*  The first event occurs at the very start of the spell, when
//*  the spell's casting time begins; most spells have 0 casting
//*  time, so in most cases this first event occurs at the same
//*  time as the second one, which runs when the unit actually
//*  begins casting a spell by starting its spell animation. The
//*  third event occurs when the spell effect actually takes place,
//*  which happens sometime into the unit's spell animation
//*  depending on the unit's "Animation - Cast Point" property.
//*  The fourth event runs if the unit finishes casting the spell
//*  uninterrupted, which might be important for channeling spells.
//*  The last event runs when the unit stops casting the spell,
//*  regardless of whether it finished casting or was interrupted.
//*
//*  If you specify a spell id when registering a response then
//*  that response will only run when that ability is cast; only
//*  one function per ability per event is supported, if you
//*  register more responses then only the last one registered will
//*  be called. If, however, you pass 0 as the ability id parameter
//*  then the registered function will run for all spells. Up to
//*  8190 functions can be registered this way for each event.
//*  These functions will be called before the ability's specific
//*  function in the order they were registered.
//*
//*  This library provides its own event responses that work
//*  better than the Blizzard's bugged native cast event responses.
//*  They still won't work after a wait, but unlike Blizzard's
//*  natives they will work on all spell events.
//*
//*  Here are usage examples for all event responses:
//*
//*    local integer a = SpellEvent.AbilityId
//*    local unit u = SpellEvent.CastingUnit
//*    local unit t = SpellEvent.TargetUnit
//*    local item i = SpellEvent.TargetItem
//*    local destructable d = SpellEvent.TargetDestructable
//*    local location l = SpellEvent.TargetLoc
//*    local real x = SpellEvent.TargetX
//*    local real y = SpellEvent.TargetY
//*    local boolean b = SpellEvent.CastFinished
//*
//*  SpellEvent.TargetLoc is provided for odd people who insist on
//*  using locations, note that if you use it you have to cleanup
//*  the returned location yourself.
//*
//*  SpellEvent.CastFinished boolean is intended only for the
//*  EndCast event as it tells you whether the spell finished or
//*  was interrupted.
//*
//*  Note that a few spells such as Berserk and Wind Walk behave
//*  somewhat differently from regular spells: they are cast
//*  instantly without regard for cast animation times, they do not
//*  interrupt the unit's current order, as well as any spell it
//*  may be casting. SpellEvent 1.1 now handles such spells without
//*  errors provided they are truly instant (without casting time).
//*
//*  It also turned out that a few rare abilities like Charge Gold
//*  & Lumber trigger a spell effect event, but not any other.
//*  SpellEvent 1.2 no longer ignores these lone effect events.
//*****************************************************************

    // use the RegisterSpell*Response functions to add spell event responses to the library
    public function interface Response takes nothing returns nothing

// ================================================================

    private keyword effectDone
    private keyword init
    private keyword get

    private struct spellEvent
        private static HandleTable casterTable
        boolean effectDone=false

        integer AbilityId
        unit CastingUnit
        unit TargetUnit
        item TargetItem=null
        destructable TargetDestructable=null
        real TargetX=0.0
        real TargetY=0.0
        boolean CastFinished=false
        
        // Some abilities like Berserk can be cast instantly without interrupting
        // the caster's current order, which includes any spells the caster may
        // already be casting. The following member allows the system to recover
        // the original spellEvent when such an instant spell overwrites it.
        private spellEvent interrupt

        method operator TargetLoc takes nothing returns location
            return Location(.TargetX, .TargetY)
        endmethod
        
        private static method create takes nothing returns spellEvent
            return spellEvent.allocate()
        endmethod
        static method init takes nothing returns spellEvent
            local spellEvent s=spellEvent.allocate()
            set s.AbilityId = GetSpellAbilityId()
            set s.CastingUnit = GetTriggerUnit()
            set s.TargetUnit = GetSpellTargetUnit()
            if s.TargetUnit != null then
                set s.TargetX = GetUnitX(s.TargetUnit)
                set s.TargetY = GetUnitY(s.TargetUnit)
            else
                set s.TargetDestructable = GetSpellTargetDestructable()
                if s.TargetDestructable != null then
                    set s.TargetX = GetDestructableX(s.TargetDestructable)
                    set s.TargetY = GetDestructableY(s.TargetDestructable)
                else
                    set s.TargetItem = GetSpellTargetItem()
                    if s.TargetItem != null then
                        set s.TargetX = GetItemX(s.TargetItem)
                        set s.TargetY = GetItemY(s.TargetItem)
                    else
                        set s.TargetX = GetSpellTargetX()
                        set s.TargetY = GetSpellTargetY()
                    endif
                endif
            endif
            set s.interrupt=spellEvent.casterTable[s.CastingUnit]
            set spellEvent.casterTable[s.CastingUnit]=integer(s)
            return s
        endmethod
        method onDestroy takes nothing returns nothing
            if .interrupt==0 then
                call spellEvent.casterTable.flush(.CastingUnit)
            else
                set spellEvent.casterTable[.CastingUnit]=.interrupt
            endif
            set .CastingUnit=null
        endmethod

        static method get takes unit caster returns spellEvent
            return spellEvent(spellEvent.casterTable[caster])
        endmethod
        static method onInit takes nothing returns nothing
            set .casterTable=HandleTable.create()
        endmethod
    endstruct
    
    globals
        spellEvent SpellEvent=0
    endglobals
    
// ================================================================

    //! textmacro spellEvent_make takes name
    globals
        private Response array $name$CallList
        private integer $name$CallCount=0
        private Table $name$Table
    endglobals

    private function $name$Calls takes integer id returns nothing
        local integer i=0
        local spellEvent previous=SpellEvent
        set SpellEvent=spellEvent.get(GetTriggerUnit())
        loop
            exitwhen i>=$name$CallCount
            call $name$CallList[i].evaluate()
            set i=i+1
        endloop
        if $name$Table.exists(id) then
            call Response($name$Table[id]).evaluate()
        endif
        set SpellEvent=previous
    endfunction

    function RegisterSpell$name$Response takes integer spellId, Response r returns nothing
        if spellId==0 then
            set $name$CallList[$name$CallCount]=r
            set $name$CallCount=$name$CallCount+1
        else
            set $name$Table[spellId]=integer(r)
        endif
    endfunction
    //! endtextmacro

    //! runtextmacro spellEvent_make("Channel")
    //! runtextmacro spellEvent_make("Cast")
    //! runtextmacro spellEvent_make("Effect")
    //! runtextmacro spellEvent_make("Finish")
    //! runtextmacro spellEvent_make("EndCast")

// ================================================================

    globals
        // Morph abilities like Metamorphosis will cause an additional spell effect
        // event to run when the caster morphs back to its original form. To avoid
        // such duplicates, SpellEvent is designed to ignore any effect event that
        // does not have a matching channel event preceding it.
        // However, there are also rare abilities, like Charge Gold&Lumber, which
        // only cause an effect event to run, so these events must not be ignored
        // even though they occur without a matching channel event. This Table
        // tracks ability IDs of spells that did cause a channel event so that when
        // a spell is cast that doesn't cause one, its effect event is not ignored.
        private Table CastAfterChannel
    endglobals

    private function Channel takes nothing returns nothing
        call spellEvent.init()
        call ChannelCalls(GetSpellAbilityId())
    endfunction

    private function Cast takes nothing returns nothing
        call CastCalls(GetSpellAbilityId())
    endfunction

    private function Effect takes nothing returns nothing
        local spellEvent s=spellEvent.get(GetTriggerUnit())
        local integer id=GetSpellAbilityId()
        if s!=0 and not s.effectDone then
            set s.effectDone=true
            call EffectCalls(id)
            if not CastAfterChannel.exists(id) then
                set CastAfterChannel[id]=1
            endif
        elseif not CastAfterChannel.exists(id) then
            set s = spellEvent.init()
            call EffectCalls(id)
            call s.destroy()
        endif
    endfunction

    private function Finish takes nothing returns nothing
        set spellEvent.get(GetTriggerUnit()).CastFinished=true
        call FinishCalls(GetSpellAbilityId())
    endfunction

    private function EndCast takes nothing returns nothing
        call EndCastCalls(GetSpellAbilityId())
        call spellEvent.get(GetTriggerUnit()).destroy()
    endfunction

// ================================================================

    private function InitTrigger takes playerunitevent e, code c returns nothing
        local trigger t=CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ( t, e )
        call TriggerAddAction(t, c)
        set t=null
    endfunction
    private function Init takes nothing returns nothing
        set ChannelTable=Table.create()
        set CastTable=Table.create()
        set EffectTable=Table.create()
        set FinishTable=Table.create()
        set EndCastTable=Table.create()
        call InitTrigger(EVENT_PLAYER_UNIT_SPELL_CHANNEL, function Channel)
        call InitTrigger(EVENT_PLAYER_UNIT_SPELL_CAST, function Cast)
        call InitTrigger(EVENT_PLAYER_UNIT_SPELL_EFFECT, function Effect)
        call InitTrigger(EVENT_PLAYER_UNIT_SPELL_FINISH, function Finish)
        call InitTrigger(EVENT_PLAYER_UNIT_SPELL_ENDCAST, function EndCast)
        set CastAfterChannel=Table.create()
    endfunction

endlibrary
Expand Usage example:
Expand Yes, if all your spells are made with triggers and use this library then you can even do this:
__________________

Last edited by Anitarf : 01-20-2012 at 10:39 PM.
Anitarf is offline   Reply With Quote
Sponsored Links - Login to hide this ad!
Old 04-08-2009, 01:10 AM   #2
Anitarf
Procrastination Incarnate


Development Director
 
Join Date: Feb 2004
Posts: 8,028

Submissions (19)

Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)

2008 Spell olympics - Fire - SilverApproved Map: Old School Alliance TacticsHero Contest #2 - 3rd PlaceSpell making session 2 winner

Default

I agree, getting all cast event responses to work for Finish and EndCast events is the main advantage here, not the speed. Regarding that, this system will definitely be faster at a certain number of spells, but it might take a lot of spells and even then the advantage will be marginal. I guess my point regarding speed is that it won't be slower (except in maps with too few spells to matter) than an individual trigger per spell, so the features the script brings don't cost you anything in terms of map performance.

Edit: Added a usage example to the first post.
__________________

Last edited by Anitarf : 04-08-2009 at 01:29 AM.
Anitarf is offline   Reply With Quote
Old 04-08-2009, 02:08 AM   #3
Rising_Dusk
Obscurity, the Art


Projects Director
Project Leader: OD
 
Join Date: Feb 2006
Posts: 9,726

Submissions (27)

Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)

Hero Contest #3 - 1st PlaceApproved Map: Desert of ExileApproved Map: Advent of the ZenithHero Contest #2 - 1st PlaceHero Contest - Third place>

Send a message via AIM to Rising_Dusk Send a message via MSN to Rising_Dusk
Default

I really dislike the naming of "SpellEvent_GetTargetUnit()" and stuff, but I'm awful glad it flows so well with standard WC3 naming for functions. I think it might be better if instead of RegisterFinishResponse(ABILITY_ID, function OnCast), it had something like RegisterSpellResponse(ABILITY_ID, RESPONSE_TYPE_FINISHED, function OnCast). I realize it's a bit more typing, but I think it's more appropriate to change the integer in there than to change the function call you're using.

Outside of that, this is awesome. Approved.

EDIT:
You mispelled the keyword 'library' in the first post. Please fix that. (I could do it for you, but I would prefer you to)
__________________
Home Page
DoE v1.14c Download
AotZ v2.03d Download
OD v0.10x Download

Coming soon eventually...

Personal To-Do List:
ICARUS
Aot3

WC3C Chat
Chat IP: 66.103.20.109
Earthbound 2 in English
vJass Manual

"DAMAGE_TYPE_POISON motherfucker!" ~Anitarf

Last edited by Rising_Dusk : 04-08-2009 at 02:10 AM.
Rising_Dusk is offline   Reply With Quote
Old 04-08-2009, 02:37 AM   #4
Bobo_The_Kodo
oO
 
Bobo_The_Kodo's Avatar
 
Join Date: Jul 2008
Posts: 579

Bobo_The_Kodo has a spectacular aura about (109)Bobo_The_Kodo has a spectacular aura about (109)Bobo_The_Kodo has a spectacular aura about (109)Bobo_The_Kodo has a spectacular aura about (109)

Default

Quote:
RegisterSpellResponse(ABILITY_ID, RESPONSE_TYPE_FINISHED, function OnCast)

I agree with this

The overly long function names associated with SpellEvent_blahblahblah is a pain

Also, with the constant as ALL_CAPS_LIKE_THIS fits more with the warcraft event natives, which is probably a good thing to keep :)
Bobo_The_Kodo is offline   Reply With Quote
Old 04-08-2009, 02:47 PM   #5
Anitarf
Procrastination Incarnate


Development Director
 
Join Date: Feb 2004
Posts: 8,028

Submissions (19)

Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)

2008 Spell olympics - Fire - SilverApproved Map: Old School Alliance TacticsHero Contest #2 - 3rd PlaceSpell making session 2 winner

Default

Quote:
Originally Posted by Rising_Dusk
I think it might be better if instead of RegisterFinishResponse(ABILITY_ID, function OnCast), it had something like RegisterSpellResponse(ABILITY_ID, RESPONSE_TYPE_FINISHED, function OnCast).
Don't approve the resource, then, since once it's approved I need to maintain backwards compatibility and there's no way I can ever change that, and I'm perfectly willing to do that if enough people think that makes for a better user interface, after all it would be a pretty lame public resource if people didn't use it because they didn't like the API. So, I'm moving this back to submissions to get more opinions.
__________________
Anitarf is offline   Reply With Quote
Old 04-08-2009, 02:58 PM   #6
Sinnergy
User
 
Sinnergy's Avatar
 
Join Date: Apr 2009
Posts: 173

Sinnergy has little to show at this moment (8)

Send a message via Yahoo to Sinnergy
Default

nice, does this works like vexorian's caster system spell events? which i stopped using because it seems like there is a blizzard bug which causes my spell to malfunction? or is this better?
__________________
Current Project:
Rise of Sinnergy v1.00
Sinnergy is offline   Reply With Quote
Old 04-08-2009, 03:05 PM   #7
Vexorian
Free Software Terrorist
 
Vexorian's Avatar


Technical Director
 
Join Date: Apr 2003
Posts: 14,905

Submissions (37)

Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)

Hero Contest #3 - 2nd Place

Default

Quote:
I wondered when someone would submit something like this. It definitely has its uses, but I don't think you should make "marginal speed gain" a selling point (and considering the presence of cache and .evaluate, I'm actually skeptical of that claim in the first place, not that it matters either way in this kind of system).
As multiple triggers mean at least multiple Evaluate calls, I wouldn't be so skeptical.

I don't like the blizz event response-like madness or that it has to save all event responses everytime, even though most of them are not needed or are empty.

If the syntax for using event responses will be the ugly blizz-like one, I'd prefer to just use blizz' event responses rather than have to use those new hard to remember event responses that also consume extra time during events, I mean really, at least it would make it easier to port old spells to this.

Not a fan of making it SpellEvent_RegisterEventResponse instead of just RegisterEventResponse, it is a little large, and public stuff are already non-scoped, but oh well.
__________________
Zoom (requires log in)Wc3 map optimizer 5.0
Someone should fix .wav sound in this thing.
Zoom (requires log in)JassHelper 0.A.2.A
Turns your simple code into something that is complicated enough to work.
Faster != more useful
Vexorian is offline   Reply With Quote
Old 04-08-2009, 03:19 PM   #8
emjlr3
Rehabbing
 
emjlr3's Avatar
 
Join Date: Jun 2005
Posts: 1,386

Submissions (14)

emjlr3 is a jewel in the rough (151)emjlr3 is a jewel in the rough (151)

Mapping Contest First Place

Send a message via AIM to emjlr3 Send a message via MSN to emjlr3
Default

why the need for the SpellEvent prefix ?
__________________
emjlr3 is offline   Reply With Quote
Old 04-08-2009, 04:17 PM   #9
Rising_Dusk
Obscurity, the Art


Projects Director
Project Leader: OD
 
Join Date: Feb 2006
Posts: 9,726

Submissions (27)

Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)

Hero Contest #3 - 1st PlaceApproved Map: Desert of ExileApproved Map: Advent of the ZenithHero Contest #2 - 1st PlaceHero Contest - Third place>

Send a message via AIM to Rising_Dusk Send a message via MSN to Rising_Dusk
Default

Quote:
Originally Posted by Anitarf
Don't approve the resource, then, since once it's approved I need to maintain backwards compatibility and there's no way I can ever change that, and I'm perfectly willing to do that if enough people think that makes for a better user interface, after all it would be a pretty lame public resource if people didn't use it because they didn't like the API. So, I'm moving this back to submissions to get more opinions.
You wouldn't lose backwards compatibility by adding a function call with different parameters, though. There'd be nothing to remove. I guess if you removed the public prefix for those functions, then it might be different.

You also still haven't fixed the spelling of library in the beginning of the script.
__________________
Home Page
DoE v1.14c Download
AotZ v2.03d Download
OD v0.10x Download

Coming soon eventually...

Personal To-Do List:
ICARUS
Aot3

WC3C Chat
Chat IP: 66.103.20.109
Earthbound 2 in English
vJass Manual

"DAMAGE_TYPE_POISON motherfucker!" ~Anitarf
Rising_Dusk is offline   Reply With Quote
Old 04-10-2009, 12:57 PM   #10
Anitarf
Procrastination Incarnate


Development Director
 
Join Date: Feb 2004
Posts: 8,028

Submissions (19)

Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)

2008 Spell olympics - Fire - SilverApproved Map: Old School Alliance TacticsHero Contest #2 - 3rd PlaceSpell making session 2 winner

Default

Quote:
Originally Posted by Vexorian
I don't like the blizz event response-like madness or that it has to save all event responses everytime, even though most of them are not needed or are empty.
Well, unless I require users to specify for every single spell what event responses it is expected to use (which I don't think is very practical), there's no way to avoid this. I could also make global switches for which event responses are used but in that case, as soon as one spell requires the targeted destructable, the system would have to call that event response on all spell events. In either of these two cases, you'd need a bunch of if statements checking whether an event response should be used, so it wouldn't be considerably faster than calling all event response functions indiscriminately every time.

There's also the option of only storing the event responses that don't work on the Finish and EndCast events and for the rest letting the user call the natives as needed, but the problem is that the only event responses that work on those events are GetSpellAbilityId (which the script needs to call anyway) and GetTriggerUnit (which pretty much every spell needs to call), so it's both faster to store those too as well as it standardizes all event responses to the same new format.

As for this format, I think wrapper functions which resemble the original event responses and get inlined anyway are better than direct struct access; I could do something with methods/method operators, where the user functions would take the struct as a parameter and then users would call them, something to the effect of set u = GetSpell.TargetUnit()//optional@ , but that doesn't look particularly different from wrapper functions to me.

Quote:
If the syntax for using event responses will be the ugly blizz-like one, I'd prefer to just use blizz' event responses rather than have to use those new hard to remember event responses that also consume extra time during events, I mean really, at least it would make it easier to port old spells to this.
Well, first of all, do event responses still work when you call a function via .evaluate/.execute? Second, how would reading a global array consume more time than calling an event response? Third, in neither of the previous two points is an issue, what's stopping you from using the original natives anyway?

Well, in any case, I'm open for syntax suggestions, that's why this is here.

Quote:
Not a fan of making it SpellEvent_RegisterEventResponse instead of just RegisterEventResponse, it is a little large, and public stuff are already non-scoped, but oh well.
Well, if the function names are specific enough to be unlikely to cause conflicts then the public keyword isn't needed, but on the other hand if you decide to use the public keyword you can shorten the function name. SpellEvent_RegisterEventResponse can easily be shortened to SpellEvent_RegisterResponse, while RegisterEventResponse alone is kinda too general and would need to be lengthened to RegisterSpellEventResponse anyway, which in the end brings us to about the same length. Now the only question is whether the underscore is too much of an aesthetic problem.


Quote:
Originally Posted by Rising_Dusk
You wouldn't lose backwards compatibility by adding a function call with different parameters, though. There'd be nothing to remove. I guess if you removed the public prefix for those functions, then it might be different.
I'm not really a fan of having two sets of functions that do the same thing. If there were some functional difference that'd be one thing but to have two functions that only differ in style is too much.

Quote:
You also still haven't fixed the spelling of library in the beginning of the script.
Oh, right, fixed now.
__________________

Last edited by Anitarf : 04-10-2009 at 12:57 PM.
Anitarf is offline   Reply With Quote
Old 04-10-2009, 02:13 PM   #11
Vexorian
Free Software Terrorist
 
Vexorian's Avatar


Technical Director
 
Join Date: Apr 2003
Posts: 14,905

Submissions (37)

Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)

Hero Contest #3 - 2nd Place

Default

Quote:
set u = GetSpell.TargetUnit()
That's still blizz like stuff. I would wish this used arguments, you know, if you specified what responses you'd like, then arguments would be possible, maybe too hard.

Quote:
Well, first of all, do event responses still work when you call a function via .evaluate/.execute?
All but GetTriggeringTrigger.

Quote:
Second, how would reading a global array consume more time than calling an event response?
I don't care about speed, it is more about having to remember brand new event response names.

Quote:
Well, unless I require users to specify for every single spell what event responses it is expected to use (which I don't think is very practical)
I actually think it is practical, could use bitflags, well bitflags could be a little slow.

What annoys me the most is thelocation call, the others are in theory not that bad. Also, why store it as location? It could at least be helpful if it converted it to x,y already. It would be nice if you could tell it not to call the location thing.

Quote:
local spellEvent s=spellEvent.allocate()
set s.spellId=GetSpellAbilityId()
set s.caster=GetTriggerUnit()
set s.target=GetSpellTargetUnit()
set s.targetItem=GetSpellTargetItem()
set s.targetDest=GetSpellTargetDestructable()
set s.targetLoc=GetSpellTargetLoc()
set s.finished=false
Well, most of the times, you don't need any of them and could use the responses directly (SPELL_EFFECT/CAST) , the other times, you don't need all of them.

Guess spellId and caster are constants, but there are only 4 other things, it is not that bad:

Collapse JASS:
struct targetType extends array
    static constant integer Unit         = 1
    static constant integer Item         = 2
    static constant integer Destructable = 4
    static constant integer Widget       = 7
    static constant integer Point        = 8
endstruct

//...
call SpellEvent_RegisterFinishResponse(ABILITY_ID,  targetType.Unit +targetType.Item) //maybe it hits both things?

//...
            local spellEvent s=spellEvent.allocate()
            set s.spellId=GetSpellAbilityId()
            set s.caster=GetTriggerUnit()
            if ( flag >= 8) then
                  set s.targetLoc=GetSpellTargetLoc()
                  set flag=flag-8
            endif
            if ( flag >= 4) then
                  set s.targetDest=GetSpellTargetDestructable()
                  set flag=flag-4
            endif
            if ( flag >= 2) then
                  set s.targetItem=GetSpellTargetItem()
                  set flag=flag-2
            endif
            if ( flag >= 1) then
                  set s.target=GetSpellTargetUnit()
            endif

            set s.finished=false
            set spellEvent.casterTable[s.caster]=integer(s)
            return s


Just suggesting here.
__________________
Zoom (requires log in)Wc3 map optimizer 5.0
Someone should fix .wav sound in this thing.
Zoom (requires log in)JassHelper 0.A.2.A
Turns your simple code into something that is complicated enough to work.
Faster != more useful
Vexorian is offline   Reply With Quote
Old 04-10-2009, 03:04 PM   #12
Anitarf
Procrastination Incarnate


Development Director
 
Join Date: Feb 2004
Posts: 8,028

Submissions (19)

Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)

2008 Spell olympics - Fire - SilverApproved Map: Old School Alliance TacticsHero Contest #2 - 3rd PlaceSpell making session 2 winner

Default

Quote:
Originally Posted by Vexorian
That's still blizz like stuff. I would wish this used arguments, you know, if you specified what responses you'd like, then arguments would be possible, maybe too hard.
I'm not sure I understand. You mean the functions following the function interface would take the event responses as arguments? That'd be either a very long argument list or you'd need different functions for different types of spells (and a point-or-unit target spell like shockwave would still have a long argument list). I thought you hated long argument lists.

Quote:
I don't care about speed, it is more about having to remember brand new event response names.
Then how's it being Blizz like a problem? That last example was a Blizzard native with a "." in the middle. It was essentially the same thing, you can even use auto-completion and then add the dot after GetSpell (well, except for the GetSpell.TargetX/Y which wouldn't have a native counterpart). It's as intuitive as it can get.

Quote:
I actually think it is practical, could use bitflags, well bitflags could be a little slow.

What annoys me the most is thelocation call, the others are in theory not that bad. Also, why store it as location? It could at least be helpful if it converted it to x,y already. It would be nice if you could tell it not to call the location thing.
I don't see why would it annoy you. To get rid of it you need an additional GC call (to get the flag from the spellID in your example) and depending on the map there might be plenty of cases where after that you still need to call GetSpellTargetLoc. So the speed gain from doing it like this is marginal at best and you just said you don't care about speed.

What about something like this:
Collapse JASS:
        set s.targetUnit = GetSpellTargetUnit()
        if s.targetUnit == null then
            set s.targetDest = GetSpellTargetDestructable()
            if s.targetDest == null then
                set s.targetItem = GetSpellTargetItem()
                if s.targetItem == null then
                    set l = GetSpellTargetLoc()
                    set s.targetX = GetLocationX()
                    set s.targetY = GetLocationY()
                    call RemoveLocation(l)
                    set l = null
                endif
            endif
        else //optional, would also be there for items and destructables but I don't feel like typing it
            set s.targetX = GetUnitX()
            set s.targetY = GetUnitY()
        endif
__________________
Anitarf is offline   Reply With Quote
Old 04-10-2009, 03:11 PM   #13
Vexorian
Free Software Terrorist
 
Vexorian's Avatar


Technical Director
 
Join Date: Apr 2003
Posts: 14,905

Submissions (37)

Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)

Hero Contest #3 - 2nd Place

Default

it is annoying, it does handle allocation and deallocation, that's annoying.

It is vJass, you only need one gc call.
__________________
Zoom (requires log in)Wc3 map optimizer 5.0
Someone should fix .wav sound in this thing.
Zoom (requires log in)JassHelper 0.A.2.A
Turns your simple code into something that is complicated enough to work.
Faster != more useful
Vexorian is offline   Reply With Quote
Old 04-10-2009, 03:28 PM   #14
Anitarf
Procrastination Incarnate


Development Director
 
Join Date: Feb 2004
Posts: 8,028

Submissions (19)

Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)Anitarf has a brilliant future (883)

2008 Spell olympics - Fire - SilverApproved Map: Old School Alliance TacticsHero Contest #2 - 3rd PlaceSpell making session 2 winner

Default

Quote:
Originally Posted by Vexorian
it is annoying, it does handle allocation and deallocation, that's annoying.
Why?

Quote:
It is vJass, you only need one gc call.
True, but it complicates the library somewhat.

Anyway, what about my last example? Would a GetSpellTargetLoc() call for point-target spells (where it's needed anyway) and no-target spells (where it isn't), but not for widget-target spells be acceptable?
__________________
Anitarf is offline   Reply With Quote
Old 05-07-2009, 03:24 PM   #15
Vexorian
Free Software Terrorist
 
Vexorian's Avatar


Technical Director
 
Join Date: Apr 2003
Posts: 14,905

Submissions (37)

Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)Vexorian has a reputation beyond repute (1060)

Hero Contest #3 - 2nd Place

Default

Ok, do the Loc stuff.
__________________
Zoom (requires log in)Wc3 map optimizer 5.0
Someone should fix .wav sound in this thing.
Zoom (requires log in)JassHelper 0.A.2.A
Turns your simple code into something that is complicated enough to work.
Faster != more useful
Vexorian is offline   Reply With Quote
Reply


Thread Tools Search this Thread
Search this Thread:

Advanced Search

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off


All times are GMT. The time now is 11:13 AM.


Donate

Affiliates
The Hubb http://bylur.com - Warcraft, StarCraft, Diablo and DotA Blog & Forums The JASS Vault Clan WEnW Campaign Creations Clan CBS GamesModding Flixreel Videos

Powered by vBulletin (Copyright ©2000 - 2014, Jelsoft Enterprises Ltd).
Hosted by www.OICcam.com
IT Support and Services provided by Executive IT Services