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

Render contiguous boundaries #1285

Merged
merged 1 commit into from
Sep 19, 2024
Merged

Render contiguous boundaries #1285

merged 1 commit into from
Sep 19, 2024

Conversation

kostmo
Copy link
Member

@kostmo kostmo commented May 28, 2023

Closes #1271

Demo

scripts/play.sh --scenario data/scenarios/Testing/1271-wall-boundaries.yaml --autoplay

Screenshot from 2024-09-18 09-27-14

Implementation

This implements neighbor-aware boundary rendering strictly on the display layer. Underlying entities remain unchanged (for the purpose of world cell contents and e.g. scan). Eventually we should be able to retire all of the niche boundary glyph entities.

Only a single function getBoundaryDisplay is exposed for computing the glyph appropriate for adjacent boundaries. It takes a function from AbsoluteDir -> Bool indicating whether the neighbor cell in a particular direction is (also) a "boundary" entity.

The internal glyphForNeighbors function is implemented with pattern-matching such that exhaustive coverage of cases is guaranteed.

@kostmo kostmo force-pushed the feature/wall-boundaries branch 4 times, most recently from 4646cb0 to 42eb3fd Compare September 18, 2024 06:47
@kostmo kostmo changed the title [WIP] Render contiguous boundaries Render contiguous boundaries Sep 18, 2024
@kostmo kostmo force-pushed the feature/wall-boundaries branch 4 times, most recently from 96a06ae to 118e874 Compare September 18, 2024 16:39
@kostmo kostmo marked this pull request as ready for review September 18, 2024 16:46
Copy link
Member

@xsebek xsebek left a comment

Choose a reason for hiding this comment

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

@kostmo Looks good! 👍

I was initially put off by having the variants hardcoded, but on further thought this simple approach makes sense.

The PR description confused me though, is the discussion up to date?

Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

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

Looks good to me, though I can't help thinking that the complication with the orientationMap necessitates adding yet more complication in the form of a boundaryOverride field in a Display.

This is just speculation for a future PR... what if a Display contained just a single Char, and an "orientation map" was stored at a higher level as a function from direction to Display? Likewise, then Display wouldn't have to know anything about boundary overrides --- if you wanted to show a boundary you would just take the Display you got and replace its Char with the appropriate box drawing character. [The reason we can't do that now is that you can't just "replace a Display's character" since it might store a whole orientation map worth of different characters...]

@kostmo kostmo added the merge me Trigger the merge process of the Pull request. label Sep 19, 2024
@mergify mergify bot merged commit 99ab1ce into main Sep 19, 2024
14 checks passed
@mergify mergify bot deleted the feature/wall-boundaries branch September 19, 2024 03:25
@kostmo kostmo mentioned this pull request Sep 23, 2024
mergify bot pushed a commit that referenced this pull request Sep 23, 2024
Makes use of #1285 to simplify wall rendering in several scenarios.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Draw annotated contiguous boundaries with proper wall glyphs
3 participants