[2016-04-17] The Redesign of Client Interface and Private Headers

[hemagx 69]

Rationale:
For ages we got awful looking code when it come to work with socket and packet, also this code was totally manually handled.
so many possible mistake could happen and lot of hardcoded numbers, now with this update this will not happen anymore and compiler will take care of all of this.
also with this update we introduce first Private header, the lclif ( Login Client Interface ) header, the private headers meant for functions should not be accessed from outside the source file, however this still accessible by plugins.
this also will prevent a long not needed re-compile time for whole project when the changes only happens in private headers.
 
Contents:
A total new client interface for login server (lclif.c/lclif.h).
The first private header which meant for client interface in login server (lclif.p.h).
 
Impact:
The impact of this changes can be huge if you have heavy modified login server as many functions removed, moved or rewrote.
please be careful merging this update.
 
Details:
Before we were dealing with packets in so ugly way and unsafe with hardcoded numbers and easy mistakes that were hard to be spoted, also we mixed code to parse packets with code to process them.
With this update we separate packet parse and packet processing, and also we started to use structs instead of manual (offset-based) packet parsing and creation!
 
an example of how we were writing a packet to client before, and now.
 
Before:
Here's an example of the server-list packet being sent to the player upon successful login

	WFIFOHEAD(fd,47+32*server_num);
	WFIFOW(fd,0) = 0x69;
	WFIFOW(fd,2) = 47+32*server_num;
	WFIFOL(fd,4) = sd->login_id1;
	WFIFOL(fd,8) = sd->account_id;
	WFIFOL(fd,12) = sd->login_id2;
	WFIFOL(fd,16) = 0; // in old version, that was for ip (not more used)
	//memcpy(WFIFOP(fd,20), sd->lastlogin, 24); // in old version, that was for name (not more used)
	memset(WFIFOP(fd,20), 0, 24);
	WFIFOW(fd,44) = 0; // unknown
	WFIFOB(fd,46) = sex_str2num(sd->sex);
	for (i = 0, n = 0; i < ARRAYLENGTH(server); ++i) {
		uint32 subnet_char_ip;

		if (!sockt->session_is_valid(server[i].fd))
			continue;

		subnet_char_ip = login->lan_subnet_check(ip);
		WFIFOL(fd,47+n*32) = htonl((subnet_char_ip) ? subnet_char_ip : server[i].ip);
		WFIFOW(fd,47+n*32+4) = sockt->ntows(htons(server[i].port)); // [!] LE byte order here [!]
		memcpy(WFIFOP(fd,47+n*32+6), server[i].name, 20);
		WFIFOW(fd,47+n*32+26) = server[i].users;

		if( server[i].type == CST_PAYING && sd->expiration_time > time(NULL) )
			WFIFOW(fd,47+n*32+28) = CST_NORMAL;
		else
			WFIFOW(fd,47+n*32+28) = server[i].type;

		WFIFOW(fd,47+n*32+30) = server[i].new_;
		n++;
	}
	WFIFOSET(fd,47+32*server_num);

 
So many numbers! And a real mess, imagine the harm that one wrong number can cause! even crash for client or server !
And the code is barely readable or meaningful.
 
After the update:
 

	length = sizeof(*packet) + sizeof(packet->ServerList[0]) * server_num;
	ip = sockt->session[sd->fd]->client_addr;

	//Allocate the packet
	WFIFOHEAD(sd->fd, length);
	packet = WP2PTR(sd->fd);

	packet->PacketType = PACKET_ID_AC_ACCEPT_LOGIN;
	packet->PacketLength = length;
	packet->AuthCode = sd->login_id1;
	packet->AID = sd->account_id;
	packet->userLevel = sd->login_id2;
	packet->lastLoginIP = 0x0;
	memset(packet->lastLoginTime, '\0', sizeof(packet->lastLoginTime));
	packet->Sex = sex_str2num(sd->sex);
	for (i = 0, n = 0;  i < ARRAYLENGTH(server); ++i) {
		uint32 subnet_char_ip;

		if (!sockt->session_is_valid(server[i].fd))
			continue;

		subnet_char_ip = login->lan_subnet_check(ip);
		packet->ServerList[n].ip = htonl((subnet_char_ip) ? subnet_char_ip : server[i].ip);
		packet->ServerList[n].port = sockt->ntows(htons(server[i].port)); // [!] LE byte order here [!]
		safestrncpy(packet->ServerList[n].name, server[i].name, 20);
		packet->ServerList[n].usercount = server[i].users;

		if( server[i].type == CST_PAYING && sd->expiration_time > time(NULL) )
			packet->ServerList[n].property = CST_NORMAL;
		else
			packet->ServerList[n].property = server[i].type;

		packet->ServerList[n].state = server[i].new_;
		++n;
	}
	WFIFOSET(sd->fd, length);

 
I bet now you can place everything and know what you're dealing with, with no numbers to mistake with at all!
 
This is how we build a packet, parsing a packet became easier too.
 
Before:

version = RFIFOL(fd,2);
safestrncpy(username, (const char*)RFIFOP(fd,6), NAME_LENGTH);
safestrncpy(password, (const char*)RFIFOP(fd,30), NAME_LENGTH);
clienttype = RFIFOB(fd,54);

 
After:

DEFPACKET(CA_LOGIN)
{
	const struct PACKET_CA_LOGIN *packet = RP2PTR(fd);
	
	sd->version = packet->Version;
	sd->clienttype = packet->clienttype;
	safestrncpy(sd->userid, packet->ID, NAME_LENGTH);
	safestrncpy(sd->passwd, packet->Passwd, PASSWD_LEN);

	if (login->config->use_md5_passwds)
		MD5_String(sd->passwd, sd->passwd);
	sd->passwdenc = PWENC_NONE;

	login->client_login(fd, sd);
	return PACKET_VALID;
}

 
we just took care of parse packet and give it to login server to process it, just as it should be, the code is cleaner and easier to read and understand.
 
Merge Date:

Sat, 16 Apr 2016 15:37:47 +0200

 

Related Pull Requests:
- #1255 - https://github.com/H...cules/pull/1255 - Login clif rewrite [hemagx]

Related Commits:

- bbcb040 - https://github.com/HerculesWS/Hercules/commit/bbcb040 - Sat, 16 Apr 2016 15:37:47 +0200 Merge pull request #1255 from HerculesWS/login-clif_rewrite [ibrahem Hossam]
- 8448e3f - https://github.com/HerculesWS/Hercules/commit/8448e3f - Sat, 16 Apr 2016 15:22:21 +0200 HPM Hooks Update [Haru]
- 15c9710 - https://github.com/HerculesWS/Hercules/commit/15c9710 - Sat, 16 Apr 2016 15:21:18 +0200 Moved packet_db to the private interface of lclif [Haru]
- 45bbb3d - https://github.com/HerculesWS/Hercules/commit/45bbb3d - Sat, 16 Apr 2016 03:33:31 +0200 Added missing documentation [Haru]
- 6de2242 - https://github.com/HerculesWS/Hercules/commit/6de2242 - Sat, 16 Apr 2016 03:33:20 +0200 HPM Hooks Update [Haru]
- fb26278 - https://github.com/HerculesWS/Hercules/commit/fb26278 - Fri, 15 Apr 2016 20:14:43 +0200 Added lclif packet handlers to the lclif interface [Haru]
- 37cc46c - https://github.com/HerculesWS/Hercules/commit/37cc46c - Thu, 31 Mar 2016 00:16:48 +0200 HPM Hooks Update [Haru]
- 7555700 - https://github.com/HerculesWS/Hercules/commit/7555700 - Mon, 28 Mar 2016 21:54:46 +0200 Rewrite client interface for login server (part 7) [hemagx]
- ceef84e - https://github.com/HerculesWS/Hercules/commit/ceef84e - Thu, 14 Apr 2016 13:38:12 +0200 HPM Hooks Update [Haru]
- c8ff1e7 - https://github.com/HerculesWS/Hercules/commit/c8ff1e7 - Tue, 12 Apr 2016 14:35:21 +0200 Rewrite client interface for login server (part 6) [hemagx]
- d8da35d - https://github.com/HerculesWS/Hercules/commit/d8da35d - Tue, 12 Apr 2016 15:25:50 +0200 Rewrite client interface for login server (part 5) [hemagx]
- c765365 - https://github.com/HerculesWS/Hercules/commit/c765365 - Tue, 12 Apr 2016 15:24:30 +0200 Rewrite client interface for login server (part 4) [hemagx]
- d60ef91 - https://github.com/HerculesWS/Hercules/commit/d60ef91 - Mon, 28 Mar 2016 21:54:46 +0200 Rewrite client interface for login server (part 3) [hemagx]
- 9defcee - https://github.com/HerculesWS/Hercules/commit/9defcee - Mon, 28 Mar 2016 21:54:46 +0200 Rewrite client interface for login server (part 2) [hemagx]
- 480e959 - https://github.com/HerculesWS/Hercules/commit/480e959 - Mon, 28 Mar 2016 21:54:46 +0200 Rewrite client interface for login server (part 1) [hemagx]

[hemagx 69]

unhided