Detect/prevent GUI modification/deletion from other mods.

[Creidhne]

Hi,
after hours of investigating a bug report of my mod, I discovered something unexpected: other mods can destroy LuaGuiElement(s) of my mod.

While I could understand that in some situations, it might be convenient for mod A to modify the GUI of mod B, in my situation this is totally unwanted & undesirable. This basically leaves my mod in a corrupted state (causing crashes at some later point, or just letting a player stuck in a surface with no way to leave).

I reported the situation to the author of the mod affecting my GUI, and I'm sure we'll come to a nice solution soon. But I'm afraid this won't be the last time (I've seen other threads of other mods reporting similar issues). Basically I have two possible "solutions" to this problem right now:

• Check on every tick/event that my GUI was not corrupted, and rebuild it if necessary (quite a painful, inefficient & inelegant solution).
• Investigate the next bug reports like this one: find the conflicting mod, and see with its author if he can fix the mod (or mark it as incompatible). Problem is that it still leaves my users with a corrupted game. And the investigation consists in auditing the code of a full modlist (which takes a little too much time for my tastes).

So I was wondering if there was a clever solution to this problem. Ideally some flag on my mod that prevent another mod from modifying/deleting my GUI (I think he should be the one getting an error right when the corruption happens, not me hours after the corruption happened). But any way to find the root cause faster from my side would be much appreciated.

[DaveMcW]

This is not your fault. Mark the mod as incompatible, and publicly shame the mod for messing with your GUI.

[ickputzdirwech]

and publicly shame the mod for messing with your GUI.

I doubt any mod author would do such things on purpose. This behaviour wouldn’t really solve the issue, instead it would discourage other mod authors and hurt the community.

[DaveMcW]

instead it would discourage other mod authors and hurt the community.

That is what messing with other mods' GUI does.

[Creidhne]

No need to shame anybody, the other mod got an update today addressing the issue. Though after looking at the code the "gui.center.clear()" just moved into a if-then statement, so I'm not sure I'll be sleeping very well in the near future.

My original question still stands: how to detect/prevent these kind of things faster ?

[raiguard]

I don't think straight-up preventing the modification of others' GUIs is the right answer. The correct solution would be to improve the documentation around GUIs in general, so people don't (accidentally) abuse the system.

For example, running clear() on one of the root GUIs is obviously a no-go. If a mod is doing that, then people can simply not use that mod until it's fixed.

[Creidhne]

Excellent, I've found another mod doing the same thing. The joy is real.

For example, running clear() on one of the root GUIs is obviously a no-go. If a mod is doing that, then people can simply not use that mod until it's fixed.

The issue is about finding "that mod". If it takes multiple hours for a modder to find the culprit in a modlist, how do you expect the average player to spontaneously not use the mod causing that ?

People will certainly disable a mod until it's fixed. But that'll likely be the one dumping the stacktrace, not "that mod" (note that in my situation, "that mod" isn't exactly a small fish in Factorio's community).

I still think it would be much more convenient if Factorio could make an error like "Mod A is trying to modify Mod B's GUI, but B has forbidden that". This would just make the diagnostic clear for the users and both modders.

[Creidhne]

I doubt any mod author would do such things on purpose.

Well, the mod author I contacted has a different point of view:

If any mods are broken by a player.gui.center.clear() call then those mods should be updated to handle that situation. When a mod uses the center GUI is it usually to take the main focus of the player and it is typical for it to clear that GUI before adding an central window

I guess it's time to inaugurate my incompatibility list.

[Qon]

What mods are we talking about?

[Creidhne]

What mods are we talking about?

First mod was Informatron, which should be fixed from v0.1.12
Second mod is Space Exploration.

[Qon]

Both from the same author...
I don't think Earendel cares about players or modders who don't play just like he wants them to play his mods.

Edit: Informatron now does center.clear() in informatron_0.1.12/scripts/informatron.lua:327 without defining variable center. So his mod is going to crash unless he does somthing weird with strings in the global scope that can't be searched for.
But that's his problem now, he gets the stack trace blame for that.

But the other use of clear() is only on a frame created by his own mod so that should be fine.

Haven't checked SE, only Informatron.

[eradicator]

For example, running clear() on one of the root GUIs is obviously a no-go. If a mod is doing that, then people can simply not use that mod until it's fixed.

Imho clearing the center root isn't that bad. It doesn't leave any "currupted state", just an empty one. It's actually required in some cases where a mod needs to open a center gui because i.e. the player pressed a button. If there's already another gui in center both will look ugly when they're displayed next to each other, and the player expects that only one center gui can be open at a time (same as you can't open your inventory and machine at the same time). Also checking if your own mods root element exists is the same single trivial "if player.gui.center.children['bla'] then" check that you're alreardy doing on almost every other event. I.e. when checking event.entity.name=='bla'. I've actually made a "center gui shaker" once to notify the player that he needs to manually close some other gui first, but that's quite an ugly solution imho.

Manipulating the *inside* of other mods guis is ofc broken. And for top/left/screen/etc there's no "only one" expectation so above doesn't apply, but it's still trivial to do one additional check.

TL;DR: The easiest method to deal with it is to properly check existance and .valid before you try to use a shared resource (gui, rendering, inventory, etc).

[sparr]

Check on every tick/event that my GUI was not corrupted, and rebuild it if necessary (quite a painful, inefficient & inelegant solution).

What if there was a way to get an event when your mod's gui elements are destroyed, so you know when to recreate them?

[sparr]

a player stuck in a surface with no way to leave

Consider moving critical "button" GUI elements from the main GUI to the user-configurable list of buttons to the right of the quickbar? I've seen a few mods doing this and I love it.

[raiguard]

Imho clearing the center root isn't that bad.

Please, never ever ever ever ever ever ever EVER just straight-up destroy another mod's GUI. It's up to that mod to handle their GUI properly (using player.opened) if they want that behavior. Messing with another mod's GUI is not something to be encouraged for any reason.

[eradicator]

Imho clearing the center root isn't that bad.

Please, never ever ever ever ever ever ever EVER just straight-up destroy another mod's GUI. It's up to that mod to handle their GUI properly (using player.opened) if they want that behavior. Messing with another mod's GUI is not something to be encouraged for any reason.

I'm not aware of any friendlier mechanism to tell other mods that they need to close their gui *right now*. My own stuff is ofc designed under the assumption that destroying the root is the normal way for others mods to close my gui. What other method do you propose? And please don't tell me i'm supposed to contact every single mod author to request a remote call interface...

[sparr]

tell other mods that they need to close their gui *right now*.

Why do you think this is something you should be able to tell them?

[eradicator]

tell other mods that they need to close their gui *right now*.

Why do you think this is something you should be able to tell them?

Because if i for example have a gui that is opened by pressing AButton. Then when the player presses AButton the gui needs to open. Entity guis are trivial to properly close by setting opened=nil.

[raiguard]

I'm not aware of any friendlier mechanism to tell other mods that they need to close their gui *right now*. My own stuff is ofc designed under the assumption that destroying the root is the normal way for others mods to close my gui. What other method do you propose? And please don't tell me i'm supposed to contact every single mod author to request a remote call interface...

If those mods aren't properly using player.opened then that's on them, and they need to fix it. All you're doing by destroying others' GUIs is messing up their state and causing potential problems. For example, if you closed one of my mods' GUIs it would cause all sorts of crashes because I expect the GUI to be there. And I'm not going to add protections for every single event to make sure of that fact.

Messing with my GUIs is a sure-fire ticket to getting marked as incompatible. I'm sure many other people feel the same way.

[Creidhne]

Manipulating the *inside* of other mods guis is ofc broken.

TL;DR: The easiest method to deal with it is to properly check existance and .valid before you try to use a shared resource (gui, rendering, inventory, etc).

If you trully consider the GUIs as a shared resource, then it heavily contradicts with your 1st statement (which is pretty much the definition of an exclusive resource).

The shared resource here is the screen space. And I think the proper way to share it is to simply ... share it. I won't unilaterally decide that whatever I'm displaying is so important that it deserves to clear other mod's UI into oblivion. The killed GUI might be a vital choice in a scripted scenario (respawn menu, surface-switching logic) which are only displayed for specific events, and don't have any button to make them reappear. Or some other mod might have also killed your menu button, because apparently killing other's GUI is fine.

Also there is no universal "easiest method". The problem here is a conflict between modders needs/philosophy. The "easiest method" for modders A is to tell modders B to stop doing their stuff, and inversely. Moreover "valid" doesn't adress the issue in my 1st post. It doesn't prevent GUI modification, and doesn't detect them either (unless you run it every N tick, but it's ugly and still doesn't tell you which mod caused the mess).

What if there was a way to get an event when your mod's gui elements are destroyed, so you know when to recreate them?

If I rebuild GUIs whenever other mods delete them, what would be the point of letting them delete it in the 1st place ?

This "solution" would just be an inefficient way to treat a symptom. The core problem is that any mod has full access to the GUI of other mods, and I'm pretty confident that there are exactly 0 mods which fully support arbitrary modifications of their GUIs: at best it'll make their GUI ugly/unusable, but going to full crashes or soft-lock isn't science fiction.

It seems there is a high agreement that arbitrary modifications of other's GUI without prior agreement is a bad practice. For now the only "valid" usage (note that the validity was just proclaimed by a subset of modders of unknown size, relative to a subset of modders of unknown size who proclaimed it as invalid), would be to clear the center root GUI whenever some modder has unilaterally decided that whatever he wants to display is "obsiously" more important than whatever those other mods want to display.

So if a Factorio's developer were to drop here and take some time to implement new things to adress the issue, I would prefer that they fix directly the core problem.

• Define a precise list of actions that every mod should support (for example it could include clear/hide elements of a Gui root, though I think that's the wrong solution to the center war). Add the API calls that does only that, and events for mods to handle that cleanly.
• Add a protected mode for mods which don't allow any other external modification. This protected mode could be mod-wide, or specific to a GUI element and its children. Anything is better than nothing. Error message should contain the name of the mod owning the protected GUI, and the mod attempting the illegal modification.

Consider moving critical "button" GUI elements [to] the right of the quickbar

That's assuming no other modder decide to clear stuff on the right of the quickbar. As long as some modders have decided that they deserve exclusive access to any part of the GUI, nowhere's safe...