Cvars: Adjust few things

- Hook is not as post, because allowing blocking change is not really that useful and this makes a context more simpler.
- Setting a min bound > max bound and reversely throw an error
- set_pcvar_bounds set cvar value on new min/max bound.
- Remove error "A cvar can't be binded with several variables", it's okay to no error such situation.

amxmodx/CvarManager.cpp

@@ -14,10 +14,7 @@
 
 CvarManager g_CvarManager;
 
-/** 
+DETOUR_DECL_STATIC2(Cvar_DirectSet, void, struct cvar_s*, var, const char*, value)
- * Returns true to call original function, otherwise false to block it.
- */
-bool Cvar_DirectSet_Custom(cvar_t* var, const char* value)
 {
 	CvarInfo* info = nullptr;
 
@@ -25,12 +22,13 @@ bool Cvar_DirectSet_Custom(cvar_t* var, const char* value)
 		|| strcmp(var->string, value) == 0                // Make sure old and new values are different to not trigger callbacks.
 		|| !g_CvarManager.CacheLookup(var->name, &info))  // No data in cache, nothing to do.
 	{
-		return true;
+		DETOUR_STATIC_CALL(Cvar_DirectSet)(var, value);
+		return;
 	}
 
 	if (info->bound.hasMin || info->bound.hasMax) // cvar_s doesn't have min/max mechanism, so we check things here.
 	{
-		float fvalue = atof(value); 
+		float fvalue = atof(value);
 		bool oob = false;
 
 		if (info->bound.hasMin && fvalue < info->bound.minVal)
@@ -47,36 +45,20 @@ bool Cvar_DirectSet_Custom(cvar_t* var, const char* value)
 		if (oob) // Found value out of bound, set new value and block original call.
 		{
 			CVAR_SET_FLOAT(var->name, fvalue);
-			return false;
+			return;
 		}
 	}
-
+	
+	ke::AString oldValue; // We save old value since it will be likely changed after original function called.
+	
 	if (!info->hooks.empty())
 	{
-		int lastResult = 0;
+		oldValue = var->string;
-
-		for (size_t i = 0; i < info->hooks.length(); ++i)
-		{
-			CvarHook* hook = info->hooks[i];
-
-			if (hook->forward->state == AutoForward::FSTATE_OK) // Our callback can be enable/disabled by natives.
-			{
-				int result = executeForwards(hook->forward->id, reinterpret_cast<cvar_t*>(var), var->string, value);
-
-				if (result >= lastResult)
-				{
-					lastResult = result;
-				}
-			}
-		}
-
-		if (lastResult) // A plugin wants to block the forward.
-		{
-			return false;
-		}
 	}
 
-	if (!info->binds.empty()) // Time to update our available binds.
+	DETOUR_STATIC_CALL(Cvar_DirectSet)(var, value);
+
+	if (!info->binds.empty()) 
 	{
 		for (size_t i = 0; i < info->binds.length(); ++i)
 		{
@@ -86,33 +68,35 @@ bool Cvar_DirectSet_Custom(cvar_t* var, const char* value)
 			{
 				case CvarBind::CvarType_Int:
 				{
-					*bind->varAddress = atoi(value);
+					*bind->varAddress = atoi(var->string);
 					break;
 				}
 				case CvarBind::CvarType_Float:
 				{
-					float fvalue = atof(value);
+					float fvalue = atof(var->string);
 					*bind->varAddress = amx_ftoc(fvalue);
 					break;
 				}
 				case CvarBind::CvarType_String:
 				{
-					set_amxstring_simple(bind->varAddress, value, bind->varLength);
+					set_amxstring_simple(bind->varAddress, var->string, bind->varLength);
 					break;
 				}
 			}
 		}
 	}
 
-	// Nothing to block.
+	if (!info->hooks.empty())
-	return true;
-}
-
-DETOUR_DECL_STATIC2(Cvar_DirectSet, void, struct cvar_s*, var, const char*, value)
-{
-	if (Cvar_DirectSet_Custom(var, value))
 	{
-		DETOUR_STATIC_CALL(Cvar_DirectSet)(var, value);
+		for (size_t i = 0; i < info->hooks.length(); ++i)
+		{
+			CvarHook* hook = info->hooks[i];
+
+			if (hook->forward->state == AutoForward::FSTATE_OK) // Our callback can be enable/disabled by natives.
+			{
+				executeForwards(hook->forward->id, reinterpret_cast<cvar_t*>(var), oldValue.chars(), var->string);
+			}
+		}
 	}
 }
 
@@ -165,12 +149,8 @@ void CvarManager::CreateCvarHook(void)
 
 	if (functionAddress)
 	{
+		// Disabled by default.
 		m_HookDetour = DETOUR_CREATE_STATIC_FIXED(Cvar_DirectSet, (void *)functionAddress);
-
-		if (m_HookDetour)
-		{
-			m_HookDetour->EnableDetour();
-		}
 	}
 }
 
@@ -375,7 +355,7 @@ void CvarManager::OnConsoleCommand()
 
 	int argcount = CMD_ARGC();
 
-	// amxx cvars <partial cvar name> <index from listing>
+	// amxx cvars [partial plugin name] [index from listing]
 	// E.g.:
 	//   amxx cvars test   <- list all cvars from plugin name starting by "test"
 	//   amxx cvars 2      <- show informations about cvar in position 2 from "amxx cvars" list

amxmodx/cvars.cpp

@@ -367,13 +367,8 @@ bool bind_pcvar(CvarInfo* info, CvarBind::CvarType type, AMX* amx, cell varofs,
 			if (bind->varAddress == address)
 			{
 				LogError(amx, AMX_ERR_NATIVE, "A same variable can't be binded with several cvars");
+				return false;
 			}
-			else
-			{
-				LogError(amx, AMX_ERR_NATIVE, "A cvar can't be binded with several variables");
-			}
-
-			return false;
 		}
 	}
 
@@ -521,22 +516,65 @@ static cell AMX_NATIVE_CALL set_pcvar_bounds(AMX *amx, cell *params)
 
 	bool set = params[3] > 0 ? true : false;
 	int pluginId = g_plugins.findPluginFast(amx)->getId();
+	float value = 0;
+	bool should_update = false;
 
 	switch (params[2])
 	{
 		case CvarBound_Lower:
+		{
 			info->bound.hasMin = set;
-			info->bound.minVal = set ? amx_ctof(params[4]) : 0;
+
-			info->bound.minPluginId = pluginId;
+			if (set)
+			{
+				value = amx_ctof(params[4]);
+
+				if (info->bound.hasMax && value > info->bound.maxVal)
+				{
+					LogError(amx, AMX_ERR_NATIVE, "A lower bound can't be above an upper bound");
+					return 0;
+				}
+
+				should_update = true;
+
+				info->bound.minVal = value;
+				info->bound.minPluginId = pluginId;
+			}
+
 			break;
+		}
 		case CvarBound_Upper:
+		{
 			info->bound.hasMax = set;
-			info->bound.maxVal = set ? amx_ctof(params[4]) : 0;
+
-			info->bound.maxPluginId = pluginId;
+			if (set)
+			{
+				value = amx_ctof(params[4]);
+
+				if (info->bound.hasMin && value < info->bound.minVal)
+				{
+					LogError(amx, AMX_ERR_NATIVE, "An upper bound can't be below a lower bound");
+					return 0;
+				}
+
+				should_update = true;
+
+				info->bound.maxVal = value;
+				info->bound.maxPluginId = pluginId;
+			}
+
 			break;
+		}
 		default:
+		{
 			LogError(amx, AMX_ERR_NATIVE, "Invalid CvarBounds value: %d", params[2]);
 			return 0;
+		}
+	}
+
+	if (should_update)
+	{
+		CVAR_SET_FLOAT(ptr->name, value);
 	}
 
 	return 1;

plugins/include/cvars.inc

@@ -91,6 +91,9 @@ native get_cvar_pointer(const cvar[]);
 /**
  * Creates a hook for when a cvar's value is changed.
  *
+ * @note You cannot prevent cvar changes from happening, you can only change the value again.
+ * @note Be careful not to set a cvar inside a cvar change hook such that 
+ *       it re-invokes he same callback. This results in infinite recursion.
  * @note Callback will be called in the following manner:
  *
  *       public cvar_change_callback(pcvar, const old_value[], const new_value[])

public/memtools/CDetour/detourhelpers.h

@@ -34,7 +34,7 @@
 
 #if defined(__linux__) || defined(__APPLE__)
 	#include <sys/mman.h>
-    #include <unistd.h>
+	#include <unistd.h>
 	#ifndef PAGE_SIZE
 		#define	PAGE_SIZE	4096
 	#endif