Skip to content

Commit d3b0567

Browse files
gotmachinegotmachine
committed
Patch splitted and ported over from PR #118 
1 parent 794335c commit d3b0567

3 files changed

Lines changed: 181 additions & 1 deletion

File tree

‎GameData/KSPCommunityFixes/Settings.cfg‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,11 @@ KSP_COMMUNITY_FIXES
496496
// To use add `Description` attribute to the field.
497497
KSPFieldEnumDesc = false
498498
499+
// Allow dynamically defining additional BaseFields on a Part or PartModule and having the backing
500+
// field for that BaseField in another class / instance than the targetted Part or Module. Look for
501+
// the examples and documentation in the patch source.
502+
BaseFieldListUseFieldHost = true
503+
499504
// ##########################
500505
// Localization tools
501506
// ##########################

‎KSPCommunityFixes/KSPCommunityFixes.csproj‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<?xml version="1.0" encoding="utf-8"?>
1+
<?xml version="1.0" encoding="utf-8"?>
22
<Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
33
<Import Project="..\packages\Krafs.Publicizer.2.2.1\build\Krafs.Publicizer.props" Condition="Exists('..\packages\Krafs.Publicizer.2.2.1\build\Krafs.Publicizer.props')" />
44
<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
@@ -155,6 +155,7 @@
155155
<Compile Include="Library\LocalizationUtils.cs" />
156156
<Compile Include="Library\Numerics.cs" />
157157
<Compile Include="Library\ObjectPool.cs" />
158+
<Compile Include="Modding\BaseFieldListUseFieldHost.cs" />
158159
<Compile Include="Modding\KSPFieldEnumDesc.cs" />
159160
<Compile Include="Modding\ModUpgradePipeline.cs" />
160161
<Compile Include="Performance\ForceSyncSceneSwitch.cs" />

‎KSPCommunityFixes/Modding/BaseFieldListUseFieldHost.cs‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
using HarmonyLib;
2+
using System;
3+
4+
/*
5+
The purpose of this patch if to allow BaseField and associated features (PAW controls, persistence, etc) to work when
6+
a custom BaseField is added to a BaseFieldList (ie, a Part or PartModule) with a host instance other than the BaseFieldList
7+
owner. This allow to dynamically add fields defined in another class to a Part or PartModule and to benefit from all the
8+
associated KSP sugar :
9+
- PAW UI controls
10+
- Value and symmetry events
11+
- Automatic persistence on the Part/PartModule hosting the BaseFieldList
12+
13+
The whole thing seems actually designed with such a scenario in mind, but for some reason some BaseField and BaseFieldList
14+
methods are using the BaseFieldList.host instance instead of the BaseField.host instance (as for why BaseFieldList has a
15+
"host" at all, I've no idea and this seems to be a design oversight). There is little to no consistency in which host
16+
reference is used, they are even sometimes mixed in the same method. For example, BaseFieldList.Load() uses BaseFieldList.host
17+
in its main body, then calls BaseFieldList.SetOriginalValue() which is relying on BaseField.host.
18+
19+
Changing every place where a `host` reference is acquired to ensure the BaseField.host reference is used allow to use a custom
20+
host instance, and shouldn't result in any behavior change. This being said, the stock code can theoretically allow a plugin
21+
to instantiate a custom BaseField with a null host and have it kinda functional if that field is only used to SetValue() /
22+
Getvalue() and as long as the field isn't persistent and doesn't have any associated UI_Control. This feels like an extremly
23+
improbable scenario, so this is probably fine.
24+
*/
25+
26+
namespace KSPCommunityFixes.Modding
27+
{
28+
[PatchPriority(Order = 0)]
29+
class BaseFieldListUseFieldHost : BasePatch
30+
{
31+
private static AccessTools.FieldRef<object, Callback<object>> BaseField_OnValueModified_FieldRef;
32+
33+
protected override Version VersionMin => new Version(1, 12, 3);
34+
35+
protected override void ApplyPatches()
36+
{
37+
BaseField_OnValueModified_FieldRef = AccessTools.FieldRefAccess<Callback<object>>(typeof(BaseField<KSPField>), nameof(BaseField<KSPField>.OnValueModified));
38+
if (BaseField_OnValueModified_FieldRef == null)
39+
throw new MissingFieldException($"BaseFieldListUseFieldHost patch could not find the BaseField.OnValueModified event backing field");
40+
41+
// Note : fortunately, we don't need to patch the generic BaseField.GetValue<T>(object host) method because it calls
42+
// the non-generic GetValue method
43+
AddPatch(PatchType.Override, AccessTools.FirstMethod(typeof(BaseField<KSPField>), (m) => m.Name == nameof(BaseField<KSPField>.GetValue) && !m.IsGenericMethod), nameof(BaseField_GetValue_Override));
44+
45+
AddPatch(PatchType.Override, typeof(BaseField<KSPField>), nameof(BaseField<KSPField>.SetValue), nameof(BaseField_SetValue_Override));
46+
47+
// BaseField.Read() is a public method called from :
48+
// - BaseFieldList.Load()
49+
// - BaseFieldList.ReadValue() (2 overloads)
50+
// The method is really tiny so there is a potential inlining risk (doesn't happen in my tests, but this stuff can be platform
51+
// dependent). It's only really critical to have BaseFieldList.Load() being patched, the ReadValue() methods are unused in the
52+
// stock codebase, and it is doubtfull anybody would ever call them.
53+
AddPatch(PatchType.Override, typeof(BaseField), nameof(BaseField.Read));
54+
55+
// We also patch BaseFieldList.Load() because :
56+
// - Of the above mentioned inlining risk
57+
// - Because it pass the (arguably wrong) host reference to the UI_Control.Load() method, and even though none
58+
// of the various overloads make use of that argument we might want to be consistent.
59+
AddPatch(PatchType.Override, typeof(BaseFieldList), nameof(BaseFieldList.Load));
60+
}
61+
62+
static object BaseField_GetValue_Override(BaseField<KSPField> instance, object host)
63+
{
64+
try
65+
{
66+
// In case the field host is null, use the parameter
67+
if (ReferenceEquals(instance._host, null))
68+
return instance._fieldInfo.GetValue(host);
69+
70+
// Uses the field host reference instead of the reference passed as a parameter
71+
return instance._fieldInfo.GetValue(instance._host);
72+
}
73+
catch
74+
{
75+
PDebug.Error("Value could not be retrieved from field '" + instance._name + "'");
76+
return null;
77+
}
78+
}
79+
80+
static bool BaseField_SetValue_Override(BaseField<KSPField> instance, object newValue, object host)
81+
{
82+
try
83+
{
84+
// In case the field host is null, use the parameter
85+
if (ReferenceEquals(instance._host, null))
86+
instance._fieldInfo.SetValue(host, newValue);
87+
88+
// Uses the field host reference instead of the reference passed as a parameter
89+
instance._fieldInfo.SetValue(instance._host, newValue);
90+
91+
// Note : since BaseField.OnValueModified is a "field-like event", it is relying on a compiler-generated
92+
// private backing field, and the public event "synctatic suger" member can only be invoked from the declaring
93+
// class, so we can't directly call "__instance.OnValueModified()" here. Additionally, the compiler has the extremly
94+
// bad taste to name the backing field "OnValueModified" too, resulting in an "ambiguous reference" if we try to use
95+
// it through the publicized assembly. So we have to resort to creating a FieldRef open delegate for that backing field.
96+
BaseField_OnValueModified_FieldRef(instance).Invoke(newValue);
97+
return true;
98+
}
99+
catch (Exception ex)
100+
{
101+
PDebug.Error(string.Concat("Value '", newValue, "' could not be set to field '", instance._name, "'"));
102+
PDebug.Error(ex.Message + "\n" + ex.StackTrace + "\n" + ex.Data);
103+
return false;
104+
}
105+
}
106+
107+
static void BaseField_Read_Override(BaseField instance, string value, object host)
108+
{
109+
if (ReferenceEquals(instance._host, null))
110+
BaseField.ReadPvt(instance._fieldInfo, value, host);
111+
112+
BaseField.ReadPvt(instance._fieldInfo, value, instance._host);
113+
}
114+
115+
static void BaseFieldList_Load_Override(BaseFieldList instance, ConfigNode node)
116+
{
117+
for (int i = 0; i < node.values.Count; i++)
118+
{
119+
ConfigNode.Value value = node.values[i];
120+
BaseField baseField = instance[value.name];
121+
if (baseField == null || baseField.hasInterface || baseField.uiControlOnly)
122+
continue;
123+
124+
object host = ReferenceEquals(baseField._host, null) ? instance.host : baseField._host;
125+
126+
// The original code calls BaseField.Read() here. We bypass it to avoid
127+
// any inlining risk and call directly the underlying static method.
128+
BaseField.ReadPvt(baseField._fieldInfo, value.value, host);
129+
130+
if (baseField.uiControlFlight.GetType() != typeof(UI_Label))
131+
{
132+
ConfigNode controlNode = node.GetNode(value.name + "_UIFlight");
133+
if (controlNode != null)
134+
baseField.uiControlFlight.Load(controlNode, host);
135+
}
136+
else if (baseField.uiControlEditor.GetType() != typeof(UI_Label))
137+
{
138+
ConfigNode controlNode = node.GetNode(value.name + "_UIEditor");
139+
if (controlNode != null)
140+
baseField.uiControlEditor.Load(controlNode, host);
141+
}
142+
}
143+
for (int j = 0; j < node.nodes.Count; j++)
144+
{
145+
ConfigNode configNode = node.nodes[j];
146+
BaseField baseField = instance[configNode.name];
147+
if (baseField == null || !baseField.hasInterface || baseField.uiControlOnly)
148+
continue;
149+
150+
object host = ReferenceEquals(baseField._host, null) ? instance.host : baseField._host;
151+
152+
object value = baseField.GetValue(host);
153+
if (value == null)
154+
continue;
155+
156+
(value as IConfigNode)?.Load(configNode);
157+
158+
if (baseField.uiControlFlight.GetType() != typeof(UI_Label))
159+
{
160+
ConfigNode controlNode = node.GetNode(configNode.name + "_UIFlight");
161+
if (controlNode != null)
162+
baseField.uiControlFlight.Load(controlNode, host);
163+
}
164+
else if (baseField.uiControlEditor.GetType() != typeof(UI_Label))
165+
{
166+
ConfigNode controlNode = node.GetNode(configNode.name + "_UIEditor");
167+
if (controlNode != null)
168+
baseField.uiControlEditor.Load(controlNode, host);
169+
}
170+
}
171+
instance.SetOriginalValue();
172+
}
173+
}
174+
}

0 commit comments

Comments
 (0)