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

[b3brodie]

Summary

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

Purpose of change

Allow mods to redefine their own content within their own overall mod folder, removing the need for authors to edit other mods to add compatibility for their own

Describe the solution

Pass the secondary id through the src field and strip it in locations where it is required

Describe alternatives you've considered

This Monster: #76960
Moving the check_duplicate_entries() method far closer to the initial item load

Testing

Loaded game with custom files within the mod_interactions folder that directly copy from the parent mod's types to check that everything functions.

• Example: many types of things when examined for information will print the source mod. With this change, the source mod will only read the parent mods name, ie 'Magiclysm', without including the extra #bombastic_perks.
  Checked that the tests are functional
  Looked through the code for uses of the mod source. Most of them are either determining if the source is dda or not, which this will have no change on.

Additional context

[ehughsbaird]

These need to be handled (the rest by converting them to use the first one):
xedra_evolved#magiclysm is not a valid mod_id and so they'll not be able to find ->name() for it. I suggest in the case that it needs to be split like Xedra Evolved/Magiclysm, having both names are displayed for maximal clarity.

Cataclysm-DDA/src/mod_manager.h

Lines 70 to 78 in 1fb836e

	template<typename src_id>
	std::string get_origin( const std::vector<std::pair<src_id, mod_id>> &src )
	{
	std::string origin_str = enumerate_as_string( src.begin(),
	src.end(), []( const std::pair<src_id, mod_id> &source ) {
	return string_format( "'%s'", source.second->name() );
	}, enumeration_conjunction::arrow );
	return string_format( _( "Origin: %s" ), origin_str );
	}

Cataclysm-DDA/src/item.cpp

Lines 2403 to 2406 in 1fb836e

	info.emplace_back( "BASE", string_format( _( "Origin: %s" ), enumerate_as_string( type->src,
	[]( const std::pair<itype_id, mod_id> &source ) {
	return string_format( "'%s'", source.second->name() );
	}, enumeration_conjunction::arrow ) ) );

Cataclysm-DDA/src/monster.cpp

Lines 888 to 892 in 1fb836e

	oss << get_tag_from_color( c_white ) << _( "Origin: " );
	oss << enumerate_as_string( type->src.begin(),
	type->src.end(), []( const std::pair<mtype_id, mod_id> &source ) {
	return string_format( "'%s'", source.second->name() );
	}, enumeration_conjunction::arrow );

Cataclysm-DDA/src/monster.cpp

Lines 979 to 984 in 1fb836e

	std::string mods = enumerate_as_string( type->src.begin(),
	type->src.end(),
	[]( const std::pair<mtype_id, mod_id> &source ) {
	return string_format( "'%s'", source.second->name() );
	},
	enumeration_conjunction::arrow );

Cataclysm-DDA/src/newcharacter.cpp

Lines 2142 to 2146 in 1fb836e

	const std::string mod_src = enumerate_as_string( sorted_profs[cur_id]->src, [](
	const std::pair<profession_id, mod_id> &source ) {
	return string_format( "'%s'", source.second->name() );
	}, enumeration_conjunction::arrow );
	assembled += string_format( _( "Origin: %s" ), mod_src ) + "\n";

Cataclysm-DDA/src/newcharacter.cpp

Lines 3354 to 3358 in 1fb836e

	const std::string mod_src = enumerate_as_string( current_scenario->src,
	[]( const std::pair<string_id<scenario>, mod_id> &source ) {
	return string_format( "'%s'", source.second->name() );
	}, enumeration_conjunction::arrow );
	assembled += string_format( _( "Origin: %s" ), mod_src ) + "\n";

Cataclysm-DDA/src/overmapbuffer.cpp

Lines 1576 to 1580 in 1fb836e

	const std::string mod_src = enumerate_as_string( oter->get_type_id().obj().src,
	[]( const std::pair<oter_type_str_id, mod_id> &source ) {
	return string_format( "'%s'", source.second->name() );
	}, enumeration_conjunction::arrow );
	format_string += "\n" + string_format( _( "Origin: %s" ), mod_src );

This seems like it should be using the check_duplicate_entries mod_tracker function.

Cataclysm-DDA/src/recipe_dictionary.cpp

Lines 445 to 452 in 1fb836e

	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 );

[b3brodie]

These need to be handled (the rest by converting them to use the first one): xedra_evolved#magiclysm is not a valid mod_id and so they'll not be able to find ->name() for it. I suggest in the case that it needs to be split like Xedra Evolved/Magiclysm, having both names are displayed for maximal clarity.

Cataclysm-DDA/src/mod_manager.h

Lines 70 to 78 in 1fb836e

	template<typename src_id>
	std::string get_origin( const std::vector<std::pair<src_id, mod_id>> &src )
	{
	std::string origin_str = enumerate_as_string( src.begin(),
	src.end(), []( const std::pair<src_id, mod_id> &source ) {
	return string_format( "'%s'", source.second->name() );
	}, enumeration_conjunction::arrow );
	return string_format( _( "Origin: %s" ), origin_str );
	}

Cataclysm-DDA/src/item.cpp

Lines 2403 to 2406 in 1fb836e

	info.emplace_back( "BASE", string_format( _( "Origin: %s" ), enumerate_as_string( type->src,
	[]( const std::pair<itype_id, mod_id> &source ) {
	return string_format( "'%s'", source.second->name() );
	}, enumeration_conjunction::arrow ) ) );

Cataclysm-DDA/src/monster.cpp

Lines 888 to 892 in 1fb836e

	oss << get_tag_from_color( c_white ) << _( "Origin: " );
	oss << enumerate_as_string( type->src.begin(),
	type->src.end(), []( const std::pair<mtype_id, mod_id> &source ) {
	return string_format( "'%s'", source.second->name() );
	}, enumeration_conjunction::arrow );

Cataclysm-DDA/src/monster.cpp

Lines 979 to 984 in 1fb836e

	std::string mods = enumerate_as_string( type->src.begin(),
	type->src.end(),
	[]( const std::pair<mtype_id, mod_id> &source ) {
	return string_format( "'%s'", source.second->name() );
	},
	enumeration_conjunction::arrow );

Cataclysm-DDA/src/newcharacter.cpp

Lines 2142 to 2146 in 1fb836e

	const std::string mod_src = enumerate_as_string( sorted_profs[cur_id]->src, [](
	const std::pair<profession_id, mod_id> &source ) {
	return string_format( "'%s'", source.second->name() );
	}, enumeration_conjunction::arrow );
	assembled += string_format( _( "Origin: %s" ), mod_src ) + "\n";

Cataclysm-DDA/src/newcharacter.cpp

Lines 3354 to 3358 in 1fb836e

	const std::string mod_src = enumerate_as_string( current_scenario->src,
	[]( const std::pair<string_id<scenario>, mod_id> &source ) {
	return string_format( "'%s'", source.second->name() );
	}, enumeration_conjunction::arrow );
	assembled += string_format( _( "Origin: %s" ), mod_src ) + "\n";

Cataclysm-DDA/src/overmapbuffer.cpp

Lines 1576 to 1580 in 1fb836e

	const std::string mod_src = enumerate_as_string( oter->get_type_id().obj().src,
	[]( const std::pair<oter_type_str_id, mod_id> &source ) {
	return string_format( "'%s'", source.second->name() );
	}, enumeration_conjunction::arrow );
	format_string += "\n" + string_format( _( "Origin: %s" ), mod_src );

This seems like it should be using the check_duplicate_entries mod_tracker function.

Cataclysm-DDA/src/recipe_dictionary.cpp

Lines 445 to 452 in 1fb836e

	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 );

Adjusted the recipes function to use the check_duplicate_entries function.

As for the rest of the areas where mod sources are printed, these are actually already working due to the change in "const MOD_INFORMATION &string_id<MOD_INFORMATION>::obj()" function. I added a debugmsg to print this in action:

I've had some issues directly tracking how ->name() calls the obj() function before printing the string, but since it does call the obj() function that means that its performing a find from the world mod_map. Since the string printed on ->name() is found via the world mod map which only contains the actual loaded mods, I'm not seeing a good way of printing a pseudo 'Magiclysm/Bombastic Perks" name rather than just "Magiclysm". Ultimately since it is Magiclysm that's the source I don't feel this is too large of an information issue.

[ehughsbaird]

As for the rest of the areas where mod sources are printed, these are actually already working due to the change in "const MOD_INFORMATION &string_id<MOD_INFORMATION>::obj()" function

My bad, I missed the implications of this.

I've had some issues directly tracking how ->name() calls the obj() function before printing the string, but since it does call the obj() function that means that its performing a find from the world mod_map. Since the string printed on...

The operators are defined and the function declared here

Cataclysm-DDA/src/string_id.h

Lines 281 to 292 in 7d4507a

	/**
	* Returns the actual object this id refers to. May show a debug message if the id is invalid.
	*/
	const T &obj() const;
	const T &operator*() const {
	return obj();
	}
	const T *operator->() const {
	return &obj();
	}

With the ::obj function you modified providing the definition for that function.

If all the places I pointed out are unified to that one function, it's a matter of doing the splitting in that function and then doing a name lookup for each id.

Another approach could be creating a MOD_INFORMATION on demand when loading a mod interaction folder - it's a little gross, but so is what is happening now. For whatever my review is worth, I think this is good now.

[Maleclypse]

Is this ready to merge or conversation ongoing?

[ehughsbaird]

LGTM