[Nexgen 1.12 // TCP Transfer // empty Strings SOLUTION]

Discussions about Coding and Scripting
Post Reply
User avatar
Sp0ngeb0b
Adept
Posts: 376
Joined: Wed Feb 13, 2008 9:16 pm
Location: Cologne
Contact:

[Nexgen 1.12 // TCP Transfer // empty Strings SOLUTION]

Post by Sp0ngeb0b »

[Nexgen 1.12 // TCP Transfer // serious bug with empty Strings on arrays (formatCmdArg) // SOLUTION]

This info is only related to programmers!

Introduction
Although I don't know how many of you people are still developing for UT and especially for Nexgen, but I don't want to keep this information for myself anyways as it might help others alot. While developing my newest Nexgen Plugin, I made extensive use of the Server <-> Client TCP communication system introduced in Nexgen 1.12. A feature which has not really established itself in the modding community, which is pretty incomprehensible from my point of view. It offers a way way faster data transfer between Server and Client, compared to this the original UT netcode sucks even more. Anyways, while working with it I ran into some strange stuff, which brought me to digging deep into Defrost's work in Nexgen 1.12 ...


What was the problem?
The data transfer worked pretty well generally, but there was one issue I discovered which made me wonder: I wasn't able to save empty strings clientside. They somehow turned into strange numbers while beeing transfered to the server side. After intensive bug hunting on my plugin I was pretty sure the issue was not on my side... And then a light dawned on me: The strange numbers turned out to be the index number of the specifc array entry! It was pretty clear that somehow during the TCP sending process, the empty string must have been completely ignored and therefore the following Index data must have been erroneously recognized as the new value.

A look into the formatCmdArg() function of NexgenUtil.uc confirms this suspicion:

Code: Select all

/***************************************************************************************************
 *
 *  $DESCRIPTION  Formats the specified string so that it can safely be added to a command string as
 *                an argument of that command.
 *  $PARAM        arg  The argument string that is to be formatted.
 *  $RETURN       The properly formatted argument string.
 *
 **************************************************************************************************/
static function string formatCmdArg(coerce string arg) {
	local string result;
	
	result = arg;
	
	// Escape argument if necessary.
	if (arg == "") {
		arg = "\"\"";
	} else {
		result = arg;
		result = class'NexgenUtil'.static.replace(result, "\\", "\\\\");
		result = class'NexgenUtil'.static.replace(result, "\"", "\\\"");
		result = class'NexgenUtil'.static.replace(result, chr(0x09), "\\t");
		result = class'NexgenUtil'.static.replace(result, chr(0x0A), "\\n");
		result = class'NexgenUtil'.static.replace(result, chr(0x0D), "\\r");
		
		if (instr(arg, " ") > 0) {
			result = "\"" $ result $ "\"";
		}
	}
	
	// Return result.
	return result;
}

As you can see in line 8 of the function code, the wrong variable got assigned (arg instead of result). Therefore, the real expression for an empty string ( \"\" ) isn't returned, but instead just an empty string which isn't recognized on the other machine.


Solution

I don't want to overrun you with to many details, since you would probably need some time to understand how Nexgen's TCP stuff works (I needed a lot of time aswell).
There are basically 2 ways to fix this:
  • 1) Ofcourse, you can fix the invalid formatCmdArg() function directly. But this would mean you have to recompile the whole core package of Nexgen + all its plugins, which I wanted to avoid. This method is only recommended if you want to create your own custom Nexgen Core version.
    But if there's ever gonna be an updated, official version of Nexgen (1.13), we need to make sure to include this fix!
  • 2) Manual fix for a plugin:
Into your PluginController class, insert this:

Code: Select all

/***************************************************************************************************
 *
 *  $DESCRIPTION  Fixed serverside set() function of NexgenSharedDataSyncManager. Uses correct
 *                formatting.
 *
 **************************************************************************************************/
function setFixed(string dataContainerID, string varName, coerce string value, optional int index, optional Object author) {
	local NexgenSharedDataContainer dataContainer;
	local NexgenClient client;
	local NexgenExtendedClientController xClient;
	local string oldValue;
	local string newValue;

  // Get the data container.
	dataContainer = dataSyncMgr.getDataContainer(dataContainerID);
	if (dataContainer == none) return;

	oldValue = dataContainer.getString(varName, index);
	dataContainer.set(varName, value, index);
	newValue = dataContainer.getString(varName, index);

	// Notify clients if variable has changed.
	if (newValue != oldValue) {
		for (client = control.clientList; client != none; client = client.nextClient) {
			xClient = getXClient(client);
			if (xClient != none && xClient.bInitialSyncComplete && dataContainer.mayRead(xClient, varName)) {
				if (dataContainer.isArray(varName)) {
					xClient.sendStr(xClient.CMD_SYNC_PREFIX @ xClient.CMD_UPDATE_VAR
						              @ static.formatCmdArgFixed(dataContainerID)
						              @ static.formatCmdArgFixed(varName)
						              @ index
						              @ static.formatCmdArgFixed(newValue));
				} else {
					xClient.sendStr(xClient.CMD_SYNC_PREFIX @ xClient.CMD_UPDATE_VAR
						              @ static.formatCmdArgFixed(dataContainerID)
						              @ static.formatCmdArgFixed(varName)
						              @ static.formatCmdArgFixed(newValue));
				}
			}
		}
	}

	// Also notify the server side controller of this event.
	if (newValue != oldValue) {
		varChanged(dataContainer, varName, index, author);
	}
}



/***************************************************************************************************
 *
 *  $DESCRIPTION  Corrected version of the static formatCmdArg function in NexgenUtil. Empty strings
 *                are formated correctly now (original source of all trouble).
 *
 **************************************************************************************************/
static function string formatCmdArgFixed(coerce string arg) {
	local string result;

	result = arg;

	// Escape argument if necessary.
	if (result == "") {
		result = "\"\"";                      // Fix (originally, arg was assigned instead of result -_-)
	} else {
		result = class'NexgenUtil'.static.replace(result, "\\", "\\\\");
		result = class'NexgenUtil'.static.replace(result, "\"", "\\\"");
		result = class'NexgenUtil'.static.replace(result, chr(0x09), "\\t");
		result = class'NexgenUtil'.static.replace(result, chr(0x0A), "\\n");
		result = class'NexgenUtil'.static.replace(result, chr(0x0D), "\\r");

		if (instr(arg, " ") > 0) {
			result = "\"" $ result $ "\"";
		}
	}

	// Return result.
	return result;
}

Into your Client class, add the following:

Code: Select all

/***************************************************************************************************
 *
 *  $DESCRIPTION  Fixed version of the setVar function in NexgenExtendedClientController.
 *                Empty strings are now formated correctly before beeing sent to the server.
 *
 **************************************************************************************************/
simulated function setVar(string dataContainerID, string varName, coerce string value, optional int index) {
	local NexgenSharedDataContainer dataContainer;
	local string oldValue;
	local string newValue;

	// Get data container.
	dataContainer = dataSyncMgr.getDataContainer(dataContainerID);

	// Check if variable can be updated.
	if (dataContainer == none || !dataContainer.mayWrite(self, varName)) return;

	// Update variable value.
	oldValue = dataContainer.getString(varName, index);
	dataContainer.set(varName, value, index);
	newValue = dataContainer.getString(varName, index);

	// Send new value to server.
	if (newValue != oldValue) {
		if (dataContainer.isArray(varName)) {
			sendStr(CMD_SYNC_PREFIX @ CMD_UPDATE_VAR
			        @ class'PluginController'.static.formatCmdArgFixed(dataContainerID)
			        @ class'PluginController'.static.formatCmdArgFixed(varName)
			        @ index
			        @ class'PluginController'.static.formatCmdArgFixed(newValue));
		} else {
			sendStr(CMD_SYNC_PREFIX @ CMD_UPDATE_VAR
			        @ class'PluginController'.static.formatCmdArgFixed(dataContainerID)
			        @ class'PluginController'.static.formatCmdArgFixed(varName)
			        @ class'PluginController'.static.formatCmdArgFixed(newValue));
		}
	}
}


/***************************************************************************************************
 *
 *  $DESCRIPTION  Corrected version of the exec_UPDATE_VAR function in NexgenExtendedClientController.
 *                Due to the invalid format function, empty strings weren't sent correctly and were
 *                therefore not identifiable for the other machine (server). This caused the var index
 *                beeing erroneously recognized as the new var value on the server.
 *                Since the serverside set() function in NexgenSharedDataSyncManager also uses the
 *                invalid format functions, we implemented a fixed function in PluginController. The
 *                client side set() function can still be called safely without problems.
 *
 **************************************************************************************************/
simulated function exec_UPDATE_VAR(string args[10], int argCount) {
	local int varIndex;
	local string varName;
	local string varValue;
	local NexgenSharedDataContainer container;
	local int index;

	// Get arguments.
	if (argCount == 3) {
		varName = args[1];
		varValue = args[2];
	} else if (argCount == 4) {
		varName = args[1];
		varIndex = int(args[2]);
		varValue = args[3];
	} else {
		return;
	}

	if (role == ROLE_Authority) {
  	// Server side, call fixed set() function
  	PluginController(xControl).setFixed(args[0], varName, varValue, varIndex, self);
  } else {
  
    // Client Side
    dataSyncMgr.set(args[0], varName, varValue, varIndex, self);

    container = dataSyncMgr.getDataContainer(args[0]);

		// Signal event to client controllers.
		for (index = 0; index < client.clientCtrlCount; index++) {
			if (NexgenExtendedClientController(client.clientCtrl[index]) != none) {
				NexgenExtendedClientController(client.clientCtrl[index]).varChanged(container, varName, varIndex);
			}
		}

		// Signal event to GUI.
		client.mainWindow.mainPanel.varChanged(container, varName, varIndex);
  }
}
This will force the correct formating of empty strings. Use the known way of updating variables clientside (setVar()). If you want to update a variable serverside, make sure to use the new setFixed() function in the PluginController.


I hope this might help somebody at some point.
Website, Forum & UTStats

Image
******************************************************************************
Nexgen Server Controller || My plugins & mods on GitHub
******************************************************************************
ShaiHulud
Adept
Posts: 459
Joined: Sat Dec 22, 2012 6:37 am

Re: [Nexgen 1.12 // TCP Transfer // empty Strings SOLUTION]

Post by ShaiHulud »

Good detective work Sp0nge, this is the kind of hands-dirty bug hunting I love getting my teeth into :D I'm sure this will be beneficial to others at some point, so thanks for sharing your detailed findings.
UT99.org

Re: [Nexgen 1.12 // TCP Transfer // empty Strings SOLUTION]

Post by UT99.org »

billybill wrote:Thanks, haven't looked into this too much except for the nexgenServerAffiliates.

IIRC this plugin extends the HTTP downloading class which opens a socket and retrieves text file information for evaluating, without the downloaded packets being saved

I assume one cannot write the file directly without log prefix and that this is why UDemo uses DLLs to download the files needed.

If not, there was a thread about writing (.)int files from the server to the clients, and this would prove very much useful. There's is the logging-output class which several modders have made use of. It is options to change where the file is saved and what it's name and extension is. I am guessing it needs to prefix the log or has to obey some rules [but worth a mention in this thread if anyone wants to make use of the TCPTransfer extension]. Thanks for finding this bug :)
Post Reply