Multiplayer Modification Checklist Bug

[uluo-wes]

Game and System Information

• Version: 1.14.17 & 1.16.3
• Downloaded from: Steam
• Build info: No custom build. Downloaded through Steam, swapped between beta 1.14.x (stable) and current 1.16.3 in Steam Betas option in game properties. Since info and log files are long I uploaded both below.
• OS: Windows 10 21H2
  1.14.17 Build Info.txt
  1.14.17 Log.txt
  1.16.3 Build Info.txt
  1.16.3 Log.txt

Description of the bug

When checking or unchecking mods in the multiplayer window the game freezes or hangs for a second and does one of the following:

1. Checks/Unchecks the selected mod only. (On rare occasion it's smooth with no hanging).
2. Checks/Unchecks up to 5+ other mods randomly, does nothing to the mod selected for some reason.
3. Checks/Unchecks the mod selected and another two spaces up or down.
4. When "3." occurs and I try to checking the selected mod again after unchecking both, the same mod two spaces up checks first, when clicking again the selected mod checks. Seems to happen in a weird sequence.

Steps to reproduce the behavior

1. Download addons for 1.14.17 and 1.16.3, ideally the same addons updated for both versions.
2. Open multiplayer, select any map.
3. Check/Uncheck mods.

Expected behavior

Checking and unchecking mods only does so for the selected mod and doesn't automatically do so for random mods in the list.

Additional context

It's possible I messed something up having multiple installs of different versions of Wesnoth on Steam and two installed locally on two different drives causing unexpected bugs.

[knyghtmare]

20220620 14:39:53 error config: found modification with id=Dead_Bodies twice
20220620 14:39:53 error config: found modification with id=lay_ambushes twice
20220620 14:39:53 error config: found modification with id=wind_modification twice
20220620 14:39:55 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:39:56 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:39:57 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:39:57 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:39:58 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:01 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:01 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:01 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:02 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:02 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:04 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:04 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:05 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:08 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:08 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:09 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:09 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:12 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:12 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:13 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:14 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:14 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:16 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:16 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:16 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:17 error mp/create: ignoring on_mod_toggle that is already set
20220620 14:40:17 error mp/create: ignoring on_mod_toggle that is already set

At a glance I think the mods Dead Bodies and Wind Mod are firing most of the error messages for 1.16.3
Rest of it is deprecation and a lot of warnings from Warning Strings from AWW.

May I suggest you to uninstall Dead Bodies, Wind Mod and Ambush Rework (all mods by Pipapopinguin) and see if the problem persists?
Context: this author has been known to make very buggy mods (seeing the forum reports) and you are using 3 of them.

[uluo-wes]

Uninstalling all of Pipapopinguin's mods fixed the issues for not only my two Steam installs but the two local versions as well.

[knyghtmare]

Closing this since the uninstallation of Dead Bodies, Wind Mod and Ambush Rework solved the issue.
For future reference:
this is the third time modifications by Pipapopinguin have caused "bugs" or issues like this.

[soliton-]

It would be still good to know what exactly the issue is. It sounds like a bug that an addon can cause this.

[soliton-]

Closing this since the uninstallation of Dead Bodies, Wind Mod and Ambush Rework solved the issue. For future reference: this is the third time modifications by Pipapopinguin have caused "bugs" or issues like this.

Do you know which one is the cause? Or each one does it? Does the author know?

[knyghtmare]

Closing this since the uninstallation of Dead Bodies, Wind Mod and Ambush Rework solved the issue. For future reference: this is the third time modifications by Pipapopinguin have caused "bugs" or issues like this.

Do you know which one is the cause? Or each one does it? Does the author know?

I suspect all three. Installed only Corpse Mod and the bug was caused instantly.

The author was notified of the previous 2 cases of their addons causing bugs. I suppose they have to be notified for the third time.

[soliton-]

The ones that I looked at all include the same files multiple times (for example corpse.cfg in that addon). I suspect that's the issue. Still sounds like a bug in wesnoth to cause such weird effects.

[knyghtmare]

Still sounds like a bug in wesnoth to cause such weird effects.

Perhaps not. I have installed mods by Mattss, Vasya N, Ravana, ForestD and myself and none of them have caused this kind of issue.

[soliton-]

Some other addons not causing issues means this issue is not a bug in wesnoth? What's the relation even?

If there is a way to mess up the UI with one addon so that clicking on modifications of completely unrelated addons is messed up then that's clearly a bug in wesnoth. So far that's how it sounds like it to me.

[soliton-]

So

[modification]
    id=foo
    name=Foo
[/modification]

[modification]
    id=foo
    name=Foo (duplicate)
[/modification]

[modification]
    id=foo2
    name=Foo 2
[/modification]

shows the issue. When you click on Foo 2 first Foo above is selected. When you click a second time Foo 2 gets selected. On the third click both get unselected at the same time. I can't reproduce the 5+ (de)selection but with a couple more duplicates and random clicking that may well happen.

While the wiki states the modification id must be unique that's quite some requirement since all addons can add as many modifications as they like... (In the case of Corpse Mod the issue is the suspected duplicate include so easy to fix there though.)

It certainly would be good if wesnoth behaved a little nicer. It does print error config: found modification with id=foo twice but at that point it should ignore the duplicate modification (or something else sensible) and not do random stuff.

[Pentarctagon]

So the fix would be that when populating that list for the UI:

• Ignore modifications with duplicate IDs.
• Print an error to the log/stderr saying it ignored a duplicate for the modification ID each time it encounters a duplicate
  ?

[soliton-]

Yes, I think that would be the simplest fix. Not sure something more sensible could be done really.

[soliton-]

Actually a design that forces unique IDs per addon would be even better. We could prepend the addon ID (addon dir name) to the modification ID. Then there won't be collisions unless the author does it in their own addon.

[Pentarctagon]

Would that potentially break other things? I guess if it's done immediately on loading the modification then it's probably fine internally, but what about allow_modification/disallow_modification/ignore_incompatible_modification/require_modification?

[soliton-]

I would accept either ID (with addon prefix or not) in those places so that it works the same as now but in the future UMC authors could gradually switch to the safer addon prefixed IDs.

[Discontinuum]

I have a feeling most of ID duplication is when the addon coded a bit improperly like Dead Bodies. Maybe ignore the second one? Or can it be done so the behaviour is as now, but UI isn't broken? Dead Bodies doesn't produce 2 mods in the mod list, but it breaks the selection

[soliton-]

For the case of duplicate IDs in the same addon there is no better way than to ignore I'd say. And ideally complain a bit more visibly so the author notices it easier.

[ProditorMagnus]

If the addons are not fixed in reasonable time they need to be deleted.

All add-ons uploaded must actually be functional. This does not mean add-ons must be bug free or that they must be complete, but they cannot:
Cause errors elsewhere, when not playing the add-on's content.

[soliton-]

If there are broken addons feel free to report them but let's not clutter up this issue with that.

[Discontinuum]

Why can't the game just ignore the second addon with the same id?

[soliton-]

Because nobody has implemented that so far.

[gfgtdf]

In case someone wants to work on it:
The problem is probably that when populating the modlist in

wesnoth/src/game_initialization/create_engine.cpp

Line 783 in 8977c1e

{

The code removes dunblicates
But when the other list of populated in

wesnoth/src/game_initialization/depcheck.cpp

Line 76 in 8977c1e

config info;

The code doesn't remove dublicates so the two lists are out of sync.

[gfgtdf]

Of course this is not an ideal solution, one could ask why do we need 2 lists (that can be out of sync) in the first place, and if we have two lists, why don't we at least populate the one list from the other list to make sure they are equal, one could try to refactor the code so that there are no longer two list etc.

[gfgtdf]

After seeing the actual usecases, (which is an addon that includes the same file multiple times via macro inclusion ) i think it would maybe be better if we just showed none of the addons in that case, to make them aware that something is broken.

[soliton-]

Don't show the modification? I don't think that's clearer than a visible message in the GUI on what's wrong. A very simple way would be to add a colored warning in front of the name or something.