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

exclude ruined buildings from quests #1543

Merged
merged 1 commit into from Aug 29, 2019

Conversation

matkoniecz
Copy link
Member

Such buildings are often not really answerable and value of answers will be low. Large number of questions will end in failure to reply.

Some ruins were built was reasons that are not clear now, a roof may be caved in making the roof shape question unanswerable (and not really fixable by tagging it), building levels are weird at the best (is 0 correct in case of completely missing roof) etc.

Though I admit that the real reason is that at least in some places many object could have historic=* tag (that is already filtering out objects in building type quest), but historic key is untagged. Many of such objects have ruins=yes.

fixes #1541

Such buildings are often not really answerable and value of answers will be low. Large number of questions will end in failure to reply.

Some ruins were built was reasons that are not clear now, a roof may be caved in making the roof shape question unanswerable (and not really fixable by tagging it), building levels are weird at the best (is 0 correct in case of completely missing roof) etc.

Though I admit that the real reason is that at least in some places many object could have historic=* tag (that is already filtering out objects in building type quest), but historic key is untagged. Many of such objects have ruins=yes.

fixes streetcomplete#1541
@@ -17,7 +17,7 @@ class AddBuildingLevels(o: OverpassMapDataDao) : SimpleOverpassQuestType<Buildin
"commercial","office","warehouse","industrial","manufacture","parking","farm",
"farm_auxiliary","barn","cabin").joinToString("|") + "\n" +
" and !building:levels and !height and !building:height\n " +
" and !man_made and location!=underground\n "
" and !man_made and location!=underground and ruins!=yes\n "
Copy link
Member

Choose a reason for hiding this comment

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

Why not this?

Copy link
Member Author

Choose a reason for hiding this comment

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

https://www.openstreetmap.org/way/131358967 is an extreme example and one of things that prompted my patch eliminating ruins=yes - just wall part remaining from something that was ancient marketplace/mall and SC will now happily create quest about it.

And what worse there is no tagging that will get rid of it, in this case even adding historic=* will not help and given no access to it is not viable to tag with height (maybe there is some official data/3D model on a suitable licence?).

But even without such ridiculous cases - there are often building in advanced decay where floor count is far from obvious and wasting time on tagging them is likely to be pointless.

EDIT: I fully agree with "I'd say that floor count is the current floor count, not one that might have been one time.".

Copy link
Member

Choose a reason for hiding this comment

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

I'd say that floor count is the current floor count, not one that might have been one time.

But looking at the site on google maps 3d, I understand the problem.

Basically, I'd say the tag is wrong - it is not a building. It is a piece of wall. But I guess there are no correct tags for this maybe? Anyway, I rest my case. Let's exclude ruins from these quests.

@westnordost westnordost merged commit f4e4f65 into streetcomplete:master Aug 29, 2019
@matkoniecz matkoniecz deleted the ruins branch August 29, 2019 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exclude ruins=yes - from address and building quest
2 participants