Cleaned up `CreatePlayer`.

This is now as close as it gets without switching to enum types/parameters everywhere, so the compiler can optimize all accesses/checks accordingly. This is especially noticeable in the way the code for the switch is generated (line 775). In the original binary you can see it does weird stuff if c is not one of the classes' enum value (probably due to optimization since the switch should be exhaustive). I've tried switching the type of c to _ui_classes, also removing the UI_NUM_CLASSES value to see if that'd be enough to let it generate the optimized code, but nope, seems like we need to change it all at once. Currently, _pClass is a char, but an enum value would fit there as well, size-wise (alignment). That's also why I'm guessing there had to be another enum for player classes, without the UI_NUM_CLASSES value...
8 years ago · 4ae7211fa0
4 changed files with 156 additions and 160 deletions
--- a/Source/player.cpp
+++ b/Source/player.cpp
@ -675,168 +675,162 @@ void __fastcall ClearPlrRVars(PlayerStruct *p)
 	p->dwReserved[6] = 0;
 }

-void __fastcall CreatePlayer(int pnum, char c)
+// TODO: c should be of type _ui_classes in order to allow
+// the compiler to optimize the size and accesses to c
+// (especially the switch statements)
+void __fastcall CreatePlayer(int pnum, int c)
 {
-	unsigned int v2; // edi
-	char v3; // bl
-	int v4; // esi
-	int v5; // eax
-	int v6; // ecx
-	char v7; // al
-	char v8; // al
-	char v9; // al
-	char v10; // al
-	int v11; // edi
-	signed int v12; // ebp
-	int v13; // eax
-	int v14; // eax
-	int v15; // eax
-	int v16; // eax
-	int v17; // eax
-	bool v18; // zf
-	char v19; // [esp+Ch] [ebp-8h]
-	int arglist; // [esp+10h] [ebp-4h]
+	int i;

-	v2 = pnum;
-	v3 = c;
-	v4 = pnum;
-	v19 = c;
-	arglist = pnum;
 	ClearPlrRVars(&plr[pnum]);
-	v5 = GetTickCount();
-	SetRndSeed(v5);
-	if ( v2 >= MAX_PLRS )
-		TermMsg("CreatePlayer: illegal player %d", v2);
-	v6 = v3;
-	_LOBYTE(plr[v4]._pClass) = v3;
-	v7 = StrengthTbl[v6];
-	if ( v7 < 0 )
-		v7 = 0;
-	plr[v4]._pStrength = v7;
-	plr[v4]._pBaseStr = v7;
-	v8 = MagicTbl[v6];
-	if ( v8 < 0 )
-		v8 = 0;
-	plr[v4]._pMagic = v8;
-	plr[v4]._pBaseMag = v8;
-	v9 = DexterityTbl[v6];
-	if ( v9 < 0 )
-		v9 = 0;
-	plr[v4]._pDexterity = v9;
-	plr[v4]._pBaseDex = v9;
-	v10 = VitalityTbl[v6];
-	if ( v10 < 0 )
-		v10 = 0;
-	v11 = v10;
-	plr[v4]._pVitality = v10;
-	plr[v4]._pBaseVit = v10;
-	plr[v4]._pStatPts = 0;
-	plr[v4].pTownWarps = 0;
-	plr[v4].pDungMsgs = 0;
-	plr[v4].pLvlLoad = 0;
-	plr[v4].pDiabloKillLevel = 0;
-	if ( v19 == 1 )
-	{
-		v12 = 200;
-		v13 = plr[v4]._pLevel * (plr[v4]._pStrength + plr[v4]._pDexterity);
+	SetRndSeed(GetTickCount());
+
+	if ( pnum >= MAX_PLRS )
+	{
+		TermMsg("CreatePlayer: illegal player %d", pnum);
+	}
+	plr[pnum]._pClass = c;
+
+	plr[pnum]._pStrength = StrengthTbl[c] < 0 ? 0 : StrengthTbl[c];
+	plr[pnum]._pBaseStr = plr[pnum]._pStrength;
+
+	plr[pnum]._pMagic = MagicTbl[c] < 0 ? 0 : MagicTbl[c];
+	plr[pnum]._pBaseMag = plr[pnum]._pMagic;
+
+	plr[pnum]._pDexterity = DexterityTbl[c] < 0 ? 0 : DexterityTbl[c];
+	plr[pnum]._pBaseDex = plr[pnum]._pDexterity;
+
+	int vit = VitalityTbl[c] < 0 ? 0 : VitalityTbl[c];
+	plr[pnum]._pVitality = vit;
+	plr[pnum]._pBaseVit = vit;
+
+	plr[pnum]._pStatPts = 0;
+	plr[pnum].pTownWarps = 0;
+	plr[pnum].pDungMsgs = 0;
+	plr[pnum].pLvlLoad = 0;
+	plr[pnum].pDiabloKillLevel = 0;
+
+	if ( c == UI_ROGUE )
+	{
+		plr[pnum]._pDamageMod = (plr[pnum]._pLevel * (plr[pnum]._pStrength + plr[pnum]._pDexterity)) / 200;
 	}
 	else
 	{
-		v13 = plr[v4]._pStrength * plr[v4]._pLevel;
-		v12 = 100;
-	}
-	plr[v4]._pDamageMod = v13 / v12;
-	plr[v4]._pBaseToBlk = ToBlkTbl[v6];
-	plr[v4]._pHitPoints = (v11 + 10) << 6;
-	if ( !v19 )
-		plr[v4]._pHitPoints = (v11 + 10) << 7;
-	if ( v19 == 1 )
-		plr[v4]._pHitPoints += plr[v4]._pHitPoints >> 1;
-	v14 = plr[v4]._pHitPoints;
-	plr[v4]._pMaxHP = v14;
-	plr[v4]._pHPBase = v14;
-	plr[v4]._pMaxHPBase = v14;
-	v15 = plr[v4]._pMagic << 6;
-	plr[v4]._pMana = v15;
-	if ( v19 == 2 )
-		plr[v4]._pMana = 2 * v15;
-	if ( v19 == 1 )
-		plr[v4]._pMana += plr[v4]._pMana >> 1;
-	v16 = plr[v4]._pMana;
-	plr[v4]._pMaxMana = v16;
-	plr[v4]._pManaBase = v16;
-	plr[v4]._pMaxManaBase = v16;
-	v17 = ExpLvlsTbl[1];
-	plr[v4]._pLevel = 1;
-	plr[v4]._pMaxLvl = 1;
-	plr[v4]._pExperience = 0;
-	plr[v4]._pMaxExp = 0;
-	plr[v4]._pNextExper = v17;
-	plr[v4]._pArmorClass = 0;
-	plr[v4]._pMagResist = 0;
-	plr[v4]._pFireResist = 0;
-	plr[v4]._pLghtResist = 0;
-	plr[v4]._pLightRad = 10;
-	plr[v4]._pInfraFlag = 0;
-	if ( !v19 )
-	{
-		plr[v4]._pAblSpells[0] = 0x2000000;
-LABEL_26:
-		plr[v4]._pAblSpells[1] = 0;
-LABEL_27:
-		plr[v4]._pMemSpells[0] = 0;
-		goto LABEL_28;
+		plr[pnum]._pDamageMod = (plr[pnum]._pStrength * plr[pnum]._pLevel) / 100;
 	}
-	if ( v19 == 1 )
+
+	plr[pnum]._pBaseToBlk = ToBlkTbl[c];
+
+	plr[pnum]._pHitPoints = (vit + 10) << 6;
+	if ( c == UI_WARRIOR )
 	{
-		plr[v4]._pAblSpells[0] = 0x8000000;
-		goto LABEL_26;
+		plr[pnum]._pHitPoints = (vit + 10) << 7;
 	}
-	if ( v19 != 2 )
-		goto LABEL_27;
-	plr[v4]._pAblSpells[0] = 0x4000000;
-	plr[v4]._pAblSpells[1] = 0;
-	plr[v4]._pMemSpells[0] = 1;
-LABEL_28:
-	plr[v4]._pMemSpells[1] = 0;
-	memset(plr[v4]._pSplLvl, 0, sizeof(plr[v4]._pSplLvl));
-	v18 = _LOBYTE(plr[v4]._pClass) == 2;
-	_LOBYTE(plr[v4]._pSpellFlags) = 0;
-	if ( v18 )
-		plr[v4]._pSplLvl[1] = 2;
-	plr[v4]._pSplHotKey[0] = -1;
-	plr[v4]._pSplHotKey[1] = -1;
-	plr[v4]._pSplHotKey[2] = -1;
-	if ( v19 )
+	if ( c == UI_ROGUE )
 	{
-		if ( v19 == 1 )
-		{
-			plr[v4]._pgfxnum = 4;
-		}
-		else if ( v19 == 2 )
-		{
-			plr[v4]._pgfxnum = 8;
-		}
+		plr[pnum]._pHitPoints += plr[pnum]._pHitPoints >> 1;
 	}
-	else
+
+	plr[pnum]._pMaxHP = plr[pnum]._pHitPoints;
+	plr[pnum]._pHPBase = plr[pnum]._pHitPoints;
+	plr[pnum]._pMaxHPBase = plr[pnum]._pHitPoints;
+
+	plr[pnum]._pMana = plr[pnum]._pMagic << 6;
+	if ( c == UI_SORCERER )
+	{
+		plr[pnum]._pMana *= 2;
+	}
+
+	if ( c == UI_ROGUE )
+	{
+		plr[pnum]._pMana += plr[pnum]._pMana >> 1;
+	}
+
+	plr[pnum]._pMaxMana = plr[pnum]._pMana;
+	plr[pnum]._pManaBase = plr[pnum]._pMana;
+	plr[pnum]._pMaxManaBase = plr[pnum]._pMana;
+
+	plr[pnum]._pLevel = 1;
+	plr[pnum]._pMaxLvl = 1;
+	plr[pnum]._pExperience = 0;
+	plr[pnum]._pMaxExp = 0;
+	plr[pnum]._pNextExper = ExpLvlsTbl[1];
+	plr[pnum]._pArmorClass = 0;
+	plr[pnum]._pMagResist = 0;
+	plr[pnum]._pFireResist = 0;
+	plr[pnum]._pLghtResist = 0;
+	plr[pnum]._pLightRad = 10;
+	plr[pnum]._pInfraFlag = 0;
+
+	switch ( c )
+	{
+		case UI_WARRIOR:
+			plr[pnum]._pAblSpells[0] = 0x2000000;
+			plr[pnum]._pAblSpells[1] = 0;
+			plr[pnum]._pMemSpells[0] = 0;
+			break;
+		case UI_ROGUE:
+			plr[pnum]._pAblSpells[0] = 0x8000000;
+			plr[pnum]._pAblSpells[1] = 0;
+			plr[pnum]._pMemSpells[0] = 0;
+			break;
+		case UI_SORCERER:
+			plr[pnum]._pAblSpells[0] = 0x4000000;
+			plr[pnum]._pAblSpells[1] = 0;
+			plr[pnum]._pMemSpells[0] = 1;
+			break;
+	}
+	plr[pnum]._pMemSpells[1] = 0;
+
+	for ( i = 0; i < sizeof(plr[pnum]._pSplLvl); i++ )
+	{
+		plr[pnum]._pSplLvl[i] = 0;
+	}
+
+	_LOBYTE(plr[pnum]._pSpellFlags) = 0;
+
+	if ( plr[pnum]._pClass == UI_SORCERER )
+	{
+		plr[pnum]._pSplLvl[1] = 2;
+	}
+
+	// interestingly, only the first three hotkeys are reset
+	for ( i = 0; i < 3; i++ )
+	{
+		plr[pnum]._pSplHotKey[i] = -1;
+	}
+
+	switch ( c )
+	{
+		case UI_WARRIOR:
+			plr[pnum]._pgfxnum = 3;
+			break;
+		case UI_ROGUE:
+			plr[pnum]._pgfxnum = 4;
+			break;
+		case UI_SORCERER:
+			plr[pnum]._pgfxnum = 8;
+			break;
+	}
+
+	for ( i = 0; i < sizeof(plr[pnum]._pLvlVisited); i++ )
+	{
+		plr[pnum]._pLvlVisited[i] = 0;
+	}
+
+	for ( i = 0; i < sizeof(plr[pnum]._pSLvlVisited); i++ )
 	{
-		plr[v4]._pgfxnum = 3;
-	}
-	*(_DWORD *)plr[v4]._pLvlVisited = 0;
-	*(_DWORD *)&plr[v4]._pLvlVisited[4] = 0;
-	*(_DWORD *)&plr[v4]._pLvlVisited[8] = 0;
-	*(_DWORD *)&plr[v4]._pLvlVisited[12] = 0;
-	plr[v4]._pLvlVisited[16] = 0;
-	*(_DWORD *)plr[v4]._pSLvlVisited = 0;
-	*(_DWORD *)&plr[v4]._pSLvlVisited[4] = 0;
-	*(_WORD *)&plr[v4]._pSLvlVisited[8] = 0;
-	plr[v4]._pLvlChanging = 0;
-	plr[v4].pTownWarps = 0;
-	plr[v4].pLvlLoad = 0;
-	plr[v4].pBattleNet = 0;
-	plr[v4].pManaShield = 0;
-	InitDungMsgs(arglist);
-	CreatePlrItems(arglist);
+		plr[pnum]._pSLvlVisited[i] = 0;
+	}
+
+	plr[pnum]._pLvlChanging = 0;
+	plr[pnum].pTownWarps = 0;
+	plr[pnum].pLvlLoad = 0;
+	plr[pnum].pBattleNet = 0;
+	plr[pnum].pManaShield = 0;
+
+	InitDungMsgs(pnum);
+	CreatePlrItems(pnum);
 	SetRndSeed(0);
 }

@ -848,13 +842,13 @@ int __fastcall CalcStatDiff(int pnum)
 	v1 = pnum;
 	v2 = SLOBYTE(plr[v1]._pClass);
 	return MaxStats[v2][0]
-		 + MaxStats[v2][1]
-		 + MaxStats[v2][2]
-		 + MaxStats[v2][3]
-		 - plr[v1]._pBaseVit
-		 - plr[v1]._pBaseDex
-		 - plr[v1]._pBaseMag
-		 - plr[v1]._pBaseStr;
+		+ MaxStats[v2][1]
+		+ MaxStats[v2][2]
+		+ MaxStats[v2][3]
+		- plr[v1]._pBaseVit
+		- plr[v1]._pBaseDex
+		- plr[v1]._pBaseMag
+		- plr[v1]._pBaseStr;
 }

 void __fastcall NextPlrLevel(int pnum)
--- a/Source/player.h
+++ b/Source/player.h
@ -30,7 +30,7 @@ void __fastcall NewPlrAnim(int pnum, unsigned char *Peq, int numFrames, int Dela
 void __fastcall ClearPlrPVars(int pnum);
 void __fastcall SetPlrAnims(int pnum);
 void __fastcall ClearPlrRVars(PlayerStruct *p);
-void __fastcall CreatePlayer(int pnum, char c);
+void __fastcall CreatePlayer(int pnum, int c);
 int __fastcall CalcStatDiff(int pnum);
 void __fastcall NextPlrLevel(int pnum);
 void __fastcall AddPlrExperience(int pnum, int lvl, int exp);
--- a/defs.h
+++ b/defs.h
@ -5,7 +5,9 @@

 #define LIGHTSIZE				6912 // 27 * 256

-#define MAX_PLRS				4
+// must be unsigned to generate unsigned comparisons with pnum
+#define MAX_PLRS				4U
+
 #define MAX_CHARACTERS			10
 #define MAX_LVLMTYPES			16
 // #define MAX_PATH				260
--- a/structs.h
+++ b/structs.h
@ -887,7 +887,7 @@ struct PlayerStruct
 	int _pVar6;
 	int _pVar7;
 	int _pVar8;
-	unsigned char _pLvlVisited[17]; // NUMLEVELS
+	unsigned char _pLvlVisited[NUMLEVELS];
 	unsigned char _pSLvlVisited[10];
 	char gap20F[9];
 	int _pGFXLoad;