From e98c340a7c2ee6be4eef2f6fc5a55261b8f75527 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 20:47:40 +0100 Subject: [PATCH 01/27] Remove manual JSON object member check: Just get the value and let the JSON system deal with the existence check and the error message. Also throw errors via the JSON system so it creates a nice message with error location. --- src/mapgen.cpp | 39 ++++++++++++--------------------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 619b6b77d8fa4..4310a5ce7d83e 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -309,36 +309,21 @@ load_mapgen_function( const JsonObject &jio, const std::string &id_base, } jio.allow_omitted_members(); return nullptr; // nothing - } else if( jio.has_string( "method" ) ) { - const std::string mgtype = jio.get_string( "method" ); - if( mgtype == "builtin" ) { // c-function - if( jio.has_string( "name" ) ) { - const std::string mgname = jio.get_string( "name" ); - if( const auto ptr = get_mapgen_cfunction( mgname ) ) { - ret = std::make_shared( ptr, mgweight ); - oter_mapgen[id_base].push_back( ret ); - } else { - debugmsg( "oter_t[%s]: builtin mapgen function \"%s\" does not exist.", id_base.c_str(), - mgname ); - } - } else { - debugmsg( "oter_t[%s]: Invalid mapgen function (missing \"name\" value).", id_base.c_str() ); - } - } else if( mgtype == "json" ) { - if( jio.has_object( "object" ) ) { - JsonObject jo = jio.get_object( "object" ); - std::string jstr = jo.str(); - ret = std::make_shared( jstr, mgweight, offset ); - oter_mapgen[id_base].push_back( ret ); - } else { - debugmsg( "oter_t[%s]: Invalid mapgen function (missing \"object\" object)", id_base.c_str() ); - } + } + const std::string mgtype = jio.get_string( "method" ); + if( mgtype == "builtin" ) { + if( const auto ptr = get_mapgen_cfunction( jio.get_string( "name" ) ) ) { + ret = std::make_shared( ptr, mgweight ); + oter_mapgen[id_base].push_back( ret ); } else { - debugmsg( "oter_t[%s]: Invalid mapgen function type: %s", id_base.c_str(), mgtype.c_str() ); + jio.throw_error( "function does not exist", "name" ); } + } else if( mgtype == "json" ) { + const std::string jstr = jio.get_object( "object" ).str(); + ret = std::make_shared( jstr, mgweight, offset ); + oter_mapgen[id_base].push_back( ret ); } else { - debugmsg( "oter_t[%s]: Invalid mapgen function (missing \"method\" value, must be \"builtin\" or \"json\").", - id_base.c_str() ); + jio.throw_error( "invalid value: must be \"builtin\" or \"json\")", "method" ); } return ret; } From 8a3af6582754dc65b8d07918b239c0aa5a508927 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 21:03:30 +0100 Subject: [PATCH 02/27] Move some repeated code into a function get_mapgen_function. Basically the whole look up of the mapgen function pointer within the `oter_mapgen` and so on. --- src/mapgen.cpp | 255 ++++++++++++++++++++++--------------------------- 1 file changed, 114 insertions(+), 141 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 4310a5ce7d83e..cb6a1d950a18a 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -264,6 +264,31 @@ void check_mapgen_definitions() } } +// @p hardcoded_weight Weight for an additional entry. If that entry is chosen, a null pointer is returned. +static mapgen_function *get_mapgen_function( const std::string &key, + const int hardcoded_weight = 0 ) +{ + const auto fmapit = oter_mapgen.find( key ); + if( fmapit == oter_mapgen.end() ) { + return nullptr; + } + const std::vector> &vector = fmapit->second; + // Creating the entry in the map is only done when an entry in the vector is about to be made, + // so the map should not contain empty vectors. + assert( !vector.empty() ); + const auto weightit = oter_mapgen_weights.find( key ); + const int rlast = weightit->second.rbegin()->first; + const int roll = rng( 1, rlast + hardcoded_weight ); + if( roll > rlast ) { + return nullptr; + } + const int fidx = weightit->second.lower_bound( roll )->second; + assert( static_cast( fidx ) < vector.size() ); + const std::shared_ptr &ptr = vector[fidx]; + assert( ptr ); + return ptr.get(); +} + ///////////////////////////////////////////////////////////////////////////////// ///// json mapgen functions ///// 1 - init(): @@ -3451,18 +3476,8 @@ void map::draw_lab( mapgendata &dat ) //A lab area with only one entrance if( boarders == 1 ) { - const std::string function_key = "lab_1side"; // terrain_type->get_mapgen_id(); - const auto fmapit = oter_mapgen.find( function_key ); - - if( fmapit != oter_mapgen.end() && !fmapit->second.empty() ) { - std::map >::const_iterator weightit = oter_mapgen_weights.find( - function_key ); - const int rlast = weightit->second.rbegin()->first; - const int roll = rng( 1, rlast ); - - const int fidx = weightit->second.lower_bound( roll )->second; - - fmapit->second[fidx]->generate( dat ); + if( const auto ptr = get_mapgen_function( "lab_1side" ) ) { + ptr->generate( dat ); if( tw == 2 ) { rotate( 2 ); } @@ -3478,75 +3493,57 @@ void map::draw_lab( mapgendata &dat ) maybe_insert_stairs( dat.above(), t_stairs_up ); maybe_insert_stairs( terrain_type, t_stairs_down ); } else { - const std::string function_key = "lab_4side"; - const auto fmapit = oter_mapgen.find( function_key ); const int hardcoded_4side_map_weight = 1500; // weight of all hardcoded maps. - bool use_hardcoded_4side_map = false; - - if( fmapit != oter_mapgen.end() && !fmapit->second.empty() ) { - std::map >::const_iterator weightit = oter_mapgen_weights.find( - function_key ); - const int rlast = weightit->second.rbegin()->first; - const int roll = rng( 1, rlast + hardcoded_4side_map_weight ); - - if( roll <= rlast ) { - const int fidx = weightit->second.lower_bound( roll )->second; - fmapit->second[fidx]->generate( dat ); - - // If the map template hasn't handled borders, handle them in code. - // Rotated maps cannot handle borders and have to be caught in code. - // We determine if a border isn't handled by checking the east-facing - // border space where the door normally is -- it should be a wall or door. - tripoint east_border( 23, 11, abs_sub.z ); - if( !has_flag_ter( "WALL", east_border ) && - !has_flag_ter( "DOOR", east_border ) ) { - // TODO: create a ter_reset function that does ter_set, - // furn_set, and i_clear? - ter_id lw_type = tower_lab ? t_reinforced_glass : t_concrete_wall; - ter_id tw_type = tower_lab ? t_reinforced_glass : t_concrete_wall; - ter_id rw_type = tower_lab && rw == 2 ? t_reinforced_glass : - t_concrete_wall; - ter_id bw_type = tower_lab && bw == 2 ? t_reinforced_glass : - t_concrete_wall; - for( int i = 0; i < SEEX * 2; i++ ) { - ter_set( point( 23, i ), rw_type ); - furn_set( point( 23, i ), f_null ); - i_clear( tripoint( 23, i, get_abs_sub().z ) ); - - ter_set( point( i, 23 ), bw_type ); - furn_set( point( i, 23 ), f_null ); - i_clear( tripoint( i, 23, get_abs_sub().z ) ); - - if( lw == 2 ) { - ter_set( point( 0, i ), lw_type ); - furn_set( point( 0, i ), f_null ); - i_clear( tripoint( 0, i, get_abs_sub().z ) ); - } - if( tw == 2 ) { - ter_set( point( i, 0 ), tw_type ); - furn_set( point( i, 0 ), f_null ); - i_clear( tripoint( i, 0, get_abs_sub().z ) ); - } - } - if( rw != 2 ) { - ter_set( point( 23, 11 ), t_door_metal_c ); - ter_set( point( 23, 12 ), t_door_metal_c ); + if( const auto ptr = get_mapgen_function( "lab_4side", hardcoded_4side_map_weight ) ) { + ptr->generate( dat ); + // If the map template hasn't handled borders, handle them in code. + // Rotated maps cannot handle borders and have to be caught in code. + // We determine if a border isn't handled by checking the east-facing + // border space where the door normally is -- it should be a wall or door. + tripoint east_border( 23, 11, abs_sub.z ); + if( !has_flag_ter( "WALL", east_border ) && + !has_flag_ter( "DOOR", east_border ) ) { + // TODO: create a ter_reset function that does ter_set, + // furn_set, and i_clear? + ter_id lw_type = tower_lab ? t_reinforced_glass : t_concrete_wall; + ter_id tw_type = tower_lab ? t_reinforced_glass : t_concrete_wall; + ter_id rw_type = tower_lab && rw == 2 ? t_reinforced_glass : + t_concrete_wall; + ter_id bw_type = tower_lab && bw == 2 ? t_reinforced_glass : + t_concrete_wall; + for( int i = 0; i < SEEX * 2; i++ ) { + ter_set( point( 23, i ), rw_type ); + furn_set( point( 23, i ), f_null ); + i_clear( tripoint( 23, i, get_abs_sub().z ) ); + + ter_set( point( i, 23 ), bw_type ); + furn_set( point( i, 23 ), f_null ); + i_clear( tripoint( i, 23, get_abs_sub().z ) ); + + if( lw == 2 ) { + ter_set( point( 0, i ), lw_type ); + furn_set( point( 0, i ), f_null ); + i_clear( tripoint( 0, i, get_abs_sub().z ) ); } - if( bw != 2 ) { - ter_set( point( 11, 23 ), t_door_metal_c ); - ter_set( point( 12, 23 ), t_door_metal_c ); + if( tw == 2 ) { + ter_set( point( i, 0 ), tw_type ); + furn_set( point( i, 0 ), f_null ); + i_clear( tripoint( i, 0, get_abs_sub().z ) ); } } + if( rw != 2 ) { + ter_set( point( 23, 11 ), t_door_metal_c ); + ter_set( point( 23, 12 ), t_door_metal_c ); + } + if( bw != 2 ) { + ter_set( point( 11, 23 ), t_door_metal_c ); + ter_set( point( 12, 23 ), t_door_metal_c ); + } + } - maybe_insert_stairs( dat.above(), t_stairs_up ); - maybe_insert_stairs( terrain_type, t_stairs_down ); - } else { // then weighted roll was in the hardcoded section - use_hardcoded_4side_map = true; - } // end json maps + maybe_insert_stairs( dat.above(), t_stairs_up ); + maybe_insert_stairs( terrain_type, t_stairs_down ); } else { // then no json maps for lab_4side were found - use_hardcoded_4side_map = true; - } // end if no lab_4side was found. - if( use_hardcoded_4side_map ) { switch( rng( 1, 3 ) ) { case 1: // Cross shaped @@ -4041,69 +4038,51 @@ void map::draw_lab( mapgendata &dat ) bw = is_ot_match( "lab", dat.south(), ot_match_type::contains ) ? 1 : 2; lw = is_ot_match( "lab", dat.west(), ot_match_type::contains ) ? 0 : 2; - const std::string function_key = "lab_finale_1level"; - const auto fmapit = oter_mapgen.find( function_key ); const int hardcoded_finale_map_weight = 500; // weight of all hardcoded maps. - bool use_hardcoded_finale_map = false; - - if( fmapit != oter_mapgen.end() && !fmapit->second.empty() ) { - std::map >::const_iterator weightit = oter_mapgen_weights.find( - function_key ); - const int rlast = weightit->second.rbegin()->first; - const int roll = rng( 1, rlast + hardcoded_finale_map_weight ); - - if( roll <= rlast ) { - const int fidx = weightit->second.lower_bound( roll )->second; - fmapit->second[fidx]->generate( dat ); - - // If the map template hasn't handled borders, handle them in code. - // Rotated maps cannot handle borders and have to be caught in code. - // We determine if a border isn't handled by checking the east-facing - // border space where the door normally is -- it should be a wall or door. - tripoint east_border( 23, 11, abs_sub.z ); - if( !has_flag_ter( "WALL", east_border ) && !has_flag_ter( "DOOR", east_border ) ) { - // TODO: create a ter_reset function that does ter_set, furn_set, and i_clear? - ter_id lw_type = tower_lab ? t_reinforced_glass : t_concrete_wall; - ter_id tw_type = tower_lab ? t_reinforced_glass : t_concrete_wall; - ter_id rw_type = tower_lab && rw == 2 ? t_reinforced_glass : t_concrete_wall; - ter_id bw_type = tower_lab && bw == 2 ? t_reinforced_glass : t_concrete_wall; - for( int i = 0; i < SEEX * 2; i++ ) { - ter_set( point( 23, i ), rw_type ); - furn_set( point( 23, i ), f_null ); - i_clear( tripoint( 23, i, get_abs_sub().z ) ); - - ter_set( point( i, 23 ), bw_type ); - furn_set( point( i, 23 ), f_null ); - i_clear( tripoint( i, 23, get_abs_sub().z ) ); - - if( lw == 2 ) { - ter_set( point( 0, i ), lw_type ); - furn_set( point( 0, i ), f_null ); - i_clear( tripoint( 0, i, get_abs_sub().z ) ); - } - if( tw == 2 ) { - ter_set( point( i, 0 ), tw_type ); - furn_set( point( i, 0 ), f_null ); - i_clear( tripoint( i, 0, get_abs_sub().z ) ); - } - } - if( rw != 2 ) { - ter_set( point( 23, 11 ), t_door_metal_c ); - ter_set( point( 23, 12 ), t_door_metal_c ); + if( const auto ptr = get_mapgen_function( "lab_finale_1level", hardcoded_finale_map_weight ) ) { + ptr->generate( dat ); + + // If the map template hasn't handled borders, handle them in code. + // Rotated maps cannot handle borders and have to be caught in code. + // We determine if a border isn't handled by checking the east-facing + // border space where the door normally is -- it should be a wall or door. + tripoint east_border( 23, 11, abs_sub.z ); + if( !has_flag_ter( "WALL", east_border ) && !has_flag_ter( "DOOR", east_border ) ) { + // TODO: create a ter_reset function that does ter_set, furn_set, and i_clear? + ter_id lw_type = tower_lab ? t_reinforced_glass : t_concrete_wall; + ter_id tw_type = tower_lab ? t_reinforced_glass : t_concrete_wall; + ter_id rw_type = tower_lab && rw == 2 ? t_reinforced_glass : t_concrete_wall; + ter_id bw_type = tower_lab && bw == 2 ? t_reinforced_glass : t_concrete_wall; + for( int i = 0; i < SEEX * 2; i++ ) { + ter_set( point( 23, i ), rw_type ); + furn_set( point( 23, i ), f_null ); + i_clear( tripoint( 23, i, get_abs_sub().z ) ); + + ter_set( point( i, 23 ), bw_type ); + furn_set( point( i, 23 ), f_null ); + i_clear( tripoint( i, 23, get_abs_sub().z ) ); + + if( lw == 2 ) { + ter_set( point( 0, i ), lw_type ); + furn_set( point( 0, i ), f_null ); + i_clear( tripoint( 0, i, get_abs_sub().z ) ); } - if( bw != 2 ) { - ter_set( point( 11, 23 ), t_door_metal_c ); - ter_set( point( 12, 23 ), t_door_metal_c ); + if( tw == 2 ) { + ter_set( point( i, 0 ), tw_type ); + furn_set( point( i, 0 ), f_null ); + i_clear( tripoint( i, 0, get_abs_sub().z ) ); } } - } else { // then weighted roll was in the hardcoded section - use_hardcoded_finale_map = true; - } // end json maps + if( rw != 2 ) { + ter_set( point( 23, 11 ), t_door_metal_c ); + ter_set( point( 23, 12 ), t_door_metal_c ); + } + if( bw != 2 ) { + ter_set( point( 11, 23 ), t_door_metal_c ); + ter_set( point( 12, 23 ), t_door_metal_c ); + } + } } else { // then no json maps for lab_finale_1level were found - use_hardcoded_finale_map = true; - } // end if no lab_4side was found. - - if( use_hardcoded_finale_map ) { // Start by setting up a large, empty room. for( int i = 0; i < SEEX * 2; i++ ) { for( int j = 0; j < SEEY * 2; j++ ) { @@ -7277,14 +7256,8 @@ std::pair, std::map> get_changed_ids_from_up bool run_mapgen_func( const std::string &mapgen_id, mapgendata &dat ) { - const auto fmapit = oter_mapgen.find( mapgen_id ); - if( fmapit != oter_mapgen.end() && !fmapit->second.empty() ) { - std::map >::const_iterator weightit = oter_mapgen_weights.find( - mapgen_id ); - const int rlast = weightit->second.rbegin()->first; - const int roll = rng( 1, rlast ); - const int fidx = weightit->second.lower_bound( roll )->second; - fmapit->second[fidx]->generate( dat ); + if( const auto ptr = get_mapgen_function( mapgen_id ) ) { + ptr->generate( dat ); return true; } return false; From aa339c1285665fe0d9f8199caa825b6610a8db07 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 21:11:35 +0100 Subject: [PATCH 03/27] Move some code as function into mapgen.cpp This is mostly done to avoid having to expose `oter_mapgen` in the header. --- src/mapgen.cpp | 10 ++++++++++ src/mapgen.h | 7 +++++++ src/overmap.cpp | 5 +---- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index cb6a1d950a18a..1286a24c3620d 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -7262,3 +7262,13 @@ bool run_mapgen_func( const std::string &mapgen_id, mapgendata &dat ) } return false; } + +int register_mapgen_function( const std::string &key ) +{ + if( const auto ptr = get_mapgen_cfunction( key ) ) { + std::vector> &vector = oter_mapgen[key]; + vector.push_back( std::make_shared( ptr ) ); + return vector.size() - 1; + } + return -1; +} diff --git a/src/mapgen.h b/src/mapgen.h index 6c9e31034673d..91f924ea59109 100644 --- a/src/mapgen.h +++ b/src/mapgen.h @@ -378,6 +378,13 @@ std::shared_ptr load_mapgen_function( const JsonObject &jio, */ void load_mapgen( const JsonObject &jo ); void reset_mapgens(); +/** + * Attempts to register the build-in function @p key as mapgen for the overmap terrain @p key. + * If there is no matching function, it does nothing (no error message) and returns -1. + * Otherwise it returns the index of the added entry in the vector of @ref oter_mapgen. + */ +// @TODO this should go away. It is only used for old build-in mapgen. Mapgen should be done via JSON. +int register_mapgen_function( const std::string &key ); /* * stores function ref and/or required data */ diff --git a/src/overmap.cpp b/src/overmap.cpp index 932ccf174a185..afda9f77e0219 100644 --- a/src/overmap.cpp +++ b/src/overmap.cpp @@ -532,10 +532,7 @@ static void load_overmap_terrain_mapgens( const JsonObject &jo, const std::strin bool default_mapgen = jo.get_bool( "default_mapgen", true ); int default_idx = -1; if( default_mapgen ) { - if( const auto ptr = get_mapgen_cfunction( fmapkey ) ) { - oter_mapgen[fmapkey].push_back( std::make_shared( ptr ) ); - default_idx = oter_mapgen[fmapkey].size() - 1; - } + default_idx = register_mapgen_function( fmapkey ); } if( jo.has_array( jsonkey ) ) { for( JsonObject jio : jo.get_array( jsonkey ) ) { From 0c9dcb627f000097caf775aacdcf47a29e4ff01d Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 21:12:33 +0100 Subject: [PATCH 04/27] Remove redundant declaration. Those variables are already declared in a header or not even used within this cpp file. --- src/map_extras.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/map_extras.cpp b/src/map_extras.cpp index caf08d0a45d4b..2ef322a6be8f6 100644 --- a/src/map_extras.cpp +++ b/src/map_extras.cpp @@ -2832,9 +2832,6 @@ void map_extra::load( const JsonObject &jo, const std::string & ) optional( jo, was_loaded, "autonote", autonote, false ); } -extern std::map> > oter_mapgen; -extern std::map> > - nested_mapgen; extern std::map> > update_mapgen; From 05bf7d0513f697b44451e3fb78527519c4e8bf1d Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 21:13:00 +0100 Subject: [PATCH 05/27] Remove commented out code. --- src/map_extras.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/map_extras.cpp b/src/map_extras.cpp index 2ef322a6be8f6..0cd274ad9dd3e 100644 --- a/src/map_extras.cpp +++ b/src/map_extras.cpp @@ -2847,13 +2847,6 @@ void map_extra::check() const break; } case map_extra_method::mapgen: { - /* - const auto fmapit = oter_mapgen.find( generator_id ); - const oter_id extra_oter( generator_id ); - if( ( fmapit == oter_mapgen.end() || !fmapit->second.empty() ) && !extra_oter.is_valid() ) { - debugmsg( "invalid mapgen function (%s) defined for map extra (%s)", generator_id, id.str() ); - } - */ break; } case map_extra_method::update_mapgen: { From b906f939df36ebd8356ba81c82955f31347c605b Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 21:15:00 +0100 Subject: [PATCH 06/27] Move access to oter_mapgen into mapgen.cpp Again: this is to avoid having to expose `oter_mapgen` in the header. --- src/mapgen.cpp | 5 +++++ src/mapgen.h | 4 ++++ src/overmap.cpp | 3 +-- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 1286a24c3620d..73c19cc4f637a 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -7272,3 +7272,8 @@ int register_mapgen_function( const std::string &key ) } return -1; } + +bool has_mapgen_for( const std::string &key ) +{ + return oter_mapgen.count( key ) != 0; +} diff --git a/src/mapgen.h b/src/mapgen.h index 91f924ea59109..8d01c465bc561 100644 --- a/src/mapgen.h +++ b/src/mapgen.h @@ -385,6 +385,10 @@ void reset_mapgens(); */ // @TODO this should go away. It is only used for old build-in mapgen. Mapgen should be done via JSON. int register_mapgen_function( const std::string &key ); +/** + * Check that @p key is present in @ref oter_mapgen. + */ +bool has_mapgen_for( const std::string &key ); /* * stores function ref and/or required data */ diff --git a/src/overmap.cpp b/src/overmap.cpp index afda9f77e0219..6198c588e8f33 100644 --- a/src/overmap.cpp +++ b/src/overmap.cpp @@ -789,9 +789,8 @@ void overmap_terrains::check_consistency() } const bool exists_hardcoded = elem.is_hardcoded(); - const bool exists_loaded = oter_mapgen.find( mid ) != oter_mapgen.end(); - if( exists_loaded ) { + if( has_mapgen_for( mid ) ) { if( test_mode && exists_hardcoded ) { debugmsg( "Mapgen terrain \"%s\" exists in both JSON and a hardcoded function. Consider removing the latter.", mid.c_str() ); From 4517596c55033bd7bb2a854202731aa17a61dfc3 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 21:19:49 +0100 Subject: [PATCH 07/27] Remove exposing variables from mapgen.cpp in the header. Merges the comments from the header with the comments in the cpp file for the very same objects. --- src/mapgen.cpp | 3 ++- src/mapgen.h | 8 -------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 73c19cc4f637a..1642fee23dbb7 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -194,7 +194,7 @@ void mapgen_function_builtin::generate( mapgendata &mgd ) ///// all sorts of ways to apply our hellish reality to a grid-o-squares /* - * ptr storage. + * stores function ref and/or required data */ std::map> > oter_mapgen; std::map> > nested_mapgen; @@ -202,6 +202,7 @@ std::map> /* * index to the above, adjusted to allow for rarity + * random selector list for the nested vector above, as per individual mapgen_function_::weight value */ std::map > oter_mapgen_weights; diff --git a/src/mapgen.h b/src/mapgen.h index 8d01c465bc561..b543a00b1bf0e 100644 --- a/src/mapgen.h +++ b/src/mapgen.h @@ -389,14 +389,6 @@ int register_mapgen_function( const std::string &key ); * Check that @p key is present in @ref oter_mapgen. */ bool has_mapgen_for( const std::string &key ); -/* - * stores function ref and/or required data - */ -extern std::map> > oter_mapgen; -/* - * random selector list for the nested vector above, as per individual mapgen_function_::weight value - */ -extern std::map > oter_mapgen_weights; /* * Sets the above after init, and initializes mapgen_function_json instances as well */ From 36a36742263001653d06827f0dd9229b9dd42483 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 21:20:09 +0100 Subject: [PATCH 08/27] Add separate function to register mapgen object: register_mapgen_function --- src/mapgen.cpp | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 1642fee23dbb7..9d9e85b4fc893 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -265,6 +265,15 @@ void check_mapgen_definitions() } } +static int register_mapgen_function( const std::string &key, + const std::shared_ptr ptr ) +{ + assert( ptr ); + std::vector> &vector = oter_mapgen[key]; + vector.push_back( ptr ); + return vector.size() - 1; +} + // @p hardcoded_weight Weight for an additional entry. If that entry is chosen, a null pointer is returned. static mapgen_function *get_mapgen_function( const std::string &key, const int hardcoded_weight = 0 ) @@ -340,14 +349,14 @@ load_mapgen_function( const JsonObject &jio, const std::string &id_base, if( mgtype == "builtin" ) { if( const auto ptr = get_mapgen_cfunction( jio.get_string( "name" ) ) ) { ret = std::make_shared( ptr, mgweight ); - oter_mapgen[id_base].push_back( ret ); + register_mapgen_function( id_base, ret ); } else { jio.throw_error( "function does not exist", "name" ); } } else if( mgtype == "json" ) { const std::string jstr = jio.get_object( "object" ).str(); ret = std::make_shared( jstr, mgweight, offset ); - oter_mapgen[id_base].push_back( ret ); + register_mapgen_function( id_base, ret ); } else { jio.throw_error( "invalid value: must be \"builtin\" or \"json\")", "method" ); } @@ -404,7 +413,7 @@ void load_mapgen( const JsonObject &jo ) for( const std::string mapgenid : row_items ) { const auto mgfunc = load_mapgen_function( jo, mapgenid, -1, offset ); if( mgfunc ) { - oter_mapgen[ mapgenid ].push_back( mgfunc ); + register_mapgen_function( mapgenid, mgfunc ); } offset.x++; } @@ -421,7 +430,7 @@ void load_mapgen( const JsonObject &jo ) const auto mgfunc = load_mapgen_function( jo, mapgenid, -1 ); if( mgfunc ) { for( auto &i : mapgenid_list ) { - oter_mapgen[ i ].push_back( mgfunc ); + register_mapgen_function( i, mgfunc ); } } } @@ -7267,9 +7276,7 @@ bool run_mapgen_func( const std::string &mapgen_id, mapgendata &dat ) int register_mapgen_function( const std::string &key ) { if( const auto ptr = get_mapgen_cfunction( key ) ) { - std::vector> &vector = oter_mapgen[key]; - vector.push_back( std::make_shared( ptr ) ); - return vector.size() - 1; + return register_mapgen_function( key, std::make_shared( ptr ) ); } return -1; } From daaf576bab21091b1cb6e49bb81dc4aafadb43e6 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 21:26:07 +0100 Subject: [PATCH 09/27] =?UTF-8?q?Change=20oter=5Fmapgen=20to=20contain=20?= =?UTF-8?q?=C3=ADnstances=20of=20a=20dedicated=20class?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Those instances can have their own members and stuff. --- src/mapgen.cpp | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 9d9e85b4fc893..1d1063299485b 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -193,10 +193,19 @@ void mapgen_function_builtin::generate( mapgendata &mgd ) ///// mapgen_function class. ///// all sorts of ways to apply our hellish reality to a grid-o-squares +class mapgen_basic_container : public std::vector> +{ + public: + int add( const std::shared_ptr ptr ) { + assert( ptr ); + push_back( ptr ); + return size() - 1; + } +}; /* * stores function ref and/or required data */ -std::map> > oter_mapgen; +std::map oter_mapgen; std::map> > nested_mapgen; std::map> > update_mapgen; @@ -268,10 +277,7 @@ void check_mapgen_definitions() static int register_mapgen_function( const std::string &key, const std::shared_ptr ptr ) { - assert( ptr ); - std::vector> &vector = oter_mapgen[key]; - vector.push_back( ptr ); - return vector.size() - 1; + return oter_mapgen[key].add( ptr ); } // @p hardcoded_weight Weight for an additional entry. If that entry is chosen, a null pointer is returned. From 818b34da4ce9f04108b2783c11b0a6cf210fa2ed Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 21:41:11 +0100 Subject: [PATCH 10/27] Merge oter_mapgen_weights into oter_mapgen: Instead of having a separate map, this reuses the already existing entries in `oter_mapgen`. --- src/mapgen.cpp | 26 ++++++++++++-------------- src/mapgen.h | 2 +- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 1d1063299485b..202665e8d4e81 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -196,6 +196,12 @@ void mapgen_function_builtin::generate( mapgendata &mgd ) class mapgen_basic_container : public std::vector> { public: + /** + * index to the above, adjusted to allow for rarity + * random selector list for the nested vector above, as per individual mapgen_function_::weight value + */ + std::map weights_; + int add( const std::shared_ptr ptr ) { assert( ptr ); push_back( ptr ); @@ -210,21 +216,14 @@ std::map> std::map> > update_mapgen; /* - * index to the above, adjusted to allow for rarity - * random selector list for the nested vector above, as per individual mapgen_function_::weight value - */ -std::map > oter_mapgen_weights; - -/* - * setup oter_mapgen_weights which mapgen uses to diceroll. Also setup mapgen_function_json + * setup mapgen_basic_container::weights_ which mapgen uses to diceroll. Also setup mapgen_function_json */ void calculate_mapgen_weights() // TODO: rename as it runs jsonfunction setup too { - oter_mapgen_weights.clear(); for( auto &omw : oter_mapgen ) { int funcnum = 0; int wtotal = 0; - oter_mapgen_weights[ omw.first ] = std::map(); + omw.second.weights_.clear(); for( auto fit = omw.second.begin(); fit != omw.second.end(); ++fit ) { // int weight = ( *fit )->weight; @@ -236,7 +235,7 @@ void calculate_mapgen_weights() // TODO: rename as it runs jsonfunction setup } ( *fit )->setup(); wtotal += weight; - oter_mapgen_weights[ omw.first ][ wtotal ] = funcnum; + omw.second.weights_[ wtotal ] = funcnum; dbg( D_INFO ) << "wcalc " << omw.first << "(" << funcnum << "): +" << weight << " = " << wtotal; ++funcnum; } @@ -288,17 +287,16 @@ static mapgen_function *get_mapgen_function( const std::string &key, if( fmapit == oter_mapgen.end() ) { return nullptr; } - const std::vector> &vector = fmapit->second; + const mapgen_basic_container &vector = fmapit->second; // Creating the entry in the map is only done when an entry in the vector is about to be made, // so the map should not contain empty vectors. assert( !vector.empty() ); - const auto weightit = oter_mapgen_weights.find( key ); - const int rlast = weightit->second.rbegin()->first; + const int rlast = vector.weights_.rbegin()->first; const int roll = rng( 1, rlast + hardcoded_weight ); if( roll > rlast ) { return nullptr; } - const int fidx = weightit->second.lower_bound( roll )->second; + const int fidx = vector.weights_.lower_bound( roll )->second; assert( static_cast( fidx ) < vector.size() ); const std::shared_ptr &ptr = vector[fidx]; assert( ptr ); diff --git a/src/mapgen.h b/src/mapgen.h index b543a00b1bf0e..ec114176152df 100644 --- a/src/mapgen.h +++ b/src/mapgen.h @@ -374,7 +374,7 @@ std::shared_ptr load_mapgen_function( const JsonObject &jio, const std::string &id_base, int default_idx, const point &offset = point_zero ); /* * Load the above directly from a file via init, as opposed to riders attached to overmap_terrain. Added check - * for oter_mapgen / oter_mapgen_weights key, multiple possible ( ie, [ "house", "house_base" ] ) + * for oter_mapgen key, multiple possible ( ie, [ "house", "house_base" ] ) */ void load_mapgen( const JsonObject &jo ); void reset_mapgens(); From 8a7d9bac71270673304a881374540bf2d4ea2529 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 21:47:16 +0100 Subject: [PATCH 11/27] Store pointers directly in oter_mapgen weights map instead of indices into the vector itself. This avoids the repeated look up within the vector and it avoids a potential error situation: the index taken from `weight_` could be invalid. --- src/mapgen.cpp | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 202665e8d4e81..dc6d0063b5f4d 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -200,7 +200,7 @@ class mapgen_basic_container : public std::vector weights_; + std::map> weights_; int add( const std::shared_ptr ptr ) { assert( ptr ); @@ -221,23 +221,18 @@ std::map> void calculate_mapgen_weights() // TODO: rename as it runs jsonfunction setup too { for( auto &omw : oter_mapgen ) { - int funcnum = 0; int wtotal = 0; omw.second.weights_.clear(); for( auto fit = omw.second.begin(); fit != omw.second.end(); ++fit ) { - // int weight = ( *fit )->weight; if( weight < 1 ) { - dbg( D_INFO ) << "wcalc " << omw.first << "(" << funcnum << "): (rej(1), " << weight << ") = " << - wtotal; - ++funcnum; + dbg( D_INFO ) << "wcalc " << omw.first << ": (rej(1), " << weight << ") = " << wtotal; continue; // rejected! } ( *fit )->setup(); wtotal += weight; - omw.second.weights_[ wtotal ] = funcnum; - dbg( D_INFO ) << "wcalc " << omw.first << "(" << funcnum << "): +" << weight << " = " << wtotal; - ++funcnum; + omw.second.weights_[ wtotal ] = *fit; + dbg( D_INFO ) << "wcalc " << omw.first << ": +" << weight << " = " << wtotal; } } // Not really calculate weights, but let's keep it here for now @@ -296,9 +291,7 @@ static mapgen_function *get_mapgen_function( const std::string &key, if( roll > rlast ) { return nullptr; } - const int fidx = vector.weights_.lower_bound( roll )->second; - assert( static_cast( fidx ) < vector.size() ); - const std::shared_ptr &ptr = vector[fidx]; + const std::shared_ptr &ptr = vector.weights_.lower_bound( roll )->second; assert( ptr ); return ptr.get(); } From 2abf56737a4f025741e69131e526806fc64c1124 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 21:56:06 +0100 Subject: [PATCH 12/27] Use weighted_int_list for oter_mapgen weights We already have this class, so we should make use of it. --- src/mapgen.cpp | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index dc6d0063b5f4d..deb9c158ecabd 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -196,11 +196,7 @@ void mapgen_function_builtin::generate( mapgendata &mgd ) class mapgen_basic_container : public std::vector> { public: - /** - * index to the above, adjusted to allow for rarity - * random selector list for the nested vector above, as per individual mapgen_function_::weight value - */ - std::map> weights_; + weighted_int_list> weights_; int add( const std::shared_ptr ptr ) { assert( ptr ); @@ -221,19 +217,16 @@ std::map> void calculate_mapgen_weights() // TODO: rename as it runs jsonfunction setup too { for( auto &omw : oter_mapgen ) { - int wtotal = 0; omw.second.weights_.clear(); - for( auto fit = omw.second.begin(); fit != omw.second.end(); ++fit ) { - int weight = ( *fit )->weight; + for( const std::shared_ptr &ptr : omw.second ) { + const int weight = ptr->weight; if( weight < 1 ) { - dbg( D_INFO ) << "wcalc " << omw.first << ": (rej(1), " << weight << ") = " << wtotal; continue; // rejected! } - ( *fit )->setup(); - wtotal += weight; - omw.second.weights_[ wtotal ] = *fit; - dbg( D_INFO ) << "wcalc " << omw.first << ": +" << weight << " = " << wtotal; + omw.second.weights_.add( ptr, weight ); + ptr->setup(); } + // @TODO clear: omw.second.clear(); } // Not really calculate weights, but let's keep it here for now for( auto &pr : nested_mapgen ) { @@ -282,18 +275,18 @@ static mapgen_function *get_mapgen_function( const std::string &key, if( fmapit == oter_mapgen.end() ) { return nullptr; } - const mapgen_basic_container &vector = fmapit->second; - // Creating the entry in the map is only done when an entry in the vector is about to be made, - // so the map should not contain empty vectors. - assert( !vector.empty() ); - const int rlast = vector.weights_.rbegin()->first; - const int roll = rng( 1, rlast + hardcoded_weight ); - if( roll > rlast ) { + const weighted_int_list> &weights = fmapit->second.weights_; + if( hardcoded_weight > 0 ) { + if( rng( 1, weights.get_weight() + hardcoded_weight ) > weights.get_weight() ) { + return nullptr; + } + } + const std::shared_ptr *ptr = weights.pick(); + if( !ptr ) { return nullptr; } - const std::shared_ptr &ptr = vector.weights_.lower_bound( roll )->second; - assert( ptr ); - return ptr.get(); + assert( *ptr ); + return ptr->get(); } ///////////////////////////////////////////////////////////////////////////////// From 832dba8b963d31a8a02436d0893656606aa65b0c Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 21:59:35 +0100 Subject: [PATCH 13/27] Move code into a member function mapgen_basic_container::pick --- src/mapgen.cpp | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index deb9c158ecabd..8d97275722ffe 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -203,6 +203,23 @@ class mapgen_basic_container : public std::vector 0 && + rng( 1, weights_.get_weight() + hardcoded_weight ) > weights_.get_weight() ) { + return nullptr; + } + const std::shared_ptr *const ptr = weights_.pick(); + if( !ptr ) { + return nullptr; + } + assert( *ptr ); + return ptr->get(); + } }; /* * stores function ref and/or required data @@ -275,18 +292,7 @@ static mapgen_function *get_mapgen_function( const std::string &key, if( fmapit == oter_mapgen.end() ) { return nullptr; } - const weighted_int_list> &weights = fmapit->second.weights_; - if( hardcoded_weight > 0 ) { - if( rng( 1, weights.get_weight() + hardcoded_weight ) > weights.get_weight() ) { - return nullptr; - } - } - const std::shared_ptr *ptr = weights.pick(); - if( !ptr ) { - return nullptr; - } - assert( *ptr ); - return ptr->get(); + return fmapit->second.pick( hardcoded_weight ); } ///////////////////////////////////////////////////////////////////////////////// From 8f68fe43a6375c53e8b3cbca00b77de7c87d5cc9 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 22:02:38 +0100 Subject: [PATCH 14/27] Move code into a member function: mapgen_basic_container::setup --- src/mapgen.cpp | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 8d97275722ffe..6801506b17e13 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -220,6 +220,18 @@ class mapgen_basic_container : public std::vectorget(); } + void setup() { + weights_.clear(); + for( const std::shared_ptr &ptr : *this ) { + const int weight = ptr->weight; + if( weight < 1 ) { + continue; // rejected! + } + weights_.add( ptr, weight ); + ptr->setup(); + } + // @TODO clear: clear(); + } }; /* * stores function ref and/or required data @@ -234,16 +246,7 @@ std::map> void calculate_mapgen_weights() // TODO: rename as it runs jsonfunction setup too { for( auto &omw : oter_mapgen ) { - omw.second.weights_.clear(); - for( const std::shared_ptr &ptr : omw.second ) { - const int weight = ptr->weight; - if( weight < 1 ) { - continue; // rejected! - } - omw.second.weights_.add( ptr, weight ); - ptr->setup(); - } - // @TODO clear: omw.second.clear(); + omw.second.setup(); } // Not really calculate weights, but let's keep it here for now for( auto &pr : nested_mapgen ) { From 93af65b9b0996aaff5e425e805de70847896cec3 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 22:02:42 +0100 Subject: [PATCH 15/27] Move code into a member function: mapgen_basic_container::check_consistency Note: the function name is chosen for consistency (no pun), our other consistency checking function are named that way. --- src/mapgen.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 6801506b17e13..38d660c73f1dc 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -232,6 +232,11 @@ class mapgen_basic_container : public std::vectorcheck( key ); + } + } }; /* * stores function ref and/or required data @@ -265,9 +270,7 @@ void calculate_mapgen_weights() // TODO: rename as it runs jsonfunction setup void check_mapgen_definitions() { for( auto &oter_definition : oter_mapgen ) { - for( auto &mapgen_function_ptr : oter_definition.second ) { - mapgen_function_ptr->check( oter_definition.first ); - } + oter_definition.second.check_consistency( oter_definition.first ); } for( auto &oter_definition : nested_mapgen ) { for( auto &mapgen_function_ptr : oter_definition.second ) { From 8db4d4e0fd74cef0cbdfa75c57eeba79b1f133a0 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 22:08:15 +0100 Subject: [PATCH 16/27] Make all data members of mapgen_basic_container private. No need to expose them. The last direct access to the vector had to be wrapped within a new member function. --- src/mapgen.cpp | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 38d660c73f1dc..a8b950ae715e1 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -193,15 +193,25 @@ void mapgen_function_builtin::generate( mapgendata &mgd ) ///// mapgen_function class. ///// all sorts of ways to apply our hellish reality to a grid-o-squares -class mapgen_basic_container : public std::vector> +class mapgen_basic_container { - public: + private: + std::vector> mapgens_; weighted_int_list> weights_; + public: int add( const std::shared_ptr ptr ) { assert( ptr ); - push_back( ptr ); - return size() - 1; + mapgens_.push_back( ptr ); + return mapgens_.size() - 1; + } + void erase( const size_t index ) { + if( index > mapgens_.size() ) { + return; + } + assert( mapgens_[index] ); + // Can't actually erase it as this might invalid the index stored elsewhere + mapgens_[index]->weight = 0; } /** * Pick a mapgen function randomly. @@ -221,8 +231,7 @@ class mapgen_basic_container : public std::vectorget(); } void setup() { - weights_.clear(); - for( const std::shared_ptr &ptr : *this ) { + for( const std::shared_ptr &ptr : mapgens_ ) { const int weight = ptr->weight; if( weight < 1 ) { continue; // rejected! @@ -230,10 +239,11 @@ class mapgen_basic_container : public std::vectorsetup(); } - // @TODO clear: clear(); + // Not needed anymore, pointers are now stored in weights_ (or not used at all) + mapgens_.clear(); } void check_consistency( const std::string &key ) { - for( auto &mapgen_function_ptr : *this ) { + for( auto &mapgen_function_ptr : mapgens_ ) { mapgen_function_ptr->check( key ); } } @@ -340,7 +350,7 @@ load_mapgen_function( const JsonObject &jio, const std::string &id_base, if( jio.has_string( "name" ) ) { const std::string mgname = jio.get_string( "name" ); if( mgname == id_base ) { - oter_mapgen[id_base][ default_idx ]->weight = 0; + oter_mapgen[id_base].erase( default_idx ); } } } From f0b11ce04e1a00b7c98d7ce2a9b9efaf7cf11165 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 22:16:36 +0100 Subject: [PATCH 17/27] Make a separate class for oter_mapgen And move all the functions that deal with it into that class. --- src/mapgen.cpp | 91 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 55 insertions(+), 36 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index a8b950ae715e1..eaa18f854f8e6 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -248,10 +248,50 @@ class mapgen_basic_container } } }; + +class mapgen_factory +{ + private: + std::map mapgens_; + + public: + void reset() { + mapgens_.clear(); + } + void setup() { + for( auto &omw : mapgens_ ) { + omw.second.setup(); + } + } + void check_consistency() { + for( auto &oter_definition : mapgens_ ) { + oter_definition.second.check_consistency( oter_definition.first ); + } + } + bool has( const std::string &key ) const { + return mapgens_.count( key ) != 0; + } + int add( const std::string &key, const std::shared_ptr ptr ) { + return mapgens_[key].add( ptr ); + } + // @p hardcoded_weight Weight for an additional entry. If that entry is chosen, a null pointer is returned. + mapgen_function *pick( const std::string &key, const int hardcoded_weight = 0 ) const { + const auto fmapit = mapgens_.find( key ); + if( fmapit == mapgens_.end() ) { + return nullptr; + } + return fmapit->second.pick( hardcoded_weight ); + } + void erase( const std::string &key, const size_t index ) { + mapgens_[key].erase( index ); + } +}; + +static mapgen_factory oter_mapgen; + /* * stores function ref and/or required data */ -std::map oter_mapgen; std::map> > nested_mapgen; std::map> > update_mapgen; @@ -260,9 +300,7 @@ std::map> */ void calculate_mapgen_weights() // TODO: rename as it runs jsonfunction setup too { - for( auto &omw : oter_mapgen ) { - omw.second.setup(); - } + oter_mapgen.setup(); // Not really calculate weights, but let's keep it here for now for( auto &pr : nested_mapgen ) { for( std::unique_ptr &ptr : pr.second ) { @@ -279,9 +317,7 @@ void calculate_mapgen_weights() // TODO: rename as it runs jsonfunction setup void check_mapgen_definitions() { - for( auto &oter_definition : oter_mapgen ) { - oter_definition.second.check_consistency( oter_definition.first ); - } + oter_mapgen.check_consistency(); for( auto &oter_definition : nested_mapgen ) { for( auto &mapgen_function_ptr : oter_definition.second ) { mapgen_function_ptr->check( oter_definition.first ); @@ -294,23 +330,6 @@ void check_mapgen_definitions() } } -static int register_mapgen_function( const std::string &key, - const std::shared_ptr ptr ) -{ - return oter_mapgen[key].add( ptr ); -} - -// @p hardcoded_weight Weight for an additional entry. If that entry is chosen, a null pointer is returned. -static mapgen_function *get_mapgen_function( const std::string &key, - const int hardcoded_weight = 0 ) -{ - const auto fmapit = oter_mapgen.find( key ); - if( fmapit == oter_mapgen.end() ) { - return nullptr; - } - return fmapit->second.pick( hardcoded_weight ); -} - ///////////////////////////////////////////////////////////////////////////////// ///// json mapgen functions ///// 1 - init(): @@ -350,7 +369,7 @@ load_mapgen_function( const JsonObject &jio, const std::string &id_base, if( jio.has_string( "name" ) ) { const std::string mgname = jio.get_string( "name" ); if( mgname == id_base ) { - oter_mapgen[id_base].erase( default_idx ); + oter_mapgen.erase( id_base, default_idx ); } } } @@ -361,14 +380,14 @@ load_mapgen_function( const JsonObject &jio, const std::string &id_base, if( mgtype == "builtin" ) { if( const auto ptr = get_mapgen_cfunction( jio.get_string( "name" ) ) ) { ret = std::make_shared( ptr, mgweight ); - register_mapgen_function( id_base, ret ); + oter_mapgen.add( id_base, ret ); } else { jio.throw_error( "function does not exist", "name" ); } } else if( mgtype == "json" ) { const std::string jstr = jio.get_object( "object" ).str(); ret = std::make_shared( jstr, mgweight, offset ); - register_mapgen_function( id_base, ret ); + oter_mapgen.add( id_base, ret ); } else { jio.throw_error( "invalid value: must be \"builtin\" or \"json\")", "method" ); } @@ -425,7 +444,7 @@ void load_mapgen( const JsonObject &jo ) for( const std::string mapgenid : row_items ) { const auto mgfunc = load_mapgen_function( jo, mapgenid, -1, offset ); if( mgfunc ) { - register_mapgen_function( mapgenid, mgfunc ); + oter_mapgen.add( mapgenid, mgfunc ); } offset.x++; } @@ -442,7 +461,7 @@ void load_mapgen( const JsonObject &jo ) const auto mgfunc = load_mapgen_function( jo, mapgenid, -1 ); if( mgfunc ) { for( auto &i : mapgenid_list ) { - register_mapgen_function( i, mgfunc ); + oter_mapgen.add( i, mgfunc ); } } } @@ -461,7 +480,7 @@ void load_mapgen( const JsonObject &jo ) void reset_mapgens() { - oter_mapgen.clear(); + oter_mapgen.reset(); nested_mapgen.clear(); update_mapgen.clear(); } @@ -3498,7 +3517,7 @@ void map::draw_lab( mapgendata &dat ) //A lab area with only one entrance if( boarders == 1 ) { - if( const auto ptr = get_mapgen_function( "lab_1side" ) ) { + if( const auto ptr = oter_mapgen.pick( "lab_1side" ) ) { ptr->generate( dat ); if( tw == 2 ) { rotate( 2 ); @@ -3516,7 +3535,7 @@ void map::draw_lab( mapgendata &dat ) maybe_insert_stairs( terrain_type, t_stairs_down ); } else { const int hardcoded_4side_map_weight = 1500; // weight of all hardcoded maps. - if( const auto ptr = get_mapgen_function( "lab_4side", hardcoded_4side_map_weight ) ) { + if( const auto ptr = oter_mapgen.pick( "lab_4side", hardcoded_4side_map_weight ) ) { ptr->generate( dat ); // If the map template hasn't handled borders, handle them in code. // Rotated maps cannot handle borders and have to be caught in code. @@ -4061,7 +4080,7 @@ void map::draw_lab( mapgendata &dat ) lw = is_ot_match( "lab", dat.west(), ot_match_type::contains ) ? 0 : 2; const int hardcoded_finale_map_weight = 500; // weight of all hardcoded maps. - if( const auto ptr = get_mapgen_function( "lab_finale_1level", hardcoded_finale_map_weight ) ) { + if( const auto ptr = oter_mapgen.pick( "lab_finale_1level", hardcoded_finale_map_weight ) ) { ptr->generate( dat ); // If the map template hasn't handled borders, handle them in code. @@ -7278,7 +7297,7 @@ std::pair, std::map> get_changed_ids_from_up bool run_mapgen_func( const std::string &mapgen_id, mapgendata &dat ) { - if( const auto ptr = get_mapgen_function( mapgen_id ) ) { + if( const auto ptr = oter_mapgen.pick( mapgen_id ) ) { ptr->generate( dat ); return true; } @@ -7288,12 +7307,12 @@ bool run_mapgen_func( const std::string &mapgen_id, mapgendata &dat ) int register_mapgen_function( const std::string &key ) { if( const auto ptr = get_mapgen_cfunction( key ) ) { - return register_mapgen_function( key, std::make_shared( ptr ) ); + return oter_mapgen.add( key, std::make_shared( ptr ) ); } return -1; } bool has_mapgen_for( const std::string &key ) { - return oter_mapgen.count( key ) != 0; + return oter_mapgen.has( key ); } From 72190ded8559e6af2c803e1b68b545be107e8e24 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 22:17:23 +0100 Subject: [PATCH 18/27] Replace `auto` with actual type --- src/mapgen.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index eaa18f854f8e6..f3025955214c4 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -259,12 +259,12 @@ class mapgen_factory mapgens_.clear(); } void setup() { - for( auto &omw : mapgens_ ) { + for( std::pair &omw : mapgens_ ) { omw.second.setup(); } } void check_consistency() { - for( auto &oter_definition : mapgens_ ) { + for( std::pair &oter_definition : mapgens_ ) { oter_definition.second.check_consistency( oter_definition.first ); } } From 35c07f755ff0b3fd30e0b6be7e7399e62aa34370 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 22:18:12 +0100 Subject: [PATCH 19/27] Consistent naming: "omw" just like in the other function --- src/mapgen.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index f3025955214c4..d984521be1a95 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -264,8 +264,8 @@ class mapgen_factory } } void check_consistency() { - for( std::pair &oter_definition : mapgens_ ) { - oter_definition.second.check_consistency( oter_definition.first ); + for( std::pair &omw : mapgens_ ) { + omw.second.check_consistency( omw.first ); } } bool has( const std::string &key ) const { From d724bdd661c7dc81f2e451c9bba0bab4b8408f6d Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 22:18:47 +0100 Subject: [PATCH 20/27] Consistent naming: just "iter" as there is no need to hint the data type (and what does the "f" even stand for?) --- src/mapgen.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index d984521be1a95..34b98d64a759c 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -276,11 +276,11 @@ class mapgen_factory } // @p hardcoded_weight Weight for an additional entry. If that entry is chosen, a null pointer is returned. mapgen_function *pick( const std::string &key, const int hardcoded_weight = 0 ) const { - const auto fmapit = mapgens_.find( key ); - if( fmapit == mapgens_.end() ) { + const auto iter = mapgens_.find( key ); + if( iter == mapgens_.end() ) { return nullptr; } - return fmapit->second.pick( hardcoded_weight ); + return iter->second.pick( hardcoded_weight ); } void erase( const std::string &key, const size_t index ) { mapgens_[key].erase( index ); From bd2e793839eb4cd2994a9756ec2e3d8319367569 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 26 Jan 2020 22:50:58 +0100 Subject: [PATCH 21/27] Add usage checks for loaded mapgen instances. This will find mapgen instances that were loaded from JSON, but have no usage within the game (e.g. because the belong to undefined overmap terrain). Register some mapgen ids that are only used within the C++ code as used --- src/mapgen.cpp | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 34b98d64a759c..6103c8f97d50a 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -15,6 +15,7 @@ #include #include "clzones.h" +#include "generic_factory.h" #include "computer.h" #include "coordinate_conversions.h" #include "coordinates.h" @@ -254,6 +255,25 @@ class mapgen_factory private: std::map mapgens_; + static std::set get_usages() { + std::set result; + for( const oter_t &elem : overmap_terrains::get_all() ) { + result.insert( elem.get_mapgen_id() ); + result.insert( elem.id.str() ); + } + // Why do I have to repeat the MapExtras here? Wouldn't "MapExtras::factory" be enough? + for( const map_extra &elem : MapExtras::mapExtraFactory().get_all() ) { + if( elem.generator_method == map_extra_method::mapgen ) { + result.insert( elem.generator_id ); + } + } + // Used in C++ code only, see calls to `oter_mapgen.pick()` below + result.insert( "lab_1side" ); + result.insert( "lab_4side" ); + result.insert( "lab_finale_1level" ); + return result; + } + public: void reset() { mapgens_.clear(); @@ -264,8 +284,14 @@ class mapgen_factory } } void check_consistency() { + // Cache all strings that may get looked up here so we don't have to go through + // all the sources for them upon each loop. + const std::set usages = get_usages(); for( std::pair &omw : mapgens_ ) { omw.second.check_consistency( omw.first ); + if( usages.count( omw.first ) == 0 ) { + debugmsg( "Mapgen %s is not used by anything!", omw.first ); + } } } bool has( const std::string &key ) const { @@ -3517,6 +3543,7 @@ void map::draw_lab( mapgendata &dat ) //A lab area with only one entrance if( boarders == 1 ) { + // If you remove the usage of "lab_1side" here, remove it from mapgen_factory::get_usages above as well. if( const auto ptr = oter_mapgen.pick( "lab_1side" ) ) { ptr->generate( dat ); if( tw == 2 ) { @@ -3535,6 +3562,7 @@ void map::draw_lab( mapgendata &dat ) maybe_insert_stairs( terrain_type, t_stairs_down ); } else { const int hardcoded_4side_map_weight = 1500; // weight of all hardcoded maps. + // If you remove the usage of "lab_4side" here, remove it from mapgen_factory::get_usages above as well. if( const auto ptr = oter_mapgen.pick( "lab_4side", hardcoded_4side_map_weight ) ) { ptr->generate( dat ); // If the map template hasn't handled borders, handle them in code. @@ -4080,6 +4108,7 @@ void map::draw_lab( mapgendata &dat ) lw = is_ot_match( "lab", dat.west(), ot_match_type::contains ) ? 0 : 2; const int hardcoded_finale_map_weight = 500; // weight of all hardcoded maps. + // If you remove the usage of "lab_finale_1level" here, remove it from mapgen_factory::get_usages above as well. if( const auto ptr = oter_mapgen.pick( "lab_finale_1level", hardcoded_finale_map_weight ) ) { ptr->generate( dat ); From 1f5668b85f0af9efee3e955ee7699975b4466f73 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Mon, 27 Jan 2020 00:06:08 +0100 Subject: [PATCH 22/27] Add some documentation. --- src/mapgen.cpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 6103c8f97d50a..bed02cb7c2767 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -231,6 +231,11 @@ class mapgen_basic_container assert( *ptr ); return ptr->get(); } + /** + * Calls @ref mapgen_function::setup and sets up the internal weighted list using + * the **current** value of @ref mapgen_function::weight. This value may have + * changed since it was first added, so this is needed to recalculate the weighted list. + */ void setup() { for( const std::shared_ptr &ptr : mapgens_ ) { const int weight = ptr->weight; @@ -255,6 +260,7 @@ class mapgen_factory private: std::map mapgens_; + /// Collect all the possible and expected keys that may get used with @ref pick. static std::set get_usages() { std::set result; for( const oter_t &elem : overmap_terrains::get_all() ) { @@ -278,6 +284,7 @@ class mapgen_factory void reset() { mapgens_.clear(); } + /// @see mapgen_basic_container::setup void setup() { for( std::pair &omw : mapgens_ ) { omw.second.setup(); @@ -294,13 +301,19 @@ class mapgen_factory } } } + /** + * Checks whether we have an entry for the given key. + * Note that the entry itself may not contain any valid mapgen instance + * (could all have been removed via @ref erase). + */ bool has( const std::string &key ) const { return mapgens_.count( key ) != 0; } + /// @see mapgen_basic_container::add int add( const std::string &key, const std::shared_ptr ptr ) { return mapgens_[key].add( ptr ); } - // @p hardcoded_weight Weight for an additional entry. If that entry is chosen, a null pointer is returned. + /// @see mapgen_basic_container::pick mapgen_function *pick( const std::string &key, const int hardcoded_weight = 0 ) const { const auto iter = mapgens_.find( key ); if( iter == mapgens_.end() ) { @@ -308,6 +321,7 @@ class mapgen_factory } return iter->second.pick( hardcoded_weight ); } + /// @see mapgen_basic_container::erase void erase( const std::string &key, const size_t index ) { mapgens_[key].erase( index ); } From ada1a13252f25a475e77f2a16e5dffce419d2b77 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 2 Feb 2020 00:11:30 +0100 Subject: [PATCH 23/27] Encapsulate the `mapgen_function` function pointers within `mapgen_factory` Don't expose them at all. All access to them is done internally. --- src/mapgen.cpp | 43 ++++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index bed02cb7c2767..0a7282b2ad4ab 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -215,21 +215,26 @@ class mapgen_basic_container mapgens_[index]->weight = 0; } /** - * Pick a mapgen function randomly. + * Pick a mapgen function randomly and call its generate function. + * This basically runs the mapgen functions with the given @ref mapgendata + * as argument. + * @return Whether the mapgen function has been run. It may not get run if + * the list of mapgen functions is effectively empty. * @p hardcoded_weight Weight for an additional entry. If that entry is chosen, - * a null pointer is returned. If unsure, just use 0 for it. + * false is returned. If unsure, just use 0 for it. */ - mapgen_function *pick( const int hardcoded_weight ) const { + bool generate( mapgendata &dat, const int hardcoded_weight ) const { if( hardcoded_weight > 0 && rng( 1, weights_.get_weight() + hardcoded_weight ) > weights_.get_weight() ) { - return nullptr; + return false; } const std::shared_ptr *const ptr = weights_.pick(); if( !ptr ) { - return nullptr; + return false; } assert( *ptr ); - return ptr->get(); + ( *ptr )->generate( dat ); + return true; } /** * Calls @ref mapgen_function::setup and sets up the internal weighted list using @@ -273,7 +278,7 @@ class mapgen_factory result.insert( elem.generator_id ); } } - // Used in C++ code only, see calls to `oter_mapgen.pick()` below + // Used in C++ code only, see calls to `oter_mapgen.generate()` below result.insert( "lab_1side" ); result.insert( "lab_4side" ); result.insert( "lab_finale_1level" ); @@ -313,13 +318,13 @@ class mapgen_factory int add( const std::string &key, const std::shared_ptr ptr ) { return mapgens_[key].add( ptr ); } - /// @see mapgen_basic_container::pick - mapgen_function *pick( const std::string &key, const int hardcoded_weight = 0 ) const { + /// @see mapgen_basic_container::generate + bool generate( mapgendata &dat, const std::string &key, const int hardcoded_weight = 0 ) const { const auto iter = mapgens_.find( key ); if( iter == mapgens_.end() ) { - return nullptr; + return false; } - return iter->second.pick( hardcoded_weight ); + return iter->second.generate( dat, hardcoded_weight ); } /// @see mapgen_basic_container::erase void erase( const std::string &key, const size_t index ) { @@ -3558,8 +3563,7 @@ void map::draw_lab( mapgendata &dat ) //A lab area with only one entrance if( boarders == 1 ) { // If you remove the usage of "lab_1side" here, remove it from mapgen_factory::get_usages above as well. - if( const auto ptr = oter_mapgen.pick( "lab_1side" ) ) { - ptr->generate( dat ); + if( oter_mapgen.generate( dat, "lab_1side" ) ) { if( tw == 2 ) { rotate( 2 ); } @@ -3577,8 +3581,7 @@ void map::draw_lab( mapgendata &dat ) } else { const int hardcoded_4side_map_weight = 1500; // weight of all hardcoded maps. // If you remove the usage of "lab_4side" here, remove it from mapgen_factory::get_usages above as well. - if( const auto ptr = oter_mapgen.pick( "lab_4side", hardcoded_4side_map_weight ) ) { - ptr->generate( dat ); + if( oter_mapgen.generate( dat, "lab_4side", hardcoded_4side_map_weight ) ) { // If the map template hasn't handled borders, handle them in code. // Rotated maps cannot handle borders and have to be caught in code. // We determine if a border isn't handled by checking the east-facing @@ -4123,9 +4126,7 @@ void map::draw_lab( mapgendata &dat ) const int hardcoded_finale_map_weight = 500; // weight of all hardcoded maps. // If you remove the usage of "lab_finale_1level" here, remove it from mapgen_factory::get_usages above as well. - if( const auto ptr = oter_mapgen.pick( "lab_finale_1level", hardcoded_finale_map_weight ) ) { - ptr->generate( dat ); - + if( oter_mapgen.generate( dat, "lab_finale_1level", hardcoded_finale_map_weight ) ) { // If the map template hasn't handled borders, handle them in code. // Rotated maps cannot handle borders and have to be caught in code. // We determine if a border isn't handled by checking the east-facing @@ -7340,11 +7341,7 @@ std::pair, std::map> get_changed_ids_from_up bool run_mapgen_func( const std::string &mapgen_id, mapgendata &dat ) { - if( const auto ptr = oter_mapgen.pick( mapgen_id ) ) { - ptr->generate( dat ); - return true; - } - return false; + return oter_mapgen.generate( dat, mapgen_id ); } int register_mapgen_function( const std::string &key ) From 9a4eb672cabf81d7b66fea8dbe1539c12ea1c73e Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 2 Feb 2020 11:32:24 +0100 Subject: [PATCH 24/27] Remove mapgen entries for overmap terrain "null". This terrain should never appears and is therefor never generated. Removing it here may free up some memory. --- src/mapgen.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 0a7282b2ad4ab..01e6fc89a85bb 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -294,6 +294,8 @@ class mapgen_factory for( std::pair &omw : mapgens_ ) { omw.second.setup(); } + // Dummy entry, overmap terrain null should never appear and is therefor never generated. + mapgens_.erase( "null" ); } void check_consistency() { // Cache all strings that may get looked up here so we don't have to go through From 7793608d5c5d41ee91573ae57dffe2715521860f Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 2 Feb 2020 14:37:43 +0100 Subject: [PATCH 25/27] Follow recommendation of clang-tidy --- src/mapgen.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 01e6fc89a85bb..fb7b4655ec586 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -436,7 +436,7 @@ load_mapgen_function( const JsonObject &jio, const std::string &id_base, ret = std::make_shared( jstr, mgweight, offset ); oter_mapgen.add( id_base, ret ); } else { - jio.throw_error( "invalid value: must be \"builtin\" or \"json\")", "method" ); + jio.throw_error( R"(invalid value: must be "builtin" or "json")", "method" ); } return ret; } From 0c650d146c50b2149aec186444c371699c5d5e17 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 2 Feb 2020 15:58:30 +0100 Subject: [PATCH 26/27] Make parameter offset of load_mapgen_function mandatory. Adds the default value to all calls to it that did not supply a value. --- src/mapgen.cpp | 4 ++-- src/mapgen.h | 2 +- src/overmap.cpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index fb7b4655ec586..8fee0a39ff965 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -505,7 +505,7 @@ void load_mapgen( const JsonObject &jo ) } if( !mapgenid_list.empty() ) { const std::string mapgenid = mapgenid_list[0]; - const auto mgfunc = load_mapgen_function( jo, mapgenid, -1 ); + const auto mgfunc = load_mapgen_function( jo, mapgenid, -1, point_zero ); if( mgfunc ) { for( auto &i : mapgenid_list ) { oter_mapgen.add( i, mgfunc ); @@ -514,7 +514,7 @@ void load_mapgen( const JsonObject &jo ) } } } else if( jo.has_string( "om_terrain" ) ) { - load_mapgen_function( jo, jo.get_string( "om_terrain" ), -1 ); + load_mapgen_function( jo, jo.get_string( "om_terrain" ), -1, point_zero ); } else if( jo.has_string( "nested_mapgen_id" ) ) { load_nested_mapgen( jo, jo.get_string( "nested_mapgen_id" ) ); } else if( jo.has_string( "update_mapgen_id" ) ) { diff --git a/src/mapgen.h b/src/mapgen.h index ec114176152df..a73284aeead6e 100644 --- a/src/mapgen.h +++ b/src/mapgen.h @@ -371,7 +371,7 @@ class mapgen_function_json_nested : public mapgen_function_json_base * Load mapgen function of any type from a json object */ std::shared_ptr load_mapgen_function( const JsonObject &jio, - const std::string &id_base, int default_idx, const point &offset = point_zero ); + const std::string &id_base, int default_idx, const point &offset ); /* * Load the above directly from a file via init, as opposed to riders attached to overmap_terrain. Added check * for oter_mapgen key, multiple possible ( ie, [ "house", "house_base" ] ) diff --git a/src/overmap.cpp b/src/overmap.cpp index 6198c588e8f33..cd7046e6b50b6 100644 --- a/src/overmap.cpp +++ b/src/overmap.cpp @@ -536,7 +536,7 @@ static void load_overmap_terrain_mapgens( const JsonObject &jo, const std::strin } if( jo.has_array( jsonkey ) ) { for( JsonObject jio : jo.get_array( jsonkey ) ) { - load_mapgen_function( jio, fmapkey, default_idx ); + load_mapgen_function( jio, fmapkey, default_idx, point_zero ); } } } From e2b0dd89d179a3d1be33b3090e422c762dc2e7ab Mon Sep 17 00:00:00 2001 From: BevapDin Date: Sun, 2 Feb 2020 16:33:53 +0100 Subject: [PATCH 27/27] Remove default_idx parameter from load_mapgen_function. This feature is not available anymore. It was added by 04e45eabbdd80a238e61c771fcf0c05073b5ca54 without any explanation. --- src/mapgen.cpp | 30 ++++-------------------------- src/mapgen.h | 2 +- src/overmap.cpp | 8 ++------ 3 files changed, 7 insertions(+), 33 deletions(-) diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 8fee0a39ff965..a3ab89369b589 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -206,14 +206,6 @@ class mapgen_basic_container mapgens_.push_back( ptr ); return mapgens_.size() - 1; } - void erase( const size_t index ) { - if( index > mapgens_.size() ) { - return; - } - assert( mapgens_[index] ); - // Can't actually erase it as this might invalid the index stored elsewhere - mapgens_[index]->weight = 0; - } /** * Pick a mapgen function randomly and call its generate function. * This basically runs the mapgen functions with the given @ref mapgendata @@ -328,10 +320,6 @@ class mapgen_factory } return iter->second.generate( dat, hardcoded_weight ); } - /// @see mapgen_basic_container::erase - void erase( const std::string &key, const size_t index ) { - mapgens_[key].erase( index ); - } }; static mapgen_factory oter_mapgen; @@ -405,21 +393,11 @@ static void set_mapgen_defer( const JsonObject &jsi, const std::string &member, * load a single mapgen json structure; this can be inside an overmap_terrain, or on it's own. */ std::shared_ptr -load_mapgen_function( const JsonObject &jio, const std::string &id_base, - int default_idx, const point &offset ) +load_mapgen_function( const JsonObject &jio, const std::string &id_base, const point &offset ) { int mgweight = jio.get_int( "weight", 1000 ); std::shared_ptr ret; if( mgweight <= 0 || jio.get_bool( "disabled", false ) ) { - const std::string mgtype = jio.get_string( "method" ); - if( default_idx != -1 && mgtype == "builtin" ) { - if( jio.has_string( "name" ) ) { - const std::string mgname = jio.get_string( "name" ); - if( mgname == id_base ) { - oter_mapgen.erase( id_base, default_idx ); - } - } - } jio.allow_omitted_members(); return nullptr; // nothing } @@ -489,7 +467,7 @@ void load_mapgen( const JsonObject &jo ) point offset; for( JsonArray row_items : ja ) { for( const std::string mapgenid : row_items ) { - const auto mgfunc = load_mapgen_function( jo, mapgenid, -1, offset ); + const auto mgfunc = load_mapgen_function( jo, mapgenid, offset ); if( mgfunc ) { oter_mapgen.add( mapgenid, mgfunc ); } @@ -505,7 +483,7 @@ void load_mapgen( const JsonObject &jo ) } if( !mapgenid_list.empty() ) { const std::string mapgenid = mapgenid_list[0]; - const auto mgfunc = load_mapgen_function( jo, mapgenid, -1, point_zero ); + const auto mgfunc = load_mapgen_function( jo, mapgenid, point_zero ); if( mgfunc ) { for( auto &i : mapgenid_list ) { oter_mapgen.add( i, mgfunc ); @@ -514,7 +492,7 @@ void load_mapgen( const JsonObject &jo ) } } } else if( jo.has_string( "om_terrain" ) ) { - load_mapgen_function( jo, jo.get_string( "om_terrain" ), -1, point_zero ); + load_mapgen_function( jo, jo.get_string( "om_terrain" ), point_zero ); } else if( jo.has_string( "nested_mapgen_id" ) ) { load_nested_mapgen( jo, jo.get_string( "nested_mapgen_id" ) ); } else if( jo.has_string( "update_mapgen_id" ) ) { diff --git a/src/mapgen.h b/src/mapgen.h index a73284aeead6e..8e051518b3673 100644 --- a/src/mapgen.h +++ b/src/mapgen.h @@ -371,7 +371,7 @@ class mapgen_function_json_nested : public mapgen_function_json_base * Load mapgen function of any type from a json object */ std::shared_ptr load_mapgen_function( const JsonObject &jio, - const std::string &id_base, int default_idx, const point &offset ); + const std::string &id_base, const point &offset ); /* * Load the above directly from a file via init, as opposed to riders attached to overmap_terrain. Added check * for oter_mapgen key, multiple possible ( ie, [ "house", "house_base" ] ) diff --git a/src/overmap.cpp b/src/overmap.cpp index cd7046e6b50b6..a507431fe369c 100644 --- a/src/overmap.cpp +++ b/src/overmap.cpp @@ -529,14 +529,10 @@ static void load_overmap_terrain_mapgens( const JsonObject &jo, const std::strin { const std::string fmapkey( id_base + suffix ); const std::string jsonkey( "mapgen" + suffix ); - bool default_mapgen = jo.get_bool( "default_mapgen", true ); - int default_idx = -1; - if( default_mapgen ) { - default_idx = register_mapgen_function( fmapkey ); - } + register_mapgen_function( fmapkey ); if( jo.has_array( jsonkey ) ) { for( JsonObject jio : jo.get_array( jsonkey ) ) { - load_mapgen_function( jio, fmapkey, default_idx, point_zero ); + load_mapgen_function( jio, fmapkey, point_zero ); } } }