Skip to content

Commit

Permalink
Update/fix HasSyncWorker method (#444)
Browse files Browse the repository at this point in the history
This will prevent a bunch of errors (mostly harmless, main downside is that it would allow duplicate sync workers to be registered).

- The method we were called is no longer static, so we need to get the instance of `SyncSerialization`
- `SyncSerialization` was moved from Client to Common, but it does not really matter as we need to get the instance from Client anyway
- Added code to get the field holding `SyncSerialization` instance, with additional checks to make sure it's not null, the field is static, and the actual instance is not null
- Changed the code to access the method to use the type of the field we've accessed, rather than using the full path
- Added code to check if the method is static and show a warning if it is (as we're expecting it to be non-static)
- Simplified error logging by using a local function that will call `Log.Error` with the same text at the beginning, rather than that text being copy-pasted all the time
  • Loading branch information
SokyranTheDragon authored May 24, 2024
1 parent 3700e0d commit 02a3b98
Showing 1 changed file with 36 additions and 6 deletions.
42 changes: 36 additions & 6 deletions Source/MpSyncWorkers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,24 +110,54 @@ private static void SyncDesignationManager(SyncWorker sync, ref DesignationManag

private static bool HasSyncWorker(Type type)
{
const string methodPath = "Multiplayer.Client.SyncSerialization:CanHandle";
const string fieldPath = "Multiplayer.Client.Multiplayer:serialization";
const string methodName = "CanHandle";
void Error(string message) => Log.Error($"Failed to check if sync worker for type {type} is already registered in MP - {message}");

// Don't cache the method, it'll be used very rarely (assuming it'll even be used at all), so there's no point in having a field for it.
var method = AccessTools.DeclaredMethod(methodPath, new[] { typeof(SyncType) });
// Don't cache the field/method, they will be used very rarely (assuming they will even be used at all), so there's no point in having a field for it.
var field = AccessTools.DeclaredField(fieldPath);

if (field == null)
{
Error($"failed to find field {fieldPath}");
return false;
}

if (!field.IsStatic)
{
Error($"field {fieldPath} is not static");
return false;
}

var instance = field.GetValue(null);

if (instance == null)
{
Error($"field {fieldPath} is null");
return false;
}

var method = AccessTools.DeclaredMethod(field.FieldType, methodName, [typeof(SyncType)]);

if (method == null)
{
Log.Error($"Failed to check if sync worker for type {type} is already registered in MP - failed to find method {methodPath}");
Error($"failed to find method {methodName}");
return false;
}

if (method.ReturnType != typeof(bool))
{
Log.Error($"Failed to check if sync worker for type {type} is already registered in MP - return type is not bool but {method.ReturnType} for method {methodPath}");
Error($"return type is not bool but {method.ReturnType} for method {methodName}");
return false;
}

return (bool)method.Invoke(null, new object[] { new SyncType(type) });
if (method.IsStatic)
{
Log.Warning($"Method {methodName} is static, but we expected it to be non-static - the results may potentially be incorrect.");
return (bool)method.Invoke(null, [new SyncType(type)]);
}

return (bool)method.Invoke(instance, [new SyncType(type)]);
}
}
}

0 comments on commit 02a3b98

Please sign in to comment.