[Snippet] GetLastCastAbility

Level 23
Joined
Apr 16, 2012
Messages
4,041
Hope this will be more useful then my first submission

JASS:
library GetLastCastAbility uses Table, RegisterPlayerUnitEvent, UnitIndexer
/*   by edo494 version 1.8d
*
*============================================================================
*
* Requires:
*
* Table by Bribe - http://www.hiveworkshop.com/forums/jass-resources-412/snippet-new-table-188084/
*
* RegisterPlayerUnitEvent by Magtheridon96 - http://www.hiveworkshop.com/forums/jass-resources-412/snippet-registerplayerunitevent-203338/
*
* UnitIndexer by Nestharus - http://www.hiveworkshop.com/forums/jass-resources-412/system-unit-indexer-172090/
*
*============================================================================
*   
*   This library includes: 
*
*       - Allows you to retrieve last cast spells ID(returns integer)
*       - Allows you to retrieve last cast spells target
*       - Allows you to retrieve last cast spells caster
*       - Allows you to retrieve last cast spells X and Y coordiantions
*       - Allows you to retrieve last cast spells ID by specific unit
*       - Allows you to retrieve target of cast spell by specific unit
*       - Allows you to retrieve X and Y coordinates of last cast spell by specific unit
*       - Additionally this allows you to pass unit's index and returns the unit's last cast ability
*
*   This also features "BAN/UNBAN" SPELL which you can add spells into it
*   If units cast those spells it will not get registered.
*
*       This feature can be used for things like Spell Steal for either
*       not registering the spell steal(because you dont want to steal that dont you?)
*       or for spells that you dont want to be spell stealed.
*
*   Also if you want to catch only heros spells, there is texted if in MainBody function which
*   you can use.
* 
*============================================================================
*
*   Issues:
*       You cant register passives for instance Bash because they
*       technically never fire the event Unit - a Unit starts the effect of an ability.
*
*       Also if you cast spell with no target the GetLastCastAbilityX, Y and
*       GetLastCastSpellTarget will return 0, 0, null
*
*============================================================================
*   
*   The API is:
*   
*       function GetLastCastAbility takes nothing returns integer
*       function GetLastCastAbilityTarget takes nothing returns unit
*       function GetLastCastAbilityUnit takes nothing returns unit
*       function GetLastCastAbilityX takes nothing returns real
*       function GetLastCastAbilityY takes nothing returns real
*
*   Based on Unit:
*       
*       function GetUnitLastCastAbility takes unit returns integer
*       function GetUnitLastCastAbilityTarget takes unit returns unit
*       function GetUnitLastCastAbilityX takes unit returns real
*       function GetUnitLastCastAbilityX takes unit returns real
*
*   Based on Unit's index:
*
*       function GetUnitLastCastAbilityById takes integer returns integer
*       function GetUnitLastCastAbilityTargetById takes integer returns unit
*       function GetUnitLastCastAbilityXById takes integer returns real
*       function GetUnitLastCastAbilityXById takes integer returns real
*
*   Ban API:
*
*       function LastAbilityAddFilter takes integer returns nothing
*       function LastAbilityRemoveFilter takes integer returns nothing
*
*============================================================================
*
*   Change Log:
*
*       1.0    - Intial Release
*
*       1.1    - Added BanAbility
*              - Added UnbanAbility
*              - Added Get(Unit)LastCastAbilityX and Y
*              - Changed GetLastCast***what***ByUnit to GetUnitLastCast***what***
*
*       1.2    - Added TempBanAbility function
*              - Removed Get(Unit)LastCastAbility and Get(Unit)LastCastAbilityLoc
*
*       1.3    - Changed AbilityId to Ability
*              - Added support for Table
*
*       1.4    - Changed Size of myTab from 0x1000000(lol this wont work anyways) to 0x200
*
*       1.5    - Resized the Table from 0x200 to 5
*              - Removed TempBan function
*
*       1.6    - Changed Ban/Unban Ability to LastAbilityAddFilter/LastAbilityRemoveFilter
*              - Implemented support for RegisterPlayerUnitEvent Library
*              - changed Scope Initializer to Module Initializer
*
*       1.7    - Added "Deinxeding" when unit dies
*
*       1.7a   - Improved speed a very little bit
*
*       1.8    - Remade the idea of GetUnitLastCastAbility
*
*       1.8a   - Removed Useless stuff
*              - Changed the way how the nulling variables works(Special thanks to Bribe)
*
*       1.8b   - TableArray -> Table (special thanks to Yixx)
*
*       1.8b.1 - Changed how GetLastCast***what*** works(thanks to Magtheridon96)
*              - Removed local unit u from Deindex function
*
*       1.8b.2 - unit lastCastAbilityUnit -> integer lastCastAbilityCaster
*
*       1.8c   - Reorganized the structure of the header
*              - Added GetUnitLastCast ability taking unit's index instead of actual unit
*              - Changed names of textmacroes(added GetLastCastAbility_ to their name)
*
*       1.8d   - removed local integer from mainbody function
*
*============================================================================
*/

globals
    private integer lastCastAbilityCaster = 0
    
    /*
    *   table for Filter/UnFilter
    */
    
    private Table myTab
    
    /*
    *   variables for GetUnitLastCast
    */
    
    private integer array lastCastUnitAbility
    private unit array lastCastUnitTarget
    private real array lastCastUnitX
    private real array lastCastUnitY
endglobals


    //! textmacro GetLastCastAbility_GETLAST takes Name, RSName, RName
        function GetLastCast$Name$ takes nothing returns $RSName$
            return $RName$
        endfunction
    //! endtextmacro
    
    //! runtextmacro GetLastCastAbility_GETLAST("Ability", "integer", "lastCastUnitAbility[lastCastAbilityCaster]")
    //! runtextmacro GetLastCastAbility_GETLAST("AbilityUnit", "unit", "GetUnitById(lastCastAbilityCaster)")
    //! runtextmacro GetLastCastAbility_GETLAST("AbilityTarget", "unit", "lastCastUnitTarget[lastCastAbilityCaster]")
    //! runtextmacro GetLastCastAbility_GETLAST("AbilityX", "real", "lastCastUnitY[lastCastAbilityCaster]")
    //! runtextmacro GetLastCastAbility_GETLAST("AbilityY", "real", "lastCastUnitY[lastCastAbilityCaster]")
    
    
    //! textmacro GetLastCastAbility_GETUNITLAST takes name, return, what
        function GetUnitLastCast$name$ takes unit u returns $return$
            return lastCastUnit$what$[GetUnitUserData(u)]
        endfunction
    //! endtextmacro
    
    //! runtextmacro GetLastCastAbility_GETUNITLAST("Ability", "integer", "Ability")
    //! runtextmacro GetLastCastAbility_GETUNITLAST("X", "real", "X")
    //! runtextmacro GetLastCastAbility_GETUNITLAST("Y", "real", "Y")
    //! runtextmacro GetLastCastAbility_GETUNITLAST("Target", "unit", "Target")
    
    
    //! textmacro GetLastCastAbility_GETUNITLASTBYID takes name, return, what
        function GetUnitLastCast$name$ById takes integer index returns $return$
            return lastCastUnit$what$[index]
        endfunction
    //! endtextmacro
    
    //! runtextmacro GetLastCastAbility_GETUNITLASTBYID("Ability", "integer", "Ability")
    //! runtextmacro GetLastCastAbility_GETUNITLASTBYID("X", "real", "X")
    //! runtextmacro GetLastCastAbility_GETUNITLASTBYID("Y", "real", "Y")
    //! runtextmacro GetLastCastAbility_GETUNITLASTBYID("Target", "unit", "Target")


        function LastAbilityAddFilter takes integer abilcode returns nothing
            set myTab[abilcode] = 1
        endfunction
        
        
        function LastAbilityRemoveFilter takes integer abilcode returns nothing
            set myTab[abilcode] = 0
        endfunction

        
        private function MainBody takes nothing returns nothing
            //if IsUnitType(GetTriggerUnit(), UNIT_TYPE_HERO) then
                if myTab[GetSpellAbilityId()] != 1 then
                    set lastCastAbilityCaster = GetUnitId(GetTriggerUnit())
                    set lastCastUnitAbility[lastCastAbilityCaster] = GetSpellAbilityId()
                    set lastCastUnitTarget[lastCastAbilityCaster] = GetSpellTargetUnit()
                    set lastCastUnitX[lastCastAbilityCaster] = GetSpellTargetX()
                    set lastCastUnitY[lastCastAbilityCaster] = GetSpellTargetY()
                endif
            //endif
        endfunction

    private module GetLastAbility
        private static method onInit takes nothing returns nothing            
            /*
            *   for ban list
            */
            
            set myTab = Table.create()
            
            /*
            *   Registering the starts the effect of an ability to the function
            */
            
            call RegisterPlayerUnitEvent(EVENT_PLAYER_UNIT_SPELL_EFFECT, function MainBody)
        endmethod
    endmodule
    
    private struct GetLast
        implement GetLastAbility
    endstruct
    
//***************************************************************
//***************************************************************

    //When unit dies, null the numbers
    
    private function DiesFunc takes nothing returns boolean
        local integer i = GetIndexedUnitId()
        set lastCastUnitAbility[i] = 0
        set lastCastUnitTarget[i] = null
        set lastCastUnitX[i] = 0
        set lastCastUnitY[i] = 0
        return false
    endfunction

//***************************************************************
    
    private module DiesInit
        private static method onInit takes nothing returns nothing
            call RegisterUnitIndexEvent(Condition(function DiesFunc), UnitIndexer.DEINDEX)
        endmethod
    endmodule

    private struct DIES
        implement DiesInit
    endstruct 
endlibrary

ZINC version:
JASS:
//! zinc
library GetLastCastAbility requires Table, RegisterPlayerUnitEvent, UnitIndexer

/*   by edo494 version 1.0.a.z
*
*============================================================================
*
* Requires:
*
* Table by Bribe - http://www.hiveworkshop.com/forums/jass-resources-412/snippet-new-table-188084/
*
* RegisterPlayerUnitEvent by Magtheridon96 - http://www.hiveworkshop.com/forums/jass-resources-412/snippet-registerplayerunitevent-203338/
*
* UnitIndexer by Nestharus - http://www.hiveworkshop.com/forums/jass-resources-412/system-unit-indexer-172090/
*
*============================================================================
*   
*   This library includes: 
*
*       - Allows you to retrieve last cast spells ID(returns integer)
*       - Allows you to retrieve last cast spells target
*       - Allows you to retrieve last cast spells caster
*       - Allows you to retrieve last cast spells X and Y coordiantions
*       - Allows you to retrieve last cast spells ID by specific unit
*       - Allows you to retrieve target of cast spell by specific unit
*       - Allows you to retrieve X and Y coordinates of last cast spell by specific unit
*       - Additionally this allows you to pass unit's index and returns the unit's last cast ability
*
*   This also features "BAN/UNBAN" SPELL which you can add spells into it
*   If units cast those spells it will not get registered.
*
*       This feature can be used for things like Spell Steal for either
*       not registering the spell steal(because you dont want to steal that dont you?)
*       or for spells that you dont want to be spell stealed.
*
*   Also if you want to catch only heros spells, there is texted if in MainBody function which
*   you can use.
* 
*============================================================================
*
*   Issues:
*       You cant register passives for instance Bash because they
*       technically never fire the event Unit - a Unit starts the effect of an ability.
*
*       Also if you cast spell with no target the GetLastCastAbilityX, Y and
*       GetLastCastSpellTarget will return 0, 0, null
*
*============================================================================
*   
*   The API is:
*   
*       function GetLastCastAbility takes nothing returns integer
*       function GetLastCastAbilityTarget takes nothing returns unit
*       function GetLastCastAbilityUnit takes nothing returns unit
*       function GetLastCastAbilityX takes nothing returns real
*       function GetLastCastAbilityY takes nothing returns real
*
*   Based on Unit:
*       
*       function GetUnitLastCastAbility takes unit returns integer
*       function GetUnitLastCastAbilityTarget takes unit returns unit
*       function GetUnitLastCastAbilityX takes unit returns real
*       function GetUnitLastCastAbilityX takes unit returns real
*
*   Based on Unit's index:
*
*       function GetUnitLastCastAbilityById takes integer returns integer
*       function GetUnitLastCastAbilityTargetById takes integer returns unit
*       function GetUnitLastCastAbilityXById takes integer returns real
*       function GetUnitLastCastAbilityXById takes integer returns real
*
*   Ban API:
*
*       function LastAbilityAddFilter takes integer returns nothing
*       function LastAbilityRemoveFilter takes integer returns nothing
*
*============================================================================
*
*   Change Log:
*
*       1.0.z    - Intial Release
*
*       1.0.a.z  - removed the local integer from mainbody function 
*
*============================================================================
*/
{
        private integer lastCastAbilityCaster = 0;
        private Table myTab;
        private integer lastCastUnitAbility[8192];
        private unit lastCastUnitTarget[8192];
        private real lastCastUnitX[8192];
        private real lastCastUnitY[8192];
        
        //! textmacro GETLAST_GetLast takes Name, RSName, RName
            public function GetLastCast$Name$() -> $RSName$ { return $RName$; }
        //! endtextmacro
       
        //! runtextmacro GETLAST_GetLast("Ability", "integer", "lastCastUnitAbility[lastCastAbilityCaster]")
        //! runtextmacro GETLAST_GetLast("AbilityUnit", "unit", "GetUnitById(lastCastAbilityCaster)")
        //! runtextmacro GETLAST_GetLast("AbilityTarget", "unit", "lastCastUnitTarget[lastCastAbilityCaster]")
        //! runtextmacro GETLAST_GetLast("AbilityX", "real", "lastCastUnitY[lastCastAbilityCaster]")
        //! runtextmacro GETLAST_GetLast("AbilityY", "real", "lastCastUnitY[lastCastAbilityCaster]")
       
        //! textmacro GETCAST_LAST_UNIT takes Name, Return, what
            public function GetUnitLastCast$Name$(unit u) -> $Return$ { return lastCastUnit$what$[GetUnitUserData(u)]; }
        //! endtextmacro
        
        //! runtextmacro GETCAST_LAST_UNIT("Ability", "integer", "Ability")
        //! runtextmacro GETCAST_LAST_UNIT("X", "real", "X")
        //! runtextmacro GETCAST_LAST_UNIT("Y", "real", "Y")
        //! runtextmacro GETCAST_LAST_UNIT("Target", "unit", "Target")
    
        //! textmacro GetLastCastAbility_GETUNITLASTBYID takes name, return, what
            public function GetUnitLastCast$name$ById(integer index) -> $return$ { return lastCastUnit$what$[index]; }
        //! endtextmacro

        //! runtextmacro GetLastCastAbility_GETUNITLASTBYID("Ability", "integer", "Ability")
        //! runtextmacro GetLastCastAbility_GETUNITLASTBYID("X", "real", "X")
        //! runtextmacro GetLastCastAbility_GETUNITLASTBYID("Y", "real", "Y")
        //! runtextmacro GetLastCastAbility_GETUNITLASTBYID("Target", "unit", "Target")
        
        
        public function LastAbilityAddFilter(integer abilcode) { myTab[abilcode] = 1; }
        
        public function LastAbilityRemoveFilter(integer abilcode) { myTab[abilcode] = 0; }
        
        function MainBody() 
        {
            if (myTab[GetSpellAbilityId()] != 1)
            {
                lastCastAbilityCaster = GetUnitId(GetTriggerUnit());
                lastCastUnitAbility[lastCastAbilityCaster] = GetSpellAbilityId();
                lastCastUnitTarget[lastCastAbilityCaster] = GetSpellTargetUnit();
                lastCastUnitX[lastCastAbilityCaster] = GetSpellTargetX();
                lastCastUnitY[lastCastAbilityCaster] = GetSpellTargetY();
            }
        }
        
        function DiesFunc() -> boolean
        {
            integer i = GetIndexedUnitId();
            lastCastUnitAbility[i] = 0;
            lastCastUnitTarget[i] = null;
            lastCastUnitX[i] = 0;
            lastCastUnitY[i] = 0;
            return false;
        }
        
        function onInit()
        {
            myTab = Table.create();
            RegisterPlayerUnitEvent(EVENT_PLAYER_UNIT_SPELL_EFFECT, function MainBody);
            RegisterUnitIndexEvent(Condition(function DiesFunc), UnitIndexer.DEINDEX);
        }
    }

//! endzinc
 
Last edited:
Would be cool is this used SpellEffectEvent by Bribe and coordinates rather than locations.

You may argue that less requirements = good, but we don't believe that here in the JASS Realm >:D

Also, instead of relying on the user setting up an array of 'banned' abilities, you could have a function that stores a boolean in a hashtable to indicate that a certain ability is banned.

The user would call the function from any other trigger:

JASS:
struct SetupAbilitiesCast extends array

    private static method onInit takes nothing returns nothing
        call GetLastCastAbility_Ban('A000')
        call GetLastCastAbility_Ban('A004')
        // etc...
    endmethod

endstruct

The ban function would just be something that stores a 'true' boolean into your own hashtable.

This method is better than having an array of abilities because the algorithm has a complexity of O(1) as opposed to the O(n) that your method has.

When an ability is cast, just load the boolean to determine whether you should skip this instance or not.

ACTUALLY, on second thought, it would be much better to have users register abilities that they want this system to run for.
Yes, it's more work for the users, but it makes more sense.

And why are you putting your API functions inside that scope?

Functions inside a scope are private by default :C

By the way, I'd recommend using Table by Bribe too.
It's really awesome and more beautiful than those shitty Hashtable natives c:
 
Level 23
Joined
Apr 16, 2012
Messages
4,041
Would be cool is this used SpellEffectEvent by Bribe and coordinates rather than locations.
Well, I didnt even know there is something like that :D Ill look into that one

You may argue that less requirements = good, but we don't believe that here in the JASS Realm >:D
Nope :D

Also, instead of relying on the user setting up an array of 'banned' abilities, you could have a function that stores a boolean in a hashtable to indicate that a certain ability is banned.

The user would call the function from any other trigger:

JASS:
struct SetupAbilitiesCast extends array

    private static method onInit takes nothing returns nothing
        call GetLastCastAbility_Ban('A000')
        call GetLastCastAbility_Ban('A004')
        // etc...
    endmethod

endstruct

The ban function would just be something that stores a 'true' boolean into your own hashtable.
Thats actually a damn good idea :D

This method is better than having an array of abilities because the algorithm has a complexity of O(1) as opposed to the O(n) that your method has.
Im still newb and Ive saw you and others talking about O(1), O(n)... but well, I dont know what that mean :D Explanation would be great

ACTUALLY, on second thought, it would be much better to have users register abilities that they want this system to run for.
Yes, it's more work for the users, but it makes more sense.
Im not working with vJass long enough to know such a things, so could you give me some veeery basic example?

And why are you putting your API functions inside that scope?
Functions inside a scope are private by default :C
Sure?

JASS:
//library GetLastCastAbility:



a
// scope GetLastCastAbility_GetLastCastSpell begins
    
//textmacro instance: GETLAST("Ability", "ability", "lastCastAbility")
        function GetLastCastAbility takes nothing returns ability
            return lastCastAbility
        endfunction
//end of: GETLAST("Ability", "ability", "lastCastAbility")
//textmacro instance: GETLAST("AbilityId", "integer", "lastCastAbilityId")
        function GetLastCastAbilityId takes nothing returns integer
            return lastCastAbilityId
        endfunction
//end of: GETLAST("AbilityId", "integer", "lastCastAbilityId")
//textmacro instance: GETLAST("AbilityUnit", "unit", "lastCastAbilityUnit")
        function GetLastCastAbilityUnit takes nothing returns unit
            return lastCastAbilityUnit
        endfunction
//end of: GETLAST("AbilityUnit", "unit", "lastCastAbilityUnit")
//textmacro instance: GETLAST("AbilityTarget", "unit", "lastCastAbilityTarget")
        function GetLastCastAbilityTarget takes nothing returns unit
            return lastCastAbilityTarget
        endfunction
//end of: GETLAST("AbilityTarget", "unit", "lastCastAbilityTarget")
//textmacro instance: GETLAST("AbilityTargetLoc", "location", "lastCastAbilityLoc")
        function GetLastCastAbilityTargetLoc takes nothing returns location
            return lastCastAbilityLoc
        endfunction
//end of: GETLAST("AbilityTargetLoc", "location", "lastCastAbilityLoc")
    
    
//textmacro instance: BYID("Ability", "ability", "AbilityHandle", "97612")
        function GetLastCastAbilityByUnit takes unit u returns ability
            return LoadAbilityHandle(lastcast, GetHandleId(u), 97612)
        endfunction
//end of: BYID("Ability", "ability", "AbilityHandle", "97612")
//textmacro instance: BYID("AbilityTarget", "unit", "UnitHandle", "97666")
        function GetLastCastAbilityTargetByUnit takes unit u returns unit
            return LoadUnitHandle(lastcast, GetHandleId(u), 97666)
        endfunction
//end of: BYID("AbilityTarget", "unit", "UnitHandle", "97666")
//textmacro instance: BYID("AbilityId", "integer", "Integer", "97548")
        function GetLastCastAbilityIdByUnit takes unit u returns integer
            return LoadInteger(lastcast, GetHandleId(u), 97548)
        endfunction
//end of: BYID("AbilityId", "integer", "Integer", "97548")
//textmacro instance: BYID("AbilityTargetLoc", "location", "LocationHandle", "97652")    
        function GetLastCastAbilityTargetLocByUnit takes unit u returns location
            return LoadLocationHandle(lastcast, GetHandleId(u), 97652)
        endfunction
//end of: BYID("AbilityTargetLoc", "location", "LocationHandle", "97652")    

        
        function GetLastCastAbility_GetLastCastSpell__MainBody takes nothing returns nothing
            local integer looper= 0
          //if IsUnitType(GetTriggerUnit(), UNIT_TYPE_HERO) then
            loop
                if GetSpellAbilityId() == banned[looper] then
                    return
                endif
                exitwhen looper == bannedmax
                set looper=looper + 1
            endloop
            call FlushChildHashtable(lastcast, GetHandleId(lastCastAbilityUnit))
            call MoveLocation(lastCastAbilityLoc, GetSpellTargetX(), GetSpellTargetY())
            set lastCastAbility=GetSpellAbility()
            set lastCastAbilityId=GetSpellAbilityId()
            set lastCastAbilityUnit=GetTriggerUnit()
            set lastCastAbilityTarget=GetSpellTargetUnit()
            call SaveAbilityHandle(lastcast, GetHandleId(lastCastAbilityUnit), 97612, lastCastAbility)
            call SaveInteger(lastcast, GetHandleId(lastCastAbilityUnit), 97548, lastCastAbilityId)
            call SaveUnitHandle(lastcast, GetHandleId(lastCastAbilityUnit), 97666, lastCastAbilityTarget)
            call SaveLocationHandle(lastcast, GetHandleId(lastCastAbilityUnit), 97652, lastCastAbilityLoc)
          //endif
        endfunction

        function OnInit takes nothing returns nothing
            local trigger t= CreateTrigger()
            call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
            call TriggerAddAction(t, function GetLastCastAbility_GetLastCastSpell__MainBody)
            set t=null
            set banned[1]='A000'
        endfunction
        
// scope GetLastCastAbility_GetLastCastSpell ends
    

//library GetLastCastAbility ends
took from jasshleper compiler when I made the misstake
The function MainBody is private on purpose, I dont want users to call it anywhere they want.
I've actually tested it and it works fine as it is(I mean the private/public thingy)

By the way, I'd recommend using Table by Bribe too.
It's really awesome and more beautiful than those shitty Hashtable natives c:
Ill look into that one

Anyways, thx for great suggestions. Ill update it as soon as possible
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
- why are you putting a scope inside the library?
- make the function onInit and variables private
- the banned[1] could be;
JASS:
function AbilityBan takes integer abilityID returns nothing
   if AbilCount==bannedmax then
      call BJDebugMsg("ERRORRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRR")
   else
      set banned[AbilCount] = abilityID
      set AbilCount = AbilCount + 1
   endif            
endfunction
no Mags is not sure, he maybe talking about structs :)...
 
Last edited:
Level 15
Joined
Aug 7, 2009
Messages
1,407
Also, instead of relying on the user setting up an array of 'banned' abilities, you could have a function that stores a boolean in a hashtable to indicate that a certain ability is banned.

You don't need to store it in a hashtable, just use the method Bribe's SpellEffectEvent uses, that's possibly the fastest way.

And yea, I agree with the custom "BanSpell" function (and possibly an "UnbanSpell" one as well, might come in handy), it's definitely easier and much more user-friendly than having to write a huge list of banned spells, things tend to get lost quite easily that way. Just keep it simple, 1 trigger per 1 spell, and you can ban the necessary dummy spells from each spell trigger using that function, that can also be made inline-friendly, so you won't lose any speed.
 
Well, I don't use scopes, so I forgot how they work xD
I only use structs for map-specific things and certain modules, and libraries for major systems and data repositories.

@Luorax:
It would stilll be using hashtables :p

Imo, the fastest model for this resource is this:

JASS:
/*****************************************
*
*   API:
*   ----
*
*       struct LastCastAbility extends array
*
*           static thistype last
*               - The instance of the last ability cast.
*
*           integer abilityId
*           unit caster
*           unit target
*           real targetX
*           real targetY
*               - Data for last-cast instances.
*
*           static method operator [] takes unit whichUnit returns thistype
*               - Returns the instance of the unit.
*
*           static method register takes integer abilityId returns nothing
*               - Registers an ability for the system.
*
*       function GetLastCastAbilityId takes nothing returns integer
*       function GetLastCastAbilityUnit takes nothing returns unit
*       function GetLastCastAbilityTarget takes nothing returns unit
*       function GetLastCastAbilityTargetX takes nothing returns real
*       function GetLastCastAbilityTargetY takes nothing returns real
*           - Retrieval of data for the last cast ability.
*
*       public function Register takes integer abilityId returns nothing
*           - Alternate function for registering a spell to the system.
*
*****************************************/
library LastCastAbility requires UnitIndexer, SpellEffectEvent
    
    struct LastCastAbility extends array
        
        static thistype last = 0
        
        integer abilityId
        unit caster
        unit target
        real targetX
        real targetY
        
        /*
        *   I am only using this function to 
        *   save a bit of memory in case any of 
        *   the referenced handles is removed.
        */
        private static method deindex takes nothing returns nothing
            set thistype(GetIndexedUnitId()).caster = null
            set thistype(GetIndexedUnitId()).target = null
        endmethod
        
        implement UnitIndexStruct
        
        private static method run takes nothing returns nothing
            local thistype this = GetUnitUserData(GetTriggerUnit())
            
            set this.abilityId = GetSpellAbilityId()
            set this.caster = GetUnitById(this)
            set this.target = GetSpellTargetUnit()
            set this.targetX = GetSpellTargetX()
            set this.targetY = GetSpellTargetY()
            
            set last = this
        endmethod
        
        static method register takes integer abilityId returns nothing
            call RegisterSpellEffectEvent(abilityId, function thistype.run)
        endmethod
    endstruct
    
    function GetLastCastAbilityId takes nothing returns integer
        return LastCastAbility.last.abilityId
    endfunction
    
    function GetLastCastAbilityUnit takes nothing returns unit
        return LastCastAbility.last.caster
    endfunction
    
    function GetLastCastAbilityTarget takes nothing returns unit
        return LastCastAbility.last.target
    endfunction
    
    function GetLastCastAbilityTargetX takes nothing returns real
        return LastCastAbility.last.targetX
    endfunction
    
    function GetLastCastAbilityTargetY takes nothing returns real
        return LastCastAbility.last.targetY
    endfunction
    
    public function Register takes integer abilityId returns nothing
        call LastCastAbility.register(abilityId)
    endfunction
endlibrary

It's also incredibly clean, sweet and short ^_^

edit
Fixed. xD
 
Level 30
Joined
Jul 10, 2007
Messages
6,307
I know, but it'd make banning/unbanning spells faster. Just because you're using hashtables, you don't have to use any everytime you can :)

And why are you including bannning/unbanning spells? That should be a separate resource.

This is called last cast ability or w/e, so it should return the last ability that a unit used.

edit
Mag hit it perfectly with what he thinks you should revise your resource to be.

Focus only on what the resource should do, don't add in extra crap no matter how much you want it in there. That's the best advice I can give to you =).
 
Level 23
Joined
Apr 16, 2012
Messages
4,041
I have updated it slightly btw.

I dont think its really needed to use structs, Im not amongst those who abuse the powers of structures at every oppoturnity.
It does what I made it for to do.

Also I dont say its not fact but how can be table faster then normal hashtable function call?
Also how can structs be faster then normal function calls?
 
Nice job.

I have updated it slightly btw.

I dont think its really needed to use structs, Im not amongst those who abuse the powers of structures at every oppoturnity.
It does what I made it for to do.

Also I dont say its not fact but how can be table faster then normal hashtable function call?
Also how can structs be faster then normal function calls?

Table isn't faster than a normal hashtable call, it is just more "efficient" memory-wise to use since it uses only 1 hashtable for everything. So if you have like 10 systems or spells which each use their own hashtable, you end up using 10 hashtables. But if you switch them all to use table, you use only 1. It isn't a big deal, but it is a pretty nice thing for mapmaking. You don't have to swap completely to it, however. You can just abuse static ifs and make table an optional requirement. :)

As for structs, Mag used it because he used UnitIndexer instead of a hashtable. By doing so, he was able to save everything to the unit id rather than their handle id.

However, your version is perfectly fine. The speed difference is negligible and the API is the same so it doesn't really matter. (actually, Mag's doesn't have the unit-specific functions)

-----

As for the system itself, you don't need to use SaveAbilityHandle. The type ability is not used in JASS (or GUI, for that matter).

You can also just save the X/Y of the spell individually and scrap saving the location, since locations are scarcely used in JASS anyway.

Also make the globals all private so that they won't be modified outside the scope.

Otherwise, it looks good. :)
 
Level 30
Joined
Jul 10, 2007
Messages
6,307
How is banning/unbanning spells from this snippet a different resource? It is essential to have a banning function, you want to return the last ability used, which shouldn't be a dummy spell.

Dummy Spells are cast by Dummy Units :\

If you're going to do that, rename it to FilterSpell or w/e so that it's not misleading. Also, you should never be able to unfilter a spell, that's pointless. After all, these are dummy spells we are talking about, so permanently filtering them is fine.


The reason mag wants you to use Table is because you are using an entire hashtable for one tiny resource. You do realize that a map can only have 255 hashtables in it right? Why does your resource get to have its own hashtable?

Use Table, your resource doesn't necessitate the need for its own hashtable.
 
Also how can structs be faster then normal function calls?

Simply using structs is not exactly faster, but it allows you to make things faster.
For example, instead of hashtables, I'm using a UnitIndexer and hence, only arrays, of course, your version using a hashtable is fine.

It would be better if it used Table, but that's not exactly the most important change.

Right now, what's important is getting rid of the 'ability' caching and the 'location' caching, because they are pretty useless. The location is only useful if you want to get the Z of the target location.
 
Level 15
Joined
Aug 7, 2009
Messages
1,407
Dummy Spells are cast by Dummy Units :\

In the most cases, but not always. Some of my heroes also have dummy spells, to cancel spells, or to display cooldowns - for me though it's simple to filter them, as all the hero spells are registered to my Ability struct, and I'm not using other pitiful dummy abilities to place bonuses, heals, dots, or other kind of things.

Unfiltering them was just an idea, I don't see a reason why it shouldn't be included - one might find it useful. An example: an ultimate spell, with only one level, where the second, invisible level is a dummy one to decrease its cooldown to 0.5 seconds. It's based on Berserk, so it doesn't interrupt movement. The hero's passive makes their normal spells able to crit, and whenever they do so, the ultimate's cooldown is reduced to 0.5 seconds - it's readded, set to level 2 and the hero is ordered to cast it.

In that case, you don't want it to be the last spell cast on the list, because it wasn't cast, so you have to temporarily add it to the list of banned spells (of course you can also make a dummy spell, but in this scenario it's unnecessary). Just because you can't find use for something, others may do, and you're making resources for others, not only for yourself.
 
A totally different resource called AbilityList would be cool.
There could be a ton of other resources that could work off of that list, so having it as an entire resource is a good idea.

It's not a requirement though.

I think having the ability to register abilities is better than having the ability to ban them.

This way, you would have 1 call per 1 spell trigger on map init.
 
Level 23
Joined
Apr 16, 2012
Messages
4,041
it would be nice, but I cant have banability and allowability at same time
Also think about it:
Yes its 1 call per 1 spell trigger but, if we want basic abilities all to be registered, we need to call this function 778 times :D + the custom ones.
You wont ban 778 functions, I personally dont know any map that has 700 custom abilities.
Btw update incomming(not today tho.)
 
Last edited:
Speaking of DotA, it is shit at the code level.

Pure shit.

Dynamic triggers are created for each spell cast with events registered to them, and they actually use their eval/exec counts to do counters >.>

It's incredibly inefficient.

It doesn't even use a system like SpellEffectEvent to reduce lag on multiple spell-casts.

I can confirm that the map does use Table by Vexorian because only one hashtable is in the globals block, but I can't confirm the use of anything else.

I'm still not even sure the map uses a Damage detection system or a Unit Indexer >.>

edit
Oh, and sorry for the OT.
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
in addition to the off-topic, the time dota was created was just like stone age, now it's
already nano technology, new systems are created which is faster and more efficient, if
dota will be remade, these nanos will be applied and who knows 'maybe' it's still on top of
the wc3chain...
 
Level 23
Joined
Apr 16, 2012
Messages
4,041
I find it easier to work with 2D tablearrays because its more similar to hashtables(for me at least) :D
edit:
Hopefully fixed, I acutally tested it and GetUnitLastCastAbility, GetLastCastAbility, BanAbility and TempBanAbility works so the others should as well
 
Last edited:
JASS:
        private function TempBanUnban takes nothing returns nothing
            local timer t = GetExpiredTimer()
            local integer abilcode = myTab[8][GetHandleId(t)]
            set myTab[7][abilcode] = 0
            call DestroyTimer(t)
            set t = null
        endfunction
        
        
        function TempBanAbility takes integer abilcode, real dur returns nothing
            local timer t = CreateTimer()
            set myTab[7][abilcode] = 1
            set myTab[8][GetHandleId(t)] = abilcode
            call TimerStart(t, dur, false, function TempBanUnban)
            set t = null
        endfunction

You really don't need this :/
It's better to have shorter code.

All this extra 'crap' as Nestharus would call it is really not needed.

edit
Also, I'd rather have it so that you have to register all the abilities that this thing can run for :p
This way, you need 1 call in every spell trigger rather than having random calls everywhere.
It makes everything more orderly.
 
Level 23
Joined
Apr 16, 2012
Messages
4,041
well, as I mentioned, if you want to call RegisterAbility rather then BanAbility its not that great really, you have to allow 700 premade abilities, its the same as banning 700 abilities and you're not going to ban 700 abilities ever. Most map doesnt even has 700 custom abilities.

Having this crap© doesnt slow it, its additional functions not something inside the main code, and there may be times when you dont want to register ability for short time(lets say you have something like COUNTER SPELL STEAL!! - preventing you to be spell stealed MUHEHE - :D) - I may remove it if you insist on it.
 
Level 30
Joined
Jul 10, 2007
Messages
6,307
Having this crap© doesnt slow it, its additional functions not something inside the main code, and there may be times when you dont want to register ability for short time(lets say you have something like COUNTER SPELL STEAL!! - preventing you to be spell stealed MUHEHE - :D) - I may remove it if you insist on it.

Additional functionality like that belongs in different libs.
 
Level 23
Joined
Apr 16, 2012
Messages
4,041
I am going to argue,
If everyone got idea how to improve their snippets/wrappers/systems/whatever and they would be like this TempBanAbility and they created new thread for it there would be around hundreds of them.
Its like 14 lines of code :D
 
Level 30
Joined
Jul 10, 2007
Messages
6,307
Trust me edo, having a large number of libraries that all do a small number of things is the way to go if you want to be able to debug things very rapidly and easily.

I've been doing this while optimizing and adding features to a very primitive game engine model, and debugging it has been as easy as .. fruitcake?

Completely true. You are doing what I call code spread, when you include functionality of unrelated systems in certain systems. When you decide to change it later on, the code is completely tangled up in everything, making it an absolute pain. You want things to be as standalone as possible, otherwise things are a pain to debug, they are a pain to improve, and they are a pain to redesign.
 
Level 30
Joined
Jul 10, 2007
Messages
6,307
Change Ban/Unban to LastAbilityAddFilter/LastAbilityRemoveFilter.


Regardless, these function names are def not ok as they are too vague. They can easily collide with other functions from other libraries or general map functions.
JASS:
*       function BanAbility takes integer returns nothing
*       function UnbanAbility takes integer returns nothing

edit
This needs to be in a module initializer

JASS:
        private function OnInit takes nothing returns nothing
            local trigger t = CreateTrigger(  )
            set myTab = TableArray[5]
            call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
            call TriggerAddAction( t, function MainBody )
            set t = null
        endfunction

You should also be using this http://www.hiveworkshop.com/forums/jass-resources-412/snippet-registerplayerunitevent-203338/
 
Last edited:
Level 23
Joined
Apr 16, 2012
Messages
4,041
Level 30
Joined
Jul 10, 2007
Messages
6,307
The next thing, use a Unit Indexer instead of all of the GetHandleId calls and the hashtable. You can just use GetUnitUserData to make it so that the user can pick whatever UnitIndexer they want.

Also, you need to clean out the data when the unit deindexes. If a new unit is made and it shares the handle of an old unit and the last cast is used on it, it'll show invalid data from before. This means that you must use a UnitIndexer in order to get the deindex event. I recommend the Unit Indexer I did as it's rather standard on here and it's the easiest one to start using

JASS:
//optional
private method index takes nothing returns nothing
endmethod

//optional
private method deindex takes nothing returns nothing
endmethod

//optional
private static method filter takes unit filterUnit returns boolean
    return true
endmethod

implement UnitIndexStruct

In fact, once you use a UnitIndexer, Table will no longer be needed as you can just use fields to store the data.
 
Level 23
Joined
Apr 16, 2012
Messages
4,041
WHY, WHY wont you post all the things at once :D
It will soon be 10.0 because of that :D
That is good point but that can happen even with unit indexer, if unit indexed at position 1 dies and then is immedietally spawned new one with index 1 and you steal from it right afterwards you will still steal the last cast spell of previous unit indexed at position 1
Also doesnt any Unit Indexer works only for up to 8192 units? with handles you can have relativly infinite number

I dont say I wont use it but also I dont say I will, Ill see :D
 
Level 30
Joined
Jul 10, 2007
Messages
6,307
That is good point but that can happen even with unit indexer, if unit indexed at position 1 dies and then is immedietally spawned new one with index 1 and you steal from it right afterwards you will still steal the last cast spell of previous unit indexed at position 1

no... deindex != death... learn your stuff

deindex == unit no longer exists, like RemoveUnit or unit decayed.
 
Top