From 60dc628e4874515179fcac442caa0f3b40f3914c Mon Sep 17 00:00:00 2001
From: Peter Nelson <peter1138@openttd.org>
Date: Wed, 24 Apr 2024 20:28:00 +0100
Subject: [PATCH] Codechange: Replace SaveLoad var length arrays with switch
 block and sizeof.

SlCalcConvMemLen(), SlCalcConfFileLen() and CalcOldVarLen() follow a pattern of looking up part of a value in an array.

These function returns the size of bytes of a variable type, but is not very clear. Replace with a switch block instead.

Removes lengthof, array indices, and magic numbers.
---
 src/saveload/oldloader.cpp | 20 ++++++++++++++++----
 src/saveload/saveload.cpp  | 37 ++++++++++++++++++++++++++-----------
 2 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/src/saveload/oldloader.cpp b/src/saveload/oldloader.cpp
index 8ec6f803c4..2150c9b902 100644
--- a/src/saveload/oldloader.cpp
+++ b/src/saveload/oldloader.cpp
@@ -33,12 +33,24 @@ static inline OldChunkType GetOldChunkType(OldChunkType type)     {return (OldCh
 static inline OldChunkType GetOldChunkVarType(OldChunkType type)  {return (OldChunkType)(GB(type, 8, 8) << 8);}
 static inline OldChunkType GetOldChunkFileType(OldChunkType type) {return (OldChunkType)(GB(type, 16, 8) << 16);}
 
+/**
+ * Return expected size in bytes of a OldChunkType
+ * @param type OldChunkType to get size of.
+ * @return size of type in bytes.
+ */
 static inline uint8_t CalcOldVarLen(OldChunkType type)
 {
-	static const uint8_t type_mem_size[] = {0, 1, 1, 2, 2, 4, 4, 8};
-	uint8_t length = GB(type, 8, 8);
-	assert(length != 0 && length < lengthof(type_mem_size));
-	return type_mem_size[length];
+	switch (GetOldChunkVarType(type)) {
+		case OC_VAR_I8: return sizeof(int8_t);
+		case OC_VAR_U8: return sizeof(uint8_t);
+		case OC_VAR_I16: return sizeof(int16_t);
+		case OC_VAR_U16: return sizeof(uint16_t);
+		case OC_VAR_I32: return sizeof(int32_t);
+		case OC_VAR_U32: return sizeof(uint32_t);
+		case OC_VAR_I64: return sizeof(int64_t);
+		case OC_VAR_U64: return sizeof(uint64_t);
+		default: NOT_REACHED();
+	}
 }
 
 /**
diff --git a/src/saveload/saveload.cpp b/src/saveload/saveload.cpp
index 28b65ba920..e32c64a4ac 100644
--- a/src/saveload/saveload.cpp
+++ b/src/saveload/saveload.cpp
@@ -599,17 +599,25 @@ static uint8_t GetSavegameFileType(const SaveLoad &sld)
  */
 static inline uint SlCalcConvMemLen(VarType conv)
 {
-	static const uint8_t conv_mem_size[] = {1, 1, 1, 2, 2, 4, 4, 8, 8, 0};
-
 	switch (GetVarMemType(conv)) {
+		case SLE_VAR_BL: return sizeof(bool);
+		case SLE_VAR_I8: return sizeof(int8_t);
+		case SLE_VAR_U8: return sizeof(uint8_t);
+		case SLE_VAR_I16: return sizeof(int16_t);
+		case SLE_VAR_U16: return sizeof(uint16_t);
+		case SLE_VAR_I32: return sizeof(int32_t);
+		case SLE_VAR_U32: return sizeof(uint32_t);
+		case SLE_VAR_I64: return sizeof(int64_t);
+		case SLE_VAR_U64: return sizeof(uint64_t);
+		case SLE_VAR_NULL: return 0;
+
 		case SLE_VAR_STR:
 		case SLE_VAR_STRQ:
 			return SlReadArrayLength();
 
+		case SLE_VAR_NAME:
 		default:
-			uint8_t type = GetVarMemType(conv) >> 4;
-			assert(type < lengthof(conv_mem_size));
-			return conv_mem_size[type];
+			NOT_REACHED();
 	}
 }
 
@@ -621,17 +629,24 @@ static inline uint SlCalcConvMemLen(VarType conv)
  */
 static inline uint8_t SlCalcConvFileLen(VarType conv)
 {
-	static const uint8_t conv_file_size[] = {0, 1, 1, 2, 2, 4, 4, 8, 8, 2};
-
 	switch (GetVarFileType(conv)) {
+		case SLE_FILE_END: return 0;
+		case SLE_FILE_I8: return sizeof(int8_t);
+		case SLE_FILE_U8: return sizeof(uint8_t);
+		case SLE_FILE_I16: return sizeof(int16_t);
+		case SLE_FILE_U16: return sizeof(uint16_t);
+		case SLE_FILE_I32: return sizeof(int32_t);
+		case SLE_FILE_U32: return sizeof(uint32_t);
+		case SLE_FILE_I64: return sizeof(int64_t);
+		case SLE_FILE_U64: return sizeof(uint64_t);
+		case SLE_FILE_STRINGID: return sizeof(uint16_t);
+
 		case SLE_FILE_STRING:
 			return SlReadArrayLength();
 
+		case SLE_FILE_STRUCT:
 		default:
-			uint8_t type = GetVarFileType(conv);
-			if (type >= lengthof(conv_file_size)) fmt::println("{}", type);
-			assert(type < lengthof(conv_file_size));
-			return conv_file_size[type];
+			NOT_REACHED();
 	}
 }