Skip to content
This repository has been archived by the owner on Feb 28, 2024. It is now read-only.

debug log changes #39

Merged
merged 2 commits into from
May 10, 2022
Merged

debug log changes #39

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b0b34b4
debug log changes
zkxs May 9, 2022
8d44fc1
Allow object logs to DebugFunc()
zkxs May 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion NeosModLoader/AssemblyLoader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ internal static class AssemblyLoader
Assembly assembly;
try
{
Logger.DebugInternal($"load assembly {filename}");
Logger.DebugFuncInternal(() => $"load assembly {filename}");
assembly = Assembly.LoadFile(filepath);
}
catch (Exception e)
Expand Down
4 changes: 2 additions & 2 deletions NeosModLoader/DebugInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ internal static void Log()
{
Logger.MsgInternal($"NeosModLoader v{ModLoader.VERSION} starting up!{(ModLoaderConfiguration.Get().Debug ? " Debug logs will be shown." : "")}");
Logger.MsgInternal($"CLR v{Environment.Version}");
Logger.DebugInternal($"Using .NET Framework: \"{AppDomain.CurrentDomain.SetupInformation.TargetFrameworkName}\"");
Logger.DebugInternal($"Using .NET Core: \"{Assembly.GetEntryAssembly()?.GetCustomAttribute<TargetFrameworkAttribute>()?.FrameworkName}\"");
Logger.DebugFuncInternal(() => $"Using .NET Framework: \"{AppDomain.CurrentDomain.SetupInformation.TargetFrameworkName}\"");
Logger.DebugFuncInternal(() => $"Using .NET Core: \"{Assembly.GetEntryAssembly()?.GetCustomAttribute<TargetFrameworkAttribute>()?.FrameworkName}\"");
Logger.MsgInternal($"Using Harmony v{GetAssemblyVersion(typeof(HarmonyLib.Harmony))}");
Logger.MsgInternal($"Using BaseX v{GetAssemblyVersion(typeof(BaseX.floatQ))}");
Logger.MsgInternal($"Using FrooxEngine v{GetAssemblyVersion(typeof(FrooxEngine.IComponent))}");
Expand Down
2 changes: 1 addition & 1 deletion NeosModLoader/JsonConverters/EnumConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public override object ReadJson(JsonReader reader, Type objectType, object? exis
Type underlyingType = Enum.GetUnderlyingType(objectType);
if (TryConvert(reader!.Value!, underlyingType, out object? deserialized))
{
Logger.DebugInternal($"Deserializing a BaseX type: {objectType} from a {reader!.Value!.GetType()}");
Logger.DebugFuncInternal(() => $"Deserializing a BaseX type: {objectType} from a {reader!.Value!.GetType()}");
return deserialized!;
}

Expand Down
28 changes: 25 additions & 3 deletions NeosModLoader/Logger.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using BaseX;
using System;
using System.Diagnostics;
using System.Reflection;

Expand All @@ -9,25 +10,46 @@ internal class Logger
// logged for null objects
internal readonly static string NULL_STRING = "null";

internal static bool IsDebugEnabled()
{
return ModLoaderConfiguration.Get().Debug;
}

internal static void DebugFuncInternal(Func<string> messageProducer)
{
if (IsDebugEnabled())
{
LogInternal(LogType.DEBUG, messageProducer());
}
}

internal static void DebugFuncExternal(Func<string> messageProducer)
{
if (IsDebugEnabled())
{
LogInternal(LogType.DEBUG, messageProducer(), SourceFromStackTrace());
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I very much dislike the code duplication of this, but then I guess it's kind of a necessary evil, as they could deviate more in the future, and passing bool arguments isn't good either.

Copy link
Collaborator Author

@zkxs zkxs May 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the methods are a bit ugly, I agree, but I can't think of a better way of dealing with it. The good news is this crusty API is internal so we can always refactor it later if we have any ideas.


internal static void DebugInternal(string message)
{
if (ModLoaderConfiguration.Get().Debug)
if (IsDebugEnabled())
{
LogInternal(LogType.DEBUG, message);
}
}

internal static void DebugExternal(object message)
{
if (ModLoaderConfiguration.Get().Debug)
if (IsDebugEnabled())
{
LogInternal(LogType.DEBUG, message, SourceFromStackTrace());
}
}

internal static void DebugListExternal(object[] messages)
{
if (ModLoaderConfiguration.Get().Debug)
if (IsDebugEnabled())
{
LogListInternal(LogType.DEBUG, messages, SourceFromStackTrace());
}
Expand Down
7 changes: 5 additions & 2 deletions NeosModLoader/ModConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,9 @@ private static JsonSerializer CreateJsonSerializer()
};
List<JsonConverter> converters = new();
IList<JsonConverter> defaultConverters = settings.Converters;
if (defaultConverters != null)
if (defaultConverters != null && defaultConverters.Count() != 0)
{
Logger.DebugInternal($"Using {defaultConverters.Count()} default json converters");
Logger.DebugFuncInternal(() => $"Using {defaultConverters.Count()} default json converters");
converters.AddRange(defaultConverters);
}
converters.Add(new EnumConverter());
Expand Down Expand Up @@ -527,6 +527,9 @@ public void Save(bool saveDefaultValues = false)

// I actually cannot believe I have to truncate the file myself
file.SetLength(file.Position);
file.Flush();

Logger.DebugFuncInternal(() => $"Saved ModConfiguration for \"{LoadedNeosMod.NeosMod.Name}\""); // todo: add timekeeping from PR #38
}

private void FireConfigurationChangedEvent(ModConfigurationKey key, string? label)
Expand Down
4 changes: 2 additions & 2 deletions NeosModLoader/ModLoader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ internal static void LoadMods()
else if (config.Debug)
{
string owner = owners.FirstOrDefault();
Logger.DebugInternal($"method \"{patchedMethod.FullDescription()}\" has been patched by \"{owner}\"");
Logger.DebugFuncInternal(() => $"method \"{patchedMethod.FullDescription()}\" has been patched by \"{owner}\"");
}
}
}
Expand Down Expand Up @@ -195,7 +195,7 @@ private static string TypesForOwner(Patches patches, string owner)
private static void HookMod(LoadedNeosMod mod)
{
SplashChanger.SetCustom($"Starting mod [{mod.NeosMod.Name}/{mod.NeosMod.Version}]");
Logger.DebugInternal($"calling OnEngineInit() for [{mod.NeosMod.Name}]");
Logger.DebugFuncInternal(() => $"calling OnEngineInit() for [{mod.NeosMod.Name}]");
try
{
mod.NeosMod.OnEngineInit();
Expand Down
2 changes: 2 additions & 0 deletions NeosModLoader/NeosMod.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ namespace NeosModLoader
// contains members that only the modloader or the mod itself are intended to access
public abstract class NeosMod : NeosModBase
{
public static bool IsDebugEnabled() => Logger.IsDebugEnabled();
public static void DebugFunc(Func<string> messageProducer) => Logger.DebugFuncExternal(messageProducer);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the DebugFunc could be generalized a bit more, allowing any return types instead of just strings? And perhaps DebugFuncExternal could skip the debugging if null is returned, allowing that to be used as a shorthand for just running debugging functions without needing for it to be logged?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could see allowing object parameters, but I don't think dropping nulls is what people would want from a Debug log API. For example, if someone is trying to do a quick and dirty DebugFunc(() => someFrooxObject) and that object is null, I think they'd much rather have the null logged than silently dropped.

public static void Debug(string message) => Logger.DebugExternal(message); // needed for binary compatibility (REMOVE IN NEXT MAJOR VERSION)
public static void Debug(object message) => Logger.DebugExternal(message);
public static void Debug(params object[] messages) => Logger.DebugListExternal(messages);
Expand Down
6 changes: 3 additions & 3 deletions NeosModLoader/NeosVersionReset.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ private static bool SpoofCompatibilityHash(Engine engine)
int? vanillaProtocolVersionMaybe = GetVanillaProtocolVersion();
if (vanillaProtocolVersionMaybe is int vanillaProtocolVersion)
{
Logger.DebugInternal($"Vanilla protocol version is {vanillaProtocolVersion}");
Logger.DebugFuncInternal(() => $"Vanilla protocol version is {vanillaProtocolVersion}");
vanillaCompatibilityHash = CalculateCompatibilityHash(vanillaProtocolVersion);
return SetCompatibilityHash(engine, vanillaCompatibilityHash);
}
Expand Down Expand Up @@ -101,7 +101,7 @@ private static bool SetCompatibilityHash(Engine engine, string Target)
}
else
{
Logger.DebugInternal($"Changing compatibility hash from {engine.CompatibilityHash} to {Target}");
Logger.DebugFuncInternal(() => $"Changing compatibility hash from {engine.CompatibilityHash} to {Target}");
field.SetValue(engine, Target);
return true;
}
Expand All @@ -120,7 +120,7 @@ private static bool SpoofVersionString(Engine engine)
Logger.WarnInternal("Unable to write Engine._versionString");
return false;
}
Logger.DebugInternal($"Changing version string from {engine.VersionString} to {target}");
Logger.DebugFuncInternal(() => $"Changing version string from {engine.VersionString} to {target}");
field.SetValue(engine, target);
}
return true;
Expand Down