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

Overmap overhaul (step 8) #21620

Merged
merged 4 commits into from Aug 17, 2017

Conversation

4 participants
@codemime
Copy link
Member

commented Aug 13, 2017

Continues #21563.
One step closer to #19298 (see my comments there).

Observable behavior

  • Roads try to avoid the terrain where it's more difficult to pave them (rivers, swamps). The difficulty can be explicitly specified.
  • More natural curvature of roads by virtue of the tweaked "estimator" function. Apart from the weights assigned to different road types, it uses square distance instead of linear distance.

Note how the roads reasonably avoid swamps and forests:
avoid_big

The swampy bank of the river isn't a good place for the road, so it goes around:
avoid_small

Terrain-specific road types can now be defined:

  • bridges for rivers (have always been there, but now there's nothing special about them - just yet another connection for the river locations).

What can be done in the future:

  • shady roads for thick forests (fallen trees would spawn on them).
  • watery roads for swamps (puddles and stuff).

Implementation details

  • Overmap connections were JSONized too (and encapsulated into their own classes).
  • There's no need to polish any connections (roads, sewers, subway tracks, ant tunnels, e.t.c). They are built perfectly connected initially. So the polish function now governs only rivers (5 lines of code). This is probably faster, but I didn't measure the difference.
  • Highways await for proper implementation.
  • There's a new flag ORTHOGONAL which produces more angled connections (with fewer turns). It's used to make subway and sewer tunnels.
  • The flag ALLOW_OVERRIDE was, in turn, removed. There's no need for it anymore: the connections themselves "know" which locations they can/cannot override.
  • Simple pathfinding now produces path objects (the class will be extended later on).
  • "Planning" the connections (lay_out_connection) now precedes the actual building of them (build_connection).
  • Opposite direction of paths (from specials to cities, not vice versa). This will be useful to make country roads (that lead from cabins and farms).
  • Minor code improvements.

@codemime codemime force-pushed the codemime:omap_over_n9 branch to d5ed8b6 Aug 13, 2017

@codemime codemime referenced this pull request Aug 13, 2017

Closed

JSON API changes #19376

@vache

This comment has been minimized.

Copy link
Contributor

commented Aug 14, 2017

In the first example, the roads on the left side of the river don't connect to the right side at all, except to dead end at specials. May be outside the scope of this PR, but they should probably always try to connect across the dividing river.

@codemime

This comment has been minimized.

Copy link
Member Author

commented Aug 14, 2017

They probably do connect (down there, beyond the lower edge of the screen). However, connecting dead ends that are close enough to each other is a good idea.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Aug 15, 2017

The flag ALLOW_OVERRIDE was, in turn, removed. There's no need for it anymore: the connections themselves "know" which locations they can/cannot override.

Does it cover all cases?
For example, "ranch_camp_77" loses the flag, but it doesn't seem to be in any of the groups.

@@ -1229,6 +1238,11 @@ oter_id &overmap::ter(const int x, const int y, const int z)
return layer[z + OVERMAP_DEPTH].terrain[x][y];
}

oter_id &overmap::ter( const tripoint &p )

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Aug 15, 2017

Contributor

Are you using it as non-const? Would be better to scrap the non-const getters and use setters instead, to make more proper encapsulation possible.

This comment has been minimized.

Copy link
@codemime

codemime Aug 15, 2017

Author Member

Agreed, but I didn't invent this approach here, merely added an overloaded version of the function. I guess, references could improve performance under certain conditions, since both getting and setting the value perform the same steps which involve checking bounds and calculating indexes. However, the impact is probably unnoticeable.

@codemime

This comment has been minimized.

Copy link
Member Author

commented Aug 15, 2017

Does it cover all cases? For example, "ranch_camp_77" loses the flag, but it doesn't seem to be in any of the groups.

Good catch! I've missed that. Will fix.

@Coolthulhu Coolthulhu self-assigned this Aug 17, 2017

@Coolthulhu Coolthulhu merged commit 2cba5c9 into CleverRaven:master Aug 17, 2017

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.6%) to 22.196%
Details
gorgon-ghprb Build finished.
Details

@Coolthulhu Coolthulhu referenced this pull request Aug 17, 2017

Closed

Astyle-P #21613

@codemime codemime deleted the codemime:omap_over_n9 branch Aug 17, 2017

@ZhilkinSerg ZhilkinSerg referenced this pull request Sep 16, 2017

Closed

[WIP] Railroads #21792

8 of 12 tasks complete
@acidia

This comment has been minimized.

Copy link
Contributor

commented Sep 17, 2017

Having strange behavior in #21809, subway connections in specials are being connected with sewer lines instead. I think I tracked it down to overmap_connections::guess_for()... It is returning the first overmap_connection entry that is underground? I simply reversed the order of subway_tunnel and sewer_tunnel in the json and it started working as intended.

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.