Unit Custom Range Rings Widget

[Forboding Angel]

A labor of love <3

I needed the ability to display custom range rings for various things. For example, the purple ring here denotes the range for energy core collection of the Overseer in Evolution RTS, while the orange ring is showing the buildrange. The orange ring disappears when the unit is not selected.



https://code.google.com/p/evolutionrts/ ... mrings.lua

--[[

You can have as many rings as you like.

color = {0.5,0,1,0.3}, --R,G,B,A on a scale from 0 - 1. A is the opacity with 1 being fully opaque to 0 being fully transparent. Easy and quick color picker here: http://www.dematte.at/colorPicker/ Take 255 divided by the color value you want, and that is it's value on a scale from 0 - 1.

radius = 500, --How large of a radius the ring will cover.

linewidth = 1, --1 is basically 1 pixel thick. It will scale as you zoom in and out. Maximum value seems to be 32.

]]--

Code: Select all

local ringsDefs = {
    [UnitDefNames.ecommander.id] = {
        { color = {1, 0.5, 0, 0.8}, lineWidth = 2, radius = 1000 },
        { color = {0.5, 0, 1, 0.2}, lineWidth = 5, radius = 500 },
    },
}

[knorke]

seen that so often now:

function widget:DrawWorld()
local units = Spring.GetAllUnits()
for _,unitID in ipairs(units) do

Basically you are looping through all units all the time, even if only a few have a ring. Or none.
Make a table of the units you care about and only loop over that.
Doing the explode (blabla) tonumber(blabla) again and again seems needless too, save it.

Creative use of conditions.

I found DrawGroundCircle is very slow. (yesyes i sometimes use it too like for cover-in-bushes thing, but only for testing not for real drawing)
This is probally a better way:
http://code.google.com/p/zero-k/source/ ... olumes.lua
(actually did not test myself yet but defense_rings widget by vbs does it similiar)

[Niobium]

This is... bad...

Why is everything in the config a string? Is it to support old fbi format? Everything being a string is needlessly complicating the code*, especially the 'explode' function, which looks awful.

The widget itself is also pretty bad.
- GetAllUnits on every drawframe (I'd probably use UnitCreated/UnitDestroyed to keep a table of just the interesting units)
- Getting unitdef id, position, customparams etc on ALL units, before checking if you are even interested in them (which doesn't use any of that information)
- "while (true) do"...
- No reset of color or line width (line width being the important one)
- No localizing of functions (e.g. glColor = gl.Color, minor issue)
- No caching/precalculation (i.e. precalculate which unit def ids have customparams and which don't, leads to optimisations like shutting down the widget when nothing has params)
- Missing easy optimizations, like units with rings when selected (a common case) is done through 'isunitselected' in the all-units loop, rather than a separate 'for each selected unit' loop
- No visibility checks before draw calls

etc.

* Here is a code sample if tables were used:

Code: Select all

customParams: ring1color = {1, 0.5, 1, 1}
widget: glColor(customParams.ring1color)

[SanadaUjiosan]

I can't speak for the "strength" of the code, but thanks for posting this Forb, I can definitely use this :)

[knorke]

I think Niobium's post is longer than the widget

Why is everything in the config a string? Is it to support old fbi format?

customparams for unitdefs can only be strings.
Of course you could convert it to some table/numbers once at start.

[Forboding Angel]

An optional table would be a good thing. This is meant for people who know nothing and as far as I can tell there is no real performance hit.

Nio, as I stated several times, customparams can only be strings, which is kind of limiting.

Also, it's worth noting that it only loops through your own units and only does anything when ring1radius is anything other than nil in the unitdef. SO it may do a lot of looping, but it isn't really doing anything.

[knorke]

Also, it's worth noting that it only loops through your own units and only does anything when ring1radius is anything other than nil in the unitdef.

things it does even if "ring1radius is anything other than nil in the unitdef":

- Getting unitdef id, position, customparams etc on ALL units, before checking if you are even interested in them (which doesn't use any of that information)

SO it may do a lot of looping, but it isn't really doing anything.

even "empty" loops cause cpu usage.

This is meant for people who know nothing

so it should be extra correct.

and as far as I can tell there is no real performance hit.

imagine your computer is less fast and all wupgets were like that.

[Forboding Angel]

I found DrawGroundCircle is very slow. (yesyes i sometimes use it too like for cover-in-bushes thing, but only for testing not for real drawing)
This is probally a better way:
http://code.google.com/p/zero-k/source/ ... olumes.lua
(actually did not test myself yet but defense_rings widget by vbs does it similiar)

Got gl.Utilities.DrawGroundCircle working, but the thickness is all screwy. It seems like this draws filled circles, which is cool, but not what I need

Code: Select all

gl.Utilities.DrawGroundCircle(x,z, tonumber(customParams["ring"..ring_number.."radius"]),tonumber(customParams["ring"..ring_number.."sides"]))

Edit: It also draws on top of EVERYTHING.

[Forboding Angel]

Anyway, bedtime. If anyone wants to improve upon it, that would be swell.

A table of units to look over would be fine, but I dunno how to do that.

*Suddenly, a wild Flashtank appears!*

[Forboding Angel]

A table of units to look over would be fine, but I dunno how to do that.

*ahem!* *hint hint*

[knorke]

Basically have some table like this:

Code: Select all

local units = {}

to keep the unitIDs of all alive units that you care about.
Add new units,remove them as they get destroyed.
Beside just storing the unitID, often you will want to store some more stuff. (here: the radius+color of rings)

Code: Select all

UnitCreated (unitID)
if (youCareAboutThisUnit) then
units[unitID] = someDataToStoreForThisUnit

To remove from the table, just set the entry to nil:

Code: Select all

UnitDestroyed(unitID)
units[unitID] = nil
end

somewhere else, eg DrawScreen() or GameFrame()

Code: Select all

for i,unitID in pairs(units) do
--do something
end

instead of

Code: Select all

local units = Spring.GetAllUnits()
for _,unitID in ipairs(units) do

[Forboding Angel]

(There was pm convo in the lobby between knorke and myself that I won't post because people get buttmad over silly stuff, so you the casual reader don't get any background on my post)

I need specifics, not shorthand. You suggest that the variables get stored elsewhere, which is an entirely separate issue. How am I supposed to learn anything from it if you branch into 3 different areas when I ask how to do one?

I want to make an array of unitnames. Tell widget to only pay attention to those unitnames and only if they are on my team.

If these conditions are met, draw stuff.

I appreciate the help, but it's too general for it to be of significant use to me :-/

[Google_Frog]

Don't use pairs either. Keep track of two tables, one indexed by unitID and the other iterable. These tables need references to each other to handle delete operations.

[KingRaptor]

Or you could just use pairs and leave the crazy two-table stuff to Google Frog. It's really not that much slower.

[Niobium]

Don't use pairs either. Keep track of two tables, one indexed by unitID and the other iterable. These tables need references to each other to handle delete operations.

Ugh... Do not follow this advice. This pattern leads to large, ugly, unreadable, hard to understand, hard to maintain, bug prone code. In the absolute best case the performance gains are negligible, anything worse and the approach is likely slower.

[Forboding Angel]

Well I've given up on my own ability to fix this. If any of you would be nice enough to get it working It would be greatly appreciated. I desperately need this, but with 500 units moving around it dumps my fps by 50% Meaning that looping through all the units is taking a serious performance hit.

It's a damn shame, but like I said I am desperate for this. If someone would fix it, fame and fortune await.

<< Deflated and depressed.

[Niobium]

What it would look like in the other structure, missing any optimization of course:

Code: Select all

local ringsDefs = {
    [UnitDefNames.armcom.id] = {
        { color = {1, 1, 1, 0.25}, radius = 350 },
        { color = {1, 1, 0, 0.5}, lineWidth = 2, radius = 250 },
    },
}

local ringedUnits = {}

function widget:Initialize()
    for _, uId in pairs(Spring.GetAllUnits()) do
        widget:UnitEnteredLos(uId)
    end
end

function widget:UnitEnteredLos(uId)
    local uDefId = Spring.GetUnitDefID(uId)
    if uDefId then
        widget:UnitCreated(uId, uDefId)
    end
end

function widget:UnitCreated(uId, uDefId)
    local rings = ringsDefs[uDefId]
    if rings then
        ringedUnits[uId] = rings
    end
end

function widget:DrawWorld()
    
    for uId, rings in pairs(ringedUnits) do
        local ux, uy, uz = Spring.GetUnitPosition(uId)
        if ux then
            for _, ring in pairs(rings) do
                gl.Color(ring.color)
                gl.LineWidth(ring.lineWidth or 1)
                gl.DrawGroundCircle(ux, uy, uz, ring.radius, ring.divs or 32)
            end
        else
            ringedUnits[uId] = nil
        end
    end
    
    gl.LineWidth(1)
    gl.Color(1, 1, 1, 1)
end

[smoth]

Well I've given up on my own ability to fix this.

seriously?

[Forboding Angel]

Awesomesauce

Holy crap, very nice! Thankyouthankyouthankyouthankyou!!!!

Well I've given up on my own ability to fix this.

seriously?

Smoth, reading nio's code is eye opening. I never even considered doing it like this. I was completely fixated on doing it via customparams.

[Forboding Angel]

Ok, I've updated it to nio's code. I did add a check that the unit is on your team and selected in order for rings to be shown. I figure people can change that to whatever they need.

To everyone who helped with this (which is pretty much all of you), thanks. I was extremely desperate for this. It may seem like a small thing but it was very necessary. My previous method of showing range based things was extremely inefficient.

Thank you!

Updated first post with goodness.

Sorry to everyone who suffered through this with me. If I hadn't been so fixated on making it adjustable via customparams, it would have been a lot less nuts.