Suggestion General UI Widget Modification

Currently viewing this thread:

Aragas

Recruit
M&BWBWF&SNW
In the Beta e1.5.9, the modding community found out that most of the UI Widgets are now auto-generated in C# instead of being parsed from their XML definitions in the runtime.
The game has an API to switch to the previous behavior, but the .xml definitions are not the same as the auto-generated files (at least some we tested in e1.5.9), so the game throws an error, because it expects a different content in the XML files when we try to switch to the old behavior.

From our modding perspective, this is a huge backstep. The safest way for us to mod the UI was to change the XML, preventing many possible runtime issues that can now happen when we need to patch directly the autogens. Also, because they are auto-generated, most likely the patches will be broken each game update, when the devs will be re-running the auto-generation and update them.

Hardcoding the widgets will limit our modding capability. While we still can patch them individually, it will require knowledge of both reverse-engineering and potentially IL patching, whether it's a tool like Harmony or low level stuff like MonoMod/Mono.Cecil.
This will lead to a huge decrease of the quality of mods doing changing the UI, as the curve of learning how to edit the XML is nothing compared to the curve of patching C# code.

We don't understand the reasoning for the auto-generation. We suspect this is done to speed-up the Widget creation and make its memory footprint smaller, but in our opinion, this is not worth the limitation. At least without having an opt-out option!

As we understand, this is what the devs aim to:
By default, all Widgets are created from the auto-generated classes, but once a mod wants to change it, it requests via the API to switch to the XML generation to enable us the ability for modification. This way, unless needed, the game always uses the optimized autogens.
If we are correct in our understanding, his would be a nice compromise for the current situation!


We also would appreciate if you could release the Widget generator so we could use it to make our custom UI less resource demanding!

As a bonus, here is an example of how we use the API introduced in e1.5.9 right now via patching by Harmony:
C#:
internal static class GauntletMoviePatch
{
    public static void Patch(Harmony harmony)
    {
        harmony.Patch(
            AccessTools.Method("TaleWorlds.GauntletUI.Data.GauntletMovie:Load"),
            prefix: new HarmonyMethod(AccessTools.Method(typeof(GauntletMoviePatch), nameof(LoadPrefix))));
    }

    private static void LoadPrefix(string movieName, ref bool doNotUseGeneratedPrefabs)
    {
        if (movieName == "SettlementNameplate") // Widget name that will switch back to loading the XML
            doNotUseGeneratedPrefabs = true;
    }
}
 

Bloc

Duke
WB
This is an important problem indeed. And this approach is not a step towards "extending modding support".

We don't understand the reasoning for the auto-generation. We suspect this is done to speed-up the Widget creation and make its memory footprint smaller, but in our opinion, this is not worth the limitation. At least without having an opt-out option!
I also don't understand why they can't have both
i.e. something like this before loading should work in theory
C#:
if ( StaticXMLExist() ){
 // Don't bother with auto generated
} else {
// Check auto generated
}
Obviously, this is kiddie-code but it's clear what I mean.
 

Aragas

Recruit
M&BWBWF&SNW
Serialization of the XML to a generic Widget class at runtime and auto-generating it in compile-time differ a lot in performance, but it's seems more of a 'quick fix'. Optimizing the generic serialization logic is harder and more expensive.
 

emrozdemir

Developer
The game has an API to switch to the previous behavior, but the .xml definitions are not the same as the auto-generated files (at least some we tested in e1.5.9), so the game throws an error, because it expects a different content in the XML files when we try to switch to the old behavior.
I'm not sure what you mean with this. The code is generated from the xml and it goes through multiple automated generation stages before the deployment. So neither the xml or the generated code is outdated. I think this is happening not because the "SettlementNameplates" xml's content is different than the generated code, it's because only the "SettlementNameplate" is using the xml and the rest of the game is using the generated code.

Also, because they are auto-generated, most likely the patches will be broken each game update, when the devs will be re-running the auto-generation and update them.
This will still be the case if we only used the xml. Since if we change related stuff in the xml like type names, hierarchies, Brush/Sprite names etc.(depending on how the mod accesses/modifies them). If there are no changes to the xml, there will be no changes in the generated code.

We don't understand the reasoning for the auto-generation. We suspect this is done to speed-up the Widget creation and make its memory footprint smaller, but in our opinion, this is not worth the limitation. At least without having an opt-out option!
In some cases like Mono, where the optimizations on the Reflection system is not supported by the OS, the performance increase were nearly 10x. And in normal Windows/PC usages we're no longer bottlenecked by the reflection and now rather bottlenecked by the GC and Tableau init(in Debug inventory open for example, for all the items in the game) so increases in performance in those cases will continue. The performance increases we gained from this is not negligible.

All in all though, we've already added a way to disable this system through user_config and added a way to disable it through code.

For user_config, modders can add ui_do_not_use_generated_prefabs = 1 line to user_config.txt under "SteamLibrary\steamapps\common\Mount & Blade II Bannerlord\user_config.txt". If the .txt doesn't exist, you can create a new one.

For UIConfig, it should already be live in 1.5.9, the modders can set the boolean UIConfig.DoNotUseGeneratedPrefabs to true in module load, and that's it. We've had the user_config boolean since the UI code gen rollout but after that, we've seen modders' request of being able to enable/disable both the UIDebugMode(hot xml reload) and GeneratedPrefabsCode from the code so we've added these changes to support modders.

i.e. something like this before loading should work in theory
I don't think it would work since we've always sent our screen prefab xmls to shipping builds for modders to see how we do things and tweak/add their own changes themselves. So "StaticXMLExist" will always be true since they exist in the directories, it'll never use generated prefabs in this case.
 

Bloc

Duke
WB
I don't think it would work since we've always sent our screen prefab xmls to shipping builds for modders to see how we do things and tweak/add their own changes themselves. So "StaticXMLExist" will always be true since they exist in the directories, it'll never use generated prefabs in this case.
If StaticXMLExist checks entire solution/game then yes. But it can be configured in a way that it can only check non-official folders, right?
C#:
StaticXMLExist() {
   // Get all submodules
   // Check by load order with LIFO manner - if you already reached to official ones with pop-op, simply use autogenerated since rest of the list ( around 2-3 other modules ) will be official anyway
}
And again, this is not even pseudo-code and I'm not entirely sure its impact on performance but it can't be worse than what we have currently. It can be worse than reading autogenerated one ofc.
 

emrozdemir

Developer
If StaticXMLExist checks entire solution/game then yes. But it can be configured in a way that it can only check non-official folders, right?
1. Would be a bad change imho. We don't treat any of the modules differently i.e if another module has a GUI prefab xml with same name it overrides the previous. No special module treatment. Not treating any module differently, makes for a simpler code. Adding definitions like "GetIfNonOfficialXMLExists" personally irks me.
2. The code generations are not one-to-one for xmls. They're generated as complete screens. Overriding a sub xml like "Standard.TriplePopupCloseButtons" will not result in a change since that is already generated inside of let's say "InventoryScreen". So the loading of generated prefabs need to be disabled during loading of most-parent prefab(movie). All in all, even if a module overrides a sub-xml, the generated code system needs to be disabled doesn't matter if an xml is overriden or not. Since that sub xml prefab might be used in other screens as well.
 

Aragas

Recruit
M&BWBWF&SNW
This will still be the case if we only used the xml. Since if we change related stuff in the xml like type names, hierarchies, Brush/Sprite names etc.(depending on how the mod accesses/modifies them). If there are no changes to the xml, there will be no changes in the generated code.
Agree.
C# is adding another potential breakage by using variable names. Depending on the generator, they might differ slightly, like elements in the same hierarchy level switching numbers. This is still theoretical and unproved anyway.

I'm not sure what you mean with this. The code is generated from the xml and it goes through multiple automated generation stages before the deployment. So neither the xml or the generated code is outdated. I think this is happening not because the "SettlementNameplates" xml's content is different than the generated code, it's because only the "SettlementNameplate" is using the xml and the rest of the game is using the generated code.
Yea, it wasn't checked whether it's needed to do the switching to XML in cascade. This will complicate things, because we'll need to know the affected Widgets all the way down.

I'm not even sure here. Is the issue only when XML references autogens or when autogens reference XML's too?
Would it be possible to make autogens and xml widgets work with each other? So cases like when you need to switch a specific autogen won't trigger cascade switching of everything.

It's great to hear that we can potentially disable autogens globally, but considering that we can have a dotnet framework, dotnet core and mono runtimes, we wouldn't want to disable such optimizations just because it's easier for us to change stuff!
 

emrozdemir

Developer
I'm not even sure here. Is the issue only when XML references autogens or when autogens reference XML's too?
Currently they're separate. The code is generated from the XML but after that there is no connection between the two. We cannot use XML in generated code and generated code cannot use XML.

Some of the issues with switching between XML and generated code in the same movie are,

1. The code gen doesn't work like that. The generated code needs to have/know all the types and instances while generation. Essentially it needs to say "this string variable in this TextWidget is binded to this string variable in SPInventoryVM(for example)". And it needs to say this like "_textWidget.Text = _dataSource.HeroLabelName" for example. So it needs to know the specific type, instance and variable name. If it cannot know it, needs to get it in runtime, and that's reflection, what we're trying to avoid. So it goes from most parent to bottom, not individually separate.

2. The generated code needs to have direct references to the instances it is creating. Since it needs to add children, set it's ids, set it's visual properties like Size, Sprite etc. So it needs to say, "Ah, I'll be adding a TextWidget FooBar and it's size is this, it'll have this as a Brush and it'll have this amount of Children etc." Changing that into a different widget with an overriding xml and adding new children or changing it's type, will break other references to FooBar and it's children. In the generated code FooBar might have 3 children and a widget higher up might be referencing it's second child like
FooBar.AddChild(WantedChildWidget);
HigherUpWidget.WantedChildWidget = WantedChildWidget as ListPanel;
So even though the FooBar and WantedChildWidget is overriden from the xml, they're referenced in the code. This will result in an error.

There are more issues like these but I don't want to make this post too long.

There are still ways to override the screen and use the code generation system. Since the code generation is essentially compiling the screen xml and converting that into .cs files. You can make your changes to the XMLs, generate the screen prefab .cs file and pack it in your .dll in your module. The GeneratedPrefabContext collects all the generated prefabs in all the loaded assemblies so it'll use the latest generated prefab that is added. So previously, modders copied a native xml to their modules to override the previous one, and made their changes on the copied xml. Now there is one more step, just running the code generator. (If the code generation is disabled in UIConfig, there is still no need for that). You can look at how we generate the code for these screens in the "TaleWorlds.MountAndBlade.GauntletUI.CodeGenerator.exe" and try calling thse methods your own and creating your own generated prefabs.
 

Aragas

Recruit
M&BWBWF&SNW
Some of the issues with switching between XML and generated code in the same movie are,

1. The code gen doesn't work like that. The generated code needs to have/know all the types and instances while generation. Essentially it needs to say "this string variable in this TextWidget is binded to this string variable in SPInventoryVM(for example)". And it needs to say this like "_textWidget.Text = _dataSource.HeroLabelName" for example. So it needs to know the specific type, instance and variable name. If it cannot know it, needs to get it in runtime, and that's reflection, what we're trying to avoid. So it goes from most parent to bottom, not individually separate.

2. The generated code needs to have direct references to the instances it is creating. Since it needs to add children, set it's ids, set it's visual properties like Size, Sprite etc. So it needs to say, "Ah, I'll be adding a TextWidget FooBar and it's size is this, it'll have this as a Brush and it'll have this amount of Children etc." Changing that into a different widget with an overriding xml and adding new children or changing it's type, will break other references to FooBar and it's children. In the generated code FooBar might have 3 children and a widget higher up might be referencing it's second child like
FooBar.AddChild(WantedChildWidget);
HigherUpWidget.WantedChildWidget = WantedChildWidget as ListPanel;
So even though the FooBar and WantedChildWidget is overriden from the xml, they're referenced in the code. This will result in an error.

There are more issues like these but I don't want to make this post too long.
Understandable.
We did had a little discussion with our team about the runtime cost of reflection. Because the serialized XML widget can't change, any reflection action could be cached, bypassing the speed decrease.
We use this technique a lot, creating delegates for property getters/setters or methods and caching them in our static/instance constructors and calling them without any overhead!

Currently they're separate. The code is generated from the XML but after that there is no connection between the two. We cannot use XML in generated code and generated code cannot use XML.
It boils down to the auto generated Widgets being the smallest units of UI modification.
If a modder wants to change something inside of an auto generated Widget, perhaps some small inner Widget that is a separate XML file - he will need to indicate to the game factory that it switches to the old behavior when loading the widget, then do the XML magic. This seems quite acceptable!

We will investigate how we could, for example, compile and cache the final Widget result after patching to match the AutoGens performance and basically follow your behavior, but at runtime instead of compile time, because of the dynamic patches. This does look like an interesting step to take and an interesting challenge! 😁
 
I just discovered that for multiple mods which assign UIConfig.DoNotUseGeneratedPrefabs, the value of the last mod in the load order will be used. This will cause conflicts with other mods higher up in the load order which have a different value for UIConfig.DoNotUseGeneratedPrefabs.
 

Aragas

Recruit
M&BWBWF&SNW
I just discovered that for multiple mods which assign UIConfig.DoNotUseGeneratedPrefabs, the value of the last mod in the load order will be used. This will cause conflicts with other mods higher up in the load order which have a different value for UIConfig.DoNotUseGeneratedPrefabs.
You can use a Harmony prefix in the property getter of UIConfig.DoNotUseGeneratedPrefabs that will block any further attempts of editing the property as a workaround
 
Top Bottom