Some access level fixes from Robby to more closely match the historic levels, and some code cleanup

5 files changed, 59 insertions, 43 deletions

diff --git a/data/chanserv.example.conf b/data/chanserv.example.conf
index 2b82b05d8..229ca1198 100644
--- a/data/chanserv.example.conf
+++ b/data/chanserv.example.conf
@@ -232,7 +232,7 @@ privilege
 {
 	name = "ACCESS_CHANGE"
 	desc = "Allowed to modify the access list"
-	rank = 290
+	rank = 0
 	level = 10
 	flag = "f"
 }
@@ -249,7 +249,7 @@ privilege
 	name = "ACCESS_LIST"
 	desc = "Allowed to view the access list"
 	rank = 10
-	level = 10
+	level = 3
 	flag = "f"
 }
 
@@ -579,7 +579,7 @@ privilege
 	name = "MODE"
 	desc = "Allowed to use the MODE command"
 	rank = 170
-	level = 5
+	level = 9999
 	flag = "s"
 }
 
@@ -663,7 +663,7 @@ privilege
 	name = "OWNERME"
 	desc = "Allowed to (de)owner him/herself"
 	rank = 340
-	level = "9999"
+	level = 9999
 	flag = "q"
 }
 
diff --git a/modules/commands/cs_access.cpp b/modules/commands/cs_access.cpp
index 2eed525a4..ff0ee067e 100644
--- a/modules/commands/cs_access.cpp
+++ b/modules/commands/cs_access.cpp
@@ -106,22 +106,31 @@ class CommandCSAccess : public Command
 			source.Reply(_("Access level must be non-zero."));
 			return;
 		}
-
-		AccessGroup u_access = ci->AccessFor(u);
-		const ChanAccess *highest = u_access.Highest();
-		int u_level = (highest ? AccessChanAccess::DetermineLevel(highest) : 0);
-		if (level >= u_level && !u_access.Founder && !u->HasPriv("chanserv/access/modify"))
-		{
-			source.Reply(ACCESS_DENIED);
-			return;
-		}
 		else if (level <= ACCESS_INVALID || level >= ACCESS_FOUNDER)
 		{
 			source.Reply(CHAN_ACCESS_LEVEL_RANGE, ACCESS_INVALID + 1, ACCESS_FOUNDER - 1);
 			return;
 		}
 
-		bool override = !ci->AccessFor(u).HasPriv("ACCESS_CHANGE") || (level >= u_level && !u_access.Founder);
+		AccessGroup u_access = ci->AccessFor(u);
+		const ChanAccess *highest = u_access.Highest();
+
+		AccessChanAccess tmp_access(NULL);
+		tmp_access.ci = ci;
+		tmp_access.level = level;
+
+		bool override = false;
+
+		if ((!highest || *highest <= tmp_access) && !u_access.Founder)
+		{
+			if (u->HasPriv("chanserv/access/modify"))
+				override = true;
+			else
+			{
+				source.Reply(ACCESS_DENIED);
+				return;
+			}
+		}
 
 		if (mask.find_first_of("!*@") == Anope::string::npos && !findnick(mask))
 		{
@@ -141,7 +150,7 @@ class CommandCSAccess : public Command
 			if (mask.equals_ci(access->mask))
 			{
 				/* Don't allow lowering from a level >= u_level */
-				if (AccessChanAccess::DetermineLevel(access) >= u_level && !u_access.Founder && !u->HasPriv("chanserv/access/modify"))
+				if ((!highest || *access >= *highest) && !u_access.Founder && !u->HasPriv("chanserv/access/modify"))
 				{
 					source.Reply(ACCESS_DENIED);
 					return;
@@ -232,7 +241,7 @@ class CommandCSAccess : public Command
 					AccessGroup u_access = ci->AccessFor(user);
 					const ChanAccess *u_highest = u_access.Highest();
 
-					if ((u_highest ? AccessChanAccess::DetermineLevel(u_highest) : 0) <= AccessChanAccess::DetermineLevel(access) && !u_access.Founder && !this->override && !access->mask.equals_ci(user->Account()->display))
+					if ((!u_highest || *u_highest <= *access) && !u_access.Founder && !this->override && !access->mask.equals_ci(user->Account()->display))
 					{
 						Denied = true;
 						return;
@@ -256,15 +265,13 @@ class CommandCSAccess : public Command
 		{
 			AccessGroup u_access = ci->AccessFor(u);
 			const ChanAccess *highest = u_access.Highest();
-			int u_level = (highest ? AccessChanAccess::DetermineLevel(highest) : 0);
 
 			for (unsigned i = ci->GetAccessCount(); i > 0; --i)
 			{
 				ChanAccess *access = ci->GetAccess(i - 1);
 				if (mask.equals_ci(access->mask))
 				{
-					int access_level = AccessChanAccess::DetermineLevel(access);
-					if (!access->mask.equals_ci(u->Account()->display) && !u_access.Founder && u_level <= access_level && !u->HasPriv("chanserv/access/modify"))
+					if (!access->mask.equals_ci(u->Account()->display) && !u_access.Founder && (!highest || *highest <= *access) && !u->HasPriv("chanserv/access/modify"))
 						source.Reply(ACCESS_DENIED);
 					else
 					{
diff --git a/modules/commands/cs_flags.cpp b/modules/commands/cs_flags.cpp
index 35c2aac0a..52d826e97 100644
--- a/modules/commands/cs_flags.cpp
+++ b/modules/commands/cs_flags.cpp
@@ -166,7 +166,7 @@ class CommandCSFlags : public Command
 							else
 							{
 								source.Reply(_("You can not set the \002%c\002 flag."), f);
-								continue;
+								break;
 							}
 						}
 						if (add == 1)
@@ -235,10 +235,11 @@ class CommandCSFlags : public Command
 		for (unsigned i = 0, end = ci->GetAccessCount(); i < end; ++i)
 		{
 			const ChanAccess *access = ci->GetAccess(i);
-			const Anope::string &flags = FlagsChanAccess::DetermineFlags(access);
 
 			if (!arg.empty())
 			{
+				const Anope::string &flags = FlagsChanAccess::DetermineFlags(access);
+
 				if (arg[0] == '+')
 				{
 					bool pass = true;
diff --git a/modules/commands/cs_xop.cpp b/modules/commands/cs_xop.cpp
index 18adeadb0..a2f97675c 100644
--- a/modules/commands/cs_xop.cpp
+++ b/modules/commands/cs_xop.cpp
@@ -33,12 +33,12 @@ static struct XOPAccess
 		{
 			"SIGNKICK",
 			"SET",
+			"MODE",
 			"AUTOOWNER",
 			"OWNERME",
 			"PROTECT",
 			"INFO",
 			"ASSIGN",
-			"TOPIC",
 			""
 		}
 	},
@@ -56,15 +56,14 @@ static struct XOPAccess
 	},
 	{ XOP_AOP, "AOP",
 		{
-			"MODE",
+			"TOPIC",
 			"GETKEY",
 			"INVITE",
-			"UNBAN",
 			"AUTOOP",
 			"OPDEOPME",
 			"HALFOP",
 			"SAY",
-			"NOKICK",
+			"GREET",
 			""
 		}
 	},
@@ -72,9 +71,10 @@ static struct XOPAccess
 		{
 			"AUTOHALFOP",
 			"HALFOPME",
+			"VOICE",
 			"KICK",
 			"BAN",
-			"FANTASIA",
+			"UNBAN",
 			""
 		}
 	},
@@ -83,6 +83,8 @@ static struct XOPAccess
 			"AUTOVOICE",
 			"VOICEME",
 			"ACCESS_LIST",
+			"FANTASIA",
+			"NOKICK",
 			""
 		}
 	},
@@ -215,14 +217,23 @@ class XOPBase : public Command
 			return;
 		}
 
+		XOPChanAccess tmp_access(NULL);
+		tmp_access.ci = ci;
+		tmp_access.type = level;
+
 		AccessGroup access = ci->AccessFor(u);
 		const ChanAccess *highest = access.Highest();
-		int u_level = (highest ? XOPChanAccess::DetermineLevel(highest) : 0);
+		bool override = false;
 
-		if ((!access.Founder && !access.HasPriv("ACCESS_CHANGE") && !u->HasPriv("chanserv/access/modify")) || (level <= u_level && !access.Founder))
+		if ((!access.Founder && !access.HasPriv("ACCESS_CHANGE")) || ((!highest || *highest <= tmp_access) && !access.Founder))
 		{
-			source.Reply(ACCESS_DENIED);
-			return;
+			if (u->HasPriv("chanserv/access/modify"))
+				override = true;
+			else
+			{
+				source.Reply(ACCESS_DENIED);
+				return;
+			}
 		}
 
 		if (mask.find_first_of("!*@") == Anope::string::npos && !findnick(mask))
@@ -243,7 +254,7 @@ class XOPBase : public Command
 
 			if (a->mask.equals_ci(mask))
 			{
-				if (XOPChanAccess::DetermineLevel(a) >= u_level && !access.Founder && !u->HasPriv("chanserv/access/modify"))
+				if ((!highest || *a >= *highest) && !access.Founder && !u->HasPriv("chanserv/access/modify"))
 				{
 					source.Reply(ACCESS_DENIED);
 					return;
@@ -272,7 +283,6 @@ class XOPBase : public Command
 		acc->created = Anope::CurTime;
 		ci->AddAccess(acc);
 
-		bool override = (level >= u_level && !access.Founder) || !access.HasPriv("ACCESS_CHANGE");
 		Log(override ? LOG_OVERRIDE : LOG_COMMAND, u, this, ci) << "to add " << mask;
 
 		FOREACH_MOD(I_OnAccessAdd, OnAccessAdd(ci, u, acc));
@@ -303,10 +313,15 @@ class XOPBase : public Command
 			return;
 		}
 
+		XOPChanAccess tmp_access(NULL);
+		tmp_access.ci = ci;
+		tmp_access.type = level;
+
 		AccessGroup access = ci->AccessFor(u);
 		const ChanAccess *highest = access.Highest();
 		bool override = false;
-		if ((!mask.equals_ci(u->Account()->display) && !access.HasPriv("ACCESS_CHANGE") && !access.Founder) || ((!highest || level <= XOPChanAccess::DetermineLevel(highest)) && !access.Founder))
+
+		if ((!mask.equals_ci(u->Account()->display) && !access.HasPriv("ACCESS_CHANGE") && !access.Founder) || ((!highest || tmp_access >= *highest) && !access.Founder))
 		{
 			if (u->HasPriv("chanserv/access/modify"))
 				override = true;
diff --git a/src/config.cpp b/src/config.cpp
index 8e45097a7..0c5d1c2af 100644
--- a/src/config.cpp
+++ b/src/config.cpp
@@ -895,7 +895,7 @@ static bool DoPrivileges(ServerConfig *config, const Anope::string &, const Anop
 {
 	Anope::string name = values[0].GetValue();
 	Anope::string desc = values[1].GetValue();
-	Anope::string rank = values[2].GetValue();
+	int rank = values[2].GetInteger();
 
 	ValueItem vi(name);
 	if (!ValidateNotEmpty(config, "privilege", "name", vi))
@@ -905,14 +905,7 @@ static bool DoPrivileges(ServerConfig *config, const Anope::string &, const Anop
 	if (!ValidateNotEmpty(config, "privilege", "desc", vi))
 		throw ConfigException("One or more values in your configuration file failed to validate. Please see your log for more information.");
 	
-	int irank = 0;
-	try
-	{
-		irank = convertTo<int>(rank);
-	}
-	catch (const ConvertException &) { }
-
-	PrivilegeManager::AddPrivilege(Privilege(name, desc, irank));
+	PrivilegeManager::AddPrivilege(Privilege(name, desc, rank));
 	return true;
 }
 
@@ -1353,7 +1346,7 @@ ConfigItems::ConfigItems(ServerConfig *conf)
 		{"privilege",
 			{"name", "desc", "rank", ""},
 			{"", "", "", ""},
-			{DT_STRING, DT_STRING, DT_STRING, DT_STRING},
+			{DT_STRING, DT_STRING, DT_INTEGER, DT_STRING},
 			InitPrivileges, DoPrivileges, DonePrivileges},
 		{"",
 			{""},