Merge branch 'accurate' into portable

CMakeLists.txt

@@ -18,6 +18,7 @@ set(ASSETS_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/assets")
 
 option(JAPANESE "Enable the Japanese-language build (instead of the unofficial Aeon Genesis English translation)" OFF)
 option(FIX_BUGS "Fix various bugs in the game" OFF)
+option(FIX_MAJOR_BUGS "Fix bugs that invoke undefined behaviour or cause memory leaks" ON)
 option(DEBUG_SAVE "Re-enable the ability to drag-and-drop save files onto the window" OFF)
 option(DOCONFIG "Compile a DoConfig clone tool - not useful for console ports" ON)
 option(LANCZOS_RESAMPLER "Use Lanczos filtering for audio resampling instead of linear-interpolation (Lanczos is more performance-intensive, but higher quality)" OFF)
@@ -265,6 +266,10 @@ if(FIX_BUGS)
 	target_compile_definitions(CSE2 PRIVATE FIX_BUGS)
 endif()
 
+if(FIX_BUGS OR FIX_MAJOR_BUGS)
+	target_compile_definitions(CSE2 PRIVATE FIX_MAJOR_BUGS)
+endif()
+
 if(DEBUG_SAVE)
 	target_compile_definitions(CSE2 PRIVATE DEBUG_SAVE)
 endif()

src/Back.cpp

@@ -29,7 +29,7 @@ BOOL InitBack(const char *fName, int type)
 
 	if (fgetc(fp) != 'B' || fgetc(fp) != 'M')
 	{
-#ifdef FIX_BUGS
+#ifdef FIX_MAJOR_BUGS
 		// The original game forgets to close fp
 		fclose(fp);
 #endif

src/Bullet.cpp

@@ -1644,7 +1644,7 @@ void ActBullet_Edge(BULLET *bul)
 			if (bul->ani_no > 4)
 			{
 				bul->cond = 0;
-			#ifdef FIX_BUGS
+			#ifdef FIX_MAJOR_BUGS
 				return;	// The code below will use 'ani_no' to access 'rcLeft' and 'rcRight', even though it's now too high
 			#endif
 			}
@@ -2182,7 +2182,7 @@ void ActBullet_SpurTail(BULLET *bul, int level)
 	if (bul->ani_no > 2)
 	{
 		bul->cond = 0;
-	#ifdef FIX_BUGS
+	#ifdef FIX_MAJOR_BUGS
 		return;	// Avoid accessing the RECT arrays with an out-of-bounds index
 	#endif
 	}

src/Caret.cpp

@@ -105,7 +105,7 @@ void ActCaret01(CARET *crt)
 		if (++crt->ani_no > 3)
 		{
 			crt->cond = 0;
-		#ifdef FIX_BUGS
+		#ifdef FIX_MAJOR_BUGS
 			return;	// The code below will use 'ani_no' to access 'rcLeft' and 'rcRight', even though it's now too high
 		#endif
 		}
@@ -155,7 +155,7 @@ void ActCaret02(CARET *crt)
 			if (crt->ani_no > 3)
 			{
 				crt->cond = 0;
-			#ifdef FIX_BUGS
+			#ifdef FIX_MAJOR_BUGS
 				return;	// The code below will use 'ani_no' to access 'rect_left', even though it's now too high
 			#endif
 			}
@@ -173,7 +173,7 @@ void ActCaret02(CARET *crt)
 			if (crt->ani_no > 3)
 			{
 				crt->cond = 0;
-			#ifdef FIX_BUGS
+			#ifdef FIX_MAJOR_BUGS
 				return;	// The code below will use 'ani_no' to access 'rect_right', even though it's now too high
 			#endif
 			}
@@ -208,7 +208,7 @@ void ActCaret03(CARET *crt)
 		if (++crt->ani_no > 3)
 		{
 			crt->cond = 0;
-		#ifdef FIX_BUGS
+		#ifdef FIX_MAJOR_BUGS
 			return;	// The code below will use 'ani_no' to access 'rect', even though it's now too high
 		#endif
 		}
@@ -242,7 +242,7 @@ void ActCaret04(CARET *crt)
 		if (++crt->ani_no > 2)
 		{
 			crt->cond = 0;
-		#ifdef FIX_BUGS
+		#ifdef FIX_MAJOR_BUGS
 			return;	// The code below will use 'ani_no' to access 'rect', even though it's now too high
 		#endif
 		}
@@ -273,7 +273,7 @@ void ActCaret05(CARET *crt)
 	if (crt->ani_no > 6)
 	{
 		crt->cond = 0;
-	#ifdef FIX_BUGS
+	#ifdef FIX_MAJOR_BUGS
 		return;	// The code below will use 'ani_no' to access 'rect', even though it's now too high
 	#endif
 	}
@@ -306,7 +306,7 @@ void ActCaret07(CARET *crt)
 		if (++crt->ani_no > 6)
 		{
 			crt->cond = 0;
-		#ifdef FIX_BUGS
+		#ifdef FIX_MAJOR_BUGS
 			return;	// The code below will use 'ani_no' to access 'rcLeft', even though it's now too high
 		#endif
 		}
@@ -432,7 +432,7 @@ void ActCaret11(CARET *crt)
 		if (++crt->ani_no > 6)
 		{
 			crt->cond = 0;
-		#ifdef FIX_BUGS
+		#ifdef FIX_MAJOR_BUGS
 			return;	// The code below will use 'ani_no' to access 'rcRight', even though it's now too high
 		#endif
 		}
@@ -456,7 +456,7 @@ void ActCaret12(CARET *crt)
 		if (++crt->ani_no > 1)
 		{
 			crt->cond = 0;
-		#ifdef FIX_BUGS
+		#ifdef FIX_MAJOR_BUGS
 			return;	// The code below will use 'ani_no' to access 'rcLeft', even though it's now too high
 		#endif
 		}
@@ -531,7 +531,7 @@ void ActCaret14(CARET *crt)
 		if (++crt->ani_no > 4)
 		{
 			crt->cond = 0;
-		#ifdef FIX_BUGS
+		#ifdef FIX_MAJOR_BUGS
 			return;	// The code below will use 'ani_no' to access 'rect', even though it's now too high
 		#endif
 		}
@@ -557,7 +557,7 @@ void ActCaret15(CARET *crt)
 		if (++crt->ani_no > 3)
 		{
 			crt->cond = 0;
-		#ifdef FIX_BUGS
+		#ifdef FIX_MAJOR_BUGS
 			return;	// The code below will use 'ani_no' to access 'rcLeft', even though it's now too high
 		#endif
 		}

src/Ending.cpp

@@ -250,7 +250,7 @@ BOOL StartCreditScript(void)
 	fread(Credit.pData, 1, Credit.size, fp);
 	EncryptionBinaryData2((unsigned char*)Credit.pData, Credit.size);
 
-#ifdef FIX_BUGS
+#ifdef FIX_MAJOR_BUGS
 	// The original game forgot to close the file
 	fclose(fp);
 #endif

src/Game.cpp

@@ -562,7 +562,7 @@ static int ModeAction(void)
 			ActBullet();
 			ActCaret();
 			MoveFrame3();
-#ifdef FIX_BUGS
+#ifdef FIX_MAJOR_BUGS
 			// ActFlash uses frame_x and frame_y uninitialised
 			GetFramePosition(&frame_x, &frame_y);
 #endif

src/MycParam.cpp

@@ -112,7 +112,7 @@ void DamageMyChar(int damage)
 #ifdef FIX_BUGS
 	if (!(g_GameFlags & 2))
 #else
-	// I'm preeeetty sure this is a typo. The Linux port optimised it out.
+	// I'm preeeetty sure this is a typo. The Linux port optimised this entire check out.
 	if (!(g_GameFlags | 2))
 #endif
 		return;
@@ -274,7 +274,7 @@ void PutArmsEnergy(BOOL flash)
 
 	int lv = gArmsData[gSelectedArms].level - 1;
 
-#ifdef FIX_BUGS
+#ifdef FIX_MAJOR_BUGS
 	// When the player has no weapons, the default level is 0, which becomes -1.
 	// Catch it, and set it to 0 instead, so the following array-accesses aren't
 	// out-of-bounds.

src/NpChar.cpp

@@ -71,7 +71,7 @@ BOOL LoadEvent(const char *path_event)
 	fread(code, 1, 4, fp);
 	if (memcmp(code, gPassPixEve, 3) != 0)
 	{
-#ifdef FIX_BUGS
+#ifdef FIX_MAJOR_BUGS
 		// The original game forgot to close the file here
 		fclose(fp);
 #endif

src/NpcAct000.cpp

@@ -59,7 +59,7 @@ void ActNpc001(NPCHAR *npc)
 		if (npc->xm < -0x600)
 			npc->xm = -0x600;
 #else
-		// Limit speed (except pixel applied it to the X position)
+		// Limit speed (except Pixel applied it to the X position)
 		if (npc->x < -0x600)
 			npc->x = -0x600;
 #endif

src/NpcAct120.cpp

@@ -590,7 +590,7 @@ void ActNpc127(NPCHAR *npc)
 		if (++npc->ani_no > 2)
 		{
 			npc->cond = 0;
-		#ifdef FIX_BUGS
+		#ifdef FIX_MAJOR_BUGS
 			return;	// The code below will use 'ani_no' to access 'rcH' and 'rcV', even though it's now too high
 		#endif
 		}
@@ -656,7 +656,7 @@ void ActNpc128(NPCHAR *npc)
 	if (++npc->ani_no > 4)
 	{
 		npc->cond = 0;
-	#ifdef FIX_BUGS
+	#ifdef FIX_MAJOR_BUGS
 		return;	// The code below will use 'ani_no' to access 'rcLeft' and co., even though it's now too high
 	#endif
 	}
@@ -717,7 +717,7 @@ void ActNpc129(NPCHAR *npc)
 		if (++npc->ani_no > 2)
 		{
 			npc->cond = 0;
-		#ifdef FIX_BUGS
+		#ifdef FIX_MAJOR_BUGS
 			return;	// The code below will use 'ani_no' to access 'rect', even though it's now too high
 		#endif
 		}

src/NpcAct140.cpp

@@ -753,7 +753,7 @@ void ActNpc146(NPCHAR *npc)
 			{
 				SetDestroyNpChar(npc->x, npc->y, 0x1000, 8);
 				npc->cond = 0;
-			#ifdef FIX_BUGS
+			#ifdef FIX_MAJOR_BUGS
 				return;	// The code below will use 'ani_no' to access 'rect', even though it's now too high
 			#endif
 			}

src/NpcAct180.cpp

@@ -1429,7 +1429,7 @@ void ActNpc199(NPCHAR *npc)
 	if (npc->ani_no > 4)
 	{
 		npc->cond = 0;
-	#ifdef FIX_BUGS
+	#ifdef FIX_MAJOR_BUGS
 		return;	// The code below will use 'ani_no' to access 'rect', even though it's now too high
 	#endif
 	}

src/Sound.cpp

@@ -37,10 +37,10 @@ BOOL InitDirectSound(void)
 
 	if (!audio_backend_initialised)
 	{
-#ifndef FIX_BUGS
+	#ifndef FIX_BUGS
 		// This makes absolutely no sense here
 		StartOrganya("Org/Wave.dat");
-#endif
+	#endif
 		return FALSE;
 	}
 
@@ -140,7 +140,7 @@ BOOL LoadSoundObject(const char *file_name, int no)
 	//	fread(&check_box[i], sizeof(char), 1, fp);	// Holy hell, this is inefficient
 	fread(check_box, 1, 58, fp);
 
-#ifdef FIX_BUGS
+#ifdef FIX_MAJOR_BUGS
 	// The original code forgets to close 'fp'
 	if (check_box[0] != 'R' || check_box[1] != 'I' || check_box[2] != 'F' || check_box[3] != 'F')
 	{
@@ -161,7 +161,7 @@ BOOL LoadSoundObject(const char *file_name, int no)
 	unsigned char *wp;
 	wp = (unsigned char*)malloc(file_size);	// ファイルのワークスペースを作る (Create a file workspace)
 
-#ifdef FIX_BUGS
+#ifdef FIX_MAJOR_BUGS
 	if (wp == NULL)
 	{
 		fclose(fp);
@@ -208,7 +208,7 @@ BOOL LoadSoundObject(const char *file_name, int no)
 
 	if (lpSECONDARYBUFFER[no] == NULL)
 	{
-#ifdef FIX_BUGS
+#ifdef FIX_MAJOR_BUGS
 		free(wp);	// The updated Organya source code includes this fix
 #endif
 		return FALSE;	

src/TextScr.cpp

@@ -680,7 +680,7 @@ int TextScriptProc(void)
 						x = GetTextScriptNo(gTS.p_read + 9);
 
 						gNumberTextScript[0] = x;
-					#ifndef FIX_BUGS
+					#ifndef FIX_MAJOR_BUGS
 						// z is uninitialised. Probably a leftover from copypasting this from elsewhere.
 						gNumberTextScript[1] = z;
 					#endif
@@ -905,6 +905,32 @@ int TextScriptProc(void)
 						x = GetTextScriptNo(gTS.p_read + 4);
 						z = GetTextScriptNo(gTS.p_read + 9);
 
+					#ifdef FIX_MAJOR_BUGS
+						// Some versions of the Waterway TSC script contain a bug:
+						//  <FLJ850:0111
+						// This command *should* be...
+						//  <FLJ0850:0111
+						// This bug causes the values to be misread as 8510 and 1075,
+						// leading to an out-of-bounds gFlagNPC access.
+						// As well as this, the out-of-bound read has a random chance
+						// of being true or false. If it's true, then the game will
+						// try to execute script 1075, causing a crash.
+						// To fix this, we manually catch this error and use the
+						// correct values.
+						// This bug is present in...
+						//  Japanese 1.0.0.5 (and presumably earlier versions)
+						//  Aeon Genesis 1.0.0.5
+						//  Aeon Genesis 1.0.0.6
+						//  Cave Story (WiiWare)
+						//  Cave Story+ (Steam)
+						// Gee, I wonder how it snuck into the Nicalis ports. ¬_¬
+						if (!memcmp(&gTS.data[gTS.p_read + 4], "850:0111", 8))
+						{
+							x = 850;
+							z = 111;
+						}
+					#endif
+
 						if (GetNPCFlag(x))
 							JumpTextScript(z);
 						else
@@ -988,7 +1014,7 @@ int TextScriptProc(void)
 					}
 					else if (IS_COMMAND('S','P','S'))
 					{
-					#ifdef FIX_BUGS
+					#ifdef FIX_MAJOR_BUGS
 						SetNoise(2, 0);
 					#else
 						// x is not initialised. This bug isn't too bad, since that parameter's not used when the first one is set to 2, but still.