2020-09-19 02:04 CEST

View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0006023Spring engineLuapublic2018-10-28 16:37
Assigned To 
Product Version104.0 +git 
Target VersionFixed in Version 
Summary0006023: widget:TextCommand(command) getting called again after enabling/disabling widget
Descriptionwhen giving a cmd that then will toggle a widget, it will get called again with the same TextCommand parameter
TagsNo tags attached.
Checked infolog.txt for Errors
Attached Files




gajop (developer)

Can you provide some code and command steps how this can be reproduced?


Floris (reporter)

might be no longer an issue, cant reproduce


Floris (reporter)

nope, its still an issue, i just dealt with the problem at:


If you remove the "os.clock() > lastOptionCommand+1 and " part, it will happen again.
then you can test by doing /option bloom
(as a result widget:TextCommand(command) is called twice and also the bloom widget twice)


gajop (developer)

I think I figured it out.

The widgethandler maintains a list of widgets that handle a TextCommand callin, it iterates over them and invokes them.

However, since you are also adding and removing widgets in one widget's callin, you're effectively modifying the widget handler's self.TextCommandList. This is a classic error of modifying a list while iterating through it (not your error, but the widgethandler writer).

It can be fixed by modifying the widget handler to make a shallow copy of the widget list before iterating, the entire code snippet being something like:

  local textCommandList = {}
  for _, w in ipairs(self.TextCommandList) do
    table.insert(textCommandList, w)
  for _,w in ipairs(textCommandList) do
    if (w:TextCommand(command)) then
      return true

Generally though this is pretty inefficient, and a better solution would swap callin lists at the end of the iteration (if necessary). It seems all callins suffer from the same error, so this would be a big change...
I'm going to wait a bit on fixing this one. I don't want to maintain a million different addon handlers.


Kloot (developer)

Last edited: 2018-10-22 16:04

View 2 revisions

no, the solution would be to iterate in reverse, similar to how the gadget handler was fixed some years ago. I didn't (and don't) consider the WH critical enough to give it the same treatment, and too many games ship their own that will never be updated.


gajop (developer)

Haven't run the code, but it seems :RemoveWidget just removes the widget from the callinlist table: https://github.com/Balanced-Annihilation/Balanced-Annihilation/blob/master/luaui/bawidgets.lua#L801 https://github.com/Balanced-Annihilation/Balanced-Annihilation/blob/master/luaui/bawidgets.lua#L752-L759

Since we aren't removing the called widget itself, but a different widget, I don't see how the order of the iteration matters. There's going to be a bug in either adding or removing widgets.

Here's an example snippet demonstrating this functionality:

x = { 11, 12, 13, 14}

for i = #x, 1, -1 do
  print("current index: ", i, "value: ", x[i])
  if x[i] == 13 then
    print("removed index: ", 2, "value: ", x[2])
    table.remove(x, 2)
    print("total elements: ", #x)

which results in this code being produced:

current index: 4 value: 14
current index: 3 value: 13
removed index: 2 value: 12
total elements: 3
current index: 2 value: 13
removed index: 2 value: 13
total elements: 2
current index: 1 value: 11

Notice how we print the value "13" twice, because we removed a previous element.

Similar situation with wh's ripairs:

local function rev_iter(t, key)
  if (key <= 1) then
    return nil
    local nkey = key - 1
    return nkey, t[nkey]

local function ripairs(t)
  return rev_iter, t, (1 + #t)

x = { 11, 12, 13, 14}

for _, v in ripairs(x) do
  print("value: ", v)
  if v == 13 then
    table.remove(x, 2)


value: 14
value: 13
value: 13
value: 11


silentwings (reporter)

I did convert the WH in BAR to r_pairs, but not BAs one (https://github.com/Balanced-Annihilation-Reloaded/BAR.sdd/blob/master/luaui/barwidgets.lua)

You could test the BAR one to see if it shows the same bug


gajop (developer)

@silentwings: a very simple test confirms it's broken with BAR too:

Since BAR doesn't have a TextCommand callin, I simplified the test by doing this:

  widgetHandler:EnableWidget("Resource Bars")
  for i,w in r_ipairs(self.KeyPressList) do
    Spring.Echo(i, w.whInfo.name)
    if i == 7 then # Resource Bars has index 6,
      widgetHandler:DisableWidget("Resource Bars")
    if (w:KeyPress(key, mods, isRepeat, label, unicode)) then
      return true

This gives me the following output:

[f=0016847] 8, Chili Framework
[f=0016847] 7, Hotkeys
[f=0016847] Removed: LuaUI/Widgets/bgu_resbars.lua
[f=0016847] 6, Hotkeys
[f=0016847] 5, Chonsole
[f=0016847] 4, Auto Group
[f=0016847] 3, Camera Flip
[f=0016847] 2, Attack AoE
[f=0016847] 1, Chat Console

As expected, Hotkeys gets repeated twice

-Issue History
Date Modified Username Field Change
2018-08-05 15:20 Floris New Issue
2018-10-20 03:24 gajop Note Added: 0019421
2018-10-20 11:49 Floris Note Added: 0019422
2018-10-20 12:03 Floris Note Added: 0019423
2018-10-22 14:19 gajop Note Added: 0019424
2018-10-22 16:00 Kloot Note Added: 0019425
2018-10-22 16:04 Kloot Note Edited: 0019425 View Revisions
2018-10-23 01:21 gajop Note Added: 0019428
2018-10-23 08:40 silentwings Note Added: 0019429
2018-10-28 16:37 gajop Note Added: 0019451
+Issue History