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

Terrain connections for groups other than WALL #14874

Merged
merged 1 commit into from Jan 19, 2016

Conversation

Projects
None yet
3 participants
@hitbutton
Copy link
Contributor

commented Jan 17, 2016

Resolves #14585 by defining group connections similar to those provided by the flag "CONNECT_TO_WALL" for arbitrary groups of terrain.

This allows connection between the various legacy "_v" and "_h" varieties of fence, and also allows them to connect to their associated gates.

Chain fences, picket fences and railings will now all use wall symbols for ASCII players, with the associated corners and T-junctions, instead of static "-" and "|" symbols.

The reason that multiple connection groups are necessary is to prevent strange things like house walls having T-Junctions where they meet a picket fence.

A new optional value is added to terrain json definitions:
"connects_to" which currently accepts values of "WALL", "WOODFENCE", "CHAINFENCE", or "RAILING". Additional groups can be defined in mapdata.h / mapdata.cpp.

To retain backward compatibility, the flags "CONNECTS_TO_WALL" and "WALL" will continue to imply that terrain connects to the "WALL" group.

The flag "AUTO_WALL_SYMBOL" no longer implies WALL group membership, as its usage is no longer exclusive to the wall group. (All terrain that made prior use of this flag also has the "WALL" flag anyway).

I considered allowing terrain to belong to multiple groups (which would allow things like gates that connected with multiple kinds of fence, without causing those fences to connect) and dynamically defining the connection groups simply by them being stated in terrain JSON definitions. However both of those features seemed too performance intensive. Tens of thousands of terrain connection comparisons can happen every redraw, so I decided to err on the side of efficiency.

@sparr

This comment has been minimized.

Copy link
Member

commented Jan 18, 2016

Perhaps the connect groups and connect-to flags should be their own set of flags, not just normal flags?

@hitbutton

This comment has been minimized.

Copy link
Contributor Author

commented Jan 18, 2016

Connect groups are not flags. "connects_to" is a seperate JSON entry that takes a single string value.

Working with the old flags is simply for legacy purposes (e.g avoiding breaking mods, and allowing me to submit this PR without cluttering it too badly changing every single wall/door/window JSON entry in the base game and mods). In implementation, they just set the "connects_to" value when terrains are first initialised.

I did start to implement connect groups as a bitset, like flags, allowing terrain to belong to an arbitrary number of connect groups. However this would mean that every time a terrain checked if another piece of terrain connected with it, it would have to check every one of its connect groups against each connecting piece of terrain. For an operation that's performed so many times every screen draw, that was potentially far too expensive for little return.

Similarly, allowing arbitrary strings for connect groups like flags would be way too performance intensive. You could easily end up doing >14,000 string comparisons per screen draw. Which would be bad.

@hitbutton

This comment has been minimized.

Copy link
Contributor Author

commented Jan 18, 2016

Although I guess it could be possible to do a speedy bitset comparison that checks if any values are shared between the two bitsets. But it would still be less efficient than doing one int comparison, and I'm not sure there's really a strong use case for it.

@kevingranade kevingranade merged commit ababf2c into CleverRaven:master Jan 19, 2016

1 check passed

default
Details

@hitbutton hitbutton deleted the hitbutton:arbitrary_terrain_connections branch Jan 19, 2016

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.