Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace add_spawn() calls in mapgen with place_spawn() #3951

Open
kevingranade opened this issue Oct 29, 2013 · 38 comments
Open

Replace add_spawn() calls in mapgen with place_spawn() #3951

kevingranade opened this issue Oct 29, 2013 · 38 comments
Labels
[C++] Changes (can be) made in C++. Previously named `Code` <Enhancement / Feature> New features, or enhancements on existing Good First Issue This is a good first issue for a new contributor Map / Mapgen Overmap, Mapgen, Map extras, Map display (P2 - High) High priority (for ex. important bugfixes) (P5 - Long-term) Long-term WIP, may stay on the list for a while.

Comments

@kevingranade
Copy link
Member

There are several building layouts in mapgen.cpp and building_generation.cpp that have add_spawn() calls placing specific monsters on the map instead of place_spawn, which is more generic and selects monsters from a group. This causes issues with the options that modify spawn behavior, because add_spawn() doesn't support those options.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@GreyZebra
Copy link

It will take some time, but I'll get it.

@Chase-san
Copy link
Contributor

I think it also makes the monster blacklist mod function fail. Which means we can't remove certain monsters from the game in their entirety via a mod, because they are hard coded.

@codemime codemime added the Map / Mapgen Overmap, Mapgen, Map extras, Map display label Jan 15, 2017
@Leland Leland added the [C++] Changes (can be) made in C++. Previously named `Code` label Aug 19, 2017
@kevingranade kevingranade reopened this Feb 2, 2018
@kevingranade
Copy link
Member Author

Afraid there are quite a few of these left in mapgen.cpp and mission_start.cpp

@ZhilkinSerg
Copy link
Contributor

Afraid there are quite a few of these left in mapgen.cpp and mission_start.cpp

Yes, sorry. I've somehow searched for this function only in mapgen_functions.cpp. Working on the rest now.

@ZhilkinSerg
Copy link
Contributor

I won't replace add_spawn in mission_start.cpp for now as place_spawns functions is missing mission_id and name parameters, so it won't fit for mission related spawns:

void mission_start::place_zombie_mom( mission *miss )
{
    const tripoint house = random_house_in_closest_city();

    miss->target = house;
    overmap_buffer.reveal( house, 6 );

    tinymap zomhouse;
    zomhouse.load( house.x * 2, house.y * 2, house.z, false );
    zomhouse.add_spawn( mon_zombie, 1, SEEX, SEEY, false, -1, miss->uid,
                        Name::get( nameIsFemaleName | nameIsGivenName ) );
    zomhouse.save();
}

@ZhilkinSerg
Copy link
Contributor

Step 2 is in #22852. When that will be merged only 85 add_spawn calls will be left in mapgen.cpp - these will be changed in step 3.

@ZhilkinSerg
Copy link
Contributor

Step 3 is in #22854. When that will be merged only 67 add_spawn calls will be left in mapgen.cpp - these will be changed in step 4.

@ZhilkinSerg ZhilkinSerg added the (P5 - Long-term) Long-term WIP, may stay on the list for a while. label Apr 3, 2018
@brekiy
Copy link
Contributor

brekiy commented Apr 4, 2019

Hi, I'm looking to start contributing to Cataclysm. Could I finish this task, if any add_spawn calls are still around?

@ZhilkinSerg
Copy link
Contributor

ZhilkinSerg commented Apr 4, 2019

Beware, it can end bad.

@brekiy
Copy link
Contributor

brekiy commented Apr 4, 2019

Ok, I read the issue you linked. Could that issue be solved by adding some flag to place_spawn? Some boolean named spawn_single that defaults to false. If spawn_single is set to true, then it would ignore the density and place a single creature at the provided x and y coordinates. And, as it defaults to false, then the flag doesn't have to be explicitly declared so that some calls to place_spawn can stay as they are.

Then, spawns for monsters like zombies can stay as a group, and spawns intended for turrets or powerful monsters can have the flag set to true so that only one of them spawns.

@brekiy
Copy link
Contributor

brekiy commented Apr 12, 2019

if( density > 1 ) {
    place_spawns( GROUP_ZOMBIE, 2, 0, 0, SEEX * 2 - 1, SEEX * 2 - 1, density );
} else {
    add_spawn( mon_zombie, rng( 0, 5 ), SEEX * 2 - 1, SEEX * 2 - 1 );
}

@ZhilkinSerg @kevingranade
I'm confused as to why I keep seeing this snippet everywhere. From what I understand, if the spawn density during mapgen is greater than 1, then the zombies spawned are from the monster group json, which includes all sorts of zombies: regular, fat, child, cop, brute, etc. But if the density is lower than 1, then it chooses to spawn a small number of zombies using add_spawn(). Why was this spawn behavior picked? Should I remove it and just modify place_spawns so that it has a lower variance?

I've listed the relevant bits of the place_spawns() method with comments where I made changes.

// Added 'individual' flag, defaults to false in declaration
void map::place_spawns( const mongroup_id &group, const int chance,
                        const int x1, const int y1, const int x2, const int y2, const float density,
                        const bool individual)
{
    // blah

    float spawn_density = 1.0f;
    if( MonsterGroupManager::is_animal( group ) ) {
        spawn_density = get_option< float >( "SPAWN_ANIMAL_DENSITY" );
    } else {
        spawn_density = get_option< float >( "SPAWN_DENSITY" );
    }

    float multiplier = density * spawn_density;
    // Added conditional
    float thenum = individual ? 1 : (multiplier * rng_float(10.0f, 50.0f));
    int num = roll_remainder( thenum );
    
    // blah
}

@brekiy
Copy link
Contributor

brekiy commented Apr 12, 2019

On a related note, there are certain spawns of individual, specific monsters that don't have monster groups like hazmat bots and fungaloids (lines 3787, 3843 of mapgen.cpp respectively). I saw that in the old, rolled-back commits, special monster groups consisting of only those monsters existed. Should I adopt that approach or is there some other approach I'm overlooking?

@ZhilkinSerg
Copy link
Contributor

That could be some legacy code which was not refactored yet. Generally we want most of the things unhardcoded and jsonized. Starting replacing single monster spawn with monster groups would be a good start.

@brekiy
Copy link
Contributor

brekiy commented Apr 14, 2019

Thanks for the input. I'll keep at it.

@leemuar
Copy link
Contributor

leemuar commented Jan 6, 2022

Should this issue be closed now? I see no add_spawn() in current source code.

@Night-Pryanik
Copy link
Contributor

I see no add_spawn() in current source code.

There are still dozens of occurrences of add_spawn in several files.

@leemuar
Copy link
Contributor

leemuar commented Jan 7, 2022

My bad, I was using the search wrong

There are still dozens of occurrences of add_spawn in several files.

@I-am-Erk
Copy link
Member

Since there are only about eight of these left, I'm going to make this a blocker. It should be trivial to finish and close this ancient issue

@I-am-Erk I-am-Erk added the (P2 - High) High priority (for ex. important bugfixes) label Nov 23, 2022
@cathalpern
Copy link
Contributor

cathalpern commented Nov 24, 2022

I can take a stab at this once I have my local environment configured - or happy to help @aaronstevenson408 if they feel good about making the changes

@aaronstevenson408
Copy link
Contributor

@cathalpern im willing to work on it with you. i'm in the discord , you can @ me or dm me so we can organize

@cathalpern
Copy link
Contributor

For someone in the know - place_spawns actually calls add_spawn

add_spawn( mgr.name, mgr.pack_size, { p, abs_sub.z() },

Is the intention to replace this as well?
@I-am-Erk

@I-am-Erk
Copy link
Member

I would think that's the actual intended spot for it

@cathalpern
Copy link
Contributor

@I-am-Erk just to make sure I'm understanding correctly - leave the add_spawn calls within place_spawns, yes?

@cathalpern
Copy link
Contributor

Gonna put in a PR for L6185, L2370, L2363, L275 in mapgen, L332, L333, L970 in mapgen_functions tomorrow

@aaronstevenson408
Copy link
Contributor

aaronstevenson408 commented Nov 26, 2022

A list of all the replacements necessary

  1. map.cpp(7774):add_spawn( mgr, pnt );
  2. map.h(1682):void add_spawn( const mtype_id &type, int count, const tripoint &p,
  3. map.h(1685):void add_spawn( const mtype_id &type, int count, const tripoint &p, bool friendly,
  4. map.h(1688):void add_spawn( const MonsterGroupResult &spawn_details, const tripoint &p );
    5. mapgen.cpp(275):add_spawn( mgr, pt ); @cathalpern
    6. mapgen.cpp(2370):dat.m.add_spawn( mgr.name, spawn_count * pack_size.get(),@cathalpern
    7. mapgen.cpp(2377):dat.m.add_spawn( chosen_type, spawn_count * pack_size.get(),@cathalpern
    8. mapgen.cpp(6192):add_spawn( mgr.name, mgr.pack_size, { p, abs_sub.z() },@cathalpern
    9. mapgen.cpp(6349):void map::add_spawn( const MonsterGroupResult &spawn_details, const tripoint &p )@cathalpern
    10. mapgen.cpp(6351):add_spawn( spawn_details.name, spawn_details.pack_size, p, false, -1, -1, "NONE",@cathalpern
    11. mapgen.cpp(6355):void map::add_spawn( const mtype_id &type, int count, const tripoint &p, bool friendly,@cathalpern
    12. mapgen.cpp(6358):add_spawn( type, count, p, friendly, faction_id, mission_id, name, spawn_data() );@cathalpern
    13. mapgen.cpp(6361):void map::add_spawn(@cathalpern
    14. mapgen.cpp(6366):debugmsg( "Out of bounds add_spawn(%s, %d, %d, %d)", type.c_str(), count, p.x, p.y );@cathalpern
    15. mapgen.cpp(6377):debugmsg( "centadodecamonant doesn't exist in grid; within add_spawn(%s, %d, %d, %d, %d)",@cathalpern
    16. mapgen_functions.cpp(332):m->add_spawn( mon_bee, 2, { i, j, m->get_abs_sub().z() } );@cathalpern
    17. mapgen_functions.cpp(333):m->add_spawn( mon_beekeeper, 1, { i, j, m->get_abs_sub().z() } );@cathalpern
    18. mapgen_functions.cpp(970):m->add_spawn( mon_zombie_jackson, 1, { SEEX, SEEY, m->get_abs_sub().z() } );@cathalpern
    19. map_extras.cpp(347):m.add_spawn( mon_dermatik, rng( 1, 3 ), tripoint( point( SEEX * 2 - 1, SEEY * 2 - 1 ), loc.z ) ); @nornagon
    20.map_extras.cpp(442):m.add_spawn( mon_zombie_military_pilot, 1, pos );@nornagon
    21.map_extras.cpp(445):m.add_spawn( mon_zombie_bio_op, 1, pos );@nornagon
    22. map_extras.cpp(447):m.add_spawn( mon_zombie_scientist, 1, pos );@nornagon
    23. map_extras.cpp(449):m.add_spawn( mon_zombie_soldier, 1, pos );@nornagon
    24. map_extras.cpp(470):m.add_spawn( mon_zombie_military_pilot, 1, pos );@nornagon
    25. map_extras.cpp(473):m.add_spawn( mon_zombie_soldier, 1, pos );@nornagon
    26. map_extras.cpp(491):m.add_spawn( mon_zombie_military_pilot, 1, pos );@nornagon
  5. map_extras.cpp(536):m.add_spawn( mon_turret_riot, 1, { p, abs_sub.z } );@nornagon
  6. map_extras.cpp(611):m.add_spawn( mon_turret_searchlight, 1, { 13, 8, abs_sub.z } );@nornagon
  7. map_extras.cpp(620):m.add_spawn( mon_zombie_soldier, 1, *p );@nornagon
  8. map_extras.cpp(648):m.add_spawn( mon_turret_riot, 1, { 12, 1, abs_sub.z } );@nornagon
  9. map_extras.cpp(653):m.add_spawn( mon_turret_riot, 1, { SEEX * 2 - 1, 12, abs_sub.z } );@nornagon
  10. map_extras.cpp(658):m.add_spawn( mon_turret_riot, 1, { 12, SEEY * 2 - 1, abs_sub.z } );@nornagon
  11. map_extras.cpp(663):m.add_spawn( mon_turret_riot, 1, { 1, 12, abs_sub.z } );@nornagon
  12. map_extras.cpp(670):m.add_spawn( mon_turret_searchlight, 1, { 7, 11, abs_sub.z } );@nornagon
  13. map_extras.cpp(1530):m.add_spawn( mon_shia, 1, { SEEX, SEEY, loc.z } );@nornagon
  14. map_extras.cpp(1545):m.add_spawn( mon_jabberwock, 1, { SEEX, SEEY, loc.z } );@nornagon
  15. map_extras.cpp(2675):compmap.add_spawn( mon_turret_speaker, 1, trap_center );@nornagon
  16. map_field.cpp(755):pd.here.add_spawn( mgr, *spawn_point );
    39. mission_start.cpp(76):doghouse.add_spawn( mon_dog, 1, { SEEX, SEEY, house.z() }, true, -1, miss->uid ); @aaronstevenson408
    40. mission_start.cpp(89):zomhouse.add_spawn( mon_zombie, 1, { SEEX, SEEY, house.z() }, false, -1, miss->uid,@aaronstevenson408
    41. mission_start.cpp(119):tile.add_spawn( mon_zombie_master, 1, { SEEX, SEEY, site.z() }, false, -1, miss->uid,@aaronstevenson408
    42. mission_start.cpp(121):tile.add_spawn( mon_zombie_brute, 3, { SEEX, SEEY, site.z() } );@aaronstevenson408
    43. mission_start.cpp(122):tile.add_spawn( mon_zombie_dog, 3, { SEEX, SEEY, site.z() } );@aaronstevenson408
    44. mission_start.cpp(126):tile.add_spawn( mon_zombie, rng( 3, 10 ), { x, y, site.z() } );@aaronstevenson408
    45. mission_start.cpp(128):tile.add_spawn( mon_zombie_dog, rng( 0, 2 ), { SEEX, SEEY, site.z() } );@aaronstevenson408
    46. mission_start.cpp(130):tile.add_spawn( mon_zombie_necro, 2, { SEEX, SEEY, site.z() } );@aaronstevenson408
    47. mission_start.cpp(131):tile.add_spawn( mon_zombie_hulk, 1, { SEEX, SEEY, site.z() } );@aaronstevenson408
    48. monster.cpp(493):here.add_spawn( type->baby_monster, spawn_cnt, pos() );@aaronstevenson408

@aaronstevenson408
Copy link
Contributor

ones that are bolded are ones that are chosen , unbolded ones are up for grabs , im assuming that @nornagon is gonna take the ones in map extras, on confirm those will be bolded

@aaronstevenson408
Copy link
Contributor

@nornagon you are way too good , at this rate you are gonna finish this before anyone else, if you want you can take lead on this, i have to reset up my dev enviroment tomorrow so if you want to keep on at this clip take mine on

@cathalpern
Copy link
Contributor

So I rewrote most of the calls to add_spawn in mapgen.cpp and map.cpp, and have those pushed to a branch in my fork - but since that would be touching some pretty core, potentially majorly breaking game functionality, I wonder if they should be put off, and just focus on replacing the calls in other files for 0.G

@I-am-Erk
Copy link
Member

Enough progress has been made here that I am content to move this along out of blockers now. I just did not want to see it sitting here waiting another six years.

@MNG-cataclysm
Copy link
Contributor

I think with the zombie mom mission JSON-ized, this issue should be closed. I think all of the old add_spawn calls have been converted.

@ehughsbaird
Copy link
Contributor

ehughsbaird commented Nov 13, 2023

#68752 added a new one.
This probably can't be closed without an enforcement mechanism.

@Maleclypse
Copy link
Member

Cataclysm-DDA\src\map_extras.cpp:
 2166  
 2167      const tripoint suitable_location = random_entry( suitable_locations, submap_center );
 2168:     fungal_map.add_spawn( mon_fungaloid_queen, 1, suitable_location );
 2169      fungal_map.place_spawns( GROUP_FUNGI_FUNGALOID, 1,
 2170                               suitable_location.xy() + point_north_west,

@ehughsbaird
Copy link
Contributor

ehughsbaird commented Nov 13, 2023

Making it private:

diff --git a/src/map.h b/src/map.h
index 17dbf6727d..01f8086223 100644
--- a/src/map.h
+++ b/src/map.h
@@ -1719,13 +1719,7 @@ class map
         // places an NPC, if static NPCs are enabled or if force is true
         character_id place_npc( const point &p, const string_id<npc_template> &type );
         void apply_faction_ownership( const point &p1, const point &p2, const faction_id &id );
-        void add_spawn( const mtype_id &type, int count, const tripoint &p,
-                        bool friendly = false, int faction_id = -1, int mission_id = -1,
-                        const std::string &name = "NONE" );
-        void add_spawn( const mtype_id &type, int count, const tripoint &p, bool friendly,
-                        int faction_id, int mission_id, const std::string &name,
-                        const spawn_data &data );
-        void add_spawn( const MonsterGroupResult &spawn_details, const tripoint &p );
+
         void do_vehicle_caching( int z );
         // Note: in 3D mode, will actually build caches on ALL z-levels
         void build_map_cache( int zlev, bool skip_lightmap = false );
@@ -1873,6 +1867,15 @@ class map
         */
         void rotten_item_spawn( const item &item, const tripoint &p );
     private:
+       // Use place_spawns or JSON mapgen instead
+        void add_spawn( const mtype_id &type, int count, const tripoint &p,
+                        bool friendly = false, int faction_id = -1, int mission_id = -1,
+                        const std::string &name = "NONE" );
+        void add_spawn( const mtype_id &type, int count, const tripoint &p, bool friendly,
+                        int faction_id, int mission_id, const std::string &name,
+                        const spawn_data &data );
+        void add_spawn( const MonsterGroupResult &spawn_details, const tripoint &p );
+
         // Helper #1 - spawns monsters on one submap
         void spawn_monsters_submap( const tripoint &gp, bool ignore_sight, bool spawn_nonlocal = false );
         // Helper #2 - spawns monsters on one submap and from one group on this submap

There are a few calls outside of map:
Fields spawning monsters?

src/map_field.cpp: In function ‘void field_processor_monster_spawn(const tripoint&, field_entry&, field_proc_data&)’:
src/map_field.cpp:768:38: error: ‘void map::add_spawn(const MonsterGroupResult&, const tripoint&)’ is private within this context
  768 |                     pd.here.add_spawn( mgr, *spawn_point );
      |                     ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~

That map extra

src/map_extras.cpp: In function ‘bool MapExtras::mx_fungal_zone(map&, const tripoint&)’:
src/map_extras.cpp:2168:25: error: ‘void map::add_spawn(const mtype_id&, int, const tripoint&, bool, int, int, const std::string&)’ is private within this context
 2168 |     fungal_map.add_spawn( mon_fungaloid_queen, 1, suitable_location );
      |     ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Monster reproduction

src/monster.cpp: In member function ‘void monster::try_reproduce()’:
src/monster.cpp:607:31: error: ‘void map::add_spawn(const mtype_id&, int, const tripoint&, bool, int, int, const std::string&)’ is private within this context
  607 |                 here.add_spawn( type->baby_monster, spawn_cnt, pos() );
      |                 ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

JSON Mapgen:

src/mapgen.cpp: In member function ‘virtual void jmapgen_monster::apply(const mapgendata&, const jmapgen_int&, const jmapgen_int&, const std::string&) const’:
src/mapgen.cpp:2458:36: error: ‘void map::add_spawn(const mtype_id&, int, const tripoint&, bool, int, int, const std::string&, const spawn_data&)’ is private within this context
 2458 |                     dat.m.add_spawn( mgr.name, spawn_count * pack_size.get(),
      |                     ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2459 |                     { x.get(), y.get(), dat.m.get_abs_sub().z() },
      |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2460 |                     friendly, -1, mission_id, chosen_name, data );
      |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The map extra is trivial, the rest probably aren't.

@MNG-cataclysm
Copy link
Contributor

Is add_spawn heavily tied to place_spawn in terms of backend code support? If not, why not just delete all of the backend code for add_spawn and use that method as an enforcement mechanism to keep it from coming up again? Or, is the backend tied to too many things for it to be reasonable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` <Enhancement / Feature> New features, or enhancements on existing Good First Issue This is a good first issue for a new contributor Map / Mapgen Overmap, Mapgen, Map extras, Map display (P2 - High) High priority (for ex. important bugfixes) (P5 - Long-term) Long-term WIP, may stay on the list for a while.
Projects
None yet
Development

No branches or pull requests