[vJASS] Improving this snipper?

[jim7777]

already fixed
see this snippet for next fixing

I think this snippet is kinda slow or something like that or perhaps inefficient enough and may cause op limits i think?

well what i want to achieve is to let a unit/hero cast an item if it has that item

 

[watermelon_1234]

if HaveSavedInteger(ht,id,0) and @not cast@ then

The highlighted part is unnecessary since you quit the loop early if cast is true.

            if cast then
                set i = HeroAI_MAX_INVENTORY_SIZE + 1
            endif

->

            if cast then
                exitwhen true
            endif

For your UseItemX functions, the boolean cast and integer ord variables are unnecessary.
This check itemid != 0 should have been made before the loop, not inside the loop. Also, HaveSavedInteger(ht,id,0) is not really ncessary. (But if you want to keep it, it makes more sense if you had done HaveSavedInteger(ht,itemid,0) out of the loop and do it only for debug mode.)

 

[jim7777]

ok ok fixed everything

but how 'bout this snippet:

library WardsSystem uses HeroAI
    globals
        private constant integer ITEM_ID = 'I001'
        private constant real RADIUS = 800
        private hashtable ht = InitHashtable()
        private real array obsX[8194]
        private real array obsY[8194]
        private integer MAX_OBS = 0
    endglobals
    
    private function Filt takes nothing returns boolean
        return HaveSavedInteger(ht,GetUnitTypeId(GetFilterUnit()),0)
    endfunction
    
    function CheckWard takes unit hero, boolean move returns boolean
        local unit obs
        local real hx = GetUnitX(hero)
        local real hy = GetUnitY(hero)
        local boolean cast = false
        local integer i = 0
        local item it = GetItemOfTypeFromUnitBJ(hero,ITEM_ID)
        if null != it then
            loop
                exitwhen i >= MAX_OBS
                set obs = GetClosestUnitInRange(obsX[i],obsY[i],RADIUS,Filter(function Filt))
                if null == obs then
                    set cast = UnitUseItemPoint(hero,it,obsX[i],obsY[i])
                    set i = MAX_OBS + 1
                    set it = null
                    set obs = null
                    return true 
                endif
                set i = i + 1
            endloop
        endif
        if move then
            call IssuePointOrder(hero,"attack",hx + GetRandomReal(-HeroAI_MOVE_DIST, HeroAI_MOVE_DIST), hy + GetRandomReal(-HeroAI_MOVE_DIST, HeroAI_MOVE_DIST))
        endif
        set it = null
        set obs = null
        return false
    endfunction
    
    function RegisterWardPoints takes real x, real y returns nothing
        set obsX[MAX_OBS] = x
        set obsY[MAX_OBS] = y
        set MAX_OBS = MAX_OBS + 1
    endfunction
    
    private module M
        static method onInit takes nothing returns nothing
            call SaveInteger(ht,'o003',0,1)
            set obsX[MAX_OBS] = -601
            set obsY[MAX_OBS] = -1719
            set MAX_OBS = MAX_OBS + 1
        endmethod
    endmodule
    
    private struct S extends array
        implement M
    endstruct
endlibrary

 

[Magtheridon96]

Sizing the arrays like that ([8194]) does nothing but create horrible trash.
vJass will turn that array into two other arrays (one for the first 8192 slots and another for the 2)

It's always better to avoid sizing your arrays like that.

Also:

                    call UnitUseItemPoint(hero,it,obsX[i],obsY[i])
                    set i = MAX_OBS + 1
                    set it = null
                    set obs = null
                    return true

That's a minor improvement

And, you can remove your local unit obs because it would be more efficient to do this:

                if null == GetClosestUnitInRange(obsX[i],obsY[i],RADIUS,Filter(function Filt)) then
                    set cast = UnitUseItemPoint(hero,it,obsX[i],obsY[i])
                    set i = MAX_OBS + 1
                    set it = null
                    return true 
                endif

Also, I know I'm crazy, but:

        set it = null
        return move and IssuePointOrder(hero,"attack",hx + GetRandomReal(-HeroAI_MOVE_DIST, HeroAI_MOVE_DIST), hy + GetRandomReal(-HeroAI_MOVE_DIST, HeroAI_MOVE_DIST)) and false

 

[jim7777]

return move and IssuePointOrder(hero,"attack",hx + GetRandomReal(-HeroAI_MOVE_DIST, HeroAI_MOVE_DIST), hy + GetRandomReal(-HeroAI_MOVE_DIST, HeroAI_MOVE_DIST)) and false
will it check if move is allowed? kinda funny LOL

 

[Magtheridon96]

Yeah, it will do the same thing yours was doing
Jass terminates boolean expressions using "and" if the first part returns false.

 

[jim7777]

ohh cool i didn't know that

 

[Troll-Brain]

One more improvement could be to get rid of the filter, use a null filter and a FirstOfGroup/GroupRemoveUnit loop with a "static" group.

 

[jim7777]

^
I can't do that since I'm using GetClosestWidget