Removed patches from source code and moved them into game XML file. Created ROMSet.cpp. Print a more descriptive error when game XML file fails to load.

Makefiles/Makefile.SDL.Win32.GCC

@@ -138,7 +138,7 @@ OBJ = $(OBJ_DIR)/PPCDisasm.o $(OBJ_DIR)/BlockFile.o $(OBJ_DIR)/93C46.o \
 	$(OBJ_DIR)/Crypto.o \
 	$(OBJ_DIR)/Logger.o \
 	$(OBJ_DIR)/tinyxml2.o \
-	$(OBJ_DIR)/ByteSwap.o $(OBJ_DIR)/Format.o $(OBJ_DIR)/NewConfig.o $(OBJ_DIR)/ConfigBuilders.o $(OBJ_DIR)/GameLoader.o
+	$(OBJ_DIR)/ByteSwap.o $(OBJ_DIR)/Format.o $(OBJ_DIR)/NewConfig.o $(OBJ_DIR)/ConfigBuilders.o $(OBJ_DIR)/GameLoader.o $(OBJ_DIR)/ROMSet.o
 
 
 # If built-in debugger enabled, include all debugging classes

Src/GameLoader.cpp

@@ -259,6 +259,7 @@ bool GameLoader::LoadGamesFromXML(const Util::Config::Node &xml)
         continue;
       }
       RegionsByName_t &regions_by_name = m_regions_by_game[game_name];
+      PatchesByRegion_t &patches_by_region = m_patches_by_game[game_name];
       PopulateGameInfo(&m_game_info_by_game[game_name], game_node);
 
       for (auto &roms_node: game_node)
@@ -308,6 +309,30 @@ bool GameLoader::LoadGamesFromXML(const Util::Config::Node &xml)
           else
             regions_by_name[region->region_name] = region;
         }
+        
+        // ROM patches, if any
+        for (auto &patches_node: roms_node)
+        {
+          if (patches_node.Key() != "patches")
+            continue;
+          
+          for (auto &patch_node: patches_node)
+          {
+            if (MissingAttrib(*this, patch_node, "region") ||
+                MissingAttrib(*this, patch_node, "bits") ||
+                MissingAttrib(*this, patch_node, "offset") || 
+                MissingAttrib(*this, patch_node, "value"))
+              continue;
+            std::string region = patch_node["region"].ValueAs<std::string>();
+            unsigned bits = patch_node["bits"].ValueAs<unsigned>();
+            uint32_t offset = patch_node["offset"].ValueAs<uint32_t>();
+            uint64_t value = patch_node["value"].ValueAs<uint64_t>();
+            if (bits != 8 && bits != 16 && bits != 32 && bits != 64)
+              ErrorLog("%s: Ignoring ROM patch in '%s' with invalid bit length. Must be 8, 16, 32, or 64!", m_xml_filename.c_str(), game_name.c_str());
+            else
+              patches_by_region[region].push_back(ROM::BigEndianPatch(offset, value, bits));
+          }
+        }
       }
       
       // Check to ensure that some ROM regions were defined for the game
@@ -439,7 +464,10 @@ bool GameLoader::LoadDefinitionXML(const std::string &filename)
   m_xml_filename = filename;
   Util::Config::Node xml("xml");
   if (Util::Config::FromXMLFile(&xml, filename))
+  {
+    ErrorLog("Game and ROM set definitions could not be loaded! ROMs will not be detected.");
     return true;
+  }
   return ParseXML(xml);
 }
 
@@ -722,6 +750,8 @@ bool GameLoader::LoadROMs(ROMSet *rom_set, const std::string &game_name, const Z
     ErrorLog("Cannot load unknown game '%s'. Is it defined in '%s'?", game_name.c_str(), m_xml_filename.c_str());
     return true;
   }
+
+  // Load up the ROMs
   auto &regions_by_name = IsChildSet(it->second) ? m_regions_by_merged_game.find(game_name)->second : m_regions_by_game.find(game_name)->second;
   LogROMDefinition(game_name, regions_by_name);
   bool error = false;
@@ -733,12 +763,25 @@ bool GameLoader::LoadROMs(ROMSet *rom_set, const std::string &game_name, const Z
       error |= true;
     else
     {
+      // Load up the ROM region
       auto &rom = rom_set->rom_by_region[region->region_name];
       rom.data.reset(new uint8_t[region_size], std::default_delete<uint8_t[]>());
       rom.size = region_size;
       error |= LoadRegion(&rom, region, zip);
     }
   }
+  
+  // Attach the patches and do some more error checking here
+  auto &patches_by_region = m_patches_by_game.find(game_name)->second;
+  for (auto &v: patches_by_region)
+  {
+    auto &region_name = v.first;
+    auto &patches = v.second;
+    if (regions_by_name.find(region_name) == regions_by_name.end())
+      ErrorLog("%s: Ignoring ROM patch for undefined region '%s' in '%s'.", m_xml_filename.c_str(), region_name.c_str(), game_name.c_str());
+    else if (rom_set->rom_by_region.find(region_name) != rom_set->rom_by_region.end())
+      rom_set->rom_by_region[region_name].patches = patches;
+  }
   return error;
 }
 

Src/GameLoader.h

@@ -38,7 +38,13 @@ private:
     bool FindFileIndexByOffset(size_t *idx, uint32_t offset) const;
   };
   
-  std::map<std::string, Game> m_game_info_by_game;  // ROMs not loaded here
+  // Game information from XML
+  std::map<std::string, Game> m_game_info_by_game;
+
+  // ROM patches by game and by region. Cannot place into Region because child
+  // sets do not inherit parent patches, which may complicate merging.
+  typedef std::map<std::string, std::vector<ROM::BigEndianPatch>> PatchesByRegion_t;
+  std::map<std::string, PatchesByRegion_t> m_patches_by_game;
   
   // Parsed XML
   typedef std::map<std::string, Region::ptr_t> RegionsByName_t;
@@ -46,7 +52,7 @@ private:
   std::map<std::string, RegionsByName_t> m_regions_by_merged_game;  // only child sets merged w/ parents
   std::string m_xml_filename;
   
-  // Zip file info
+  // Single compressed file inside of a zip archive
   struct ZippedFile
   {
     unzFile zf = nullptr;
@@ -56,6 +62,7 @@ private:
     uint32_t crc32 = 0;
   };
 
+  // Multiple zip archives
   struct ZipArchive
   {
     std::vector<std::string> zipfilenames;

Src/Model3/Model3.cpp

@@ -2548,104 +2548,6 @@ void CModel3::Reset(void)
  Initialization, Shutdown, and ROM Management
 ******************************************************************************/
 
-// Apply patches to games
-static void Patch(uint8_t *crom, const Game &game)
-{
-  if (game.name == "scudplus" || game.name == "scudplusa") // No patching?
-  {
-    // Base offset of program in CROM: 0x710000
-  }
-  else if (game.name == "lemans24")
-  {
-    // Base offset of program in CROM: 6473C0
-    *(UINT32 *) &crom[0x6D8C4C] = 0x00000002; // comm. mode: 00=master, 01=slave, 02=satellite
-    *(UINT32 *) &crom[0x73FE38] = 0x38840004; // an actual bug in the game code
-    *(UINT32 *) &crom[0x73EB5C] = 0x60000000;
-    *(UINT32 *) &crom[0x73EDD0] = 0x60000000;
-    *(UINT32 *) &crom[0x73EDC4] = 0x60000000;
-  } 
-  else if (game.name == "lostwsga")
-  {
-    *(UINT32 *) &crom[0x7374f4] = 0x38840004; // an actual bug in the game code
-  }
-  else if (game.name == "vs215" || game.name == "vs215o" || game.name == "vs29815" || game.name == "vs29915")
-  {
-    // VS215 is a modification of VS2 that runs on Step 1.5 hardware. I
-    // suspect the code here is trying to detect the system type but am too
-    // lazy to figure it out right now.
-    *(UINT32 *) &crom[0x7001A8] = 0x48000000+(0xFFF01630-0xFFF001A8); // force a jump to FFF01630
-  }
-  else if (game.name == "vs298")
-  {
-    // Base offset of program in CROM: 600000
-    // Inexplicably, at PC=AFC1C, a call is made to FC78, which is right in the middle of some
-    // totally unrelated initialization code (ASIC checks). This causes an invalid pointer to be fetched. 
-    // Perhaps FC78 should be overwritten with other program data by then? Why is it not?
-    // Or, 300138 needs to be written with a non-zero value, it is loaded from EEPROM but is 0.
-    *(UINT32 *) &crom[0x6AFC1C] = 0x60000000;
-  }
-  else if (game.name == "srally2")
-  {
-    *(UINT32 *) &crom[0x7C0C4] = 0x60000000;
-    *(UINT32 *) &crom[0x7C0C8] = 0x60000000;
-    *(UINT32 *) &crom[0x7C0CC] = 0x60000000;
-  }
-  else if (game.name == "harleya")
-  {
-    *(UINT32 *) &crom[0x50E8D4] = 0x60000000;
-    *(UINT32 *) &crom[0x50E8F4] = 0x60000000;
-    *(UINT32 *) &crom[0x50FB84] = 0x60000000;
-  }
-  else if (game.name == "harley")
-  {
-    *(UINT32 *) &crom[0x50ECB4] = 0x60000000;
-    *(UINT32 *) &crom[0x50ECD4] = 0x60000000;
-    *(UINT32 *) &crom[0x50FF64] = 0x60000000;
-  }
-  else if (game.name == "swtrilgy")
-  {
-    *(UINT32 *) &crom[0xF0E48] = 0x60000000;
-    *(UINT32 *) &crom[0x043DC] = 0x48000090;  // related to joystick feedback
-    *(UINT32 *) &crom[0x029A0] = 0x60000000;
-    *(UINT32 *) &crom[0x02A0C] = 0x60000000;
-  }
-  else if (game.name == "swtrilgya")
-  {
-    *(UINT32 *) &crom[0xF6DD0] = 0x60000000;  // from MAME
-  }
-  else if (game.name == "eca" || game.name == "ecau")
-  {
-    *(UINT32 *) &crom[0x535580] = 0x60000000;
-    //*(UINT32 *) &crom[0x5023B4] = 0x60000000;
-    //*(UINT32 *) &crom[0x5023D4] = 0x60000000;
-  }
-}
-
-// Reverses all aligned 16-bit words, thereby switching their endianness (assumes buffer size is divisible by 2)
-static void Reverse16(uint8_t *buf, size_t size)
-{
-  for (size_t i = 0; i < size; i += 2)
-  {
-    uint8_t tmp = buf[i+0];
-    buf[i+0] = buf[i+1];
-    buf[i+1] = tmp;
-  }
-}
-
-// Reverses all aligned 32-bit words, thereby switching their endianness (assumes buffer size is divisible by 4)
-static void Reverse32(uint8_t *buf, size_t size)
-{
-  for (size_t i = 0; i < size; i += 4)
-  {
-    uint8_t tmp1 = buf[i+0];
-    uint8_t tmp2 = buf[i+1];
-    buf[i+0] = buf[i+3];
-    buf[i+1] = buf[i+2];
-    buf[i+2] = tmp2;
-    buf[i+3] = tmp1;
-  }
-}
-
 // Offsets of memory regions within Model 3's pool
 #define OFFSET_RAM          0         // 8 MB
 #define OFFSET_CROM         0x800000  // 8 MB (fixed CROM)
@@ -2681,8 +2583,6 @@ bool CModel3::LoadGame(const Game &game, const ROMSet &rom_set)
    *  - Fixed CROM: 8MB. If < 8MB, loaded only in high part of space and low
    *    part is a mirror of (banked) CROM0.
    *  - Sample ROM: 16MB. If <= 8MB, mirror to high 8MB.
-   *
-   * ROMs are released after being loaded.
    */
   if (rom_set.get_rom("vrom").size <= 32*0x100000)
   {
@@ -2807,10 +2707,6 @@ bool CModel3::LoadGame(const Game &game, const ROMSet &rom_set)
   // Security board encryption device
   m_cryptoDevice.Init(game.encryption_key, std::bind(&CModel3::ReadSecurityRAM, this, std::placeholders::_1));
   
-  // Apply ROM patches
-  //TODO: place these in XML
-  Patch(crom, game);
-  
   // Print game information
   std::set<std::string> extra_hw;
   if (DSB)

Src/ROMSet.cpp

@@ -0,0 +1,48 @@
+#include "ROMSet.h"
+#include "OSD/Logger.h"
+#include <cstring>
+
+void ROM::CopyTo(uint8_t *dest, size_t dest_size, bool apply_patches ) const
+{
+  if (!data || !size || !dest || !dest_size)
+    return;
+  
+  size_t bytes_to_copy = std::min(size, dest_size);
+  const uint8_t *src = data.get();
+  memcpy(dest, src, bytes_to_copy);
+  
+  if (apply_patches)
+  {
+    for (auto &patch: patches)
+    {
+      unsigned bytes = patch.bits / 8;
+      if (patch.offset + bytes > dest_size)
+      {
+        ErrorLog("Ignored ROM patch to offset 0x%x in region 0x%x bytes long.", patch.offset, dest_size);
+        continue;
+      }
+      uint64_t value = patch.value;
+      switch (patch.bits)
+      {
+      case 8:
+      case 16:
+      case 32:
+      case 64:
+        for (size_t i = 0; i < bytes; i++)
+        {
+          dest[patch.offset + bytes - 1 - i] = value & 0xff;
+          value >>= 8;
+        }
+        break;
+      default:
+        break;
+      }
+    }
+  }
+}
+
+ROM ROMSet::get_rom(const std::string &region) const
+{
+  auto it = rom_by_region.find(region);
+  return it == rom_by_region.end() ? ROM() : it->second;
+}

Src/ROMSet.h

@@ -4,34 +4,37 @@
 #include <memory>
 #include <string>
 #include <map>
-#include <algorithm>
+#include <vector>
-#include <cstring>
+#include <cstdint>
 
 // Holds a single ROM region
 struct ROM
 {
+  struct BigEndianPatch
+  {
+    uint32_t offset;
+    uint64_t value;
+    unsigned bits;
+    
+    BigEndianPatch(uint32_t o, uint64_t v, unsigned b)
+      : offset(o),
+        value(v),
+        bits(b)
+    {}
+  };
+  
   std::shared_ptr<uint8_t> data;
+  std::vector<BigEndianPatch> patches;
   size_t size = 0;
   
-  void CopyTo(uint8_t *dest, size_t dest_size) const
+  void CopyTo(uint8_t *dest, size_t dest_size, bool apply_patches = true) const;
-  {
-    if (!data || !size || !dest || !dest_size)
-      return;
-    size_t bytes_to_copy = std::min(size, dest_size);
-    const uint8_t *src = data.get();
-    memcpy(dest, src, bytes_to_copy);
-  }
 };
   
 struct ROMSet
 {
   std::map<std::string, ROM> rom_by_region;
   
-  ROM get_rom(const std::string &region) const
+  ROM get_rom(const std::string &region) const;
-  {
-    auto it = rom_by_region.find(region);
-    return it == rom_by_region.end() ? ROM() : it->second;
-  }
 };
 
 #endif  // INCLUDED_ROMSET_H