Helium314 / SCEE

OpenStreetMap surveyor app for experienced OSM contributors
GNU General Public License v3.0
141 stars 10 forks source link

Add Shelter type quest #473

Closed mcliquid closed 11 months ago

mcliquid commented 11 months ago

@Helium314 I'm bringing here my old shelter_type quest. See: Old PR: https://github.com/streetcomplete/StreetComplete/pull/4428 Old Issue: https://github.com/streetcomplete/StreetComplete/issues/4418

The reason for rejection at that time was that an ordinary SC user could hardly distinguish the images. I hope this is different in SCEE. What do you think?

mcliquid commented 11 months ago

Damn, I should probably not update branches in between. Is it bad that the last three commits don't belong?

HolgerJeromin commented 11 months ago

Is it bad that the last three commits don't belong?

With git rebase -i HEAD~5 you can drop them.

mcliquid commented 11 months ago

Does it work?

Helium314 commented 11 months ago

In some cases it's not so clear. The difference between gazebo and picnic shelter is just whether some sort of (picnic) table is inside? The lean-to wouldn't be clear to me without reading the wiki. I would have tagged some of the examples from the wiki as weather shelter when just looking at the images. Possibly some short text mentioning the 3 walls would help. Weather shelter seems to be some sort of generic type that may fit for most other types. Not sure, but maybe it should only be selectable via other answers to discourage users from tagging everything as weather shelter.

Helium314 commented 11 months ago

Does it work?

No, there are still other changes in the PR. I don't know enough about git to tell you how to properly sort this out...

mcliquid commented 11 months ago

I've looked around in JOSM and iD which options they will give you with naming in english / german:

iD (via search)

In general, iD will call a amenity=shelter without sub-tag a "Shelter / Wetterschutz"

JOSM Preset / Translations

Learnings

mcliquid commented 11 months ago

I've added a individual warning message: image

I tried to enlarge the titles but than it's to long and you can't read the text properly / can't see the image. I could rebuild the whole quest (like smoothness) so you could add a description too, but with 10 values that's quite a lot to scroll / read. What do you think?

Helium314 commented 11 months ago

I think it's ok, let's keep the current style.

Could you move the weather shelter into an other answer? It really seems to be a rather generic type, and I don't want users to choose this over more specific types.

Then there is the issue with the unrelated commits... that should be sorted out.

mcliquid commented 11 months ago

I've tried but failed 😢

Helium314 commented 11 months ago

You could revert those commits, so at least the changes are not part of this PR.

mcliquid commented 11 months ago

@Helium314 Reverting seems to work. And removed pavilion assets because unused.

Helium314 commented 11 months ago

I added the weather_shelter as an other answer, in a simpler way as suggested above, I hope you're ok with that.

Could you re-check the additions in authors.txt? Several link go to different images. Also the weather shelter image could be removed.

mcliquid commented 11 months ago

Thanks! All done 👍

Helium314 commented 11 months ago

Thanks 👍