Skip to content

Throw assert failure exceptions on development build #718

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Throw assert failure exceptions on development build #718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4c3e68d
Throw exceptions on assert failure on dev build
SadPencil May 16, 2025
4247998
Add AssertFailedException
SadPencil May 16, 2025
6ffa2aa
Allow overriding stack trace
SadPencil May 16, 2025
9f0020a
Apply suggestions from code review
SadPencil May 16, 2025
2c512e2
Merge branch 'develop' into throw-assert-on-dev-build
SadPencil Jul 9, 2025
315f593
Merge branch 'develop' into throw-assert-on-dev-build
SadPencil Sep 25, 2025
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
12 changes: 12 additions & 0 deletions DXMainClient/AssertFailedException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#nullable enable
using System;

namespace DTAClient
{
public class AssertFailedException : Exception
{
public AssertFailedException(string message) : base(message)
{
}
}
}
Comment on lines +1 to +12
Copy link
Contributor

Choose a reason for hiding this comment

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

Why you placed exception not in ClientCore?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the Dev class is not in ClientCore

3 changes: 2 additions & 1 deletion DXMainClient/DXGUI/Multiplayer/CnCNet/CnCNetLobby.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1399,7 +1399,8 @@ private void RefreshPlayerList(object sender, EventArgs e)
lbPlayerList.Clear();

// Note: IUserCollection.GetFirst() is not guaranteed to be implemented, unless it is a SortedUserCollection
Debug.Assert(currentChatChannel.Users is SortedUserCollection<ChannelUser>, "Channel 'users' is supposed to be a SortedUserCollection");
Dev.Assert(currentChatChannel.Users is SortedUserCollection<ChannelUser>, "Channel 'users' is supposed to be a SortedUserCollection");

var current = currentChatChannel.Users.GetFirst();
while (current != null)
{
Expand Down
4 changes: 2 additions & 2 deletions DXMainClient/DXGUI/Multiplayer/GameInformationPanel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ public void SetInfo(GenericHostedGame game)
mapTexture = mapLoader.GameModeMaps.Find(m => m.Map.UntranslatedName.Equals(game.Map, StringComparison.InvariantCultureIgnoreCase) && m.Map.IsPreviewTextureCached())?.Map?.LoadPreviewTexture();
if (mapTexture == null && noMapPreviewTexture != null)
{
Debug.Assert(!noMapPreviewTexture.IsDisposed, "noMapPreviewTexture should not be disposed.");
Dev.Assert(!noMapPreviewTexture.IsDisposed, "noMapPreviewTexture should not be disposed.");
mapTexture = noMapPreviewTexture;
disposeTextures = false;
}
Expand All @@ -223,7 +223,7 @@ public void ClearInfo()

if (mapTexture != null && disposeTextures)
{
Debug.Assert(!mapTexture.IsDisposed, "mapTexture should not be disposed.");
Dev.Assert(!mapTexture.IsDisposed, "mapTexture should not be disposed.");
mapTexture.Dispose();
mapTexture = null;
}
Expand Down
43 changes: 43 additions & 0 deletions DXMainClient/Dev.cs
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

The same question about "why not ClientCore"

Copy link
Member Author

@SadPencil SadPencil May 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

  1. The Dev.Initialize() method is called after the DXMainClient sets up it self. Also, DEVELOPMENT_BUILD is only available in DXMainClient.
        public static void Initialize()
        {
            IsDev = IsDev || ClientConfiguration.Instance.ModMode;

#if DEVELOPMENT_BUILD
            IsDev = IsDev || ClientConfiguration.Instance.ShowDevelopmentBuildWarnings;
#endif
        }
  1. The whole exception handling logic is processed in DXMainClient
MainClientConstants.DisplayErrorAction

Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
#nullable enable
using System;
using System.Diagnostics;

using ClientCore;

namespace DTAClient
{
public static class Dev
{
public static bool IsDev { get; private set; } = false;

public static void Initialize()
{
IsDev = IsDev || ClientConfiguration.Instance.ModMode;

#if DEVELOPMENT_BUILD
IsDev = IsDev || ClientConfiguration.Instance.ShowDevelopmentBuildWarnings;
#endif
}

public static void Assert(bool condition, string message)
{
if (!IsDev)
{
Debug.Assert(condition, message);
return;
}

if (!condition)
{
try
{
throw new AssertFailedException($"Assert failed. {message}");
}
catch (Exception ex)
{
PreStartup.HandleException(null, ex, Environment.StackTrace);
}
}
}
}
}
5 changes: 3 additions & 2 deletions DXMainClient/Domain/Multiplayer/Map.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public Map(string baseFilePath)

public Map(string baseFilePath, bool isCustomMap)
{
Debug.Assert(!baseFilePath.EndsWith($".{ClientConfiguration.Instance.MapFileExtension}", StringComparison.InvariantCultureIgnoreCase), $"Unexpected map path {baseFilePath}. It should not end with the map extension.");
Dev.Assert(!baseFilePath.EndsWith($".{ClientConfiguration.Instance.MapFileExtension}", StringComparison.InvariantCultureIgnoreCase), $"Unexpected map path {baseFilePath}. It should not end with the map extension.");

BaseFilePath = baseFilePath;
customMapFilePath = isCustomMap
Expand Down Expand Up @@ -400,7 +400,8 @@ public bool SetInfoFromMpMapsINI(IniFile iniFile)
if (string.IsNullOrEmpty(waypoint))
break;

Debug.Assert(int.TryParse(waypoint.Split(',')[0], out _), $"waypoint should be a number, got {waypoint}");
Dev.Assert(int.TryParse(waypoint.Split(',')[0], out _), $"waypoint should be a number, got {waypoint}");

waypoints.Add(waypoint);
}

Expand Down
2 changes: 1 addition & 1 deletion DXMainClient/Domain/Multiplayer/MapLoader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ private ConcurrentDictionary<string, Map> LoadCustomMapCache()
/// <returns>The map if loading it was successful, otherwise false.</returns>
public Map LoadCustomMap(string mapPath, out string resultMessage)
{
Debug.Assert(!mapPath.EndsWith($".{ClientConfiguration.Instance.MapFileExtension}", StringComparison.InvariantCultureIgnoreCase), $"Unexpected map path {mapPath}. It should not end with the map extension.");
Dev.Assert(!mapPath.EndsWith($".{ClientConfiguration.Instance.MapFileExtension}", StringComparison.InvariantCultureIgnoreCase), $"Unexpected map path {mapPath}. It should not end with the map extension.");

string customMapFilePath = SafePath.CombineFilePath(ProgramConstants.GamePath, FormattableString.Invariant($"{mapPath}.{ClientConfiguration.Instance.MapFileExtension}"));
FileInfo customMapFile = SafePath.GetFile(customMapFilePath);
Expand Down
3 changes: 2 additions & 1 deletion DXMainClient/Online/Channel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ public void OnUserListReceived(List<ChannelUser> userList)
existingUser.IsFriend = user.IsFriend;

// Note: IUserCollection.Reinsert() is not guaranteed to be implemented, unless it is a SortedUserCollection
Debug.Assert(users is SortedUserCollection<ChannelUser>, "Channel 'users' is supposed to be a SortedUserCollection");
Dev.Assert(users is SortedUserCollection<ChannelUser>, "Channel 'users' is supposed to be a SortedUserCollection");

users.Reinsert(user.IRCUser.Name);
}
}
Expand Down
4 changes: 2 additions & 2 deletions DXMainClient/Online/FileHashCalculator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public void CalculateHashes()
{
string fileRelativePath = SafePath.CombineFilePath(path.Name, filename);
string fileFullPath = SafePath.CombineFilePath(path.FullName, filename);
Debug.Assert(File.Exists(fileFullPath), $"File {fileFullPath} is supposed to but does not exist.");
Dev.Assert(File.Exists(fileFullPath), $"File {fileFullPath} is supposed to but does not exist.");

string hash = fh.AddHashForFileIfExists(fileRelativePath, fileFullPath);
if (!string.IsNullOrEmpty(hash))
Expand Down Expand Up @@ -262,7 +262,7 @@ public string AddHashForFileIfExists(string relativePath) =>

public string AddHashForFileIfExists(string relativePath, string filePath)
{
Debug.Assert(!relativePath.StartsWith(ProgramConstants.GamePath), $"File path {relativePath} should be a relative path.");
Dev.Assert(!relativePath.StartsWith(ProgramConstants.GamePath), $"File path {relativePath} should be a relative path.");

string hash = CalculateSHA1ForFile(filePath);
if (!string.IsNullOrEmpty(hash))
Expand Down
10 changes: 6 additions & 4 deletions DXMainClient/PreStartup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ public static void Initialize(StartupParams parameters)
MainClientConstants.DisplayErrorAction(null, error, true);
}

Dev.Initialize();

Startup startup = new();
#if DEBUG
startup.Execute();
Expand All @@ -224,7 +226,7 @@ public static void Initialize(StartupParams parameters)

}

public static void LogException(Exception ex, bool innerException = false)
public static void LogException(Exception ex, bool innerException = false, string stackTraceOverride = null)
{
if (!innerException)
Logger.Log("KABOOOOOOM!!! Info:");
Expand All @@ -235,15 +237,15 @@ public static void LogException(Exception ex, bool innerException = false)
Logger.Log("Message: " + ex.Message);
Logger.Log("Source: " + ex.Source);
Logger.Log("TargetSite.Name: " + ex.TargetSite?.Name);
Logger.Log("Stacktrace: " + ex.StackTrace);
Logger.Log("Stacktrace: " + (stackTraceOverride ?? ex.StackTrace));

if (ex.InnerException is not null)
LogException(ex.InnerException, true);
}

public static void HandleException(object sender, Exception ex)
public static void HandleException(object sender, Exception ex, string stackTraceOverride = null)
{
LogException(ex, innerException: false);
LogException(ex, innerException: false, stackTraceOverride);

string errorLogPath = SafePath.CombineFilePath(ProgramConstants.ClientUserFilesPath, "ClientCrashLogs", FormattableString.Invariant($"ClientCrashLog{DateTime.Now.ToString("_yyyy_MM_dd_HH_mm")}.txt"));
bool crashLogCopied = false;
Expand Down
Loading