robot background passthrough #1672
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kostmo
force-pushed
the
feature/terrain-as-background
branch
from
9af3d35 to
6402249
Compare
kostmo
force-pushed
the
feature/robot-background-passthrough
branch
from
f74bace to
fcd9a75
Compare
mergify bot
pushed a commit
that referenced
this pull request
Dec 12, 2023
Towards #1662 Prerequisite for #1672 To allow a robot-occupied cell to take on the underlying terrain color as the background color of its cell, the terrain color must be representable strictly as a "background". However, currently, the `dirt`, `stone`, and `grass` terrains are represented by a half-shaded **foreground** glyph upon `black` background. Currently the ["Medium Shade" unicode character](https://www.compart.com/en/unicode/U+2592) (`▒`) is used to "blend" a somewhat bright color with a black background, resulting in a moderately dark color in the terminal for `dirt`, `stone`, and `grass`. However, these same dark colors are not reproducible in the 240-color scheme without this foreground+background blending trick; the closest approximations as a background-only or foreground-only color come out quite a bit lighter. ## Visual comparison Using: scripts/play.sh -i creative --seed 2 --autoplay | Before | After | | --- | --- | |  |  | ## Possible approaches So, we need to decide whether to: 1. Accept the new lighter colors 2. Choose new, alternative terrain colors that may be darker given the 240-color palette 3. Abandon support for 240 colors and assume "full" color terminals 4. Abandon efforts to passthrough terrain color as background of robot cells
kostmo
force-pushed
the
feature/robot-background-passthrough
branch
from
fcd9a75 to
673eeb9
Compare
kostmo
force-pushed
the
feature/robot-background-passthrough
branch
from
673eeb9 to
44af2f6
Compare
kostmo
force-pushed
the
feature/robot-background-passthrough
branch
5 times, most recently
from
c83db52 to
2bb17a2
Compare
kostmo
force-pushed
the
feature/robot-background-passthrough
branch
from
2bb17a2 to
07fcc24
Compare
byorgey
reviewed
Dec 19, 2023
There was a problem hiding this comment.
I am not sure why, but with this PR it seems like all robots now look like Ω. (Edited to add: just kidding, it seems like that is already the case on main. I guess we did not test some previous PR enough... filed #1693 to track this issue.)
What is the advantage of doing this only for robots (as opposed to for entities as well)? It seems like we needed to add various special cases (displayObscured, etc.) to get it to work only for robots, rather than just generically changing the way Displays combine.
mergify bot
pushed a commit
that referenced
this pull request
Jan 5, 2024
YAML syntax in this file was actually fixed in #1672, which is not yet merged. Cherry-pick that fix as well as enhancements to the scenario. Towards #845. Aside from fixing the syntax for #1706 (comment), this provides a good "before" example to showcase the fix in #1672. scripts/run-tests.sh --test-arguments '--pattern "1034-custom-attributes"' and scripts/play.sh -i data/scenarios/Testing/1034-custom-attributes.yaml --autoplay --speed 1 
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #1662
Demo
Using:
Also: