background passthrough

[kostmo]

Closes #1662.

This is an attempt at an alternate approach from #1672.

scripts/play.sh --scenario data/scenarios/Testing/1034-custom-attributes.yaml --autoplay --speed 2 --hide-goal

Comparison

scripts/play.sh -i creative --seed 2

Before	After

Overview

This approach required threading an attribute map (aMap) through a lot of places. Fundamentally, it is because we need to arbitrate between an potentially entity-provided background color and a terrain-provided background color late in the rendering pipeline. We need to preserve whether that background originated from an Entity or a Terrain, so that we can prioritize the Entity.

However, we also want to let the Terrain win if we find that, after looking up the color of the Entity in the attribute map, the Entity has no provided background color.

Asciinema recordings

Before

After

[byorgey]

I can't yet put my finger on exactly why, but this seems unnecessarily complicated. It may be due to the fact that there is something inelegant about Display and it needs some refactoring. I will try to give it some thought.

[kostmo]

I can't yet put my finger on exactly why, but this seems unnecessarily complicated. It may be due to the fact that there is something inelegant about Display and it needs some refactoring. I will try to give it some thought.

I think it's related to the fact that terrains and entities are styled with "opaque" attributes, for which the separation into background and foreground colors is only known until after scenario parse time.

[kostmo]

Also I'd say that some of the complication is due to #2265.

[kostmo]

I will try to give it some thought.

@byorgey have you come up with anything? Would like to avoid incurring more rebase cost for this PR.

[byorgey]

I think the fundamental thing for me is that we shouldn't need to store extra information outside of Display in order to figure out how to draw things --- instead, we should redesign Display so that simply composing a stack of Display values is enough to know how to draw the result.

What if we change Display so it contains not just displayAttr :: Attribute but displayAttrFG :: Attribute and displayAttrBG :: Maybe Attribute? How do we currently handle background attributes?

Edit: more thoughts about a redesign for Display are here:

• Refactor Display to make it simpler/more robust #570 (comment)
• Render contiguous boundaries #1285 (review)

[kostmo]

I feel like the problem is less with Display and more with Attribute. Any Attribute can have both/either a foreground and a background color, stored together at one layer of indirection behind a string key. We may have to abandon the Attribute abstraction if we want the Display logic to be cleaner.

[byorgey]

I feel like the problem is less with Display and more with Attribute.

Yes, I think you're right. Though when you talk about "abandoning" the Attribute abstraction I guess I'm not sure what you mean, since I don't remember exactly how we use it. Can't we just have some attributes map to foreground + background, and some map to foreground only, and some to background only? It might require adding more attributes and changing which things use which attributes, but I'm not sure we need to abandon it altogether.

[kostmo]

Can't we just have some attributes map to foreground + background, and some map to foreground only, and some to background only?

That's a possibility, but with the current abstraction, it would rely on respecting a convention for which attributes are allowed to specify background/foreground/both. Ideally we would enforce by construction which layer an attribute pertains to.