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

Add tracking of overmap special for placed terrain #27074

Conversation

ralreegorganon
Copy link
Contributor

@ralreegorganon ralreegorganon commented Dec 12, 2018

Summary

SUMMARY: Infrastructure "Add tracking of overmap special for placed terrain"

Purpose of change

This is the first in a series of changes being split off from this branch, which provides the support for checking for the existence of overmap specials (and placement of them if necessary) after overmap generation--initially used in updating missions to be "overmap special aware / capable".

This change adds the support for capturing, persisting, and later determining a given overmap location's associated overmap special.

Describe the solution

Add a new overmap member which records the overmap special id associated with a location, update the special placement to use it, add serialization and deserialization of the mapping and add a new function for checking if a given overmap location is a given overmap special.

Additional context

I'm storing this in the overmap as a std::unordered_map<tripoint, overmap_special_id> because the way this gets used is in conjunction with other checks that evaluate a given tripoint for other criteria, and a given tripoint can have at most one special that placed the current overmap terrain.

For serialization, however, I'm storing it like this:

updated: after discussion I've revised the format for the diff'd section as described in this comment

[
  {
...rest of the overmap save file...
    "overmap_special_placements": [
-      {
-        "special": "a_particular_special_id",
-        "placements": [ { "p": [ 0, 0, 0 ] }, { "p": [ 1, 0, 0 ] }, { "p": [ 1, 1, 0 ] } ]
-      },
-      {
-        "special": "some_other_special",
-        "placements": [ { "p": [ 0, 0, -1 ] }, { "p": [ 1, 0, -1 ] }, { "p": [ 1, 1, -1 ] } ]
-      }
    ]
  }
]

The rationale here is:

  1. There are a lot of specials that get placed, as every structure placed in a city gets turned into a special behind the scenes, plus the overmap specials, but there are comparatively few distinct specials, so it makes a lot of sense to group all the placements for a given special under a single key...
  2. ...but I want the placements to still be an object rather than simply an array of coordinates. There is some discussion of a potential future need to differentiate the special placements (e.g. FEMA Camp 1 vs FEMA Camp 2)--that seems reasonable, but I'm not going to add it now. However, this structure leaves the door open to add additional properties to the serialized object (e.g. a discriminator) without having to implement a save-game migration to something different.

As for how this gets used, the subsequent changes can be tracked in this branch,

@ZhilkinSerg ZhilkinSerg added Map / Mapgen Overmap, Mapgen, Map extras, Map display [C++] Changes (can be) made in C++. Previously named `Code` labels Dec 12, 2018
@ralreegorganon ralreegorganon force-pushed the capture-omt-special-relationship branch from c0b1641 to b6d3fd3 Compare December 13, 2018 06:06
@ralreegorganon
Copy link
Contributor Author

After discussion with @ZhilkinSerg I've updated the serialization format of the "overmap_special_placements" key to be as follows:

[
  {
    "special": "Megastore",
    "placements": [
      {
        "uniqueid": 0,
        "points": [
          { "p": [ 172, 10, 0 ] },
          { "p": [ 172, 11, 0 ] },
          { "p": [ 172, 9, 0 ] },
          { "p": [ 173, 10, 0 ] },
          { "p": [ 173, 11, 0 ] },
          { "p": [ 173, 9, 0 ] },
          { "p": [ 174, 10, 0 ] },
          { "p": [ 174, 11, 0 ] },
          { "p": [ 174, 9, 0 ] }
        ]
      },
      {
        "uniqueid": 1,
        "points": [
          { "p": [ 21, 136, 0 ] },
          { "p": [ 21, 137, 0 ] },
          { "p": [ 21, 138, 0 ] },
          { "p": [ 22, 136, 0 ] },
          { "p": [ 22, 137, 0 ] },
          { "p": [ 22, 138, 0 ] },
          { "p": [ 23, 136, 0 ] },
          { "p": [ 23, 137, 0 ] },
          { "p": [ 23, 138, 0 ] }
        ]
      }
    ]
  }
]

where each placement is now an object, so that data in common for the placement (e.g. the unique id) isn't repeated for each point in the placement.

As noted in my original PR summary

There is some discussion of a potential future need to differentiate the special placements (e.g. FEMA Camp 1 vs FEMA Camp 2)--that seems reasonable, but I'm not going to add it now. However, this structure leaves the door open to add additional properties to the serialized object (e.g. a discriminator) without having to implement a save-game migration to something different.

there are no actual additional properties yet (because I don't need them yet), and so there is no unique id being assigned/saved/loaded yet, and thus all points for a given special are in a single object like this:

[
  {
    "special": "Megastore",
    "placements": [
      {
        "points": [
          { "p": [ 172, 10, 0 ] },
          { "p": [ 172, 11, 0 ] },
          { "p": [ 172, 9, 0 ] },
          { "p": [ 173, 10, 0 ] },
          { "p": [ 173, 11, 0 ] },
          { "p": [ 173, 9, 0 ] },
          { "p": [ 174, 10, 0 ] },
          { "p": [ 174, 11, 0 ] },
          { "p": [ 174, 9, 0 ] },
          { "p": [ 21, 136, 0 ] },
          { "p": [ 21, 137, 0 ] },
          { "p": [ 21, 138, 0 ] },
          { "p": [ 22, 136, 0 ] },
          { "p": [ 22, 137, 0 ] },
          { "p": [ 22, 138, 0 ] },
          { "p": [ 23, 136, 0 ] },
          { "p": [ 23, 137, 0 ] },
          { "p": [ 23, 138, 0 ] }
        ]
      }
    ]
  }
]

but we can make those future enhancements without breaking this structure.

src/overmap.cpp Outdated
@@ -2988,6 +2988,21 @@ bool overmap::check_ot_subtype( const std::string &otype, int x, int y, int z )
return is_ot_subtype( otype.c_str(), oter );
}

bool overmap::check_overmap_special_type( const overmap_special_id &id, int x, int y, int z ) const
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is a new function, please take the location as tripoint there should be no separate x,y,z values,it's one piece of data. You're not forwarding the string id as const char * and size_t either, but as a single object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, no problem. I actually wrote it with a tripoint first, and then backed off to keep the API homogeneous with the other two

bool check_ot_type( const std::string &otype, int x, int y, int z ) const;
bool check_ot_subtype( const std::string &otype, int x, int y, int z ) const;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

When updating the not-yet-PR'd changes that depend on this, I remembered the other reason I kept the API homogeneous--the overmapbuffer versions of those two functions call the corresponding versions of get_om_global that take the coordinates by reference and adjust them to be overmap local, and return the overmap, while the tripoint version of get_om_global doesn't currently return adjusted coordinates.

It's the sort of thing that it would actually probably be better (or at least less surprising) for the get_om_global functions to return a new type that has both the overmap and the adjusted coordinates, rather than changing coordinates passed by reference for the xyz version and not for the tripoint.

@ZhilkinSerg ZhilkinSerg self-assigned this Dec 14, 2018
@ralreegorganon ralreegorganon force-pushed the capture-omt-special-relationship branch from b6d3fd3 to 4945075 Compare December 14, 2018 22:02
@kevingranade kevingranade merged commit ad897e9 into CleverRaven:master Dec 15, 2018
@ZhilkinSerg ZhilkinSerg removed their assignment Dec 15, 2018
@ralreegorganon ralreegorganon deleted the capture-omt-special-relationship branch April 7, 2020 16:08
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` Map / Mapgen Overmap, Mapgen, Map extras, Map display
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants