New widget: Enemy Spotter

[Gota]

Maybe make it optional in the widget?If indeed a lot think it should be team color.
As it is it's already nice.
Hope you make some other nice widgets.

[SirMaverick]

If indeed a lot think it should be team color.

If needed a lot someone else can make a fork to provide that.

[AF]

srsly... how many times you look at which color the incoming enemy is? you will just react to them... no matter what color they are, you will do the same thing to everyone when you see enemies coming... if you are smart and you dont underestimate anyone in the game...

this widget is to spot enemies that comes at you, not enemies you attack at... so i dont see any fucking point drawing them with different color since it wont matter a shit.

I always look at the colour of an incoming swarm of units rather than just reacting to it, because that swarm might actually be allied units!

I would also want to know who sent them so I know whose base to send my swarm of units at.

[TradeMark]

because that swarm might actually be allied units!

no worries! my widget notifies this for you! hurray!

I would also want to know who sent them so I know whose base to send my swarm of units at.

lol yeah me too, i was thinking yesterday to add some nick texts who attacked you last time, maybe some list that counts which player made most damage to you, and so you can revenge them D:

could be like:

Code: Select all

Nick            Damage        Time
----------------------------------------------------------------------
Noob one        10            2 hours ago
Pro one         7464          6 mins ago
Uber pro one    124633        12 secs ago   ***REVENGE NOW OR DIE!*** (flashing text)
Noob two        244           1 hour ago
----------------------------------------------------------------------

[Argh]

<yawn>

This involved a one-liner change to my team-platters Widget, in the P.U.R.E. sourcecode distribution. Uses a single quad, can be differentiated between types (you're seeing "static" and "tank" there, "tank" has the little "arrow" to show facing).


This will be included in the next version of P.U.R.E.- I hadn't thought of just eliminating team-platters for friendlies, thanks for the idea.

[TradeMark]

you are using a texture, arent you? will lag more

[Argh]

No, actually, now that I've read your code, mine's probably significantly faster (I know it's less than 1% of CPU loading, but you should test yourself, ofc). Most of the drawing load's on the GPU, and the textures get sent once per operation, no matter how many Units are on the screen.

One way you could make this more efficient, btw, is to figure out an optimization for GetUnitDefRealRadius(). If you feel that you need all those steps, per unitDef(I assume that's to make up for issues with 3DOs), then do all that during Initialize for every Unit , and store the resulting radius (and / or manual settings, offset Z, etc.) in a table.

You want as little real math inside the draw stage as possible, basically. Table lookups, where unitDefID == table location are cheap, you're just giving it an index and grabbing the value at that key. Actual math operations should be avoided.

Furthermore, while it does require an additional table lookup, mine draws only for Units in my WG.Visible table, which is a significant speedup. WG.Visible is a global visible-Units tester, and keeps a list, precisely for stuff like this to operate. I forget whether that's in P.U.R.E. 1.2's source, but it was a fairly major speedup to do "what can this Team see" operations in only one place throughout the entire gamecode, and I released the source to do that awhile ago.

Not that my code's perfect- there are probably quite a few places I can use a faster operation, just looking at the way I built parts of the loop. I don't have time to go back through it and optimize further atm, though.

Anyhow, I don't want to say this stuff to discourage you, or to imply you haven't done anything useful. Getting something to work is part of the struggle, and then you have to optimize it. This is just what everybody must deal with- I never write anything that's perfectly fast the first time, and I really doubt anybody else does either.

[Niobium]

you are using a texture, arent you? will lag more

Coming from the guy who is showing off his widget which is laggier than all popular widgets. You haven't taken any of the plain good advice in this thread, while people who don't know better download your widget and have a forever laggier spring experience.

[Neddie]

mostly you can see skill level by the appearance of the units how they are placed.

you have piqued my interest, what does one look for?

Right now you're looking at bullshit.

[Pendrokar]

TradeMark writes Lua?

He must believe that the world will end in 2012 too and wants that his ideas comes to reality before that!!!

[TradeMark]

No, actually, now that I've read your code, mine's probably significantly faster (I know it's less than 1% of CPU loading, but you should test yourself, ofc). Most of the drawing load's on the GPU, and the textures get sent once per operation, no matter how many Units are on the screen.

its not about sending the texture you idiot... its about drawing the textures.

if i draw millions of polygons with texture, its about twice as fast as with just 1 plain color (with no texture).

One way you could make this more efficient, btw, is to figure out an optimization for GetUnitDefRealRadius(). If you feel that you need all those steps, per unitDef(I assume that's to make up for issues with 3DOs), then do all that during Initialize for every Unit , and store the resulting radius (and / or manual settings, offset Z, etc.) in a table.

You want as little real math inside the draw stage as possible, basically. Table lookups, where unitDefID == table location are cheap, you're just giving it an index and grabbing the value at that key. Actual math operations should be avoided.

Code: Select all

-- Drawing:
function widget:DrawWorldPreUnit()
   glDepthTest(true)
   glPolygonOffset(-100, -2)
   for _,unitID in ipairs(Spring.GetVisibleUnits()) do
      local teamID = spGetUnitTeam(unitID)
      if (teamID) then
         if ( not Spring.AreTeamsAllied(myTeamID, teamID) ) then
            local radius = GetUnitDefRealRadius(spGetUnitDefID(unitID))
            if (radius) then
               glDrawListAtUnit(unitID, circlePolys, false, radius, 1.0, radius)
            end
         end
      end
   end
end

now where do you see math? GetUnitDefRealRadius() uses array to take the radius. and thats the only custom function used. learn2read code... it doesnt go through all those steps in the function -_-

Code: Select all

local function GetUnitDefRealRadius(udid)
   local radius = realRadii[udid]
   if (radius) then return radius end

--

Coming from the guy who is showing off his widget which is laggier than all popular widgets. You haven't taken any of the plain good advice in this thread, while people who don't know better download your widget and have a forever laggier spring experience.

why are you whining at me? this widget is an optimized version of TeamPlatter, i dont fucking understand why you say this is the laggiest version, and go look at the code (first post), i took advices from this thread.

what a bullshit youre saying... even the stupid metal patch finder freezes spring on metal maps and you call this is the laggiest wow...

gotta be just jealous i made something good...

[Argh]

if i draw millions of polygons with texture, its about twice as fast as with just 1 plain color (with no texture).

But I'm drawing one quad, lol, not many triangles.

You obviously don't understand jack about OpenGL yet. Per fragment, the blend operation's the same thing- the texture's just providing the alpha component for the test. IOW, once the texture's loaded, it really isn't a lot slower. A little, yes, but the smaller amount of geometry makes up for it.


Mainly though, you're making all sorts of basic errors, in terms of CPU-side optimization. It doesn't matter if your OpenGL's optimized... you're wasting a lot of CPU cycles, and that's exactly what you need to avoid, with a Widget that's basically a visual aid. I mean... you're pointing at your loop, and saying, "where's the problem"...

Code: Select all

 if ( not Spring.AreTeamsAllied(myTeamID, teamID) ) then
            local radius = GetUnitDefRealRadius(spGetUnitDefID(unitID))
            if (radius) then
               glDrawListAtUnit(unitID, circlePolys, false, radius, 1.0, radius)
            end
         end

The problem is that GetUnitDefRealRadius():

Code: Select all

local function GetUnitDefRealRadius(udid)
   local radius = realRadii[udid]
   if (radius) then return radius end
   local ud = UnitDefs[udid]
   if (ud == nil) then return nil end
   local dims = spGetUnitDefDimensions(udid)
   if (dims == nil) then return nil end
   local scale = ud.hitSphereScale -- missing in 0.76b1+
   scale = ((scale == nil) or (scale == 0.0)) and 1.0 or scale
   radius = dims.radius / scale
   realRadii[udid] = radius*innersize
   return radius
end

...is very poorly optimized.

You're making all sorts of noob errors there. For one thing, do not create locals within that loop. Creating locals is expensive, and you're doing it for every non-Ally that's visible.

Moreover... this check's not necessary at all:

Code: Select all

      local teamID = spGetUnitTeam(unitID)
      if (teamID) then
         if ( not Spring.AreTeamsAllied(myTeamID, teamID) ) then

...because you can store the LocalTeam ID in a local variable at game start, and if teamID ~= LocalTeam, then.

And that's just the really obvious stuff that's wrong. Meh. I know you're new to this, and people are being dicks about it, but that doesn't mean they aren't right. You need to review some basics:

1. It's much cheaper to look up data that's stored in a table than to perform an expensive callout to Spring, which has to pass through Spring and back. Avoid using it whenever possible. That whole GetUnitDefRealRadius() is 100% not necessary- you should run that during Initialize() for every UnitDef, and store the results in a table of values, not re-do all that stuff every pass through the loop- after all, it's not like the Units have changed size since the last time you did the calculations. So that entire operation should be done ONCE, and stored in a table being used like an array.

So... your loop:

Code: Select all

 if ( not Spring.AreTeamsAllied(myTeamID, teamID) ) then
            local radius = GetUnitDefRealRadius(spGetUnitDefID(unitID))
            if (radius) then
               glDrawListAtUnit(unitID, circlePolys, false, radius, 1.0, radius)
            end
         end

Should be:

Code: Select all

function widget:DrawWorldPreUnit()
	glDepthTest(true)
	glPolygonOffset(-100, -2)
	for _,unitID in ipairs(GetVisibleUnits()) do
		teamID = spGetUnitTeam(unitID)
		if teamID ~= LocalTeam then
			radius = RadiusTable[spGetUnitDefID(unitID)]
			glDrawListAtUnit(unitID, circlePolys, false, radius, 1.0, radius)
		end
	end
end

Which... hmm... doesn't have any of that evil word "local" in it, and assumes that you've built a proper lookup table. It's also shorter by an IF-->THEN, and assumes that LocalTeam == Spring.GetLocalTeamID().

Hell, just fixing your GetUnitDefRealRadius() to operate where it should, i.e. during Initialize, would get rid of a lot of the suck... but that's about what your final loop should actually look like.

The UnitDefID lookup is unavoidable, unless you want to do a UnitCreated() loop and store the radius this way table.insert(SomeTable,{unitID = unitID, radius = radius}) instead. The only reason I don't suggest that is that then that table will get really long, and I've found that looking up the UnitDefID is usually faster. But there is zero reason at all to be building all that data more than one time.

2. Local variables within a loop should be avoided, unless the loop is very infrequent. Instead, create your locals at the start of the Widget:

local blah, blah2, blah3

And then just change their value.

[TradeMark]

But I'm drawing one quad, lol, not many triangles.

your mod has just one enemy unit? anyways, youre using GL_BLEND mode too. so you win nothing.

A little, yes, but the smaller amount of geometry makes up for it.

i dont think you win anything there, since you are using GL_BLEND too.

You're making all sorts of noob errors there.

Wait, lets make something clear here, if it isnt yet: i copypasted Trepan's TeamPlatter widget, took out teamcolors, optimized it a bit, and changed the name. Where i wrote own code is the initialize() function. Mostly its just copypaste/edit piece of work.

For one thing, do not create locals within that loop. Creating locals is expensive, and you're doing it for every non-Ally that's visible.

Oh? i didnt know that, as i said, i know nothing about lua. What i use if not locals? i create them out of the functions? global vars is bad i heard...

Moreover... this check's not necessary at all:

Code: Select all

      local teamID = spGetUnitTeam(unitID)
      if (teamID) then
         if ( not Spring.AreTeamsAllied(myTeamID, teamID) ) then

...because you can store the LocalTeam ID in a local variable at game start, and if teamID ~= LocalTeam, then.

And that's just the really obvious stuff that's wrong.

what the? only thing i see wrong is the useless if(teamID) and i dont know why is it there, i dont want to remove something that i know nothing about, thats mostly why spring has new bugs every release because people dont give a shit what they remove.

[Argh]

...

Oh? i didnt know that, as i said, i know nothing about lua. What i use if not locals? i create them out of the functions? global vars is bad i heard...

This is a good question.

Firstly... in any programming language I've ever used, creating a local variable is expensive. It involves memory assignment, etc., which isn't trivial. In Lua, it's quite expensive, more expensive than in some languages.

Global variables are also bad, because:

A. They increase the total memory load on Spring.
B. The more you use, the longer the lookup time will get. IOW, stuff like GG or WG is something we want to use, but only when we really need a true global, accessible to everything. Otherwise, use messages (this may cause harrumphing from a few folks, but it's my current opinion, and it's not an iron-clad thing, just a rule of thumb).

Therefore... good practice:

Code: Select all

local myVar1, myVar2, myVar3...

function myFunction1()
end

That's ideal. We don't want any new variables being created inside functions, ever. This ideal is not always possible, for various reasons (Lua has an upper limit on how many local variables we can have, amongst other things) but can usually be achieved.

Wait, lets make something clear here, if it isnt yet: i copypasted Trepan's TeamPlatter widget, took out teamcolors, optimized it a bit, and changed the name. Where i wrote own code is the initialize() function. Mostly its just copypaste/edit piece of work.

Well, that may be... perhaps Trepan wrote something that's pretty inefficient and slow.

It wouldn't be the only time it's ever happened. A lot of the early Widgets aren't terribly good (of course, neither were most of my early things, either). Trepan was trying to write enough functional stuff to get everybody excited about the whole concept of Widgets, after all- it wouldn't surprise me, if a lot of that stuff was written in a less-than-optimal way, just to get it functional fast.

Anyhow, whether it's something he didn't do, or your imperfect understanding of Lua and you hooked things together in a very inefficient way... doesn't really matter. If you want it to run well, you need to optimize it. I understand, you're new to all this, and thought, "hey, I'll just change a couple of lines, it'll be useful and cool", but it's never quite that simple. Welcome to programming, we all have to deal with these issues

[very_bad_soldier]

Code: Select all

local function GetUnitDefRealRadius(udid)
   local radius = realRadii[udid]
   if (radius) then return radius end
   local ud = UnitDefs[udid]
   if (ud == nil) then return nil end
   local dims = spGetUnitDefDimensions(udid)
   if (dims == nil) then return nil end
   local scale = ud.hitSphereScale -- missing in 0.76b1+
   scale = ((scale == nil) or (scale == 0.0)) and 1.0 or scale
   radius = dims.radius / scale
   realRadii[udid] = radius*innersize
   return radius
end

...is very poorly optimized.

That whole GetUnitDefRealRadius() is 100% not necessary- you should run that during Initialize() for every UnitDef, and store the results in a table of values, not re-do all that stuff every pass through the loop- after all, it's not like the Units have changed size since the last time you did the calculations. So that entire operation should be done ONCE, and stored in a table being used like an array.

You are aware that it is already caching the data by storing the calculated values in the table realRadii so the range for each unitId will only be calculated once (first two lines of that function)?
I am using that function too somewhere so I am a bit curious if it is really that very poorly optimized as you said... I dont get it at the moment frankly.

[Argh]

Yes, I am saying that. Run that function once per UnitDef, store it in a table, never do any of that ever again. Again, it's not like any of that changes per cycle, so it's static data, never rebuild it.

Meh, I don't do any of that stuff, for my platters, btw. I just look up the x value of the footprint in the UnitDef. But then again, I'm not drawing it for everything in P.U.R.E., just stuff whose UnitDefID is in a certain table (I don't want platters for Gaia trees, for example). So I just read a customParam at game start, build a table of valid Units and their type of platter, and voila, the only math's getting facing (for certain things) and an offset, IIRC.

Wish more people would get it, and just build more customParams for common stuff like this. The modder / game-designer folks really should be working with you Widget folks more closely, not just leeching from your work, imo. So far as I know, P.U.R.E. and to a lesser extent S'44 are the only games using this model for development, but I can tell you right now, it's very effective- once you have enough customParams, it's almost like every Unit has a "fingerprint", and it makes it much easier to sort them. Meh, that's off-topic... moving on.

[very_bad_soldier]

I think you just missed those two lines.
In theory I think you are right by building that table completely before game starts. In real game I think it doesnt matter cause I doubt your FPS will drop if you calc the radius of a single unit every now and then.

[Argh]

local function GetUnitDefRealRadius(udid)
local radius = realRadii[udid]
if (radius) then return radius end
local ud = UnitDefs[udid]
if (ud == nil) then return nil end
local dims = spGetUnitDefDimensions(udid)
if (dims == nil) then return nil end
local scale = ud.hitSphereScale -- missing in 0.76b1+
scale = ((scale == nil) or (scale == 0.0)) and 1.0 or scale
radius = dims.radius / scale
realRadii[udid] = radius*innersize
return radius
end

Code: Select all

if ( not Spring.AreTeamsAllied(myTeamID, teamID) ) then
            local radius = GetUnitDefRealRadius(spGetUnitDefID(unitID))
            if (radius) then
               glDrawListAtUnit(unitID, circlePolys, false, radius, 1.0, radius)
            end
         end

Instead, you could just be doing:

Code: Select all

if teamID ~= LocalTeam then
           radius = radiusTable[Spring.GetUnitDefID].radius
           glDrawListAtUnit(unitID, circlePolys, false, radius, 1.0, radius)
         end

Using a table built like this:

Code: Select all

local radius,xsize, zsize
function widget:Initialize()
	for i,k in ipairs(UnitDefs) do
		xsize = k.xsize * 6
		zsize = k.zsize * 6
		if xsize >= zsize then
			radius = xsize
			table.insert(radiusTable,k,{radius})
		else
			radius = zsize
			table.insert(radiusTable,k,{radius})
		end
	end
end

...which obviously you can make fancier, if you really need to- heck, detect whether a customParam exists, and if yes, then allow customParams to offset it, etc. I'm not saying "don't build this data in whatever fancy way you want", I'm just saying "never, ever, ever build it again in realtime code, because it's static data, it never changes, and doing any of it again is a waste of CPU".

IOW:

local radius = realRadii[udid]
if (radius) then return radius end

...is a wasteful operation. It's in the radiusTable, by definition, or it's not a valid unitDef (at least the way I wrote that).

[very_bad_soldier]

Putting every single variable used in functions into global scope is a really big mess IMO. Something I dont want normally.

local radius = realRadii[udid]
if (radius) then return radius end

...is a wasteful operation. It's in the radiusTable, by definition, or it's not a valid unitDef (at least the way I wrote that).

It is in the radiusTable by definition? The table gets filled at runtime every time a units' radius is calculated for the first time.
It basically says:
1. Check if I already calculated the radius for that unit by looking in my preCalculatedRadius-Table
2. If I dont have it precalculated, I have to calculate it now and put it to my preCalculatedRadius-Table so I dont have to calculate it again next time I get asked for the same unit

[Argh]

If it's possible that something's not in radiusTable... an optimal solution is:

Code: Select all

table.insert(radiusTable,k,{radius = 0, dontDraw = true})

Then have an AND (faster than IF) for safety:

Code: Select all

if teamID ~= LocalTeam and radiusTable[Spring.GetUnitDefID].dontDraw == false then
           radius = radiusTable[Spring.GetUnitDefID].radius
           glDrawListAtUnit(unitID, circlePolys, false, radius, 1.0, radius)
end

Making sense? That's an additional table lookup, but it's clean and safe at that point (assuming you wrote valid logic in Initialize(), ofc). And no, there's no reason at all to do what you're talking about during runtime, ever. Do it during Initialize(), that's what it's for. Stuff like Widgets should have all the data that isn't dynamic computed and stored before a single gameframe occurs, period.

And there are reasons to use variables that way, starting with memory allocation. I'm not saying, "never create a local within a function", just "avoid it wherever humanly possible".

That, and it encourages better coding practices, because you'll want to use a descriptive name, instead of some local "g" which gets created and used in only one part of a loop, with few hints about what it is and what it's for. But mainly, it's about speed. Variable creation and assignment of that memory isn't fast.