Problems with HkRename

Cannon

I’ve been tinkering with writing a rename plugin so that players are able to rename their characters from chat…by typing something like /renameme bob.

I’ve run into a problem with HkRename function. If I rename a character say A1 to A2 and then back to A2 to A1 the server sometimes (but fairly frequently) goes mad - crashes, chucks an exception, stops the player logging in - that sort of thing. I get the same behaviour using the rename command from the FLHook console.

I’ve kind of traced the problem to the HkRename function. I’ve modified this a bit and seem have got rid of the problem.

I was wondering:
1. If anyone else is able to replicate the problem - i.e. goto the FLHook console and type rename A1 to A2 then A2 back to A1 - repeat a few times. Obviously the A1 character has to exist in the first place.
2. Does the code fragment below fix the problem for you?
3. Could/should this change go into the next version of FLHook plugin? It needs lots of testing…

From HkFuncPlayers.C…

HK_ERROR HkRename(wstring wscCharname, wstring wscNewCharname, bool bOnlyDelete)
{
HK_GET_CLIENTID(iClientID, wscCharname);

if((iClientID == -1) && !HkGetAccountByCharname(wscCharname))
return HKE_CHAR_DOES_NOT_EXIST;

if(!bOnlyDelete && HkGetAccountByCharname(wscNewCharname))
return HKE_CHARNAME_ALREADY_EXISTS;

if(!bOnlyDelete && (wscNewCharname.length() > 23))
return HKE_CHARNAME_TOO_LONG;

if(!bOnlyDelete && !wscNewCharname.length())
return HKE_CHARNAME_TOO_SHORT;

CAccount *acc;
wstring wscOldCharname;
if(iClientID != -1) {
acc = Players.FindAccountFromClientID(iClientID);
wscOldCharname = Players.GetActiveCharacterName(iClientID);
} else {
wscOldCharname = wscCharname;
acc = HkGetAccountByCharname(wscCharname);
}

wstring wscAccountDirname;
HkGetAccountDirName(acc, wscAccountDirname);
wstring wscNewFilename;
HkGetCharFileName(wscNewCharname, wscNewFilename);
wstring wscOldFilename;
HkGetCharFileName(wscOldCharname, wscOldFilename);

string scNewCharfilePath = scAcctPath + wstos(wscAccountDirname) + “\” + wstos(wscNewFilename) + “.fl”;
string scOldCharfilePath = scAcctPath + wstos(wscAccountDirname) + “\” + wstos(wscOldFilename) + “.fl”;
if(!bOnlyDelete)
{
if(!flc_decode(scOldCharfilePath.c_str(), scNewCharfilePath.c_str()))
{ // file wasn’t encoded, thus simply rename it
DeleteFile(scNewCharfilePath.c_str()); // just to get sure…
CopyFile(scOldCharfilePath.c_str(), scNewCharfilePath.c_str(), FALSE);
}
}

// If only delete char we’re finished.
if (bOnlyDelete)
{
flstr *str = CreateWString(wscOldCharname.c_str());
HkLockAccountAccess(acc, true); // also kicks player on this account
bool bRet = Players.DeleteCharacterFromName(*str);
HkUnlockAccountAccess(acc);
FreeWString(str);
return HKE_OK;
}

// Otherwise this is a rename
HkLockAccountAccess(acc, true); // also kicks player on this account

DeleteFile(scOldCharfilePath.c_str()); // doesn’t delete when previously renamed, dunno why

wstring wscNewNameString = L"“;
for(uint i = 0; (i < wscNewCharname.length()); i++)
{
char cHiByte = wscNewCharname _>> 8;
char cLoByte = wscNewCharname _& 0xFF;
wchar_t wszBuf[8];
swprintf(wszBuf, L”%02X%02X", ((uint)cHiByte) & 0xFF, ((uint)cLoByte) & 0xFF);
wscNewNameString += wszBuf;
}

IniWrite(scNewCharfilePath, “Player”, “Name”, wstos(wscNewNameString));

// We’ll also need to change the character file name into the flname searchtree
// in memory(which is usually build when flserver starts). methods like
// FindAccountByCharacterName won’t work else
struct TREENODE
{
TREENODE *pLeft;
TREENODE *pParent;
TREENODE *pRight;
ulong l1;
char *szFLName;
uint lLength;
ulong l2; // ??
CAccount *acc;
};

char pEBP;
pEBP = (char)&Players + 0x30;
char *pEDI;
memcpy(&pEDI, pEBP + 4, 4);
TREENODE *leaf;
memcpy(&leaf, pEBP + 8, 4);
// edi+4 is where the root node is
TREENODE *node;
memcpy(&node, pEDI + 4, 4);

// Find the node with the OldFileName and change it to the new filename.
bool bLeft = true;
TREENODE lastparent = (TREENODE)(pEDI + 4);
while(node != leaf)
{
lastparent = node;
int iRet = strcmp(node->szFLName, wstos(wscOldFilename + L".fl").c_str());
if (iRet==0) { // this node?
strcpy(node->szFLName, wstos(wscNewFilename + L".fl").c_str());
} else if(iRet >= 0) { // leftnode?
node = node->pLeft;
bLeft = true;
} else { // rightnode?
node = node->pRight;
bLeft = false;
}
}

// finally we need to add the new char to the CAccount character list
// else it will not be shown in the accounts list in the flserver window
struct LISTNODE
{
LISTNODE *next;
LISTNODE *prev;
u_long l1;
wchar_t *wszCharname;
ulong lArray[32];
};
LISTNODE lnHead;
memcpy(&lnHead, (char)acc + 0x28, 4);

// This code is a bit different from the previous versions of FLHook in that
// rather than adding a new node to the end we rename the existing one.
LISTNODE *lnCur = lnHead->next;
while (lnCur != lnHead)
{
if (wcscmp(lnCur->wszCharname, wscOldCharname.c_str())==0)
{
wcscpy(lnCur->wszCharname, wscNewCharname.c_str());
break;
}
lnCur = lnCur->next;
}

// We’re done with the account mods. Unlock everything.
HkUnlockAccountAccess(acc);

return HKE_OK;
}__

Cannon

Reading over the post, I’ve spotted a few mistakes in my changes…nothing that really matters but would need to be fixed before run on a production server.

Wicked_Witch

@Cannon:

3. Could/should this change go into the next version of FLHook plugin? It needs lots of testing…

Don’t know if this make any sence.

I learned, if you rename a char, that the server needs a reboot.
If players are doing this, and they know it, than the server boots a lot
This is normally an Admin commant. Don’t think it’s a good idea for players.

Mindhunter

We’re using it as playercommand (/rename newname) for a while now and it works very well!

w0dk4

Interesting… it does work with this change:

// This code is a bit different from the previous versions of FLHook in that
// rather than adding a new node to the end we rename the existing one.

?

I knew partly about crashes when using the rename command, but didnt investigate up till now. I will have a closer look for the next plugin version.

w0dk4

ok, I checked your code… if its working, then it would be great indeed, but from looking at it, it should not work.

First, simply changing the node in the search tree may lead to the flserver never finding an account to a charname, therefore saying the charname is free even though it exists.
Also, I think there may be a buffer overrun if the new charname is longer than the old name (see char pointer), apart from the length parameter that needs to be changed as well.

Anyways, its pretty hard to figure out how the flserver is storing the chars in the internal database exactly, since there is no simple “storecharindatabase” function…

So maybe the best way would be to emulate a player login and then call the createnewcharacter function… and go from there…

Mindhunter

Not sure how schmackbolzen did it, all I can tell u is that /rename costs 250mil and will kick u from the server.
Works very well and i know that players are using it a lot.

I will point Schmackbolzen to this threat as soon as he comes online

w0dk4

Its actually about the rename function itself, not a player-command implementation.

There are still problems under certain circumstances with it, as Cannon told us, and I experienced those problems as well at some point. We at HC however have replaced the whole flserver chardatabase with our own, thats why we no longer have problems with renames at all.

Cannon

@w0dk4:

ok, I checked your code… if its working, then it would be great indeed, but from looking at it, it should not work.

First, simply changing the node in the search tree may lead to the flserver never finding an account to a charname, therefore saying the charname is free even though it exists.
Also, I think there may be a buffer overrun if the new charname is longer than the old name (see char pointer), apart from the length parameter that needs to be changed as well.

Anyways, its pretty hard to figure out how the flserver is storing the chars in the internal database exactly, since there is no simple “storecharindatabase” function…

So maybe the best way would be to emulate a player login and then call the createnewcharacter function… and go from there…

Buffer overrun & length definitely a problem - did think about that - but thought wrong (have re-read the original code and paid attention to it)

I see your point about the search tree - just changing the name file will break the search Â - my test server didn’t have many characters and they were in just the right order so I got away with it !

You could use my change as a case study for computer science students…find the mistakes.

Thanks for looking at it…back to the drawing board. I’ll keep tinkering.

w0dk4

ok, I know what went wrong, mc_horst made a fix for it but obviously it didnt make it into an actual FLHook release version.

The problem is that Players.DeleteChar decrements the account char counter. So, if you rename a lot on the same account, this counter will eventually be zero and negative which results in unpredictable behaviour.

So, include this to increment the counter:

// increment character counter
		memcpy(&iNumberOfChars, (char*)acc + 0x2C, 4);
		iNumberOfChars++;
		memcpy((char*)acc + 0x2C, &iNumberOfChars, 4);

Im still looking into emulating the whole process however, because IMO there may still be issues…

w0dk4

Ok, I got an emulation working, I will include it in the next version.

The emulation actually calls the CreateNewChar functions just like the flserver does, so any remaining problems should be fixed then.