Mod Compatibility 2: Prevent Error from duplicate ids within mod_interactions folder

doc/MODDING.md

@@ -35,7 +35,7 @@ Here is a full list of supported values for MOD_INFO:
 [
   {
     "type": "MOD_INFO",                             // Mandatory, you're making one of these!
-    "id": "Mod_ID",                                 // Mandatory, unique id for your mod. You should not use the same id as any other mod.
+    "id": "Mod_ID",                                 // Mandatory, unique id for your mod. You should not use the same id as any other mod.  Cannot contain the '#' character.  Mod ids must also be valid folder pathing in order to support compatibility with other mods.
     "name": "Mod's Display Name",                   // Mandatory.
     "authors": [ "Me", "A really cool friend" ],    // Optional, but you probably want to put your name here. More than one entry can be added, as shown.
     "description": "The best mod ever!",            // Optional

doc/MOD_COMPATABILITY.md

@@ -6,7 +6,7 @@ Mods are capable of dynamically loading directories based on if other mods are l
 
 ## Guide
 
-In order to dynamically load mod content, files must be placed within subdirectories named after other mod ids (capitalization is checked) within the mod_interactions folder.
+In order to dynamically load mod content, files must be placed within subdirectories named after other mod ids within the mod_interactions folder.
 
 Example:
 Mod 1: Mind Over Matter (id:mindovermatter)
@@ -19,15 +19,12 @@ Files located within the mod_interactions folders are always loaded after other
 
 ## Limitations
 
-Currently, this functionality only loads / unloads files based on if other mods are active for the particular world.  It does not suppress any other warnings beyond this function.
+Multi-mod interaction folders are not supported (ie. "mindovermatter/xedra_evolved").
 
-In particular, when designing mod compatability content an author will likely want to redefine certain ids to have new definitions, flags, etc.  If attempting to do this within the same overall mod folder, this will throw a duplicate definition error.  To get around this, instead of a mod redefining its own content within its own mod_interactions folder, the author should redefine its own content using the other mods mod_interaction folder.
+## Technical Summary
 
-Example:
-Mod 1: Mind Over Matter (id:mindovermatter)
-Mod 2: Xedra Evolved (id:xedra_evolved)
+When mods are loaded, they are loaded while ignoring every file that is within the "mod_interactions" folder.  After all mods are finished loading then the mod_interaction folder content is loaded is the same order as the initial mods.  Within the mod interaction folders, only folders with names matching loaded mod ids (case sensitive) will be loaded.
 
-If xedra wants an item to have extra damage while Mind Over Matter is loaded, the author should place the new definition in the following:
-MindOverMatter/mod_interactions/xedra_evolved/xedra_compat_data.json
+### Developers
 
-TODO: remove this limitation entirely by adjusting the check to take into account the mod_interaction id source as well
+When a definition from the mod interaction folder is loaded, the src is saved as a combination of the base mod id, a hashtag, and the interaction mod id.  For example a combined id may be "xedra_evolved#mindovermatter".

src/init.cpp

@@ -569,33 +569,32 @@ void DynamicDataLoader::load_mod_interaction_files_from_path( const cata_path &p
                  "Can't load additional data after finalization.  Must be unloaded first." );
 
     std::vector<mod_id> &loaded_mods = world_generator->active_world->active_mod_order;
-    std::vector<cata_path> files;
+    std::multimap<mod_id, cata_path> files;
 
     if( dir_exist( path.get_unrelative_path() ) ) {
 
         // obtain folders within mod_interactions to see if they match loaded mod ids
         const std::vector<cata_path> interaction_folders = get_directories( path, false );
 
         for( const cata_path &f : interaction_folders ) {
-            bool is_mod_loaded = false;
-            for( mod_id id : loaded_mods ) {
-                if( id.str() == f.get_unrelative_path().filename().string() ) {
-                    is_mod_loaded = true;
-                }
-            }
+            const mod_id associated_mod = mod_id( f.get_unrelative_path().filename().string() );
+            bool is_mod_loaded = std::find( loaded_mods.begin(), loaded_mods.end(),
+                                            associated_mod ) != loaded_mods.end();
+
             if( is_mod_loaded ) {
                 const std::vector<cata_path> interaction_files = get_files_from_path( ".json", f, true, true );
-                files.insert( files.end(), interaction_files.begin(), interaction_files.end() );
+                for( const cata_path &path : interaction_files ) {
+                    files.emplace( associated_mod, path );
+                }
             }
         }
     }
-
     // iterate over each file
-    for( const cata_path &file : files ) {
+    for( const std::pair<const mod_id, cata_path> &file : files ) {
         try {
             // parse it
-            JsonValue jsin = json_loader::from_path( file );
-            load_all_from_json( jsin, src, path, file );
+            JsonValue jsin = json_loader::from_path( file.second );
+            load_all_from_json( jsin, string_format( "%s#%s", src, file.first.str() ), path, file.second );
         } catch( const JsonError &err ) {
             throw std::runtime_error( err.what() );
         }

src/math_parser_diag.cpp

@@ -12,6 +12,7 @@
 #include "magic.h"
 #include "map.h"
 #include "math_parser_diag_value.h"
+#include "mod_manager.h"
 #include "mongroup.h"
 #include "mtype.h"
 #include "enums.h"
@@ -586,7 +587,7 @@ std::function<void( dialogue &, double )> spellcasting_adjustment_ass( char scop
             case scope_mod: {
                 const mod_id target_mod_id( filter_str );
                 for( spell *spellIt : d.actor( beta )->get_character()->magic->get_spells() ) {
-                    if( spellIt->get_src() == target_mod_id
+                    if( get_mod_base_id_from_src( spellIt->get_src() ) == target_mod_id
                         && ( whitelist.str( d ).empty() || spellIt->has_flag( whitelist.str( d ) ) )
                         && ( blacklist.str( d ).empty() || !spellIt->has_flag( blacklist.str( d ) ) )
                       ) {

src/mod_manager.cpp

@@ -24,11 +24,22 @@ static const mod_id MOD_INFORMATION_user_default( "user:default" );
 
 static const std::string MOD_SEARCH_FILE( "modinfo.json" );
 
+mod_id get_mod_base_id_from_src( mod_id src )
+{
+    mod_id base_mod_id;
+    size_t split_loc = src.str().find( '#' );
+    if( split_loc == std::string::npos ) {
+        return src;
+    } else {
+        return mod_id( src.str().substr( 0, split_loc ) );
+    }
+}
+
 template<>
 const MOD_INFORMATION &string_id<MOD_INFORMATION>::obj() const
 {
     const auto &map = world_generator->get_mod_manager().mod_map;
-    const auto iter = map.find( *this );
+    const auto iter = map.find( get_mod_base_id_from_src( *this ) );
     if( iter == map.end() ) {
         debugmsg( "Invalid mod %s requested", str() );
         static const MOD_INFORMATION dummy{};
@@ -224,6 +235,10 @@ void mod_manager::load_modfile( const JsonObject &jo, const cata_path &path )
         debugmsg( "there is already a mod with ident %s", m_ident.c_str() );
         return;
     }
+    if( m_ident.str().find( '#' ) != std::string::npos ) {
+        debugmsg( "Mod id %s contains illegal '#' character.", m_ident.str() );
+        return;
+    }
 
     translation m_name;
     jo.read( "name", m_name );

src/mod_manager.h

@@ -26,6 +26,8 @@ const std::vector<std::pair<std::string, translation>> &get_mod_list_categories(
 const std::vector<std::pair<std::string, translation>> &get_mod_list_tabs();
 const std::map<std::string, std::string> &get_mod_list_cat_tab();
 
+mod_id get_mod_base_id_from_src( mod_id src );
+
 struct MOD_INFORMATION {
     private:
         friend mod_manager;

src/recipe_dictionary.cpp

@@ -441,30 +441,9 @@ recipe &recipe_dictionary::load( const JsonObject &jo, const std::string &src,
     r.was_loaded = true;
 
     // Check for duplicate recipe_ids before assigning it to the map
-    if( out.find( r.ident() ) != out.end() ) {
-        const std::string new_mod_src = enumerate_as_string( r.src, [](
-        const std::pair<recipe_id, mod_id> &source ) {
-            return string_format( "'%s'", source.second->name() );
-        }, enumeration_conjunction::arrow );
-        const std::string old_mod_src = enumerate_as_string( out[ r.ident() ].src, [](
-        const std::pair<recipe_id, mod_id> &source ) {
-            return string_format( "'%s'", source.second->name() );
-        }, enumeration_conjunction::arrow );
-
-        const std::string base_json = string_format( "'%s'", _( "Dark Days Ahead" ) );
-        if( old_mod_src == base_json && new_mod_src != base_json ) {
-            // Assume the mod developer knows what they're doing
-        } else if( old_mod_src == new_mod_src ) {
-            // Conflict within a single source. Throw an error:
-            debugmsg( "Unable to load recipe_id %1$s. A recipe with that id already exists.\nExisting recipe source: %2$s\nNew recipe source: %3$s",
-                      r.ident().str(), old_mod_src, new_mod_src );
-        } else {
-            // Conflict between mods, leave a warning for debugging
-            DebugLog( DebugLevel::D_WARNING,
-                      DC_ALL ) <<
-                               string_format( "Recipe_id conflict: %1$s is included in both %2$s and %3$s.  Only the latter recipe will be loaded",
-                                              r.ident().str(), old_mod_src, new_mod_src );
-        }
+    auto duplicate = out.find( r.ident() );
+    if( duplicate != out.end() ) {
+        mod_tracker::check_duplicate_entries( r, duplicate->second );
     }
 
     return out[ r.ident() ] = std::move( r );