From 51348d18064d0092ce5118af906e646f82069208 Mon Sep 17 00:00:00 2001
From: quake1337 <3310937+bartico6@users.noreply.github.com>
Date: Sat, 31 Jul 2021 16:34:43 +0200
Subject: [PATCH] Fixes for PR 2397.

- GroupManager now validates core groups in the constructor.
- EnsureCoreGroupsPresent -> AssertCoreGroupsPresent.
- Fix indentiation in some places.
- Clarify the intent of this PR in CHANGELOG.md.
---
 CHANGELOG.md                 |  3 ++-
 TShockAPI/DB/GroupManager.cs | 32 ++++++++++++++++++++++++++++----
 TShockAPI/TShock.cs          |  1 -
 TShockAPI/Utils.cs           |  7 ++++---
 4 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0bf907db..09d6164a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -29,7 +29,8 @@ This is the rolling changelog for TShock for Terraria. Use past tense when addin
 * Correct rejection message in LandGolfBallInCupHandler to output the proper expected player id. (@drunderscore)
 * Clarified the error mesage that the console is presented if a rate-limit is reached over REST to indicate that "tokens" actually refers to rate-limit tokens, and not auth tokens, and added a hint as to what config setting determines this. (@hakusaro, @patsore)
 * Fixed an issue where, when the console was redirected, input was disabled and commands didn't work, in TSAPI. You can now pass `-disable-commands` to disable the input thread, but by default, it will be enabled. Fixes [#1450](https://github.com/Pryaxis/TShock/issues/1450). (@DeathCradle, @QuiCM)
-* Fixed missing group problems & error reporting on startup. (@bartico6)
+* Fixed errors on startup not being reported to console. (@bartico6)
+* The server now correctly disconnects players with missing groups instead of throwing an exception, stalling the connection (@bartico6)
 
 ## TShock 4.5.4
 * Fixed ridiculous typo in `GetDataHandlers` which caused TShock to read the wrong field in the packet for `usingBiomeTorches`. (@hakusaro, @Arthri)
diff --git a/TShockAPI/DB/GroupManager.cs b/TShockAPI/DB/GroupManager.cs
index 8133edaa..09b19f5a 100644
--- a/TShockAPI/DB/GroupManager.cs
+++ b/TShockAPI/DB/GroupManager.cs
@@ -200,9 +200,11 @@ namespace TShockAPI.DB
 			LoadPermisions();
 
 			Group.DefaultGroup = GetGroupByName(TShock.Config.Settings.DefaultGuestGroupName);
+
+			AssertCoreGroupsPresent();
 		}
 
-		internal void EnsureCoreGroupsPresent()
+		internal void AssertCoreGroupsPresent()
 		{
 			if (!GroupExists(TShock.Config.Settings.DefaultGuestGroupName))
 			{
@@ -210,11 +212,33 @@ namespace TShockAPI.DB
 				throw new Exception("The guest group could not be found.");
 			}
 
-			if(!GroupExists(TShock.Config.Settings.DefaultRegistrationGroupName))
-            {
+			if (!GroupExists(TShock.Config.Settings.DefaultRegistrationGroupName))
+			{
 				TShock.Log.ConsoleError("The default usergroup could not be found. This may indicate a typo in the configuration file, or that the group was renamed or deleted.");
 				throw new Exception("The default usergroup could not be found.");
-            }
+			}
+		}
+
+		/// <summary>
+		/// Asserts that the group reference can be safely assigned to the player object.
+		/// <para>If this assertion fails, and <paramref name="kick"/> is true, the player is disconnected. If <paramref name="kick"/> is false, the player will receive an error message.</para>
+		/// </summary>
+		/// <param name="player">The player in question</param>
+		/// <param name="group">The group we want to assign them</param>
+		/// <param name="kick">Whether or not failing this check disconnects the player.</param>
+		/// <returns></returns>
+		public bool AssertGroupValid(TSPlayer player, Group group, bool kick)
+		{
+			if (group == null)
+			{
+				if (kick)
+					player.Disconnect("Your account's group could not be loaded. Please contact server administrators about this.");
+				else
+					player.SendErrorMessage("Your account's group could not be loaded. Please contact server administrators about this.");
+				return false;
+			}
+
+			return true;
 		}
 
 		private void AddDefaultGroup(string name, string parent, string permissions)
diff --git a/TShockAPI/TShock.cs b/TShockAPI/TShock.cs
index 996463e1..2adfbf79 100644
--- a/TShockAPI/TShock.cs
+++ b/TShockAPI/TShock.cs
@@ -322,7 +322,6 @@ namespace TShockAPI
 				Regions = new RegionManager(DB);
 				UserAccounts = new UserAccountManager(DB);
 				Groups = new GroupManager(DB);
-				Groups.EnsureCoreGroupsPresent();
 				ProjectileBans = new ProjectileManagager(DB);
 				TileBans = new TileManager(DB);
 				RememberedPos = new RememberedPosManager(DB);
diff --git a/TShockAPI/Utils.cs b/TShockAPI/Utils.cs
index aad127dc..c5bc6c46 100644
--- a/TShockAPI/Utils.cs
+++ b/TShockAPI/Utils.cs
@@ -216,11 +216,12 @@ namespace TShockAPI
 		/// Asserts that the group reference can be safely assigned to the player object.
 		/// <para>If this assertion fails, and <paramref name="kick"/> is true, the player is disconnected. If <paramref name="kick"/> is false, the player will receive an error message.</para>
 		/// </summary>
-		/// <param name="player"></param>
-		/// <param name="group"></param>
+		/// <param name="player">The player in question</param>
+		/// <param name="group">The group we want to assign them</param>
+		/// <param name="kick">Whether or not failing this check disconnects the player.</param>
 		/// <returns></returns>
 		public bool AssertGroupValid(TSPlayer player, Group group, bool kick)
-        {
+		{
 			if (group == null)
 			{
 				if (kick)