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

Overhaul of map revealing items #27012

Merged
merged 12 commits into from Dec 14, 2018

Conversation

Projects
None yet
5 participants
@ZhilkinSerg
Copy link
Contributor

commented Dec 8, 2018

Summary

SUMMARY: Features "Overhaul of map revealing items"

Purpose of change

Map revealing items will now reveal overmap terrains around city nearest to its spawn location (as opposed to player location which was used previously).

Describe the solution

When items of maps category are spawned they store coordinates of nearest city as tripoint in item variable named reveal_map_center_omt. Corresponding city name is shown for this items in inventory. When item is used it reveals overmap terrain around corresponding city.

I've also moved map revealing items to separate item category (MAPS) and separate json file. Also utilized abstract for map revealing items and added disassembly recipe for trail guide.

Additional context

image

image

@@ -2321,6 +2326,9 @@ void overmap::place_cities()
tmp.s = size;
cities.push_back( tmp );

DebugLog( D_INFO, DC_ALL ) << "CITY:[" << tmp.name << "] at {" << tmp.x << "," << tmp.x <<

This comment has been minimized.

Copy link
@kevingranade

kevingranade Dec 9, 2018

Member

This spams the logs a LOT, it might be interesting but I don't think we need it happening by default.

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Dec 9, 2018

Author Contributor

Right, it was debug feature I forgot to cut-off.

@@ -294,14 +295,12 @@ class overmap
// parse data in an old overmap file
void unserialize_legacy( std::istream &fin );
void unserialize_view_legacy( std::istream &fin );
const city &get_nearest_city( const tripoint &p ) const;

This comment has been minimized.

Copy link
@kevingranade

kevingranade Dec 9, 2018

Member

Instead of making this public, please see if you can use overmapbuffer::closest_city()

@@ -48,6 +48,7 @@ struct city {
}

int get_distance_from( const tripoint &p ) const;
tripoint position() const;

This comment has been minimized.

Copy link
@kevingranade

kevingranade Dec 9, 2018

Member

Even better, if you use overmapbuffer::closest_city(), the reference it returns has this position already.

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Dec 9, 2018

Author Contributor

Will try to do that, though position in city_reference is in submap coordinates, not omt coordinates.

src/map.cpp Outdated
@@ -4177,6 +4177,12 @@ item &map::add_item_at( const tripoint &p,
submap *const current_submap = get_submap_at( p, lx, ly );
current_submap->is_uniform = false;

if( new_item.is_map() && !new_item.has_var( "reveal_map_center_omt" ) ) {
//new_item.set_var( "origin_abs", g->m.getabs( p ) );

This comment has been minimized.

Copy link
@kevingranade

kevingranade Dec 9, 2018

Member

Looks like you forgot to remove some commented out code.

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Dec 9, 2018

Author Contributor

Thanks.

@@ -2934,7 +2952,14 @@ std::string item::display_name( unsigned int quantity ) const
}
}

return string_format( "%s%s%s", name.c_str(), sidetxt.c_str(), amt.c_str() );
std::string &prefixed_name = name;

This comment has been minimized.

Copy link
@BevapDin

BevapDin Dec 9, 2018

Contributor

Why that? Can't you just use the variable name directly?

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Dec 9, 2018

Author Contributor

Right, I thought name was item field and didn't want to change that.

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor Author

commented Dec 9, 2018

We need to improve name selection algorithm (see two Bradleys nearby):

image

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor Author

commented Dec 9, 2018

Should I split json changes off current PR?

@I-am-Erk

This comment has been minimized.

Copy link
Contributor

commented Dec 10, 2018

What about worlds without cities?

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor Author

commented Dec 10, 2018

What about worlds without cities?

Old behavior - item display name isn't changed, overmap terrains are revealed around player.

@Rivet-the-Zombie Rivet-the-Zombie merged commit 1c4c8da into CleverRaven:master Dec 14, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
gorgon-ghprb Build finished.
Details

@ZhilkinSerg ZhilkinSerg deleted the ZhilkinSerg:map-items-overhaul branch Dec 14, 2018

@kevingranade

This comment has been minimized.

Copy link
Member

commented Dec 20, 2018

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/military-operation-map-bug/18016/2

"type": "reveal_map",
"radius": 180,
"terrain": [ "hiway", "road", "bridge", "fema_entrance", "bunker", "outpost", "silo", "shelter", "police" ],
"message": "You add roads and restaurants to your map."

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Dec 20, 2018

Author Contributor

Message is invalid (copy-paste error) and should be updated.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Apr 8, 2019

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/road-maps-and-tourist-maps/19593/5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.