-
-
Notifications
You must be signed in to change notification settings - Fork 112
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.
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
base: develop
Are you sure you want to change the base?
Conversation
Nightly build for this pull request:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request changes the assert mechanism in development builds by replacing Debug.Assert with a custom Dev.Assert that actively throws exceptions, thereby preventing issues like the loading screen hanging. It also updates error logging and error display behaviors to better handle exceptions, and introduces a custom AssertFailedException.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
DXMainClient/DXGUI/Multiplayer/GameLobby/GameLobbyBase.cs | Replace Debug.Assert with Dev.Assert in player option checks. |
DXMainClient/PreStartup.cs | Adds Dev.Initialize(), updates LogException and HandleException signature and logic. |
DXMainClient/Online/FileHashCalculator.cs | Replaces Debug.Assert with Dev.Assert in file existence and path checking. |
DXMainClient/Online/Channel.cs | Updates assert comment and uses Dev.Assert for user collection checks. |
DXMainClient/Domain/Multiplayer/MapLoader.cs | Replaces Debug.Assert with Dev.Assert when verifying map path conditions. |
DXMainClient/Domain/Multiplayer/Map.cs | Replaces Debug.Assert with Dev.Assert in map path and waypoint validations. |
DXMainClient/Dev.cs | Introduces Dev helper methods for asserting and initialization. |
DXMainClient/DXGUI/Multiplayer/GameInformationPanel.cs | Uses Dev.Assert to validate texture disposal states. |
DXMainClient/DXGUI/Multiplayer/CnCNet/CnCNetLobby.cs | Replaces Debug.Assert with Dev.Assert for user collection type validation. |
DXMainClient/DXGUI/Generic/MainMenu.cs | Updates error display behavior by redefining DisplayErrorAction with a delayed error box. |
DXMainClient/DXGUI/GameClass.cs | Removes redundant DisplayErrorAction assignment in favor of a centralized error action. |
DXMainClient/AssertFailedException.cs | Adds a custom exception for failed assertions. |
Co-authored-by: Copilot <[email protected]>
#nullable enable | ||
using System; | ||
|
||
namespace DTAClient | ||
{ | ||
public class AssertFailedException : Exception | ||
{ | ||
public AssertFailedException(string message) : base(message) | ||
{ | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 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
}
- The whole exception handling logic is processed in DXMainClient
MainClientConstants.DisplayErrorAction
We should rely on the asserts to build a robust program.
Fix in some cases the loading screen might hang forever. Delay using XNAMessageBox as the(moved to Fix loading screen hanging on map parsing errors #782)DisplayErrorAction
until the main menu has been loaded. Otherwise, if there are any exceptions (not only assert failures but also other exceptions) be thrown when parsing the maps, the loading screen will hang forever.Example: