2025-08-01 18:28 CEST
  

View Issue Details [ Jump to Notes ] [ Issue History ] [ Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006023Spring engineLuapublic2018-08-05 15:202018-10-28 16:37
ReporterFloris 
Assigned To 
PrioritynormalSeverityminorReproducibilityalways
StatusnewResolutionopen 
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

-Relationships
+Relationships

-Notes

~0019421

gajop (developer)

2018-10-20 03:24

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

~0019422

Floris (reporter)

2018-10-20 11:49

 might be no longer an issue, cant reproduce

~0019423

Floris (reporter)

2018-10-20 12:03

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

https://github.com/Balanced-Annihilation/Balanced-Annihilation/blob/master/luaui/Widgets_BA/gui_options.lua#L2376

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)

~0019424

gajop (developer)

2018-10-22 14:19

 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.
https://github.com/Balanced-Annihilation/Balanced-Annihilation/blob/master/luaui/bawidgets.lua#L1175

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)
  end
  for _,w in ipairs(textCommandList) do
    if (w:TextCommand(command)) then
      return true
    end
  end


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.

~0019425

Kloot (developer)

2018-10-22 16:00

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.

~0019428

gajop (developer)

2018-10-23 01:21

 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)
  end
end


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
  else
    local nkey = key - 1
    return nkey, t[nkey]
  end
end

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

x = { 11, 12, 13, 14}

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


Produces:


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

~0019429

silentwings (reporter)

2018-10-23 08:40

 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

~0019451

gajop (developer)

2018-10-28 16:37

 @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")
    end
    if (w:KeyPress(key, mods, isRepeat, label, unicode)) then
      return true
    end
  end

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
+Notes

-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