New quest: Building colour

[hangy]

Based on the same pattern as #428, adds a building colour quest.

Notable changes/additions:

• RoofColourDisplayItem and BuildingColourDisplayItem use a new common base class FilteredDisplayItem to avoid repeating the colour matrix logic
• FilteredDisplayItem is modified from the the suggestion in New quest: Roof colour #428 (comment) to avoid dupes (equality + hashCode overrides)
• Therefore, moveFavoritesToFront is enabled for roof colours, again.

[hangy]

Note: I haven't found a good real-world case to test the colour on a building:part. Any suggestion where to find one? 😅

[Helium314]

I mostly find elevators tagged as building:parts, and not much else.
But I'll have a look when testing the quest.

[Helium314]

• FilteredDisplayItem is modified from the the suggestion in New quest: Roof colour #428 (comment) to avoid dupes (equality + hashCode overrides)

So you got it working, nice!

(I'll have a proper look at the PR later this weekend)

[Helium314]

Some more comments after a very quick test and looking for applicable building parts on overpass:

• the quest icon going all the way to the bottem looks a bit strange, I think it would be better to cut it off to have a straight bottom edge, or maybe just use the path data from building quest icon (building + shadow). But I have no strong opinion here, and leave that decision to you.
• the changeset comment should be in line with the roof colour quest (specify <-> add)
• the filter could be improved to exclude some cases where the quest is hard / impossible to answer
  • exclude indoor except indoor = no
  • exclude wall = no
  • exclude roofs
  • maybe exclude building:material=glass? Though there is colored glass...
• there are some super-detailed buildings with a lot of partially overlapping building parts, where it's not always clear which building part the quest is asking about
  • complicated filters could be used to exclude such situations, but that's likely not worth the effort and will also exclude many perfectly answerable cases
  • actually if the user looks at the tags (using the "show/edit tags" other answer), they should be able to identify which part exactly is meant in most cases. So I think leaving it as it is would be fine.
  • Edit: when testing I found such a case where there were 2 building parts on top of each other, I had to check tags to see that one of these was on top, indicated by min_level, but then the quest was perfectly answerable.

[hangy]

Sorry for the late response - I wasn't home during the weekend.

• the quest icon going all the way to the bottem looks a bit strange, I think it would be better to cut it off to have a straight bottom edge, or maybe just use the path data from building quest icon (building + shadow). But I have no strong opinion here, and leave that decision to you.

The height of ic_quest_building_colour should be the same as ic_quest_building_levels, and ic_quest_building_height, because it's based on the latter.

• the changeset comment should be in line with the roof colour quest (specify <-> add)

ACK'd & fixed!

• the filter could be improved to exclude some cases where the quest is hard / impossible to answer

  • exclude indoor except indoor = no
  • exclude wall = no
  • exclude roofs
  • maybe exclude building:material=glass? Though there is colored glass...

I didn't know about some of those combinations. Excluding most of them sounds like a good idea, but I'd like to be able to tag coloured glass.

• there are some super-detailed buildings with a lot of partially overlapping building parts, where it's not always clear which building part the quest is asking about

  • complicated filters could be used to exclude such situations, but that's likely not worth the effort and will also exclude many perfectly answerable cases
  • actually if the user looks at the tags (using the "show/edit tags" other answer), they should be able to identify which part exactly is meant in most cases. So I think leaving it as it is would be fine.
  • Edit: when testing I found such a case where there were 2 building parts on top of each other, I had to check tags to see that one of these was on top, indicated by min_level, but then the quest was perfectly answerable.

I think it would be also fine to skip this quest for too complicated buildings, if it's not easy to tag in a casual survey. It's gonna be okay for other cases. 😄

[Helium314]

Sorry for the late response - I wasn't home during the weekend.

No problem at all, we don't have a time schedule anyway

The height of ic_quest_building_colour should be the same as ic_quest_building_levels, and ic_quest_building_height, because it's based on the latter.

Alright, then let's leave it as it is.

I didn't know about some of those combinations. Excluding most of them sounds like a good idea, but I'd like to be able to tag coloured glass.

I only found out about wall=no when checking buildings that could cause issues... the vast majority of wall seems to be specifying that a building has no wall, instead of specifying the type of wall as suggested in the wiki.

I think it would be also fine to skip this quest for too complicated buildings, if it's not easy to tag in a casual survey. It's gonna be okay for other cases.

Currently I'm in favor of leaving the filter as it is:
detecting complicated building:part things will be more work (requires checking geometry of nearby building parts), and almost inevitably exclude answerable cases.
With the current way, people who want to avoid some potentially unanswerable quests can tune the filter using quest settings, e.g. remove building:part from applicable elements, or exclude everything with min_level.

If you don't intend to do any more changes, I'll merge the PR.

[hangy]

If you don't intend to do any more changes, I'll merge the PR.

I think this PR is good to merge as-is. As you explained, it's probably "good enough" for most cases. Thanks for your reviews. 😄