Fleshed out forward system and extensively documented its design considerations

Implemented and did basic tests on new IPluginFunction type
Split function types into a separate file

--HG--
extra : convert_revision : svn%3A39bc706e-5318-0410-9160-8a85361fbb7c/trunk%40182

core/interfaces/IForwardSys.h

@@ -7,6 +7,11 @@
 #define SMINTERFACE_FORWARDMANAGER_NAME		"IForwardManager"
 #define SMINTERFACE_FORWARDMANAGER_VERSION	1
 
+/**
+ * There is some very important documentation at the bottom of this file.
+ * Readers interested in knowing more about the forward system, scrolling down is a must!
+ */
+
 namespace SourceMod
 {
 	enum ResultType
@@ -25,12 +30,14 @@ namespace SourceMod
 	};
 
 	/**
+	 * :TODO: finish this spec
 	 * @brief Abstracts multiple function calling.
-	 *
 	 * NOTE: Parameters should be pushed in forward order, unlike
 	 * the virtual machine/IPluginContext order.
+	 * NOTE: Some functions are repeated in here because their documentation differs
+	 * from their IPluginFunction equivalents.
 	 */
-	class IForward : public IPluginFunction
+	class IForward : public ICallable
 	{
 	public:
 		/**
@@ -40,61 +47,6 @@ namespace SourceMod
 		 */
 		virtual const char *GetForwardName() =0;
 
-		/**
-		 * @brief Pushes a cell onto the current call.
-		 *
-		 * @param cell		Parameter value to push.
-		 * @return			True if successful, false if type or count mismatch.
-		 */
-		virtual bool PushCell(cell_t cell) =0;
-
-		/**
-		 * @brief Pushes a float onto the current call.
-		 *
-		 * @param float		Parameter value to push.
-		 * @return			True if successful, false if type or count mismatch.
-		 */
-		virtual bool PushFloat(float number) =0;
-
-		/**
-		 * @brief Pushes an array of cells onto the current call, each cell individually.
-		 * NOTE: This is essentially a crippled version of PushArray().
-		 *
-		 * @param array		Array of cells.
-		 * @param numcells	Number of cells in array.
-		 * @param each		Whether or not to push as an array or individual cells.
-		 * @return			True if successful, false if too many cells were pushed.
-		 */
-		virtual bool PushCells(cell_t array[], unsigned int numcells, bool each) =0;
-
-		/**
-		 * @brief Pushes an array of cells onto the current call.  
-		 *
-		 * @param array		Array to copy, NULL if no initial array should be copied.
-		 * @param cells		Number of cells to allocate and optionally read from the input array.
-		 * @param phys_addr	Optional return address for physical array.
-		 * @param copyback	Whether or not changes should be copied back to the input array.
-		 * @return			True if successful, false otherwise.
-		 */
-		virtual bool PushArray(cell_t *inarray, size_t cells, cell_t **phys_addr, bool copyback) =0;
-
-		/**
-		 * @brief Pushes a string onto the current call.
-		 *
-		 * @param string	String to push.
-		 * @return			True if successful, false if type or count mismatch.
-		 */
-		virtual bool PushString(const char *string) =0;
-
-		/**
-		 * @brief Executes the forward, resets the pushed parameter list, and performs any copybacks.
-		 *
-		 * @param result		Pointer to store return value in (dependent on forward type).
-		 * @param last_err		Pointer to store number of successful executions in.
-		 * @return				Error code, if any.
-		 */
-		virtual int Execute(cell_t *result, unsigned int *num_functions) =0;
-
 		/**
 		 * @brief Returns the number of functions in this forward.
 		 *
@@ -105,9 +57,18 @@ namespace SourceMod
 		/**
 		 * @brief Returns the method of multi-calling this forward has.
 		 *
-		 * @return			ResultType of the forward.
+		 * @return			ExecType of the forward.
 		 */
-		virtual ResultType GetResultType() =0;
+		virtual ExecType GetExecType() =0;
+
+		/**
+		 * @brief Executes the forward.
+		 *
+		 * @param result		Pointer to store result in.
+		 * @param num_functions	Optionally filled with the number of function sucessfully executed.
+		 * @return				Error code, if any.
+		 */
+		virtual int Execute(cell_t *result, unsigned int *num_functions) =0;
 	};
 
 
@@ -127,24 +88,37 @@ namespace SourceMod
 		 * @param plugin	Plugin to remove instances of.
 		 * @return			Number of functions removed therein.
 		 */
-		virtual void RemoveFunctionsOfPlugin(IPlugin *plugin) =0;
+		virtual unsigned int RemoveFunctionsOfPlugin(IPlugin *plugin) =0;
 
 		/**
 		 * @brief Adds a function to the call list.
+		 * NOTE: Cannot be used during a call.
 		 *
 		 * @param func		Function to add.
+		 * @return			True on success, otherwise false.
 		 */
-		virtual void AddFunction(IPluginFunction *func) =0;
+		virtual bool AddFunction(IPluginFunction *func) =0;
+
+		/**
+		 * @brief Adds a function to the call list.
+		 * NOTE: Cannot be used during a call.
+		 *
+		 * @param ctx		Context to use as a look-up.
+		 * @param funcid	Function id to add.
+		 * @return			True on success, otherwise false.
+		 */
+		virtual bool AddFunction(sp_context_t *ctx, funcid_t index) =0;
 	};
 
 	enum ParamType
 	{
-		Param_Any = 0,
+		Param_Any = 0,		//Any type will be accepted
-		Param_Cell = 1,
+		Param_Cell = 1,		//Only a cell will be accepted
-		Param_Float = 2,
+		Param_Float = 2,	//Only a float value will be accepted
-		Param_String = 3,
+		Param_String = 3,	//Only a string will be accepted
-		Param_Array = 4,
+		Param_Array = 4,	//Only a 1D array will be accepted
-		Param_VarArgs = 5,
+		Param_VarArgs = 5,	//Anything will be accepted
+		ParamTypes_Total = 6,
 	};
 	
 	class IForwardManager : public SMInterface
@@ -174,7 +148,11 @@ namespace SourceMod
 		 * @param ...			If types is NULL, num_params ParamTypes should be pushed.
 		 * @return				A new IForward on success, NULL if type combination is impossible.
 		 */
-		virtual IForward *CreateForward(const char *name, ExecType et, int num_params, ParamType *types, ...) =0;
+		virtual IForward *CreateForward(const char *name, 
+										ExecType et, 
+										unsigned int num_params, 
+										ParamType *types, 
+										...) =0;
 
 		/**
 		 * @brief Creates an unmanaged forward.  This forward exists privately.
@@ -208,4 +186,81 @@ namespace SourceMod
 	};
 };
 
+/**
+ *  In the AMX Mod X model of forwarding, each forward contained a list of pairs, each pair containing
+ * a function ID and an AMX structure.  The forward structure itself did very little but hold parameter types.
+ * An execution call worked like this:
+ *  - executeForward() took in a function id and a list of parameters
+ *  - for each contained plugin:
+ *   - the list of parameters was preprocessed and pushed
+ *   - the call was made
+ *   - the list was freed and copybacks were made
+ *  - return
+ * 
+ *  The advantages to this is that the system is very easy to implement, and it's fast. The disadvantage is 
+ * varargs tend to be very unforgiving and inflexible, and thus weird problems arose with casting.  You also 
+ * lose flexibility, type checking, and the ability to reasonably use variable arguments lists in the VM.
+ *
+ *  SourceMod replaces this forward system with a far more advanced, but a bit bulkier one. The idea is that 
+ * each plugin has a table of functions, and each function is an ICallable object.  As well as being an ICallable, 
+ * each function is an IPluginFunction.  An ICallable simply describes the process of adding parameters to a 
+ * function call.  An IPluginFunction describes the process of actually calling a function and performing allocation, 
+ * copybacks, and deallocations.
+ *
+ * A very powerful forward system emerges: a Forward is just a collection of IPluginFunctions.  Thus, the same 
+ * API can be easily wrapped around a simple list, and it will look transparent to the user.
+ * Advantages:
+ * 1) "SP Forwards" from AMX Mod X are simply IPluginFunctions without a collection.
+ * 2) Forwards are function based, rather than plugin based, and are thus far more flexible at runtime..
+ * 3) [2] Individual functions can be paused and more than one function from the same plugin can be hooked.
+ * 4) [2] One hook type that used to map to many SP Forwards can now be centralized as one Forward.
+ *    This helps alleviate messes like Fakemeta.
+ * 5) Parameter pushing is type-checked and allows for variable arguments.
+ *
+ *  Note that while #2,3,4 could be added to AMX Mod X, the real binding property is #1, which makes the system
+ * object oriented, rather than AMX Mod X, which hides the objects behind static functions.  It is entirely a design
+ * issue, rather than a usability one.  The interesting part is when it gets to implementation, which is when the 
+ * problems for SourceMod arise.  Specifically, the implementation is easier in GENERAL -- the tough part is the oddball
+ * cases.
+ *
+ * Observe the calling process:
+ * - Each parameter is pushed using the ICallable interface.  For each parameter:
+ *  - For each function in the collection, the parameter is processed and pushed.
+ * - For each function in the collection:
+ *  - The call is made.
+ *  - Copy backs are performed.
+ * - Return
+ *
+ *  Astute readers will note the problems - the parameters are processed individually for each push,
+ * rather than for each call.  This means:
+ * 1) More memory is used.  Specifically, rather than N params of memory, you now have N params * M plugins.
+ *    This is because, again, parameters are processed per function object, per-push, before the call.
+ * 2) There are slightly more calls going around: one extra call for each parameter, since each push is manual.
+ * 3) Copybacks won't work as expected.  
+ *
+ *  Number 3 is hard to see.  For example, say a forward has two functions, and an array is pushed with copyback.
+ * The array gets pushed and copied into each plugin.  When the call is made, each plugin now has separate copies of 
+ * the array.  When the copyback is performed, it gets mirrored to the originall address, but not to the next plugin!
+
+ *  Implementing this is "difficult."  To be re-entrant, an IPluginFunction can't tell you anything
+ * about its copybacks after it has returned, because its internal states were reset long ago.
+ * 
+ *  In order to implement this feature, IPluginFunction::Execute function now lets you pass a listener in.
+ * This listener is notified each time an array parameter is about to be copied back.  Specifically, the forward 
+ * uses this to detect when it needs to push a new parameter out to the next plugin.  When the forward gets the 
+ * call back, it detects whether there is another plugin in the queue.  If there is, it grabs the address it will
+ * be using for the same arrays, and specifies it as the new copy-back point.  If no plugins are left, it allows
+ * the copyback to chain up to the original address.
+ * 
+ *  This wonderful little hack is somewhat reminiscent of how SourceHook parameter rewrite chains work.  It seems 
+ * ugly at first, but it is actually the correct design pattern to apply to an otherwise awful problem.
+ *
+ *  Note that there are other solutions to this that aren't based on a visitor-like pattern.  For example, the Function
+ * class could expose a "set new original address" function.  Then after arrays are pushed, the arrays are re-browsed,
+ * and function #1 gets function #2's original address, function #2 gets function #3's original address, et cetera.
+ * This extra browse step is a bit less efficient though, since the "visitor" method implies only taking action when
+ * necessary.  Furthermore, it would require exposing such a "set address" function, which should fire a red flag 
+ * that the API is doing something it shouldn't (namely, exposing the direct setting of very internal properties).
+ */
+
 #endif //_INCLUDE_SOURCEMOD_FORWARDINTERFACE_H_

core/interfaces/IPluginFunction.h

@@ -0,0 +1,177 @@
+#ifndef _INCLUDE_SOURCEMOD_PLUGINFUNCTION_INTERFACE_H_
+#define _INCLUDE_SOURCEMOD_PLUGINFUNCTION_INTERFACE_H_
+
+#include <IPluginSys.h>
+
+namespace SourceMod
+{
+	#define SMFUNC_COPYBACK_NONE	(0)			/* Never copy an array back */
+	#define SMFUNC_COPYBACK_ONCE	(1<<0)		/* Copy an array back after call */
+	#define SMFUNC_COPYBACK_ALWAYS	(1<<1)		/* Copy an array back after subsequent calls (forwards) */
+
+	/**
+	 * @brief Represents what a function needs to implement in order to be callable.
+	 */
+	class ICallable
+	{
+	public:
+		/**
+		 * @brief Pushes a cell onto the current call.
+		 *
+		 * @param cell		Parameter value to push.
+		 * @return			Error code, if any.
+		 */
+		virtual int PushCell(cell_t cell) =0;
+
+		/**
+		 * @brief Pushes a cell by reference onto the current call.
+		 * NOTE: On Execute, the pointer passed will be modified if copyback is enabled.
+		 *
+		 * @param cell		Address containing parameter value to push.
+		 * @param flags		Copy-back flags.
+		 * @return			Error code, if any.
+		 */
+		virtual int PushCellByRef(cell_t *cell, int flags) =0;
+
+		/**
+		 * @brief Pushes a float onto the current call.
+		 *
+		 * @param float		Parameter value to push.
+		 * @return			Error code, if any.
+		 */
+		virtual int PushFloat(float number) =0;
+
+		/**
+		 * @brief Pushes a float onto the current call by reference.
+		 * NOTE: On Execute, the pointer passed will be modified if copyback is enabled.
+		 *
+		 * @param float		Parameter value to push.
+		 & @param flags		Copy-back flags.
+		 * @return			Error code, if any.
+		 */
+		virtual int PushFloatByRef(float *number, int flags) =0;
+
+		/**
+		 * @brief Pushes an array of cells onto the current call, each cell individually.
+		 * NOTE: This is essentially a crippled version of PushArray().
+		 *
+		 * @param array		Array of cells.
+		 * @param numcells	Number of cells in array.
+		 * @param each		Whether or not to push as an array or individual cells.
+		 * @return			Error code, if any.
+		 */
+		virtual int PushCells(cell_t array[], unsigned int numcells, bool each) =0;
+
+		/**
+		 * @brief Pushes an array of cells onto the current call.  
+		 * NOTE: On Execute, the pointer passed will be modified if non-NULL and copy-back
+		 * is enabled.
+		 *
+		 * @param array		Array to copy, NULL if no initial array should be copied.
+		 * @param cells		Number of cells to allocate and optionally read from the input array.
+		 * @param phys_addr	Optional return address for physical array (if array was non-NULL, will be array).
+		 * @param flags		Whether or not changes should be copied back to the input array.
+		 * @return			Error code, if any.
+		 */
+		virtual int PushArray(cell_t *inarray, 
+								unsigned int cells, 
+								cell_t **phys_addr, 
+								int flags=SMFUNC_COPYBACK_NONE) =0;
+
+		/**
+		 * @brief Pushes a string onto the current call.
+		 *
+		 * @param string	String to push.
+		 * @return			Error code, if any.
+		 */
+		virtual int PushString(const char *string) =0;
+
+		/**
+		 * @brief Pushes a string onto the current call.
+		 * NOTE: On Execute, the pointer passed will be modified if copy-back is enabled.
+		 *
+		 * @param string	String to push.
+		 * @param flags		Copy-back flags.
+		 * @return			Error code, if any.
+		 */
+		virtual int PushStringByRef(char *string, int flags) =0;
+
+		/**
+		 * @brief Cancels a function call that is being pushed but not yet executed.
+		 * This can be used be reset for CallFunction() use.
+		 */
+		virtual void Cancel() =0;
+	};
+
+
+	/**
+	 * @brief Used for copy-back notification
+	 */
+	class IFunctionCopybackReader
+	{
+	public:
+		/**
+		 * @brief Called before an array is copied back.
+		 * 
+		 * @param param			Parameter index.
+		 * @param cells			Number of cells in the array.
+		 * @param source_addr	Source address in the plugin that will be copied.
+		 * @param orig_addr		Destination addres in plugin that will be overwritten.
+		 * @param flags			Copy flags.
+		 * @return				True to copy back, false otherwise.
+		 */
+		virtual bool OnCopybackArray(unsigned int param, 
+								unsigned int cells,
+								cell_t *source_addr,
+								cell_t *orig_addr,
+								int flags) =0;
+	};
+
+	/**
+	 * @brief Encapsulates a function call in a plugin.
+	 * NOTE: Function calls must be atomic to one execution context.
+	 * NOTE: This object should not be deleted.  It lives for the lifetime of the plugin.
+	 */
+	class IPluginFunction : public ICallable
+	{
+	public:
+		/**
+		 * @brief Executes the forward, resets the pushed parameter list, and performs any copybacks.
+		 *
+		 * @param result		Pointer to store return value in.
+		 * @param reader		Copy-back listener.  NULL to specify 
+		 * @return				Error code, if any.
+		 */
+		virtual int Execute(cell_t *result, IFunctionCopybackReader *reader) =0;
+
+		/**
+		 * @brief Executes the function with the given parameter array.
+		 * Parameters are read in forward order (i.e. index 0 is parameter #1)
+		 * NOTE: You will get an error if you attempt to use CallFunction() with
+		 * previously pushed parameters.
+		 *
+		 * @param param			Array of cell parameters.
+		 * @param num_params	Number of parameters to push.
+		 * @param result		Pointer to store result of function on return.
+		 * @return				SourcePawn error code (if any).
+		 */
+		virtual int CallFunction(const cell_t *params, unsigned int num_params, cell_t *result) =0;
+
+		/**
+		 * @brief Returns which plugin this function belongs to.
+		 *
+		 * @return				IPlugin pointer to parent plugin.
+		 */
+		virtual IPlugin *GetParentPlugin() =0;
+
+		/**
+		 * @brief Returns the physical address of a by-reference parameter.
+		 *
+		 * @param				Parameter index to read (beginning at 0).
+		 * @return				Address, or NULL if invalid parameter specified.
+		 */
+		virtual cell_t *GetAddressOfPushedParam(unsigned int param) =0;
+	};
+};
+
+#endif //_INCLUDE_SOURCEMOD_PLUGINFUNCTION_INTERFACE_H_

core/interfaces/IPluginSys.h

@@ -11,6 +11,8 @@
 
 namespace SourceMod
 {
+	class IPlugin;
+
 	/** 
 	 * @brief Encapsulates plugin public information.
 	 */
@@ -48,39 +50,7 @@ namespace SourceMod
 		PluginType_Global,			/* Plugin will never be unloaded or updated */
 	};
 
-
+	class IPluginFunction;
-	class IPlugin;
-
-
-	/**
-	 * @brief Encapsulates a basic function call.
-	 * NOTE: Function calls must be atomic to one execution context.
-	 * NOTE: This object should not be deleted.  It lives for the lifetime of the plugin.
-	 */
-	class IPluginFunction
-	{
-	public:
-		virtual ~IPluginFunction()
-		{
-		}
-
-		/**
-		 * @brief Executes the function with the given parameter array.
-		 * Parameters are read in forward order (i.e. index 0 is parameter #1)
-		 *
-		 * @param param			Array of cell parameters.
-		 * @param num_params	Number of parameters to push.
-		 * @param result		Pointer to store result of function on return.
-		 * @return				SourcePawn error code (if any).
-		 */
-		virtual int CallFunction(const cell_t *params, unsigned int num_params, cell_t *result) =0;
-
-		/**
-		 * @brief Returns which plugin this function belongs to.
-		 */
-		virtual IPlugin *GetParentPlugin() =0;
-	};
-
 
 	/**
 	 * @brief Encapsulates a run-time plugin as maintained by SourceMod.

core/msvc8/sourcemod_mm.vcproj

@@ -224,26 +224,6 @@
 		<Filter
 			Name="Interfaces"
 			>
-			<File
-				RelativePath="..\interfaces\IForwardSys.h"
-				>
-			</File>
-			<File
-				RelativePath="..\interfaces\ILibrarySys.h"
-				>
-			</File>
-			<File
-				RelativePath="..\interfaces\IModuleSys.h"
-				>
-			</File>
-			<File
-				RelativePath="..\interfaces\IPluginSys.h"
-				>
-			</File>
-			<File
-				RelativePath="..\interfaces\IShareSys.h"
-				>
-			</File>
 			<File
 				RelativePath="..\interfaces\ITextParsers.h"
 				>
@@ -255,6 +235,14 @@
 			<Filter
 				Name="Header Files"
 				>
+				<File
+					RelativePath="..\systems\CFunction.h"
+					>
+				</File>
+				<File
+					RelativePath="..\systems\ForwardSys.h"
+					>
+				</File>
 				<File
 					RelativePath="..\systems\LibrarySys.h"
 					>
@@ -267,6 +255,10 @@
 			<Filter
 				Name="Source Files"
 				>
+				<File
+					RelativePath="..\systems\CFunction.cpp"
+					>
+				</File>
 				<File
 					RelativePath="..\systems\LibrarySys.cpp"
 					>
@@ -276,6 +268,34 @@
 					>
 				</File>
 			</Filter>
+			<Filter
+				Name="Interfaces"
+				>
+				<File
+					RelativePath="..\interfaces\IForwardSys.h"
+					>
+				</File>
+				<File
+					RelativePath="..\interfaces\ILibrarySys.h"
+					>
+				</File>
+				<File
+					RelativePath="..\interfaces\IModuleSys.h"
+					>
+				</File>
+				<File
+					RelativePath="..\interfaces\IPluginFunction.h"
+					>
+				</File>
+				<File
+					RelativePath="..\interfaces\IPluginSys.h"
+					>
+				</File>
+				<File
+					RelativePath="..\interfaces\IShareSys.h"
+					>
+				</File>
+			</Filter>
 		</Filter>
 		<Filter
 			Name="SDK"

core/sourcemod.cpp

@@ -111,12 +111,17 @@ bool SourceModBase::InitializeSourceMod(char *error, size_t err_max, bool late)
 		return false;
 	}
 
+#if 0
 	g_SMAPI->PathFormat(file, sizeof(file), "%s/addons/sourcemod/plugins/test.smx", g_BaseDir.c_str());
 	IPlugin *pPlugin = g_PluginMngr.LoadPlugin(file, false, PluginType_Global, error, err_max);
-	IPluginFunction *func = pPlugin->GetFunctionByName("OnPluginInit");
+	IPluginFunction *func = pPlugin->GetFunctionByName("Test");
-	cell_t result;
+	cell_t result = 2;
-	func->CallFunction(NULL, 0, &result);
+	cell_t val = 6;
+	func->PushCell(1);
+	func->PushCellByRef(&val, SMFUNC_COPYBACK_ONCE);
+	func->Execute(&result, NULL);
 	g_PluginMngr.UnloadPlugin(pPlugin);
+#endif
 
 	return true;
 }

core/systems/CFunction.cpp

@@ -0,0 +1,272 @@
+#include <stdio.h>
+#include "PluginSys.h"
+
+/********************
+* FUNCTION CALLING *
+********************/
+
+void CFunction::Set(funcid_t funcid, CPlugin *plugin)
+{
+	m_funcid = funcid;
+	m_pPlugin = plugin;
+	m_curparam = 0;
+	m_errorstate = SP_ERROR_NONE;
+}
+
+int CFunction::CallFunction(const cell_t *params, unsigned int num_params, cell_t *result)
+{
+	IPluginContext *ctx = m_pPlugin->m_ctx_current.base;
+
+	while (num_params--)
+	{
+		ctx->PushCell(params[num_params]);
+	}
+
+	return ctx->Execute(m_funcid, result);
+}
+
+IPlugin *CFunction::GetParentPlugin()
+{
+	return m_pPlugin;
+}
+
+CFunction::CFunction(funcid_t funcid, CPlugin *plugin) : 
+	m_funcid(funcid), m_pPlugin(plugin), m_curparam(0), 
+	m_errorstate(SP_ERROR_NONE)
+{
+}
+
+int CFunction::PushCell(cell_t cell)
+{
+	if (m_curparam >= SP_MAX_EXEC_PARAMS)
+	{
+		return SetError(SP_ERROR_PARAMS_MAX);
+	}
+
+	m_info[m_curparam].marked = false;
+	m_params[m_curparam] = cell;
+	m_curparam++;
+
+	return SP_ERROR_NONE;
+}
+
+int CFunction::PushCellByRef(cell_t *cell, int flags)
+{
+	if (m_curparam >= SP_MAX_EXEC_PARAMS)
+	{
+		return SetError(SP_ERROR_PARAMS_MAX);
+	}
+
+	return PushArray(cell, 1, NULL, flags);
+}
+
+int CFunction::PushFloat(float number)
+{
+	cell_t val = *(cell_t *)&number;
+
+	return PushCell(val);
+}
+
+int CFunction::PushFloatByRef(float *number, int flags)
+{
+	return PushCellByRef((cell_t *)number, flags);
+}
+
+int CFunction::PushCells(cell_t array[], unsigned int numcells, bool each)
+{
+	if (!each)
+	{
+		return PushArray(array, numcells, NULL, SMFUNC_COPYBACK_NONE);
+	} else {
+		int err;
+		for (unsigned int i=0; i<numcells; i++)
+		{
+			if ((err=PushCell(array[i])) != SP_ERROR_NONE)
+			{
+				return SetError(err);
+			}
+		}
+		return SP_ERROR_NONE;
+	}
+}
+
+int CFunction::PushArray(cell_t *inarray, unsigned int cells, cell_t **phys_addr, int copyback)
+{
+	if (m_curparam >= SP_MAX_EXEC_PARAMS)
+	{
+		return SetError(SP_ERROR_PARAMS_MAX);
+	}
+
+	IPluginContext *ctx = m_pPlugin->m_ctx_current.base;
+	ParamInfo *info = &m_info[m_curparam];
+	int err;
+
+	if ((err=ctx->HeapAlloc(cells, &info->local_addr, &info->phys_addr)) != SP_ERROR_NONE)
+	{
+		return SetError(err);
+	}
+
+	info->flags = inarray ? copyback : SMFUNC_COPYBACK_NONE;
+	info->marked = true;
+	info->size = cells;
+	m_params[m_curparam] = info->local_addr;
+	m_curparam++;
+
+	if (inarray)
+	{
+		memcpy(info->phys_addr, inarray, sizeof(cell_t) * cells);
+		info->orig_addr = inarray;
+	} else {
+		info->orig_addr = info->phys_addr;
+	}
+
+	if (phys_addr)
+	{
+		*phys_addr = info->phys_addr;
+	}
+
+	return true;
+}
+
+int CFunction::PushString(const char *string)
+{
+	return _PushString(string, SMFUNC_COPYBACK_NONE);
+}
+
+int CFunction::PushStringByRef(char *string, int flags)
+{
+	return _PushString(string, flags);
+}
+
+int CFunction::_PushString(const char *string, int flags)
+{
+	if (m_curparam >= SP_MAX_EXEC_PARAMS)
+	{
+		return SetError(SP_ERROR_PARAMS_MAX);
+	}
+
+	IPluginContext *base = m_pPlugin->m_ctx_current.base;
+	ParamInfo *info = &m_info[m_curparam];
+	size_t len = strlen(string);
+	size_t cells = (len + sizeof(cell_t) - 1) / sizeof(cell_t);
+	int err;
+
+	if ((err=base->HeapAlloc(cells, &info->local_addr, &info->phys_addr)) != SP_ERROR_NONE)
+	{
+		return SetError(err);
+	}
+
+	info->marked = true;
+	m_params[m_curparam] = info->local_addr;
+	m_curparam++;	/* Prevent a leak */
+
+	//:TODO: Use UTF-8 version
+	if ((err=base->StringToLocal(info->local_addr, len, string)) != SP_ERROR_NONE)
+	{
+		return SetError(err);
+	}
+
+	info->flags = flags;
+	info->orig_addr = (cell_t *)string;
+	info->size = cells;
+
+	return SP_ERROR_NONE;
+}
+
+void CFunction::Cancel()
+{
+	if (!m_curparam)
+	{
+		return;
+	}
+
+	IPluginContext *base = m_pPlugin->m_ctx_current.base;
+
+	while (m_curparam--)
+	{
+		if (m_info[m_curparam].marked)
+		{
+			base->HeapRelease(m_info[m_curparam].local_addr);
+			m_info[m_curparam].marked = false;
+		}
+	}
+
+	m_errorstate = SP_ERROR_NONE;
+}
+
+int CFunction::Execute(cell_t *result, IFunctionCopybackReader *reader)
+{
+	int err;
+	if (m_errorstate != SP_ERROR_NONE)
+	{
+		err = m_errorstate;
+		Cancel();
+		return err;
+	}
+
+	//This is for re-entrancy!
+	cell_t temp_params[SP_MAX_EXEC_PARAMS];
+	ParamInfo temp_info[SP_MAX_EXEC_PARAMS];
+	unsigned int numparams = m_curparam;
+	bool docopies = true;
+
+	if (numparams)
+	{
+		//Save the info locally, then reset it for re-entrant calls.
+		memcpy(temp_params, m_params, numparams * sizeof(cell_t));
+		memcpy(temp_info, m_info, numparams * sizeof(ParamInfo));
+	}
+	m_curparam = 0;
+
+	if ((err = CallFunction(temp_params, numparams, result)) != SP_ERROR_NONE)
+	{
+		docopies = false;
+	}
+
+	IPluginContext *base = m_pPlugin->m_ctx_current.base;
+
+	while (numparams--)
+	{
+		if (!temp_info[numparams].marked)
+		{
+			continue;
+		}
+		if (docopies && temp_info[numparams].flags)
+		{
+			if (reader)
+			{
+				if (!reader->OnCopybackArray(numparams, 
+										temp_info[numparams].size, 
+										temp_info[numparams].phys_addr,
+										temp_info[numparams].orig_addr,
+										temp_info[numparams].flags))
+				{
+					goto _skipcopy;
+				}
+			}
+			if (temp_info[numparams].orig_addr)
+			{
+				memcpy(temp_info[numparams].orig_addr, 
+						temp_info[numparams].phys_addr, 
+						temp_info[numparams].size * sizeof(cell_t));
+			}
+		}
+_skipcopy:
+		base->HeapRelease(temp_info[numparams].local_addr);
+		temp_info[numparams].marked = false;
+	}
+
+	return err;
+}
+
+cell_t *CFunction::GetAddressOfPushedParam(unsigned int param)
+{
+	if (m_errorstate != SP_ERROR_NONE
+		|| param >= m_curparam
+		|| !m_info[param].marked)
+	{
+		return NULL;
+	}
+
+	return m_info[param].phys_addr;
+}

core/systems/CFunction.h

@@ -0,0 +1,55 @@
+#ifndef _INCLUDE_SOURCEMOD_BASEFUNCTION_H_
+#define _INCLUDE_SOURCEMOD_BASEFUNCTION_H_
+
+#include <IPluginFunction.h>
+#include "sm_globals.h"
+
+struct ParamInfo
+{
+	int flags;			/* Copy-back flags */
+	bool marked;		/* Whether this is marked as being used */
+	cell_t local_addr;	/* Local address to free */
+	cell_t *phys_addr;	/* Physical address of our copy */
+	cell_t *orig_addr;	/* Original address to copy back to */
+	ucell_t size;		/* Size of array in cells */
+};
+
+class CPlugin;
+
+class CFunction : public IPluginFunction
+{
+public:
+	CFunction(funcid_t funcid, CPlugin *plugin);
+public:
+	virtual int PushCell(cell_t cell);
+	virtual int PushCellByRef(cell_t *cell, int flags);
+	virtual int PushFloat(float number);
+	virtual int PushFloatByRef(float *number, int flags);
+	virtual int PushCells(cell_t array[], unsigned int numcells, bool each);
+	virtual int PushArray(cell_t *inarray, unsigned int cells, cell_t **phys_addr, int copyback);
+	virtual int PushString(const char *string);
+	virtual int PushStringByRef(char *string, int flags);
+	virtual cell_t *GetAddressOfPushedParam(unsigned int param);
+	virtual int Execute(cell_t *result, IFunctionCopybackReader *reader);
+	virtual void Cancel();
+	virtual int CallFunction(const cell_t *params, unsigned int num_params, cell_t *result);
+	virtual IPlugin *GetParentPlugin();
+public:
+	void Set(funcid_t funcid, CPlugin *plugin);
+private:
+	int _PushString(const char *string, int flags);
+	inline int SetError(int err)
+	{
+		m_errorstate = err;
+		return err;
+	}
+private:
+	funcid_t m_funcid;
+	CPlugin *m_pPlugin;
+	cell_t m_params[SP_MAX_EXEC_PARAMS];
+	ParamInfo m_info[SP_MAX_EXEC_PARAMS];
+	unsigned int m_curparam;
+	int m_errorstate;
+};
+
+#endif //_INCLUDE_SOURCEMOD_BASEFUNCTION_H_

core/systems/PluginSys.cpp

@@ -7,34 +7,6 @@ CPluginManager::CPluginManager()
 {
 }
 
-void CFunction::Set(funcid_t funcid, CPlugin *plugin)
-{
-	m_funcid = funcid;
-	m_pPlugin = plugin;
-}
-
-int CFunction::CallFunction(const cell_t *params, unsigned int num_params, cell_t *result)
-{
-	IPluginContext *ctx = m_pPlugin->m_ctx_current.base;
-
-	for (unsigned int i=0; i<num_params; i++)
-	{
-		ctx->PushCell(params[i]);
-	}
-
-	return ctx->Execute(m_funcid, result);
-}
-
-IPlugin *CFunction::GetParentPlugin()
-{
-	return m_pPlugin;
-}
-
-CFunction::CFunction(funcid_t funcid, CPlugin *plugin) : 
-	m_funcid(funcid), m_pPlugin(plugin)
-{
-}
-
 CPlugin *CPlugin::CreatePlugin(const char *file, 
 								bool debug_default, 
 								PluginType type, 
@@ -355,7 +327,7 @@ bool CPluginManager::UnloadPlugin(IPlugin *plugin)
 	{
 		for (uint32_t i=0; i<pPlugin->m_plugin->info.publics_num; i++)
 		{
-			delete pPlugin->m_pub_funcs[i];
+			g_PluginMngr.ReleaseFunctionToPool(pPlugin->m_pub_funcs[i]);
 		}
 		delete [] pPlugin->m_pub_funcs;
 		pPlugin->m_pub_funcs = NULL;
@@ -365,7 +337,7 @@ bool CPluginManager::UnloadPlugin(IPlugin *plugin)
 	{
 		for (unsigned int i=0; i<pPlugin->m_funcsnum; i++)
 		{
-			delete pPlugin->m_priv_funcs[i];
+			g_PluginMngr.ReleaseFunctionToPool(pPlugin->m_priv_funcs[i]);
 		}
 		delete [] pPlugin->m_priv_funcs;
 		pPlugin->m_priv_funcs = NULL;
@@ -435,6 +407,11 @@ void CPluginManager::ReleaseIterator(CPluginIterator *iter)
 
 void CPluginManager::ReleaseFunctionToPool(CFunction *func)
 {
+	if (!func)
+	{
+		return;
+	}
+	func->Cancel();
 	m_funcpool.push(func);
 }
 

core/systems/PluginSys.h

@@ -5,6 +5,7 @@
 #include <sh_list.h>
 #include <sh_stack.h>
 #include "sm_globals.h"
+#include "CFunction.h"
 
 using namespace SourceHook;
 
@@ -19,22 +20,6 @@ struct ContextPair
 	sp_context_t *ctx;
 };
 
-class CPlugin;
-
-class CFunction : public IPluginFunction
-{
-public:
-	CFunction(funcid_t funcid, CPlugin *plugin);
-public:
-	virtual int CallFunction(const cell_t *params, unsigned int num_params, cell_t *result);
-	virtual IPlugin *GetParentPlugin();
-public:
-	void Set(funcid_t funcid, CPlugin *plugin);
-private:
-	funcid_t m_funcid;
-	CPlugin *m_pPlugin;
-};
-
 class CPlugin : public IPlugin
 {
 	friend class CPluginManager;