New widget: Enemy Spotter

[TradeMark]

i dont see how that would be any better than my widget, screenshot?

[Jazcash]

i dont see how that would be any better than my widget, screenshot?

Probably not the best pic for an example but a pic none the less.

[TradeMark]

lol try using that in 8v8 DSD (or DSI)

your UI looks like shit btw.

So that widget makes the teamcolor under enemies if they are enough close to your units...? so basically if i hide some of my radar planes in your place far enough, you cant spot it with that widget o.o

i dont see any fucking point in testing how far they are, while the fastest (and best) way is to get all visible units and do the trick on every enemy unit you see...

[Jazcash]

your UI looks like shit btw.

Now now, no need to get jealous. Just because your metal maps suck balls and you suck at the game, doesn't meat you should take it out on somebody who actually uses his GUI for gameplay measures rather than for smexy shine.

Gloss doesn't make something good. It just makes it pretty.

Oh and I've got no problems using those widgets on any map. I also see you just had to throw a shoutout for your map in there as if it's anywhere near as popular as DSD.

[Argh]

@Jazcash: Erm, don't be silly. This isn't some auto-finder or maphack, it's just a minor variation on teamplatters.

@jK: Quite right, oops. Fixed.

@Lurker: OK, so I went here and read this (PDF).

I really, really suggest people read it, if they want to learn more about optimization strategies for issues particular to Lua. I'm going to quote some of it, but by no means all.

OK... so, basically, Lua's running within a C loop, once it has precompiled. We're not responsible for memory allocation or garbage collection (thank goodness), but it is happening.

OK, so, first rule of thumb: use locals to avoid upvalue calls.

Code: Select all

For instance, the code
for i = 1, 1000000 do
local x = math.sin(i)
end
runs 30% slower than this one:
local sin = math.sin
for i = 1, 1000000 do
local x = sin(i)
end

That makes sense to me.

But, in any case where multiple functions need that value, keep this in mind:

Code: Select all

Access to external locals (that is, variables that are local to an enclosing
function) is not as fast as access to local variables, but it is still faster than
access to globals. Consider the next fragment:
function foo (x)
for i = 1, 1000000 do
x = x + math.sin(i)
end
return x
end
print(foo(10))
We can optimize it by declaring sin once, outside function foo:
local sin = math.sin
function foo (x)
for i = 1, 1000000 do
x = x + sin(i)
end
return x
end
print(foo(10))
This second code runs 30% faster than the original one.

OK, that pretty much covers basic functions- there are a few more examples.

Talking about tables, it's quite clear from here that, for realtime code, there are very few justifications for building a table within a function loop. One thing I'm going to need to test at some point is the cost of hashing, vs. assignment costs. According to the PDF, keeping the table short by dynamically removing members of the table is costing a lot of hashing- so, if you have a table where you may need from 1 to 10,000 members, you might as well make it at runtime with 10,000 members and a default value. That seems counterintuitive to me, but I'll test it and see.

One typical case, that ought to look very familiar:

Code: Select all

A good place to look for chances of reducing garbage creation is in loops. For instance, if a constant table is created inside a loop, you can move it out the loop, or even out of the function enclosing its creation. Compare:
function foo (...)
for i = 1, n do
local t = {1, 2, 3, "hi"}
-- do something without changing ÔÇÖtÔÇÖ
...
end
end
local t = {1, 2, 3, "hi"} -- create ÔÇÖtÔÇÖ once and for all
function foo (...)
for i = 1, n do
-- do something without changing ÔÇÖtÔÇÖ
...
end
end

So, for stuff like the pre-computed table of key-value pairs unitDef,radius, we absolutely want that data to remain static at runtime. Which is, along with doing no new computation and losing a loop, why that speeds things up considerably.

********************************

The only real mystery here, after reading this, is that it's kind've unclear about the real costs of locals within and outside functions. IOW, there really wasn't a lot of stuff definitely answering VBS's comment about scope. Here is what I think, after looking at it.

At runtime, the very first time a function runs, after reading through the PDF, it would seem that we'd get better performance from declaring them at the start of our function loop, i.e.:

Code: Select all

local function myFunction()
local foo1, foo2, foo3
...do stuff with foo1, foo2, foo3
end

And only using:

Code: Select all

local foo1, foo2, foo3
local function myFunction()
...do stuff with foo1, foo2, foo3
end

...when we need to use those variables outside that function, or the values are static.

IOW, for static stuff, it appears we want to localize it outside our functions.

It makes no sense, in a typical Widget, to do this:

Code: Select all

function widget:DrawWorldPreUnit()
local glTexture = gl.Texture
glTexture(blah)
end

...because every time that function runs, we're reassigning that global to that local. Obviously, this won't save on performance.

For dynamic stuff, though, it would appear that it's better to localize it, at first glance. For example:

Code: Select all

local function Blah()
local X = 0
...do arithmetic that might change X's value, but cannot operate if it's nil
X = final value
end

Appears to be a bit faster than:

Code: Select all

local X = 0
local function Blah()
...do arithmetic that might change X's value, but cannot operate if it's nil
X = final value
end

...even though that seems counterintuitive.

However... if we don't want X to be reset to 0 every time Blah() runs... then we should put it outside that function, because the cost of an evaluator loop, such as this:

Code: Select all

local function Blah()
local X
if X ~= nil then
...do arithmetic that might change X's value, but cannot operate if it's nil
else
X = 0
return
end

...is considerably slower, because it has to do that comparison every time.

TBH, the more I look at that issue, the more issues I see with just blithely localizing your variables within functions, because with a lot of things, you will have to write safety functions before passing to evaluator stages, and that bloats your code with additional if-then loops- a much higher performance cost than fetching a local upvalue- and it's harder to read.

In most cases, it's probably wiser to eat the upvalue costs, imo. Not always- if it's a throwaway local and you can set it to zero at the start of your function, then go ahead, it will save an upvalue call. If it would make your program not operate correctly, though...

Basically, it's just fine to localize everything in your functions, for stuff that runs once. But... most Widget stuff is supposed to run many times a second. That kinda changes things. Safety loops are expensive, compared to grabbing an upvalue, so we want to avoid them.

********************************
On the topic of "re-use and recycle", there are obviously a lot of good reasons to do so. But, please don't take that concept to extremes. What little you're saving on performance in most cases is usually not justified by how much harder you've made your code to read.

For example:

Code: Select all

local function Blah()
local myVar
myVar = functionCall()
-->use myVar to draw something
myVar = anotherFunctionCall()
-->use myVar to tell end-user that "42" is the number
end

Please don't do that, it's hard to read and debug, and it's really not improving performance a lot. Instead, do this:

Code: Select all

local function Blah()
local myVarOne, myVarTwo
myVarOne = functionCall()
-->use myVarOne to draw something
myVarTwo = anotherFunctionCall()
-->use myVarTwo to tell end-user that "42" is the number
end

It will use ever-so-little more memory, but it's a LOT clearer what is being used to do what to whom. You can take "re-use and recycle" a little too far. When it changes the entire use context of a variable, it's too far, imo. At the very least, write a comment... you may not be here in six months, when it's time to fix your wonderful Widget... that got broken by a small specification change in Spring.

[Jazcash]

@Jazcash: Erm, don't be silly. This isn't some auto-finder

[Unintended-Trolling]

It finds the units and highlights the ones you need to kill. Teamplatters just makes the colours of the units easier to see. It's all the small things which makes the game require less skill. Sure they help a great deal, but that's why they annoy me so much.

I used to enjoy having to micro-manage everything. But now any noob can play Spring and have half of the tasks we had to do about 3-4 years ago automated.

I'll probably get a fair amount of hate for this but I just think that Lua Widgets have become more of a "Must have" rather than a system to enhance gameplay a little.

Here is what players can do without playing the game and just using widgets:

• Micro units
• Control nanos
• Load nukes and anti-nukes
• Stop themselves getting comnapped
• Cap their mexes
• Upgrade their mexes
• Control their eco thought metal makers
• Label any unit they like and coms so they don't have to even look for things anymore
• And many many other tasks that widgets do

Don't get me wrong, there are so really needed and awesome widgets out there that don't play the game for you. All the GUI's and GUI elements, setting certain units to hold position automatically, all the things that make Spring pretty like bloom, Lua Manager etc...

I just think all the awesome widgets have been made and people still make more for a few reasons such as it helps them develop their knowledge of Lua and coding, it's a nice feeling to make something that people use, it adds to your list of stuff you've done

However, because all the main widgets have been made, it forces coders to make widgets that now interfere with the gameplay.

Trade's widget isn't really that bad in terms of playing the game for you, but the point is that most players don't really need a widget like this because they take the time and effort to recognize who is the enemy and who isn't.

[/Unintended-Trolling]

[TradeMark]

You are right jaz and i agree, that was the only post from you in this thread that wasnt trolling btw.

TeamPlatter is exactly like this widget, except i dont care about which color they are, just that i see the color easier. So you can gtfo about crying how this plays the game for you, since you are using similar tool too... just that you make it harder for you.

[Argh]

With jK's fixes:

Code: Select all

function widget:GetInfo()
   return {
      name      = "Enemy Spotter v.2",
      desc      = "Draws smoothed octagon under enemy units",
      author    = "Argh,TradeMark,Trepan",
      date      = "December 17th, 2009",
      license   = "(C) WolfeGames, released under GNU GPL, v2 or later",
      layer     = 5,
      enabled   = false  --  loaded by default?
   }
end

--------------------------------------------------------------------------------
--------------------------------------------------------------------------------

-- Automatically generated local definitions

local glBeginEnd             = gl.BeginEnd
local glTexCoord =	gl.TexCoord
local glTexture = gl.Texture
local GL_QUADS = GL.QUADS
local glColor                = gl.Color
local glCreateList           = gl.CreateList
local glDeleteList           = gl.DeleteList
local glDepthTest            = gl.DepthTest
local glDrawListAtUnit       = gl.DrawListAtUnit
local glVertex               = gl.Vertex
local GetVisibleUnits     = Spring.GetVisibleUnits
local spGetUnitDefID         = Spring.GetUnitDefID
local spGetUnitTeam          = Spring.GetUnitTeam

--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
local LocalTeam = Spring.GetLocalTeamID()
local ENEMY_UNITS       = Spring.ENEMY_UNITS
local texture = "bitmaps/StaticCircle.tga"
local radiusDefs = {}

--------------------------------------------------------------------------------
--------------------------------------------------------------------------------

function widget:Initialize()
	for uDefID, uDef in pairs(UnitDefs) do
		radiusDefs[uDefID] = uDef.radius * 1.25
	end

	SimpleCircle = glCreateList(function()
		glBeginEnd(GL_QUADS, function()
			glTexCoord(0.04,0.04)
			glVertex(-1,10,-1)
			glTexCoord(0.96,0.04)
			glVertex(1,10,-1)
			glTexCoord(0.96,0.96)
			glVertex(1,10,1)
			glTexCoord(0.04,0.96)
			glVertex(-1,10,1)
		end)
	end)
end

function widget:Shutdown()
	glDeleteList(SimpleCircle)
end


function widget:DrawWorldPreUnit()
	glDepthTest(true)
	glTexture(texture)
	glColor(1.0,0,0,1.0)
	local teamID, ud
	local visible = GetVisibleUnits(ENEMY_UNITS, nil, false)

	for i = 1,#visible do
		unitID = visible[i]
		teamID = spGetUnitTeam(unitID)
		if teamID ~= LocalTeam then
			ud = spGetUnitDefID(unitID)
			radius = radiusDefs[ud]
			glDrawListAtUnit(unitID, SimpleCircle, false, radius, 1.0, radius, 0, 0, 0, 0)
		end
	end
	glTexture(false)
end

Note: you still need to make StaticCircle.tga, or download it in this thread.

[jK]

local visible = GetVisibleUnits(ENEMY_UNITS, nil, false)

for i = 1,#visible do
unitID = visible
teamID = spGetUnitTeam(unitID)
if teamID ~= LocalTeam then

[Regret]

local visible = GetVisibleUnits(ENEMY_UNITS, nil, false)

for i = 1,#visible do
unitID = visible
teamID = spGetUnitTeam(unitID)
if teamID ~= LocalTeam then

Hahahah that's hilarious D:

[Argh]

Oops, missed that one. Yay, loses another loop, too.

Code: Select all

function widget:GetInfo()
   return {
      name      = "Enemy Spotter v.2",
      desc      = "Draws smoothed octagon under enemy units",
      author    = "Argh,TradeMark,Trepan",
      date      = "December 17th, 2009",
      license   = "(C) WolfeGames, released under GNU GPL, v2 or later",
      layer     = 5,
      enabled   = false  --  loaded by default?
   }
end

--------------------------------------------------------------------------------
--------------------------------------------------------------------------------

-- Automatically generated local definitions

local glBeginEnd             = gl.BeginEnd
local glTexCoord =   gl.TexCoord
local glTexture = gl.Texture
local GL_QUADS = GL.QUADS
local glColor                = gl.Color
local glCreateList           = gl.CreateList
local glDeleteList           = gl.DeleteList
local glDepthTest            = gl.DepthTest
local glDrawListAtUnit       = gl.DrawListAtUnit
local glVertex               = gl.Vertex
local GetVisibleUnits     = Spring.GetVisibleUnits
local spGetUnitDefID         = Spring.GetUnitDefID
local spGetUnitTeam          = Spring.GetUnitTeam

--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
local LocalTeam = Spring.GetLocalTeamID()
local ENEMY_UNITS       = Spring.ENEMY_UNITS
local texture = "bitmaps/StaticCircle.tga"
local radiusDefs = {}

--------------------------------------------------------------------------------
--------------------------------------------------------------------------------

function widget:Initialize()
   for uDefID, uDef in pairs(UnitDefs) do
      radiusDefs[uDefID] = uDef.radius * 1.25
   end

   SimpleCircle = glCreateList(function()
      glBeginEnd(GL_QUADS, function()
         glTexCoord(0.04,0.04)
         glVertex(-1,10,-1)
         glTexCoord(0.96,0.04)
         glVertex(1,10,-1)
         glTexCoord(0.96,0.96)
         glVertex(1,10,1)
         glTexCoord(0.04,0.96)
         glVertex(-1,10,1)
      end)
   end)
end

function widget:Shutdown()
   glDeleteList(SimpleCircle)
end


function widget:DrawWorldPreUnit()
   glDepthTest(true)
   glTexture(texture)
   glColor(1.0,0,0,1.0)
   local teamID, ud
   local visible = GetVisibleUnits(ENEMY_UNITS, nil, false)

   for i = 1,#visible do
      unitID = visible[i]
      ud = spGetUnitDefID(unitID)
      radius = radiusDefs[ud]
      glDrawListAtUnit(unitID, SimpleCircle, false, radius, 1.0, radius, 0, 0, 0, 0)
   end
   glTexture(false)
end

[TradeMark]

i find it funny you guys are optimizing if's

[Argh]

Well, if-then evaluators for simple variables are not massively expensive, no.

But we also lost a callout to Spring for every Unit, which should speed things up considerably. I'm just guessing, but we probably gained another 5% speed or more.

In the case of this particular implementation, it's pretty pointless, as it's already really fast, but that's not really the point of the exercise.

[AF]

i find it funny you guys are optimizing if's

Code: Select all

if bannana == true
    eat pineapple
else
    for( i=0; i < 100000000000; i++)
        veryexpensiveoperation()
    end for
end if

[TradeMark]

if(radius)

i mean that, since that was the first thing they whined at "my" code, so fucking slow!!!

[jK]

I am not arguing anything about performance here ...
Argh just made some code structure failures.

[very_bad_soldier]

Argh, one thing I noticed while testing:
When enabling your widget for the first time, I had a small freeze (maybe 300 ms). Probably while your widget builds the complete unit-radius-table.
Its not dramatic but imagine there were 10 or 20 widgets that would do stuff like that. It would sum up to a very noticable freeze of multiple seconds.

In fact I dont know how it is handled when loading the widgets at gamestart. Would it lead to a small "freeze"?

[lurker]

The rule of thumb is very simple. If you're going to use it more than once, localize it, otherwise don't. As far as your thing with X and blah, the method to use it and make sure it's never nil should be one of the following.

Code: Select all

local X = 0
function Blah()
    --use X a little bit, so just leave it as an upvalue
end

Code: Select all

local X = 0
function Blah()
    local X_ = X
    --use X_ a *lot*, in a loop, needing a tiny performance tweak that makes the function two lines longer
    X = X_
end

If checks don't have much cost in an interpreted language, because the processor is already branching at each opcode. If you can avoid significant arithmetic with one, do so.


Don't worry about table sizing. It's not going to come up. The impact of resizing a table isn't that big, and the cost of making huge tables and then not using them is just going to get in the way.


Don't worry about the speed of code that only runs once per frame, outside of a loop. The only non-loop code you should put a lot of effort into tweaking is something like UnitDamaged.


Last, I refer to the top of the document Argh linked.

In Lua, as in any other programming language, we should always follow the two maxims of program optimization:
Rule #1: DonÔÇÖt do it.
Rule #2: DonÔÇÖt do it yet. (for experts only)

[Argh]

If you can avoid significant arithmetic with one, do so.

Yes. Also, OR and AND are both very, very cheap, so use that instead of arithmetic if you can.

Don't worry about the speed of code that only runs once per frame, outside of a loop.

Well, if it's on GameFrame(), you should worry a bit, simply because it's competing with the rest of the gamecode that's running in that timeslot- over an entire game design, that gets to be an issue. But you can frequently space it out over multiple gameframes, etc.

Don't worry about table sizing. It's not going to come up. The impact of resizing a table isn't that big, and the cost of making huge tables and then not using them is just going to get in the way.

IDK about that one yet, I will have to test that. According to the document, there are specific size ranges where it might be optimal to build tables with default values at runtime, and do changes to the values, instead of table.insert / table.delete.

--use X_ a *lot*, in a loop, needing a tiny performance tweak that makes the function

Where's that speedbump, though? 10? 100? 1000? Kinda curious as to how many iterations before the two extra operations + local speedup is actually faster. I have a few things where that would matter, but not much.

Argh, one thing I noticed while testing:
When enabling your widget for the first time, I had a small freeze (maybe 300 ms). Probably while your widget builds the complete unit-radius-table.
Its not dramatic but imagine there were 10 or 20 widgets that would do stuff like that. It would sum up to a very noticable freeze of multiple seconds.

In fact I dont know how it is handled when loading the widgets at gamestart. Would it lead to a small "freeze"?

Yes, but it's usually before we've run a gameframe, assuming it's enabled at startup. IIRC, we're not allowed to begin the first gameframe until all of that has completed. Now, can that possibly cause a desync, if there's a massive disparity between two computers? IDK. I would think not, my experience with Gadgets is that it doesn't do so.

In Lua, as in any other programming language, we should always follow the two maxims of program optimization:
Rule #1: DonÔÇÖt do it.
Rule #2: DonÔÇÖt do it yet. (for experts only)

I think that's good advice.

Performance improvements can wait until something works as planned. If it's really slow, but runs as planned, your problem is probably strategic, not the small stuff.

I mean, go look at P.O.P.S.- it got rebuilt practically from the ground up several times before I got serious about optimization, because the first versions weren't using the right approach to the problem.

Your chief optimization is usually not all this detailed stuff we're doing here, it's the overall strategy for avoiding non-essential computation. This little Widget's like a coder haiku, though, so it's a good place to talk about the nuts and bolts.

[TradeMark]

as very_bad_soldier mentioned here that it takes 300ms to initialize those radius things, shouldnt we have some basic methods by engine side to do those arrays for our lua widgets? so all widgets could use the same array, would reduce code repetition in widgets and make them faster too.