NES: Fixed MMC5 crashes when header defines no CHR ROM

+ Added missing variables to save state
SourMesen · Aug 14, 2023 · cc892e5 · cc892e5
1 parent f89553c
commit cc892e5
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 16 deletions.
diff --git a/Core/NES/BaseMapper.cpp b/Core/NES/BaseMapper.cpp
@@ -28,7 +28,7 @@ void BaseMapper::Reset(bool softReset) { }
 uint16_t BaseMapper::InternalGetPrgPageSize() { return std::min((uint32_t)GetPrgPageSize(), _prgSize); }
 uint16_t BaseMapper::InternalGetSaveRamPageSize() { return std::min((uint32_t)GetSaveRamPageSize(), _saveRamSize); }
 uint16_t BaseMapper::InternalGetWorkRamPageSize() { return std::min((uint32_t)GetWorkRamPageSize(), _workRamSize); }
-uint16_t BaseMapper::InternalGetChrPageSize() { return std::min((uint32_t)GetChrPageSize(), _chrRomSize); }
+uint16_t BaseMapper::InternalGetChrRomPageSize() { return std::min((uint32_t)GetChrPageSize(), _chrRomSize); }
 uint16_t BaseMapper::InternalGetChrRamPageSize() { return std::min((uint32_t)GetChrRamPageSize(), _chrRamSize); }
 
 bool BaseMapper::ValidateAddressRange(uint16_t startAddr, uint16_t endAddr)
@@ -204,14 +204,14 @@ void BaseMapper::SetPpuMemoryMapping(uint16_t startAddr, uint16_t endAddr, uint1
 	switch(type) {
 		case ChrMemoryType::Default:
 		case ChrMemoryType::ChrRom:
-			pageSize = InternalGetChrPageSize();
+			pageSize = InternalGetChrRomPageSize();
 			if(pageSize == 0) {
 				#ifdef _DEBUG
 				MessageManager::DisplayMessage("Debug", "Tried to map undefined chr rom.");
 				#endif
 				return;
 			}
-			pageCount = GetChrPageCount();
+			pageCount = GetChrRomPageCount();
 			break;
 
 		case ChrMemoryType::ChrRam:
@@ -397,7 +397,7 @@ void BaseMapper::SelectChrPage(uint16_t slot, uint16_t page, ChrMemoryType memor
 		if(memoryType == ChrMemoryType::Default) {
 			memoryType = _chrRomSize > 0 ? ChrMemoryType::ChrRom : ChrMemoryType::ChrRam;
 		}
-		pageSize = memoryType == ChrMemoryType::ChrRam ? InternalGetChrRamPageSize() : InternalGetChrPageSize();
+		pageSize = memoryType == ChrMemoryType::ChrRam ? InternalGetChrRamPageSize() : InternalGetChrRomPageSize();
 	}
 
 	uint16_t startAddr = slot * pageSize;
@@ -456,9 +456,9 @@ uint32_t BaseMapper::GetPrgPageCount()
 	return pageSize ? (_prgSize / pageSize) : 0;
 }
 
-uint32_t BaseMapper::GetChrPageCount()
+uint32_t BaseMapper::GetChrRomPageCount()
 {
-	uint16_t pageSize = InternalGetChrPageSize();
+	uint16_t pageSize = InternalGetChrRomPageSize();
 	return pageSize ? (_chrRomSize / pageSize) : 0;
 }
 
@@ -1052,8 +1052,8 @@ CartridgeState BaseMapper::GetState()
 
 	state.PrgPageCount = GetPrgPageCount();
 	state.PrgPageSize = InternalGetPrgPageSize();
-	state.ChrPageCount = GetChrPageCount();
-	state.ChrPageSize = InternalGetChrPageSize();
+	state.ChrPageCount = GetChrRomPageCount();
+	state.ChrPageSize = InternalGetChrRomPageSize();
 	state.ChrRamPageSize = InternalGetChrRamPageSize();
 	for(int i = 0; i < 0x100; i++) {
 		state.PrgMemoryOffset[i] = _prgMemoryOffset[i];

diff --git a/Core/NES/BaseMapper.h b/Core/NES/BaseMapper.h
@@ -22,7 +22,7 @@ class BaseMapper : public INesMemoryHandler, public ISerializable
 	uint16_t InternalGetPrgPageSize();
 	uint16_t InternalGetSaveRamPageSize();
 	uint16_t InternalGetWorkRamPageSize();
-	uint16_t InternalGetChrPageSize();
+	uint16_t InternalGetChrRomPageSize();
 	uint16_t InternalGetChrRamPageSize();
 	bool ValidateAddressRange(uint16_t startAddr, uint16_t endAddr);
 
@@ -132,7 +132,7 @@ class BaseMapper : public INesMemoryHandler, public ISerializable
 	string GetBatteryFilename();
 
 	uint32_t GetPrgPageCount();
-	uint32_t GetChrPageCount();
+	uint32_t GetChrRomPageCount();
 
 	uint8_t GetPowerOnByte(uint8_t defaultValue = 0);
 	uint32_t GetDipSwitches();

diff --git a/Core/NES/Mappers/Nintendo/MMC5.h b/Core/NES/Mappers/Nintendo/MMC5.h
@@ -404,6 +404,11 @@ class MMC5 : public BaseMapper
 		SV(_irqCounterTarget); SV(_irqEnabled); SV(_scanlineCounter); SV(_irqPending); SV(_ppuInFrame);
 		SV(_splitInSplitRegion); SV(_splitVerticalScroll); SV(_splitTile); SV(_splitTileNumber); SV(_needInFrame);
 
+		SV(_prevChrA);
+		SV(_ppuIdleCounter);
+		SV(_lastPpuReadAddr);
+		SV(_ntReadCounter);
+
 		if(!s.IsSaving()) {
 			UpdatePrgBanks();
 			SetFillModeTile(_fillModeTile);
@@ -493,7 +498,7 @@ class MMC5 : public BaseMapper
 					}
 				} else if(_splitInSplitRegion) {
 					//CHR tile fetches for split region
-					return _chrRom[(_verticalSplitBank % (GetChrPageCount() / 4)) * 0x1000 + (((addr & ~0x07) | (verticalSplitScroll & 0x07)) & 0xFFF)];
+					return ReadFromChr((_verticalSplitBank << 12) + (((addr & ~0x07) | (verticalSplitScroll & 0x07)) & 0xFFF));
 				}
 			}
 
@@ -518,7 +523,7 @@ class MMC5 : public BaseMapper
 							uint8_t value = InternalReadRam(0x5C00 + _exAttributeLastNametableFetch);
 
 							//"The pattern fetches ignore the standard CHR banking bits, and instead use the top two bits of $5130 and the bottom 6 bits from Expansion RAM to choose a 4KB bank to select the tile from."
-							_exAttrSelectedChrBank = ((value & 0x3F) | (_chrUpperBits << 6)) % (_chrRomSize / 0x1000);
+							_exAttrSelectedChrBank = (value & 0x3F) | (_chrUpperBits << 6);
 
 							//Return a byte containing the same palette 4 times - this allows the PPU to select the right palette no matter the shift value
 							uint8_t palette = (value & 0xC0) >> 6;
@@ -528,7 +533,7 @@ class MMC5 : public BaseMapper
 						case 1:
 						case 0:
 							//PPU tile data fetch (high byte & low byte)
-							return _chrRom[_exAttrSelectedChrBank * 0x1000 + (addr & 0xFFF)];
+							return ReadFromChr((_exAttrSelectedChrBank << 12) + (addr & 0xFFF));
 					}
 				}
 			}
@@ -662,6 +667,16 @@ class MMC5 : public BaseMapper
 		return entries;
 	}
 
+	__forceinline uint8_t ReadFromChr(uint32_t pos)
+	{
+		uint32_t size = (_chrRomSize || !_chrRam) ? _chrRomSize : _chrRamSize;
+		if(size == 0) {
+			return 0;
+		}
+
+		return ((_chrRomSize || !_chrRam) ? _chrRom : _chrRam)[pos & (size - 1)];
+	}
+
 public:
 	bool IsExtendedAttributes()
 	{
@@ -681,13 +696,13 @@ class MMC5 : public BaseMapper
 		uint8_t value = InternalReadRam(0x5C00 + ntAddr);
 
 		//"The pattern fetches ignore the standard CHR banking bits, and instead use the top two bits of $5130 and the bottom 6 bits from Expansion RAM to choose a 4KB bank to select the tile from."
-		uint16_t chrBank = ((value & 0x3F) | (_chrUpperBits << 6)) % (_chrRomSize / 0x1000);
+		uint16_t chrBank = (value & 0x3F) | (_chrUpperBits << 6);
 
-		return chrBank * 0x1000 + (chrAddr & 0xFFF);
+		return (chrBank << 12) + (chrAddr & 0xFFF);
 	}
 
 	uint8_t GetExAttributeTileData(uint16_t ntAddr, uint16_t chrAddr)
 	{
-		return _chrRom[GetExAttributeAbsoluteTileAddr(ntAddr, chrAddr)];
+		return ReadFromChr(GetExAttributeAbsoluteTileAddr(ntAddr, chrAddr));
 	}
 };