fix some critical bugs

  Aside from obvious gameplay bugs pretty much everywhere, the
game is finally in a PLAYABLE state. It should be smooth sailing
to get it stable for an initial test server.

5 files changed, 65 insertions, 73 deletions

diff --git a/src/communication.cc b/src/communication.cc
index cdfb6c7..b981d1e 100644
--- a/src/communication.cc
+++ b/src/communication.cc
@@ -241,26 +241,53 @@ void ExitLoadHistory(void){
 
 // Connection Output
 // =============================================================================
-bool WriteToSocket(TConnection *Connection, uint8 *Buffer, int Size){
-	// IMPORTANT(fusion): The caller must ensure `Buffer` have two extra bytes
-	// at the beginning and enough room at the end to properly add padding for
-	// XTEA encryption when needed. `Size` refers to the payload size which
-	// starts after the first two extra bytes. See note in `SendData`.
-
-	while((Size % 8) != 0){
-		Buffer[Size + 2] = rand_r(&Connection->RandomSeed);
+static constexpr int GetPacketSize(int DataSize){
+	// NOTE(fusion): This can be annoying but we're compiling with C++11 which
+	// means constexpr functions can only have a single return statement.
+#if __cplusplus > 201103L
+	int EncryptedSize = ((DataSize + 2) + 7) & ~7;
+	int PacketSize = EncryptedSize + 2;
+	return PacketSize;
+#else
+	return (((DataSize + 2) + 7) & ~7) + 2;
+#endif
+}
+
+bool WriteToSocket(TConnection *Connection, uint8 *Buffer, int Size, int MaxSize){
+	// IMPORTANT(fusion): The final packet will have the following layout:
+	//	PLAIN:
+	//		0 .. 2 => Encrypted Size
+	//	ENCRYPTED:
+	//		2 .. 4 => Data Size
+	//		4 ..   => Data + Padding
+	//
+	//	The caller must ensure `Buffer` has four extra bytes at the beginning so
+	// the packet and payload sizes can be written. It should also ensure that
+	// `(MaxSize % 8) == 2` so we can always add the necessary amount of padding
+	// for encryption.
+	ASSERT(Size >= 4 && Size <= MaxSize && MaxSize <= UINT16_MAX);
+
+	int DataSize = Size - 4;
+	while((Size % 8) != 2 && Size < MaxSize){
+		Buffer[Size] = rand_r(&Connection->RandomSeed);
 		Size += 1;
 	}
 
-	for(int i = 0; i < Size; i += 8){
-		Connection->SymmetricKey.encrypt(&Buffer[i + 2]);
+	if((Size % 8) != 2){
+		error("WriteToSocket: Failed to add padding (Size: %d, MaxSize: %d)\n",
+				Size, MaxSize);
+		return false;
 	}
 
-	TWriteBuffer WriteBuffer(Buffer, 2);
-	WriteBuffer.writeWord((uint16)Size);
+	TWriteBuffer WriteBuffer(Buffer, 4);
+	WriteBuffer.writeWord((uint16)(Size - 2));
+	WriteBuffer.writeWord((uint16)(DataSize));
+	for(int i = 2; i < Size; i += 8){
+		Connection->SymmetricKey.encrypt(&Buffer[i]);
+	}
 
 	int Attempts = 50;
-	int BytesToWrite = Size + 2;
+	int BytesToWrite = Size;
 	uint8 *WritePtr = Buffer;
 	while(BytesToWrite > 0){
 		int BytesWritten = (int)write(Connection->GetSocket(), WritePtr, BytesToWrite);
@@ -324,19 +351,22 @@ bool SendLoginMessage(TConnection *Connection, int Type, const char *Message, in
 	// IMPORTANT(fusion): This is doing the same thing as `SendData` but in a
 	// smaller scale and building the packet directly instead of using the
 	// connection's write buffer.
-	uint8 Data[302]; // 2 + 300
-	TWriteBuffer WriteBuffer(Data + 2, sizeof(Data) - 2);
-	WriteBuffer.writeWord(0);
-	WriteBuffer.writeByte((uint8)Type);
-	WriteBuffer.writeString(Message);
-	if(Type == 22){
-		WriteBuffer.writeByte(WaitingTime);
-	}
+	try{
+		uint8 Data[GetPacketSize(300)];
+		TWriteBuffer WriteBuffer(Data, sizeof(Data));
+		WriteBuffer.writeWord(0); // EncryptedSize
+		WriteBuffer.writeWord(0); // DataSize
+		WriteBuffer.writeByte((uint8)Type);
+		WriteBuffer.writeString(Message);
+		if(Type == 22){
+			WriteBuffer.writeByte(WaitingTime);
+		}
 
-	int Size = WriteBuffer.Position;
-	WriteBuffer.Position = 0;
-	WriteBuffer.writeWord((uint16)(Size - 2));
-	return WriteToSocket(Connection, Data, Size);
+		return WriteToSocket(Connection, Data, WriteBuffer.Position, WriteBuffer.Size);
+	}catch(const char *str){
+		error("SendLoginMessage: Fehler beim Füllen des Puffers (%s)\n", str);
+		return true;
+	}
 }
 
 bool SendData(TConnection *Connection){
@@ -345,24 +375,8 @@ bool SendData(TConnection *Connection){
 		return false;
 	}
 
-	//
-	// IMPORTANT(fusion): The final packet will have this layout:
-	//	PLAIN:
-	//		0 .. 2 => Encrypted Size
-	//	ENCRYPTED:
-	//		2 .. 4 => Data Size
-	//		4 ..   => Data + Padding
-	//
-	//	The ENCRYPTED size needs to be a multiple of 8 for XTEA encryption and
-	// `WriteToSocket` will add padding to ensure it is. This also requires us
-	// to ensure the passed buffer has enough room for it, so we don't write out
-	// of bounds.
-	//
-
 	int DataSize = Connection->NextToCommit - Connection->NextToSend;
-	int EncryptedSize = (DataSize + 2);
-	int EncryptedSizeAligned = (EncryptedSize + 7) & ~7;
-	int PacketSize = EncryptedSizeAligned + 2;
+	int PacketSize = GetPacketSize(DataSize);
 
 	// TODO(fusion): The maximum size of a packet depends on the size of
 	// `Connection->OutData` and I don't think we'd have a problem with
@@ -371,8 +385,8 @@ bool SendData(TConnection *Connection){
 	// problem.
 	uint8 *Buffer = (uint8*)alloca(PacketSize);
 	TWriteBuffer WriteBuffer(Buffer, PacketSize);
-	WriteBuffer.writeWord(0);
-	WriteBuffer.writeWord((uint16)DataSize);
+	WriteBuffer.writeWord(0); // EncryptedSize
+	WriteBuffer.writeWord(0); // DataSize
 
 	// NOTE(fusion): `Connection->OutData` is a ring buffer so we need to check
 	// if the data we're currently sending is wrapping around, in which case we'd
@@ -387,34 +401,13 @@ bool SendData(TConnection *Connection){
 		WriteBuffer.writeBytes(&Connection->OutData[0],         DataEnd - OutDataSize);
 	}
 
-	bool Result = WriteToSocket(Connection, Buffer, DataSize);
+	bool Result = WriteToSocket(Connection, Buffer, WriteBuffer.Position, WriteBuffer.Size);
 	if(Result){
 		Connection->NextToSend += DataSize;
 	}
 	return Result;
 }
 
-bool SendData(TConnection *Connection, const uint8 *Data, int Size){
-	if(Connection == NULL){
-		error("SendData: Connection ist NULL.\n");
-		return false;
-	}
-
-	// TODO(fusion): Why are we returning true for invalid parameters?
-	if(Data == NULL){
-		error("SendData: Data ist NULL.\n");
-		return true;
-	}
-
-	// TODO(fusion): Why do we leave extra two bytes unassigned at the beginning?
-	// `SendData` should already take care of building the packet properly so this
-	// is something else or a bug, if this function is even used.
-	memcpy(&Connection->OutData[2], Data, Size);
-	Connection->NextToSend = 0;
-	Connection->NextToCommit = Size + 2;
-	return SendData(Connection);
-}
-
 // Waiting List
 // =============================================================================
 bool GetWaitinglistEntry(const char *Name, uint32 *NextTry, bool *FreeAccount, bool *Newbie){
diff --git a/src/communication.hh b/src/communication.hh
index 8d2574b..df0e00a 100644
--- a/src/communication.hh
+++ b/src/communication.hh
@@ -28,10 +28,9 @@ void NetLoadCheck(void);
 void InitLoadHistory(void);
 void ExitLoadHistory(void);
 
-bool WriteToSocket(TConnection *Connection, uint8 *Buffer, int Size);
+bool WriteToSocket(TConnection *Connection, uint8 *Buffer, int Size, int MaxSize);
 bool SendLoginMessage(TConnection *Connection, int Type, const char *Message, int WaitingTime);
 bool SendData(TConnection *Connection);
-bool SendData(TConnection *Connection, const uint8 *Data, int Size);
 
 bool GetWaitinglistEntry(const char *Name, uint32 *NextTry, bool *FreeAccount, bool *Newbie);
 void InsertWaitinglistEntry(const char *Name, uint32 NextTry, bool FreeAccount, bool Newbie);
diff --git a/src/cract.cc b/src/cract.cc
index 8397d79..f30415d 100644
--- a/src/cract.cc
+++ b/src/cract.cc
@@ -1513,7 +1513,7 @@ void TCreature::NotifyChangeInventory(void){
 
 	this->Combat.CheckCombatValues();
 	if(this->Type == PLAYER){
-		int NewDelta[NARRAY(this->Skills)];
+		int NewDelta[NARRAY(this->Skills)] = {};
 		for(int Position = INVENTORY_FIRST;
 				Position <= INVENTORY_LAST;
 				Position += 1){
diff --git a/src/crnonpl.cc b/src/crnonpl.cc
index b841d18..2e56f66 100644
--- a/src/crnonpl.cc
+++ b/src/crnonpl.cc
@@ -2146,7 +2146,7 @@ bool TMonster::MovePossible(int x, int y, int z, bool Execute, bool Jump){
 	// objects away.
 	for(int Attempt = 0; Attempt < 100; Attempt += 1){
 		Object Obj = GetFirstObject(x, y, z);
-		if(!Obj.getObjectType().getFlag(BANK)){
+		if(Obj == NONE || !Obj.getObjectType().getFlag(BANK)){
 			return false;
 		}
 
diff --git a/src/sending.cc b/src/sending.cc
index 3b22cc3..d1cca27 100644
--- a/src/sending.cc
+++ b/src/sending.cc
@@ -452,7 +452,7 @@ void SendFullScreen(TConnection *Connection){
 		int ZOffset = (PlayerZ - PointZ);
 		for(int PointX = MinX; PointX <= MaxX; PointX += 1)
 		for(int PointY = MinY; PointY <= MaxY; PointY += 1){
-			SendMapPoint(Connection, PointX + ZOffset, PointY + ZOffset, PlayerZ);
+			SendMapPoint(Connection, PointX + ZOffset, PointY + ZOffset, PointZ);
 		}
 	}
 	SkipFlush(Connection);
@@ -506,7 +506,7 @@ void SendRow(TConnection *Connection, int Direction){
 		int ZOffset = (PlayerZ - PointZ);
 		for(int PointX = MinX; PointX <= MaxX; PointX += 1)
 		for(int PointY = MinY; PointY <= MaxY; PointY += 1){
-			SendMapPoint(Connection, PointX + ZOffset, PointY + ZOffset, PlayerZ);
+			SendMapPoint(Connection, PointX + ZOffset, PointY + ZOffset, PointZ);
 		}
 	}
 	SkipFlush(Connection);
@@ -568,7 +568,7 @@ void SendFloors(TConnection *Connection, bool Up){
 			int ZOffset = (PlayerZ - PointZ);
 			for(int PointX = MinX; PointX <= MaxX; PointX += 1)
 			for(int PointY = MinY; PointY <= MaxY; PointY += 1){
-				SendMapPoint(Connection, PointX + ZOffset, PointY + ZOffset, PlayerZ);
+				SendMapPoint(Connection, PointX + ZOffset, PointY + ZOffset, PointZ);
 			}
 		}
 		SkipFlush(Connection);