AutoCastSystem v1.5

This bundle is marked as awaiting update. A staff member has requested changes to it before it can be approved.
AutoCastSystem v1.5

Introduction

This system allows your AI or even your own units to automatically cast random spells at hand.
It has a simple mechanic to evaluate a situation or function that you created on when, what, where
to cast a certain spell.

Code

Tutorial (vJass)

Pros/Cons


JASS:
/*********************************************************************
    AutoCastSystem v1.5
    by mckill2009
**********************************************************************

    INSTALLATION:
        Copy ALL the required libraries and the AutoCastSystem trigger to your trigger editor
    OR       
        Copy ALL the triggers from the AutoCastSystem folder

**********************************************************************
   
    Struct API: See the tutorial/demo on how this works
   
        static method register takes unit caster returns thistype
            - this MUST be called first
            - local ACS test >>> set test = test.register(YOUR_UNIT)
       
        method spellTypeUnitTarget takes code func, integer spellOrderID returns nothing
        method spellTypePointTarget takes code func, integer spellOrderID returns nothing
        method spellTypeNoTarget takes code func, integer spellOrderID returns nothing
            - call test.spellTypeUnitTarget(function CODE_THAT_RETURNS_BOOLEAN, ORDER_ID)
            - the boolean must return true for the caster to cast spell automatically
            - increase the chance of casting a particular spell by calling this 2 or more           
            - CONS, if you created a lot of this, say 6, you need to remove 6 also if you want
                the spell not to be casted at all
           
        method removeSpellOrderUnit takes integer spellOrderID returns nothing
        method removeSpellOrderPoint takes integer spellOrderID returns nothing
        method removeSpellOrderNone takes integer spellOrderID returns nothing
            - call test.removeSpellOrderUnit(ORDER_ID)
            - if you dont want the spell to be casted anymore simply remove it ONE BY ONE
            - this only DECREASES the chance for a particular spell to be casted NOT REMOVING IT ALL
            - if you want to remove all, do this calling as many times you added a particular spellOrderID
       
        method launch takes real interval returns nothing
            - call test.launch(1.0)
            - recommended is 1 but you may set it as low as 0.03125 for a very fast trigger evaluation
                thus FAST casting a spell
           
        method removeWhenCasterDies takes boolean b returns nothing
            - default is false but you may change it to true
            - call test.removeWhenCasterDies(true)
       
        method removeAutocast takes nothing returns nothing
            - this removes the caster from the system even if it's still alive
            - call test.removeAutocast()
       
    Struct globals: you may use these for code func you create that returns a boolean
        CS.caster (readonly)
        CS.target (can modify)
        CS.SpellTargetX (can modify)
        CS.SpellTargetY (can modify)

**********************************************************************
   
    CREDITS:
       
        TimerUtils by Vexorian              http://www.wc3c.net/showthread.php?t=101322
        Table by Bribe                      http://www.hiveworkshop.com/forums/jass-resources-412/snippet-new-table-188084/
        IsUnitChanneling by Magtheridon96   http://www.hiveworkshop.com/forums/jass-resources-412/snippet-isunitchanneling-211254/

**********************************************************************/
library AutoCastSystem uses TimerUtils, Table, IsUnitChanneling

globals
    /***********************************************
    *   NON-CONFIGURABLES
    ************************************************/
    private TableArray trig
    private TableArray oID
    private Table chk
    private unit TempCaster = null
endglobals

private function UnitAlive takes unit u returns boolean
    return not (IsUnitType(u, UNIT_TYPE_DEAD) or u==null)
endfunction

private module startCSinit
    private static method onInit takes nothing returns nothing
        set trig = TableArray[8192]
        set oID = TableArray[8192]
        set chk = Table.create()
    endmethod     
endmodule

struct CS extends array
    readonly static unit caster
    static unit target
    static real SpellTargetX
    static real SpellTargetY
   
    static method returnCaster takes nothing returns nothing
        set CS.caster = TempCaster
    endmethod   
   
    implement startCSinit
endstruct

struct ACS   
    private unit caster
    private integer indexUnitMax
    private integer indexPointMax
    private integer indexNoneMax  
    private boolean remove
    private boolean removeAuto
    private boolean checkTimer
   
    private static constant integer pointIndex = 10000
    private static constant integer noneIndex = 20000
   
    private method removeEx takes integer index, integer maxIndex returns nothing
        local integer i = index
        loop
            set i = i+1
            exitwhen i==maxIndex
            set oID[this][i] = 0
            call DestroyTrigger(trig[this].trigger[i])
            set trig[this].trigger[i] = null
        endloop   
    endmethod
   
    private method destroy takes nothing returns nothing
        call .removeEx(0, .indexUnitMax)
        call .removeEx(pointIndex, .indexPointMax)
        call .removeEx(noneIndex, .indexNoneMax)       
        call chk.remove(GetHandleId(.caster))
        set .caster = null
        call .deallocate()  
    endmethod
   
    private static method period takes nothing returns nothing
        local timer t = GetExpiredTimer()
        local thistype this = GetTimerData(t)
        local integer randomizer
        local integer i
        if .remove then
            call ReleaseTimer(GetExpiredTimer())
            call .destroy()
       
        elseif UnitAlive(.caster) then
            set TempCaster = .caster
            call CS.returnCaster()
            set .caster = TempCaster
            if not IsUnitChanneling(.caster) then
                set randomizer = GetRandomInt(1, 3)
                if randomizer==1 then //Unit target
                    set i = GetRandomInt(1, .indexUnitMax)
                    if oID[this][i] > 0 then  
                        if TriggerEvaluate(trig[this].trigger[i]) then
                            call IssueTargetOrderById(.caster, oID[this][i], CS.target)
                        endif
                    endif
   
                elseif randomizer==2 then //Point target
                    set i = GetRandomInt(pointIndex, .indexPointMax)
                    if oID[this][i] > 0 then  
                        if TriggerEvaluate(trig[this].trigger[i]) then
                            call IssuePointOrderById(.caster, oID[this][i], CS.SpellTargetX, CS.SpellTargetY)
                        endif               
                    endif
           
                elseif randomizer==3 then //No target
                    set i = GetRandomInt(noneIndex, .indexNoneMax)
                    if oID[this][i] > 0 then  
                        if TriggerEvaluate(trig[this].trigger[i]) then
                            call IssueImmediateOrderById(.caster, oID[this][i])
                        endif
                    endif
                endif   
            endif     
           
        elseif .removeAuto then
            set .remove = true
        endif
        set t = null
    endmethod
   
    /***********************************************
    *   SYSTEM API's
    ************************************************/
    static method register takes unit caster returns thistype
        local thistype this
        if chk.has(GetHandleId(caster)) then   
            debug if IsUnitType(caster, UNIT_TYPE_HERO) then
                debug call DisplayTextToPlayer(GetLocalPlayer(), 0, 0, "thistype.register ERROR: "+GetHeroProperName(caster)+" is already registered!")
            else
                debug call DisplayTextToPlayer(GetLocalPlayer(), 0, 0, "thistype.register ERROR: "+GetUnitName(caster)+" is already registered!")
            endif
            return 0
        else
            set this = allocate()
            set .caster = caster
            set .remove = false
            set .indexUnitMax = 0     
            set .indexPointMax = pointIndex
            set .indexNoneMax = noneIndex
            set .removeAuto = false
            set .checkTimer = true
            set chk[GetHandleId(caster)] = this
        endif
        return this
    endmethod
   
    method spellTypeUnitTarget takes code func, integer spellOrderID returns nothing  
        set .indexUnitMax = .indexUnitMax + 1
        set oID[this][.indexUnitMax] = spellOrderID
        set trig[this].trigger[.indexUnitMax] = CreateTrigger()
        call TriggerAddCondition(trig[this].trigger[.indexUnitMax], Condition(func))
    endmethod
   
    method spellTypePointTarget takes code func, integer spellOrderID returns nothing   
        set .indexPointMax = .indexPointMax + 1
        set oID[this][.indexPointMax] = spellOrderID
        set trig[this].trigger[.indexPointMax] = CreateTrigger()
        call TriggerAddCondition(trig[this].trigger[.indexPointMax], Condition(func))
    endmethod
   
    method spellTypeNoTarget takes code func, integer spellOrderID returns nothing   
        set .indexNoneMax = .indexNoneMax + 1
        set oID[this][.indexNoneMax] = spellOrderID
        set trig[this].trigger[.indexNoneMax] = CreateTrigger()
        call TriggerAddCondition(trig[this].trigger[.indexNoneMax], Condition(func))
    endmethod
   
    method removeSpellOrderUnit takes integer spellOrderID returns nothing
        local integer i = 0
        loop
            set i = i+1
            if oID[this][i]==spellOrderID then
                set oID[this][i] = oID[this][.indexUnitMax]
                set oID[this][.indexUnitMax] = 0
                call DestroyTrigger(trig[this].trigger[.indexUnitMax])
                set trig[this].trigger[.indexUnitMax] = null
                set .indexUnitMax = .indexUnitMax - 1
                exitwhen true               
            endif
            exitwhen i==.indexUnitMax
        endloop
    endmethod
   
    method removeSpellOrderPoint takes integer spellOrderID returns nothing
        local integer i = pointIndex
        loop
            set i = i+1
            if oID[this][i]==spellOrderID then
                set oID[this][i] = oID[this][.indexPointMax]
                set oID[this][.indexPointMax] = 0
                call DestroyTrigger(trig[this].trigger[.indexPointMax])
                set trig[this].trigger[.indexPointMax] = null
                set .indexPointMax = .indexPointMax - 1
                exitwhen true               
            endif
            exitwhen i==.indexPointMax
        endloop
    endmethod
   
    method removeSpellOrderNone takes integer spellOrderID returns nothing
        local integer i = noneIndex
        loop
            set i = i+1
            if oID[this][i]==spellOrderID then
                set oID[this][i] = oID[this][.indexNoneMax]
                set oID[this][.indexNoneMax] = 0
                call DestroyTrigger(trig[this].trigger[.indexNoneMax])
                set trig[this].trigger[.indexNoneMax] = null
                set .indexNoneMax = .indexNoneMax - 1
                exitwhen true               
            endif
            exitwhen i==.indexNoneMax
        endloop
    endmethod
   
    method launch takes real interval returns nothing
        if chk.has(GetHandleId(.caster)) then  
            if .checkTimer then
                set .checkTimer = false
                call TimerStart(NewTimerEx(this), interval, true, function thistype.period)
            else
                debug call DisplayTextToPlayer(GetLocalPlayer(), 0, 0, "thistype.launch ERROR: Timer is already active!")
            endif
        else
            debug call DisplayTextToPlayer(GetLocalPlayer(), 0, 0, "thistype.launch ERROR: attempt to start a null caster.")
        endif
    endmethod
   
    method removeWhenCasterDies takes boolean b returns nothing
        set .removeAuto = b
    endmethod
   
    method removeAutocast takes nothing returns nothing
        set .remove = true
    endmethod
endstruct

endlibrary

This is only a vJass demo, you MUST start reading at function CastAITestBloodmage
JASS:
library TEST uses AutoCastSystem, RandomUnit

globals
    ACS bloodmage
    ACS warden
endglobals

private function UnitAlive takes unit u returns boolean
    return not IsUnitType(u, UNIT_TYPE_DEAD) and GetUnitTypeId(u)!=0
endfunction

private function FilterAllies takes unit flteredUnit returns boolean
    return UnitAlive(flteredUnit) and not IsUnitEnemy(CS.caster, GetOwningPlayer(flteredUnit))
endfunction

private function FilterEnemies takes unit flteredUnit returns boolean
    return UnitAlive(flteredUnit) and IsUnitEnemy(CS.caster, GetOwningPlayer(flteredUnit))
endfunction

/*******************************************************
*   CAST CONDITIONS AND UNIT FILTERING
********************************************************/
private function CheckAlly takes nothing returns boolean
    return FilterAllies(GetFilterUnit())
endfunction

private function CheckEnemies takes nothing returns boolean
    return FilterEnemies(GetFilterUnit())
endfunction

private function TargetEnemyNonStructure takes nothing returns boolean
    return FilterEnemies(GetFilterUnit()) and not IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE)
endfunction

private function TargetEnemyStructure takes nothing returns boolean
    return FilterEnemies(GetFilterUnit()) and IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE)
endfunction

private function TargetEnemyFlyers takes nothing returns boolean
    return FilterEnemies(GetFilterUnit()) and IsUnitType(GetFilterUnit(), UNIT_TYPE_FLYING)
endfunction

/*******************************************************
*   NO-TARGET
********************************************************/
private function CastChanceNoTarget takes nothing returns boolean
    local boolean b = false
    if GetRandomInt(0, 100) < 80 then
        set CS.target = GetRandomUnitInArea(GetUnitX(CS.caster), GetUnitY(CS.caster), 500, Filter(function CheckEnemies))
        if CS.target!=null then
            set b = true
        endif
    endif
    return b
endfunction

/*******************************************************
*   TARGET UNIT
********************************************************/
private function CastHealAlly takes nothing returns boolean
    set CS.target = GetRandomUnitInArea(GetUnitX(CS.caster), GetUnitY(CS.caster), 900, Filter(function CheckAlly))
    if CS.target!=null and GetWidgetLife(CS.target) < GetUnitState(CS.target, UNIT_STATE_MAX_LIFE) and GetUnitAbilityLevel(CS.target, 'Brej')==0 then
        return true
    endif
    return false
endfunction

private function CastTargetEnemy takes nothing returns boolean
    set CS.target = GetRandomUnitInArea(GetUnitX(CS.caster), GetUnitY(CS.caster), 900, Filter(function TargetEnemyNonStructure))
    return CS.target!=null
endfunction

private function CastChanceTargetEnemy takes nothing returns boolean
    local boolean b = false
    if GetRandomInt(0, 100) < 70 then
        set CS.target = GetRandomUnitInArea(GetUnitX(CS.caster), GetUnitY(CS.caster), 900, Filter(function TargetEnemyFlyers))   
        if CS.target!=null and GetUnitAbilityLevel(CS.target, 'Bena')==0 then
            set b = true
        endif       
    endif   
    return b
endfunction

/*******************************************************
*   TARGET POINT
********************************************************/
//This is how to manipulate the spell's execution, this is a condition for the spell to take place
//if it returns true the spell is casted
private function CastPointEnemy takes nothing returns boolean
    /********************************************
        Struct globals you may use, these are self explanatory
            CS.caster (readonly)
            CS.target (can modify)
            CS.SpellTargetX (can modify)
            CS.SpellTargetY (can modify)
    *********************************************/
    set CS.target = GetRandomUnitInArea(GetUnitX(CS.caster), GetUnitY(CS.caster), 900, Filter(function TargetEnemyNonStructure))
    if CS.target!=null then
        set CS.SpellTargetX = GetUnitX(CS.target)
        set CS.SpellTargetY = GetUnitY(CS.target)
        return true
    endif
   
    //if you do not have a target but just want to cast a spell in the XY of the map
    //you may set the CS.SpellTargetX and CS.SpellTargetY to a coordinate you like
    //and just return true, but remember that the XY MUST be in range of spell
    return false
endfunction

/*******************************************************
*   START READING HERE
********************************************************/
private function RemoveBloodmageFromSystem takes nothing returns nothing  
    //removes the unit from the system
    call bloodmage.removeAutocast()
    call BJDebugMsg("******************************************")
    call BJDebugMsg("Bloodmage removed from casting ALL spells")
endfunction

private function RemoveSpell takes nothing returns nothing
    call bloodmage.removeSpellOrderUnit(852218) //carrion swarm
    call BJDebugMsg("******************************************")
    call BJDebugMsg("After a while again, Blood mage will no longer cast any spells.")
    call TimerStart(CreateTimer(), 10.0, false, function RemoveBloodmageFromSystem)
endfunction

function CastAITestBloodmage takes unit u returns nothing
    //create a global or local ACS based on struct name
    //in this scenario, global is created for it to be used outside this function
    //a GLOBAL instance is highly recommended
    //units cannot be registered twice
    set bloodmage = bloodmage.register(u)   
   
    /***************************************************************************
    *   SPELL REGISTRATION, use OrderID's
    ****************************************************************************/
    //the unit will cast 6 different kinds of spell based below
    //you do not need to create a new function, you may add different spell to the same function but
    //depends on your condition though
    //you may create a condition such as CHANCE for it to be true
    //for the purpose of this demo, the cooldown is set to 2 seconds for each of the spells  
    //all spells also is at range from the standing hero and target unit or point
   
    //the function is the execution condtion while the number is the orderID of the spell
    //the function must return TRUE for the spell to take place
    //you may add the spell orderID twice or many times to increase the chance for the unit to cast a certain spell
    call bloodmage.spellTypeUnitTarget(function CastHealAlly, 852160) //rejuvenation
    call bloodmage.spellTypeUnitTarget(function CastTargetEnemy, 852189) //cripple   
    call bloodmage.spellTypeUnitTarget(function CastTargetEnemy, 852487) //life drain (channel)
   
    call bloodmage.spellTypePointTarget(function CastPointEnemy, 852488) //flame strike
    call bloodmage.spellTypePointTarget(function CastPointEnemy, 852218) //carrion swarm
   
    call bloodmage.spellTypeNoTarget(function CastChanceNoTarget, 852588) //howl of terror   
   
    /***************************************************************************
    *   STARTING THE CASTING
    ****************************************************************************/
    //the last thing you need to do, set the interval on how many seconds your caster casts the spell
    call bloodmage.launch(1.0) //recommended is 1 second, but it's all up to you
   
    //call BJDebugMsg("******************************************")
    //call BJDebugMsg("Carrion Swarm will be removed from casting after a while.")
    //removing a specific spell from the system
    //in this example, the global bloodmage instance is used
    //call TimerStart(CreateTimer(), 10.0, false, function RemoveSpell)
endfunction

function CastAITestWarden takes unit u returns nothing
    set warden = warden.register(u)
   
    call warden.spellTypePointTarget(function CastPointEnemy, 852238) //rain of fire (channel)  
    call warden.spellTypePointTarget(function CastPointEnemy, 852525) //blink   
   
    call warden.spellTypeUnitTarget(function CastChanceTargetEnemy, 852106) //ensnare
    call warden.spellTypeUnitTarget(function CastChanceTargetEnemy, 852527) //shadow strike
   
    call warden.spellTypeNoTarget(function CastChanceNoTarget, 852526) //fan of knives   
   
    call warden.launch(1.0)   
endfunction

endlibrary

Pros
- Cast a spell even if your unit is NOT an AI
- Works on channeled spells
- Casts random spells, makes the AI can use ALL of the spells
- Has time interval on when to cast

Cons
- Manually make a trigger/funcion that evaluates a certain situation in order for AI to cast a spell, meaning the evaluation MUST return TRUE
- Manually removes your spellOrderID, if you created 6, you need to remove 6



v1.5
- Added a global variable TempCaster to avoid modifying the CS.caster which is now readonly
- Added module initializer

v1.4
- Most of the suggestion of MyPad is considered
- Some useless instance variable deleted

v1.3
- ADDED timer check whether it has been launched already or not
- Most of Quilnezhas been made

v1.2
- Reorganized and renamed API's
- Triggers now destroyed and nulled when unit is removed from system

v1.1
- Added an automatic removal from the system if unit is dead



TimerUtils by Vexorian
Table by Bribe
IsUnitChanneling by Magtheridon96

For Suggestions:
Special thanks to Quilnez and MyPad their review :D
Contents

AutoCastSystem (Map)

Reviews
MyPad
A common bug was spotted among these methods, in struct ACS method removeSpellOrderUnit takes integer spellOrderID returns nothing method removeSpellOrderPoint takes integer spellOrderID returns nothing method removeSpellOrderNone takes integer...
Level 19
Joined
Mar 18, 2012
Messages
1,717
if you mean channeling, yes, the sample map has life drain and I tested it in custom channel abilities
What I meant is that each unit has animation properties i.e spell, slam, etc ...
I want to know if the spells start with a 0 seconds delay or do they wait for the animation to finish.

Yes I agree weight would be just a change of the concept.
 

Kazeon

Hosted Project: EC
Level 33
Joined
Oct 12, 2011
Messages
3,445
Hello! :)
  • JASS:
            method launch takes real interval returns nothing
            method removeWhenDead takes boolean b returns nothing
    What does "launch" do? Does it launch a missile?
    What gets removed on dead? The registered unit? Removed from what?
    The point is, give simple descriptions about those functions/methods.
  • JASS:
    private function UnitAlive takes unit u returns boolean
        return not IsUnitType(u, UNIT_TYPE_DEAD) and u!=null
    endfunction
    =>
    native UnitAlive takes unit id returns boolean
  • struct CS extends array, struct ACS extends array
    Those are not good names for public structs
  • JASS:
                if IsUnitType(caster, UNIT_TYPE_HERO) then
                    call DisplayTextToPlayer(GetLocalPlayer(), 0, 0, "thistype.register ERROR: "+GetHeroProperName(caster)+" is already registered!")
                else
                    call DisplayTextToPlayer(GetLocalPlayer(), 0, 0, "thistype.register ERROR: "+GetUnitName(caster)+" is already registered!")
                endif
    Should be done at debug mode only.
  • static method period takes nothing returns nothing
    Should be private.
  • JASS:
        method launch takes real interval returns nothing
            call TimerStart(NewTimerEx(this), interval, true, function thistype.period)
        endmethod
    You should check whether it has been launched already or not. Or at least, destroy the previous timer first.
  • method removeSpellOrderUnit takes integer spellOrderID returns nothing
    You don't destroy the triggers while you keep re-creating them here:
    JASS:
        method spellTypeUnitTarget takes code func, integer spellOrderID returns nothing  
            set .indexUnitMax = .indexUnitMax + 1
            set oID[this][.indexUnitMax] = spellOrderID
            @set trig[this].trigger[.indexUnitMax] = CreateTrigger()@
  • JASS:
        method removeSpellOrderUnit takes integer spellOrderID returns nothing
            local integer i = 0
            loop
                set i = i+1
                if oID[this][i]==spellOrderID then
                    set oID[this][i] = oID[this][.indexUnitMax]
                    set .indexUnitMax = .indexUnitMax - 1
                    @exitwhen [email protected]               
                endif
                exitwhen i==.indexUnitMax
            endloop
        endmethod
    At first I thought allowing user to register same order id is a good idea so user can have something called preferences (by registering same order id twice or anything, more than other order ids). I mean, if you register one order id twice or more, you will have higher chance that ability will be casted more than the others (which only registered once), right?

    But if you exit from the iteration directly (exitwhen true) when the desired order id has met there is an opportunity that the other same order ids are not removed. The choice is on you, will you allow multi-registering or not.
  • JASS:
        method remove takes nothing returns nothing
            set .isOn = false
        endmethod
    It doesn't remove the unit from your system (unregister), I think "enable/d/" is a better name and it should takes a boolean (true = enable, false = disable). And about unregistering unit, you should add one. Unregister should automatically remove all registered order ids. Maybe you have one already:
    JASS:
        private method destroy takes nothing returns nothing
            call chk.remove(GetHandleId(.caster))
            set .caster = null
            call .deallocate()  
        endmethod
    But it's private, user has no access to it, and you haven't destroyed all triggers yet.
This is definitedly useful. I like the addition where users are allowed to evaluate some conditions before the spell is going to be casted, but, I'm afraid the evaluation which can be done is so limited, there are several causes:
- User have no access to the "casting unit"
- User have no access to the "casted ability" or the order id.

And may I ask? Why do you randomize things? There's a big chance that the unit won't cast anything if the chosen ability is still in cooldown phase, and what happen if that ability is chosen over again? I suggest to cast them based on a specific order (i.e. based on the order when they are firstly registered to the system or allow user to have a variable called "priority" or "chances" for each ability)
 
You explained most stuff in an example/demo trigger, but not in the system itself.
I agree with Dalvengyr, that short description should also be part of your system.

Your UnitAlive method should be changed to:
JASS:
private function UnitAlive takes unit u returns boolean
    return not IsUnitType(u, UNIT_TYPE_DEAD) and GetUnitTypeId(u) != 0
endfunction
Because your's currently doesn't check for removed units.... or just use the native that Dalvengyr suggested.

Privatize as much as possible. And in the post above there are more things mentioned that you could have a look at. (like the trigger- and multiple register and then unregister question)

I think this is a useful system.
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
What does "launch" do? Does it launch a missile?
What gets removed on dead? The registered unit? Removed from what?
launch is OK, the other one I already changed the API name

struct CS extends array , struct ACS extends array
Those are not good names for public structs
nothing wrong with abbrevations, coz if I put on full name then it might conflict with other 'AutoCastSystem' struct names

I am not a fan of custom allocators as nothing wrong with the built in one

native UnitAlive takes unit id returns boolean
it's better to make it functions as I donnt want to spam the native inside one map

Should be done at debug mode only.
debug modes are optional but I want it to be implicitly displayed

You don't destroy the triggers while you keep re-creating them here:
destroying triggers are useless coz it's not functioning well as it keeps registering the same id's, better is destroying and nulling

At first I thought allowing user to register same order id is a good idea so user can have something
called preferences (by registering same order id twice or anything, more than other order ids).
I mean, if you register one order id twice or more,
you will have higher chance that ability will be casted more than the others (which only registered once), right?
no, users cannot use twice order id's anyway, coz spells will be in conflict, the 'CHANCE' you're saying is in the condition evaluate
which is outside the system (I have also included a chance demo for that).

But if you exit from the iteration directly ( exitwhen true ) when the desired order id has met there is
an opportunity that the other same order ids are not removed. The choice is on you, will you allow multi-registering or not.
like I said, users cant register (they may) but it's not a good idea coz it will conflict the spell, it's a bug blizzard made.

But it's private, user has no access to it, and you haven't destroyed all triggers yet.
I've changed the API, although the precious one, you may access the destroy by calling remove since it uses a boolean check "isOn"

This is definitedly useful. I like the addition where users are allowed to evaluate some conditions before the spell is going to be
casted, but, I'm afraid the evaluation which can be done is so limited, there are several causes:
- User have no access to the "casting unit"
- User have no access to the "casted ability" or the order id.
You may access the caster via CS.caster and I dont see any reason why you need to access the casted ability...

And may I ask? Why do you randomize things? There's a big chance that the unit won't cast anything if the chosen ability
is still in cooldown phase, and what happen if that ability is chosen over again? I suggest to cast them
based on a specific order (i.e. based on the order when they are firstly registered to the system or allow
user to have a variable called "priority" or "chances" for each ability)
coz that's the 'theme' of this system, to randomize, and like I said you may prioritize spells in the execution condition function
like 'chances' and whether the unit has buff/range/invulnerable, etc...you may even prioritize the spells based on their cooldowns and mana
 
Level 17
Joined
Nov 18, 2012
Messages
1,460
IMO, I would prefer doing my own "autocast spell" rather than use any system like this, I'm sorry. If you want this to become more flexible, maybe make registration of ability's cooldown and then add timers to check if the ability can be cast again.
 
Level 13
Joined
Mar 19, 2010
Messages
870
IMO, I would prefer doing my own "autocast spell" rather than use any system like this, I'm sorry. If you want this to become more flexible, maybe make registration of ability's cooldown and then add timers to check if the ability can be cast again.

YES!!!

Pls tell me when you're rdy. I need a system like that for my map "Forsaken Bastion's Fall". Don't forget me :)

Best Reg and gl for this cool system idea!
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
IMO, I would prefer doing my own "autocast spell" rather than use any system like this, I'm sorry. If you want this to become more flexible, maybe make registration of ability's cooldown and then add timers to check if the ability can be cast again.
ability cooldown cant be implemented coz the player might reset the ability cooldowns so the timer will be inaccurate, plus there is no detection of cooldown abilities yet...
 
In private method removeEx the exitwhen must be on top of the loop, as you don't know if there is anything registered.
If there is an empty stack once, you will try to make invalid operations, and it will stop the virtual thread. -> bug, no deallocation.

Your randomizer onPeriod:
oID[this][i] > 0
^If this is wrong you should randomize again between 2 and 3. Hope you understand my thought.
With your current method you could get 10x "1" as value and so do 10x "nothing" if there is no "unit target".
You just ignore rest at the moment, but it should be different. "empty" evaluations should be directly ignored, or others should get a new chance.
Hehe, sorry if I write it confusing.

For more safety it was better or to disallow mulitple registration (same id), or not exitloop onRemove, when found one matching id.

Why not directly call the destroy() onRemove? Why wait the period?
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
In private method removeEx the exitwhen must be on top of the loop, as you don't know if there is anything registered.
makes sense, I'll test that, though it will exit anyway at the end...

Your randomizer onPeriod:
oID[this][i] > 0
^If this is wrong you should randomize again between 2 and 3. Hope you understand my thought.
With your current method you could get 10x "1" as value and so do 10x "nothing" if there is no "unit target".
You just ignore rest at the moment, but it should be different. "empty" evaluations should be directly ignored, or others should get a new chance.
even humans do that, clicking icons with cooldowns still on the run, anyway I see what to do...

For more safety it was better or to disallow mulitple registration (same id), or not exitloop onRemove, when found one matching id.
if I allow that, sample holy light, then AI will not cast on undead units... >>> they can coz it evaluates, problem is it will stick to ONE function to evaluate...

Why not directly call the destroy() onRemove? Why wait the period?
coz there's no release timer when I do that directly...if I put the release timer in the destroy, I dont think it will go since it's in another function unless GetExpiredTimer() is following an event (which I didnt test)...
 

MyPad

Spell Reviewer
Level 21
Joined
May 9, 2014
Messages
1,649

Mmm, this has been updated. Could you detail what has changed in the system? Is this applicable to versions 1.29 above?

I found out the following:

JASS:
struct CS extends array
    ..

    @private static method onInit takes nothing returns nothing
        ..
    endmethod
endstruct

It does not use a module initializer, which could cause bugs on resources, which would have this as a dependency.

JASS:
library AutoCastSystem uses TimerUtils, Table, IsUnitChanneling

You lack RegisterPlayerUnitEvent as a requirement. Perhaps, specify its' nature to the resource at hand.

That is all I can catch at a glance. Further testing shall reveal more details on which functions to improve. I suspect that most of these issues are still relevant.

Until then,


Notes:

  • JASS:
    struct CS extends array
        ..
    
        @private static method onInit takes nothing returns nothing
            set trig = TableArray[0x2000]
            set oID = TableArray[0x2000]
            set chk = Table.create()
        endmethod
    endstruct

    As pointed out in my previous message, this is not initialized via module initializer.
    Furthermore, you can now just use the constant JASS_MAX_ARRAY_SIZE to
    ensure cross-compatibility across Wc3 versions.

  • JASS:
    private function UnitAlive takes unit u returns boolean
        return not (IsUnitType(u, UNIT_TYPE_DEAD) or GetUnitTypeId(u)==0 or u==null)
    endfunction

    I believe Quilnez already pointed this out, but you can simply use this native.

    native UnitAlive takes unit id returns boolean

  • JASS:
    struct CS extends array
        [USER=176910]@Static[/USER] unit caster
        ...
    
        private static integer pointIndex = 10000
        private static integer noneIndex = 20000

    I have a reason to believe that the caster variable should be readonly, it is not supposed to be modified in the first place.

    Also, static members pointIndex and noneIndex behave as constants, or that is what I make of it.

  • JASS:
    struct CS extends array
        ...
    
        static method register takes unit caster returns thistype
            local thistype this
            if chk.has(GetHandleId(caster)) then   
                if IsUnitType(caster, UNIT_TYPE_HERO) then
                    debug call DisplayTextToPlayer(GetLocalPlayer(), 0, 0, "thistype.register ERROR: "+GetHeroProperName(caster)+" is already registered!") 
                else
                    debug call DisplayTextToPlayer(GetLocalPlayer(), 0, 0, "thistype.register ERROR: "+GetUnitName(caster)+" is already registered!")
                endif
                return 0
            ...

    Just to be safe, let that case return the valid instance of the unit.
    Also, the if-then statement seems to be relevant only when debug-mode is enabled.
    Try to add a debug keyword before the if, else and endif in that method.

  • JASS:
    private method removeEx takes integer index, integer maxIndex returns nothing
        ...
            [USER=131480]@set[/USER] trig[this].trigger[i] = null
        endloop   
    endmethod

    You can change that from set to call trig[this].trigger.remove(i). This removes the entry
    and may have some form of benefit (though marginal it may be).

  • JASS:
    struct ACS
        ...
        private integer spellType

    I don't see that variable in use inside the system. You may have to remove it.

  • JASS:
    method removeWhenCasterDies takes boolean b returns nothing
        set .removeAuto = b
    endmethod

    In the way removeAuto is handled in the periodic callback function, it will remove the instance of the caster after the next tick, instead of checking if the unit is dead before changing the flag.

Nitpicks:

  • It would be best to place most of the documentation for the library inside it.
    • Explaining by example would work within the Test map, but a straightforward documentation can go a long way with providing understanding of the resource
      to the user.

Status:

  • Awaiting Update
 
Last edited:
Level 29
Joined
Mar 10, 2009
Messages
5,016
MyPad said:
As pointed out in my previous message, this is not initialized via module initializer.

I know that module initializes first but what's wrong with that?, maybe some spells/systems will begin with a module therefore this system will not work?...

JASS_MAX_ARRAY_SIZE will be changed, TBH i dont know about that constant though...but i know 8192...

MyPad said:
I believe Quilnez already pointed this out, but you can simply use this native.

native UnitAlive takes unit id returns boolean

That native is by far the fastest and most efficient way of cheking for Alive units but maybe because of taste.
I drink orange juice but do not drink the juice from the raw fruit (strange?)...
So please I think it would be OK to use a in UnitAlive function but I'm gonna erase the
GetUnitTypeId(u)==0

MyPad said:
- I have a reason to believe that the caster variable should be readonly, it is not supposed to be modified in the first place.
- Also, static members pointIndex and noneIndex behave as constants, or that is what I make of it.

- The private caster cannot be modified inside and outside of that struct, so it's fine...
- Sure changing to constant now...

MyPad said:
- Just to be safe, let that case return the valid instance of the unit.
- Also, the if-then statement seems to be relevant only when debug-mode is enabled.

- Is it?, it's already safe coz it will return 0 or am i missing something?...
- Add debug OK

MyPad said:
You can change that from set to call trig[this].trigger.remove(i). This removes the entry
and may have some form of benefit (though marginal it may be).

AFAIK it removes the entry from the table but doesnt null the trigger, unless it is nulling, so i'll leave it for the time being...

MyPad said:
I don't see that variable in use inside the system. You may have to remove it.

I missed that, so spellType is removed...

MyPad said:
In the way removeAuto is handled in the periodic callback function,
it will remove the instance of the caster after the next tick, instead of checking if the unit is dead before changing the flag.

That's the purpose of it, the option of removing the unit from the system even if it's still alive...

MyPad said:
It would be best to place most of the documentation for the library inside it.
Explaining by example would work within the Test map, but a straightforward documentation can go a
long way with providing understanding of the resource to the user.

IMO the API is straightforward and the documentation is the tutorial, it even mentions the pros and cons so i think it's safe to say that this is easy for the user specially all this is in the map...


THANKS for all your suggestion and updating it ASAP...
 

MyPad

Spell Reviewer
Level 21
Joined
May 9, 2014
Messages
1,649
I know that module initializes first but what's wrong with that?, maybe some spells/systems will begin with a module therefore this system will not work?...
Yep. That's the worst case scenario.

The private caster cannot be modified inside and outside of that struct, so it's fine...
The struct which I was referring to is first struct, CS. That struct is visible, which means any public members can be modified. Readonly members can be accessed but not modified.

Is it?, it's already safe coz it will return 0 or am i missing something?...
Suppose you have a function that relies on register returning 0 on a valid instance. Now, suppose that you have a lot of units whose total count exceeds the maximum array size. It will cause the instance to return 0, a false positive in the function above.

AFAIK it removes the entry from the table but doesnt null the trigger, unless it is nulling, so i'll leave it for the time being...
Well, it does remove the entry. Treat is as a variable of sorts.

IMO the API is straightforward and the documentation is the tutorial, it even mentions the pros and cons so i think it's safe to say that this is easy for the user specially all this is in the map...
I was actually referring to when importing the library and only the library directly. Of course, I agree with the explanation being sufficient, but it is only reserved to the test map itself.
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
Updated v1.5
- Added a global variable TempCaster to avoid modifying the CS.caster which is now readonly
- Added module initializer

MyPad said:
Suppose you have a function that relies on register returning 0 on a valid instance. Now, suppose that you have a lot of units whose total count exceeds the maximum array size. It will cause the instance to return 0, a false positive in the function above.
Nobody will create units that exceeds the array size or even 1000 coz of the lag it will produce and if it reaches the array size it will cause error anyway, so automatically it will not be registered...

MyPad said:
I was actually referring to when importing the library and only the library directly. Of course, I agree with the explanation being sufficient, but it is only reserved to the test map itself.
brief explanations already in the library itself...
 

MyPad

Spell Reviewer
Level 21
Joined
May 9, 2014
Messages
1,649
Nobody will create units that exceeds the array size or even 1000 coz of the lag it will produce and if it reaches the array size it will cause error anyway, so automatically it will not be registered...

Suppose this function exists, outside of the library:

JASS:
function IsCasterRegistered takes unit caster returns boolean
    return CS.register(caster) != 0
endfunction

If a unit is already registered, that function will return false, even when the expected value would be true.

Now, let's assume that the result is not stored in a global variable, and CS.register is somehow called twice on the same unit. The first time, it will return a valid instance. The second time, however, it will return null. (0)

In essence, I am suggesting a change in a certain line in the register function from:

JASS:
if chk.has(GetHandleId(caster)) then   
            debug if IsUnitType(caster, UNIT_TYPE_HERO) then
                debug call DisplayTextToPlayer(GetLocalPlayer(), 0, 0, "thistype.register ERROR: "+GetHeroProperName(caster)+" is already registered!")
            else
                debug call DisplayTextToPlayer(GetLocalPlayer(), 0, 0, "thistype.register ERROR: "+GetUnitName(caster)+" is already registered!")
            endif
            return 0

to..

JASS:
if chk.has(GetHandleId(caster)) then   
            debug if IsUnitType(caster, UNIT_TYPE_HERO) then
                debug call DisplayTextToPlayer(GetLocalPlayer(), 0, 0, "thistype.register ERROR: "+GetHeroProperName(caster)+" is already registered!")
            else
                debug call DisplayTextToPlayer(GetLocalPlayer(), 0, 0, "thistype.register ERROR: "+GetUnitName(caster)+" is already registered!")
            endif
           @  set this = chk[GetHandleId(this)]
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
Suppose this function exists, outside of the library:

JASS:
function IsCasterRegistered takes unit caster returns boolean
    return CS.register(caster) != 0
endfunction

If a unit is already registered, that function will return false, even when the expected value would be true.

Now, let's assume that the result is not stored in a global variable, and CS.register is somehow called twice on the same unit. The first time, it will return a valid instance. The second time, however, it will return null. (0)

I really dont see the point why someone will make that function outside this library but let me follow, coz the function you made is misleading, first there is no register in the struct CS so it will cause error...second you cant treat an integer as handleID...third instead of making it "!=" why not make it "=="?...

JASS:
function IsCasterRegistered takes unit caster returns boolean
    return ACS.register(caster) == 0 //CS.register(caster) != 0, there is no register in CS
endfunction

This will return TRUE If a unit is already registered as you mentioned...
 

MyPad

Spell Reviewer
Level 21
Joined
May 9, 2014
Messages
1,649
I apologize for the late response

JASS:
function IsCasterRegistered takes unit caster returns boolean
    return ACS.register(caster) == 0 //CS.register(caster) != 0, there is no register in CS
endfunction

This will return TRUE If a unit is already registered as you mentioned...

In CS, you're correct. My mistake there.
There is the possibility of that returning a false positive, though. Take null, for instance. Registering it will give it an instance, even when it should not. The second time around, passing even that instance will return true.

first there is no register in the struct CS so it will cause error...second you cant treat an integer as handleID...third instead of making it "!=" why not make it "=="?...

I will answer the second. Again, my mistake. I meant to state caster instead of this in the GetHandleId function. That way, it will still return the correct instance and not default to a short-circuit method of returning 0.
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
MyPad said:
- Registering it will give it an instance, even when it should not.
- The second time around, passing even that instance will return true.
- True, but I still dont get the point why somebody will register a null unit, when it launched it will be filtered out anyway in the UnitAlive function...
- Tested it, still returns ZERO or FALSE, not true...

MyPad said:
I meant to state caster instead of this in the GetHandleId function. That way, it will still return the correct instance and not default to a short-circuit method of returning 0.
The point of the zero return is to short-circuit or even crash the game if it needs be and if the return is the handleID, then it will become slower...


EDIT:
I'm sorry for my arrogance coz i still dont see any point of changing that :D...
 

MyPad

Spell Reviewer
Level 21
Joined
May 9, 2014
Messages
1,649
The point of the zero return is to short-circuit or even crash the game if it needs be and if the return is the handleID, then it will become slower...

Alright, but could the system release a safe value, at least in DEBUG_MODE?

As for the methods which add certain orders to the list of spells to cast for a unit, how would you fancy on doing a doubly-linked list (as opposed to the manual dynamic indexing method found in the library?)
 

MyPad

Spell Reviewer
Level 21
Joined
May 9, 2014
Messages
1,649
A common bug was spotted among these methods, in struct ACS

JASS:
method removeSpellOrderUnit takes integer spellOrderID returns nothing
method removeSpellOrderPoint takes integer spellOrderID returns nothing
method removeSpellOrderNone takes integer spellOrderID returns nothing

In those methods, the trigger being destroyed isn't the same trigger being referenced by the index I. This will lead to (n-1)/n * 100% of cases failing (when invoking these methods), with the remaining percentage only being coincidental with the max index.

In practice, with the given demo map and certain premises being held, the part when one wants to remove an automatic ability cast will fail (Carrion Swarm was reported to have been removed, but was still being cast until deregistered).

EDIT:

Until then, this shall be set to Awaiting Update

Status:

  • Awaiting Update
 
Last edited:
Top