Rangefinder library functions -- feedback please

05-01-2008, 03:52 AM

This is a set of library that I made for the purpose of checking the range of different units.

I've posted the library itself, as well as a sample trigger that uses it.

The library is used by calling the public function:
immoRange_RangeView(player whichplayer, location sourcelocation, integer radius)

radius must satisfy 0 <= radius <= immoRange_MAXRANGE

This is almost the first thing I've made, and I'm not sure if I did everything correctly/efficiently, so I'm hoping some of you can take a look at it and tell me what you think, and where it leaks.

Thanks,
ImmolatusBurn

P.S.
Sorry if I posted this incorrectly, or not in the right place, I'm rather new here.

Rangefinder Library

JASS:

library immoRange initializer maketimers
    globals
        //public config data
        public constant integer MAXRANGE = 2000 //maximum disa
        
        //return codes
        public constant integer SUCCESS = 1
        public constant integer ERROR = 0
        
        //config settings
        private constant string RANGEFINDEREFFECT = "Doodads\\LordaeronSummer\\Props\\TorchHuman\\TorchHuman.mdl" //model file for the rangefinder
        private constant integer DURATION = 3 //number of seconds that the rangefinders will last for
        private constant integer DEGREEINCREMENT = 20 //how many degrees separate each rangefinder
        
        //holds the rangefinders currently active for each player
        private effect array Player0Rangefinders
        private effect array Player1Rangefinders
        private effect array Player2Rangefinders
        private effect array Player3Rangefinders
        private effect array Player4Rangefinders
        private effect array Player5Rangefinders
        private effect array Player6Rangefinders
        private effect array Player7Rangefinders
        private effect array Player8Rangefinders
        private effect array Player9Rangefinders
        private effect array Player10Rangefinders
        private effect array Player11Rangefinders
        
        //holds the timers to handle the removal of rangefinders
        private timer array RemovalTimers
    endglobals
    
    private function maketimers takes nothing returns nothing
        local integer index = 0
        
        loop
            exitwhen(index > 11)
            set RemovalTimers[index] = CreateTimer()
            set index = index + 1
        endloop
    endfunction
    
    private function GetPlayerRangefinder takes player p, integer index returns effect
        //! textmacro ReturnPlayerRangefinder takes ID
        elseif(p == Player($ID$)) then
            return Player$ID$Rangefinders[index]
        //! endtextmacro
        
        if(p == Player(0)) then
            return Player0Rangefinders[index]
        //! runtextmacro ReturnPlayerRangefinder("1")
        //! runtextmacro ReturnPlayerRangefinder("2")
        //! runtextmacro ReturnPlayerRangefinder("3")
        //! runtextmacro ReturnPlayerRangefinder("4")
        //! runtextmacro ReturnPlayerRangefinder("5")
        //! runtextmacro ReturnPlayerRangefinder("6")
        //! runtextmacro ReturnPlayerRangefinder("7")
        //! runtextmacro ReturnPlayerRangefinder("8")
        //! runtextmacro ReturnPlayerRangefinder("9")
        //! runtextmacro ReturnPlayerRangefinder("10")
        //! runtextmacro ReturnPlayerRangefinder("11")
        endif
        
        return null
    endfunction
    
    private function SetPlayerRangefinder takes player p, integer index, effect e returns nothing
        //! textmacro SetPlayerRangefinder takes ID
        elseif(p == Player($ID$)) then
            set Player$ID$Rangefinders[index] = e
        //! endtextmacro
        
        if(p == Player(0)) then
            set Player0Rangefinders[index] = e
        //! runtextmacro SetPlayerRangefinder("1")
        //! runtextmacro SetPlayerRangefinder("2")
        //! runtextmacro SetPlayerRangefinder("3")
        //! runtextmacro SetPlayerRangefinder("4")
        //! runtextmacro SetPlayerRangefinder("5")
        //! runtextmacro SetPlayerRangefinder("6")
        //! runtextmacro SetPlayerRangefinder("7")
        //! runtextmacro SetPlayerRangefinder("8")
        //! runtextmacro SetPlayerRangefinder("9")
        //! runtextmacro SetPlayerRangefinder("10")
        //! runtextmacro SetPlayerRangefinder("11")
        endif
    endfunction
    
    private function ClearRangefinders takes player p returns nothing
        local integer index = 0
        local effect currenteffect = GetPlayerRangefinder(p, index)
        
        loop
            exitwhen(currenteffect == null)
            call DestroyEffect(currenteffect)
            call SetPlayerRangefinder(p, index, null)
            set index = index + 1
            set currenteffect = GetPlayerRangefinder(p, index)
        endloop
    endfunction
    
    private function HandleRemovalTimers takes nothing returns nothing
        local integer index = 0
        
        loop
            exitwhen(RemovalTimers[index] == GetExpiredTimer())
            set index = index + 1
        endloop
        
        call ClearRangefinders(Player(index))
    endfunction
    
    private function CreateLocalEffect takes player p, location l, integer degree, integer range returns effect
        local string s = ""
        
        if(GetLocalPlayer() == p) then
            set s = RANGEFINDEREFFECT
        endif
        
        return AddSpecialEffectLoc(s, Location(GetLocationX(l) + range * Cos(degree * bj_DEGTORAD), GetLocationY(l) + range * Sin(degree * bj_DEGTORAD)))
    endfunction

    public function RangeView takes player p, location l, integer range returns integer
        local integer degree = 0
        
        if((range < 0) or (range > MAXRANGE)) then
            return ERROR
        endif

        call ClearRangefinders(p)
        
        loop
            exitwhen(degree >= 360)
            call SetPlayerRangefinder(p, degree / DEGREEINCREMENT, CreateLocalEffect(p, l, degree, range))
            set degree = degree + DEGREEINCREMENT
        endloop
        
        call TimerStart(RemovalTimers[GetPlayerId(p)], DURATION, false, function HandleRemovalTimers)
        
        return SUCCESS
    endfunction
endlibrary

Sample Trigger

JASS:

scope ShowRange

    private function Actions takes nothing returns nothing
        local integer range
        local unit rangeunit
        local group g = CreateGroup()
        
        set range = S2I(SubString(GetEventPlayerChatString(), 7, StringLength(GetEventPlayerChatString())))
        
        call GroupEnumUnitsSelected(g, GetTriggerPlayer(), null)
        
        set rangeunit = FirstOfGroup(g)
        
        if(not (SubString(GetEventPlayerChatString(), 0, 6) == "-range")) then
            return
        endif
        
        if(range == 0) then
            call DisplayTextToPlayer(GetTriggerPlayer(), 0, 0, "|cffffcc00-range <number>|r|cffffbbbb Displays a circle with radius <number> around your selected unit")
            return
        endif
        
        if(rangeunit == null) then
            call DisplayTextToPlayer(GetTriggerPlayer(), 0, 0, "You must select a unit")
            return
        endif
        
        if((range > immoRange_MAXRANGE) or (range < 0)) then
            call DisplayTextToPlayer(GetTriggerPlayer(), 0, 0, "Range must be between 0 and " + I2S(immoRange_MAXRANGE))
            return
        endif
        
        call immoRange_RangeView(GetTriggerPlayer(), GetUnitLoc(rangeunit), range)
        
        set rangeunit = null
        call DestroyGroup(g)
        set g = null
    endfunction
    
    //===========================================================================
    public function InitTrig takes nothing returns nothing
        local trigger t = CreateTrigger(  )
        local triggeraction ta = TriggerAddAction( t, function Actions )
        
        call TriggerRegisterPlayerChatEvent(t, Player(0), "-range", false)
        call TriggerRegisterPlayerChatEvent(t, Player(1), "-range", false)
        call TriggerRegisterPlayerChatEvent(t, Player(2), "-range", false)
        call TriggerRegisterPlayerChatEvent(t, Player(3), "-range", false)
        call TriggerRegisterPlayerChatEvent(t, Player(4), "-range", false)
        call TriggerRegisterPlayerChatEvent(t, Player(5), "-range", false)
        call TriggerRegisterPlayerChatEvent(t, Player(6), "-range", false)
        call TriggerRegisterPlayerChatEvent(t, Player(7), "-range", false)
        call TriggerRegisterPlayerChatEvent(t, Player(8), "-range", false)
        call TriggerRegisterPlayerChatEvent(t, Player(9), "-range", false)
        call TriggerRegisterPlayerChatEvent(t, Player(10), "-range", false)
        call TriggerRegisterPlayerChatEvent(t, Player(11), "-range", false)
        
        set ta = null
        set t = null
    endfunction

endscope

Attached Images

upload.jpg (53.4 KB)

05-01-2008, 06:35 AM	#2
Pyrogasm	Can you try to better explain what your system does? I really don't get it from the little description you posted. Also, you shouldn't prefix your functions/variables with "immo", as it looks bad.

05-01-2008, 08:16 AM

ImmolatusBurn

I prefix them as part of a way to avoid collisions with other systems/triggers, i figure few enough people will use the same name abbreviation and library name as that.

What the trigger does is, when someone types in -range 600, for instance, it takes the unit currently selected by that player and places a circle of markers around it at the specified distance, in this case, 600. The markers dissipate after 3 seconds, or when that player uses the trigger again. I made it to be used to be able to see the attack range and such of towers.

I attached an image to the first post.

05-01-2008, 07:13 PM

Strilanc

You don't need to store the effects. Just pick an effect that doesn't die instantly and destroy them as soon as you create them. For example the glowing runes have a decent death animation for this type of thing. Getting rid of the storage will seriously simplify the library.

You should give more markers for larger ranges, and less for smaller ranges. In other words the distance between each marker should remain approximately the same as the radius grows and shrinks.

Get rid of the maximum range. If a mapper asks for a circle with 3000 range the library shouldn't say no, it should try its best to do give a circle with 3000 range. You should only fail when the input is actually wrong or going to cause problems (like less than 0).

Finally, you're leaking locations in CreateLocalEffect. That's called a lot, so you might want to fix that.

05-01-2008, 08:10 PM

Vexorian

Quote:

I prefix them as part of a way to avoid collisions with other systems/triggers,

You are using a scope, for god's sake.

Quote:

i figure few enough people will use the same name abbreviation and library name as that.

They are lame.

05-01-2008, 09:28 PM

Toadcop

Quote:

using a scope

sometimes is very limiting.
// btw why the same scope name can't be used twice O_o ? it does fully ruined my interes for it.

05-01-2008, 09:58 PM

Vexorian

Quote:

sometimes is very limiting.

That's the whole point of scopes.

Quote:

// btw why the same scope name can't be used twice O_o ? it does fully ruined my interes for it.

I am not sure if that's an actual problem with scopes or with your understanding of what scopes are.

05-02-2008, 09:22 AM

ImmolatusBurn

Ok, I think I've worked in what was said, and I removed a lot of the functions since I didn't need to store the effects anymore, here's the new library:

JASS:

library Range
    globals
        //return codes
        public constant integer SUCCESS = 1
        public constant integer ERROR = 0
        
        //config settings
        private constant string RANGEFINDEREFFECT = "Doodads\\Cinematic\\GlowingRunes\\GlowingRunes0.mdl" //model file for the rangefinder
        private constant integer DISTANCEINCREMENT = 100 //how much disance separates each rangefinder
        
        //other constants
        private constant real PI_2 = 6.28 //pi * 2
    endglobals
    
    private function CreateLocalEffect takes player p, location l, integer degree, integer range returns nothing
        local string s = ""
        local effect e
        
        if(GetLocalPlayer() == p) then
            set s = RANGEFINDEREFFECT
        endif
        
        set e = AddSpecialEffect(s, GetLocationX(l) + range * Cos(degree * bj_DEGTORAD), GetLocationY(l) + range * Sin(degree * bj_DEGTORAD))
        
        call DestroyEffect(e)
        set e = null
    endfunction

    public function RangeView takes player p, location l, integer range returns integer
        local integer degree = 0
        local integer degreeincrement = R2I((360 * DISTANCEINCREMENT) / (PI_2 * range))
        
        if(range < 0) then
            return ERROR
        endif
        
        loop
            exitwhen(degree >= 360)
            call CreateLocalEffect(p, l, degree, range)
            set degree = degree + degreeincrement
        endloop
        
        return SUCCESS
    endfunction
endlibrary

Quick question: Is it possible to set the animation speed of a normal doodad using triggers?

05-02-2008, 02:19 PM	#9
Toadcop	no i need the possibility to have access to the scope in different triggers for example... so i need to intialize a scope in 2 triggers. (to have shared functions, vars etc.) to write everything in 1 trigger is not the best solution sometimes =)

05-02-2008, 04:50 PM

#10

Strilanc

It's looking much better now, just a few more things.

You're currently working in degrees, which forces you to multiply by constants a quite a bit. Change the local effect function so the angle it accepts is in radians (just change degree * bj_DEGTORAD to degree and rename degree to angle). Not only does this get rid of that multiplication, it changes degreeincrement from R2I((360 * DISTANCEINCREMENT) / (PI_2 * range)) to a much simpler DISTANCEINCREMENT / range [it doesn't need to be an integer, btw]. You might also want to put a maximum on the increment, so very small ranges get more than 1 or 2 effects.

The CreateLocalEffect function is only used in one spot, and is quite short, so consider just inlining it inside RangeView. This will also allow you to cache the values of GetLocationX, GetLocationY, and the local player's model path.

This is more of a taste thing, but

JASS:

        set e = AddSpecialEffect(...)
        call DestroyEffect(e)
        set e = null

can be changed to the equivalent

JASS:

call DestroyEffect(AddSpecialEffect(...))

05-03-2008, 07:55 PM

#11

ImmolatusBurn

Here's the latest update, I changed the things you mentioned:

JASS:

library Range
    globals
        //return codes
        public constant integer SUCCESS = 1
        public constant integer ERROR = 0
        
        //config settings
        private constant string RANGEFINDEREFFECT = "Doodads\\Cinematic\\GlowingRunes\\GlowingRunes0.mdl"//"Doodads\\LordaeronSummer\\Props\\TorchHuman\\TorchHuman.mdl" //model file for the rangefinder
        private constant integer DISTANCEINCREMENT = 100 //how much disance separates each rangefinder
        
        //other settings
        private constant real MAXINCREMENT = .5 //maximum angleincrement value
        private constant real DEFINCREMENT = .3 //angleincrement default value if maximum is exceeded
        private constant real PI_x_2 = 6.28 //pi * 2
    endglobals
    
    public function RangeView takes player p, location l, integer range returns integer
        local real angle = 0
        local real angleincrement = (I2R(DISTANCEINCREMENT) / range)
        local string s = ""
        
        if(range < 0) then
            return ERROR
        endif
        
        if(GetLocalPlayer() == p) then
            set s = RANGEFINDEREFFECT
        endif

        if(angleincrement > MAXINCREMENT) then
            set angleincrement = DEFINCREMENT
        endif
        
        loop
            exitwhen(angle >= PI_x_2)
            call DestroyEffect(AddSpecialEffect(s, GetLocationX(l) + range * Cos(angle), GetLocationY(l) + range * Sin(angle)))
            set angle = angle + angleincrement
        endloop
        
        return SUCCESS
    endfunction
endlibrary

Think I should try to optimize it some more? Or is it fast enough?

05-03-2008, 11:23 PM

#12

Pyrogasm

There is no need for the I2R() call because integers can be safely typecast as reals. However, I would instead make them all reals instead of integers anyway. Again regarding reals, it would make sense to have the function take X/Y coordinates instead of a location argument because it only uses the X/Y anyway; also it will save you from calling GetLocationX/Y(l).

On another note, I would simply return a boolean value instead of integer globals; regarding that, where you return false, your range comparison is faulty; it should be if(range <=0) then. Finally, I would remove the use of the PI_x_2 variable and just inline it:

JASS:

library Range
    globals
        //config settings
        private constant string RANGEFINDEREFFECT = "Doodads\\Cinematic\\GlowingRunes\\GlowingRunes0.mdl"//"Doodads\\LordaeronSummer\\Props\\TorchHuman\\TorchHuman.mdl" //model file for the rangefinder
        private constant real DISTANCEINCREMENT = 100.00 //how much disance separates each rangefinder
        
        //other settings
        private constant real MAXINCREMENT = .5 //maximum angleincrement value
        private constant real DEFINCREMENT = .3 //angleincrement default value if maximum is exceeded
    endglobals
    
    public function RangeView takes player p, real x, real y, real range returns boolean
        local real angle = 0
        local real angleincrement = (DISTANCEINCREMENT / range)
        local string s = ""
        
        if(range <= 0) then
            return false
        endif
        
        if(GetLocalPlayer() == p) then
            set s = RANGEFINDEREFFECT
        endif

        if(angleincrement > MAXINCREMENT) then
            set angleincrement = DEFINCREMENT
        endif
        
        loop
            exitwhen(angle >= 6.28318)
            call DestroyEffect(AddSpecialEffect(s, x + range * Cos(angle), y + range * Sin(angle)))
            set angle = angle + angleincrement
        endloop
        
        return true
    endfunction
endlibrary

After that, the only thing that doesn't really make sense to me is the DISTANCEINCREMENT global. It really has nothing to do with specifically how far apart the effects are (sure, it affects them, but not predictably if you don't know how the library works); I want to say it should be renamed DENSITY or something, but I don't know.

Oh, and another thing: you might want to have a private constant real Pi so that people can use that in the Min/Max increment stuff (when dealing with radians, multiples of Pi are most often used:

JASS:

globals
    //config settings
    private constant string RANGEFINDEREFFECT = "Doodads\\Cinematic\\GlowingRunes\\GlowingRunes0.mdl"//"Doodads\\LordaeronSummer\\Props\\TorchHuman\\TorchHuman.mdl" //model file for the rangefinder
    private constant real DISTANCEINCREMENT = 100.00 //how much disance separates each rangefinder
        
    //other settings
    private constant real Pi = 3.14159 //Don't modify Pi; it's just a bad idea
    private constant real MAXINCREMENT = Pi/12 //How the Pi variable might be applied
    private constant real DEFINCREMENT = Pi/10 //...
endglobals

05-03-2008, 11:40 PM

#13

ImmolatusBurn

When DISTANCEINCREMENT and range are both integers, I do need the I2R in the division here
I2R(DISTANCEINCREMENT) / range because otherwise it returns the division of two integers, 0.

That being said, It probably would make sense to change them to reals in the future.

As for a private constant Pi, wouldn't it work for them to just use bj_PI instead?

I'll look into updating it with the stuff you said.

<EDIT>And the reason I used error codes was that I was initially thinking there might be multiple causes of errors, and I wanted the client trigger to be able to check which occurred, but boolean does make more sense here.</EDIT>

<EDIT2>I think the DISTANCEINCREMENT value is the actual distance separating each marker as you move along the circumference of the circle.</EDIT2>

05-04-2008, 12:21 AM

#14

Pyrogasm

They could use bj_PI instead, but it wouldn't get inlined.

Quote:

Originally Posted by ImmolatusBurn

<EDIT2>I think the DISTANCEINCREMENT value is the actual distance separating each marker as you move along the circumference of the circle.</EDIT2>

It's not. You're basically modifying your angle Theta through that. So let's say you pick the range 300 when you call that function. Then AngleIncrement gets set to 300/100 = 3 (assuming that there is no max value). Then, using trigonometric identities and an isosceles triangle, the length between one effect and the next is computed as 2*300*Sin(Theta/2) = 600*Sin(1.5) = 598.496!

Doing it with the angle limit, Theta is now 0.3, yet it still doesn't work: D = 600*Sin(0.15) = 89.6628.

Now, you can work backwards to determine the angle increment in order to place the effects the proper distance apart:

JASS:

D = 2*Range*Sin(AngleIncrement/2)
Sin(AngleIncrement/2) = D/(2*Range)
AngleIncrement = ArcSin(D/(2*Range))*2

And that's what you should be using.

05-04-2008, 01:23 AM

#15

ImmolatusBurn

If you call the function with range 300, angleincrement gets set to .33, not 3 (DISTANCEINCREMENT / range)

I used the equation d = (2 * pi * range * theta) / 360 to find the distance between each point, and then solved for theta to get the equation that I plugged into the code.

And when I said DISTANCEINCREMENT is the distance between each marker, I meant it's the distance along the edge of the circle.