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

Add Shelter type quest #473

Merged
merged 20 commits into from
Nov 1, 2023
Merged

Add Shelter type quest #473

merged 20 commits into from
Nov 1, 2023

Conversation

mcliquid
Copy link

@Helium314 I'm bringing here my old shelter_type quest. See:
Old PR: streetcomplete#4428
Old Issue: streetcomplete#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
Copy link
Author

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

@HolgerJeromin
Copy link

HolgerJeromin commented Oct 28, 2023

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

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

@mcliquid
Copy link
Author

Does it work?

@Helium314
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

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"

  • public_transport (Transit Shelter / Überdachter Wartebereich)
  • picnic_shelter (Picnic Shelter / Überdachte Picknickmöglichkeit)
  • lean_to (Lean-To / offene Schutzhütte)
  • gazebo (Gazebo / Aussichtspavillon)

JOSM Preset / Translations

  • basic_hut (Basic Hut / Biwakschachtel / Einfache Hütte)
  • gazebo (Gazbo / Aussichtstürmchen (Gazebo) / Aussichtspavillon)
  • lean_to (Lean-To / offene Schutzhütte / Schutzhütte mit typischerweise drei Wänden)
  • picnic_shelter (Picnic Shelter / Picknickhütte/ Picknickplatzdach)
  • public_transport (Öffentlicher Nahverkehr)
  • weather_shelter (Weather Shelter / Wetterschutz)
  • wildlife_hide (Wildtierschutz)
  • sun_shelter (Sun Shelter / Sonnenschutz)
  • rock_shelter (Rock Shelter / Felsvorsprung)
  • field_shelter (Field Shelter / Weidezelt)
  • animal_shelter (Animal Shelter / Tierheim)

Learnings

  • Neither JOSM nor iD use pavilion as a value
  • lean_to is really just a minimal roofing, which is specifically designed for overnight stays, but compared to the Basic Hut, it is not completely enclosed
  • For the rest, I would say it is a matter of taste which value you take. But in case of doubt I would leave the power to the people to handle respectfully with SCEE right?

@mcliquid
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

I've tried but failed 😢

@Helium314
Copy link
Owner

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

@mcliquid
Copy link
Author

mcliquid commented Nov 1, 2023

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

@Helium314
Copy link
Owner

Helium314 commented Nov 1, 2023

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
Copy link
Author

mcliquid commented Nov 1, 2023

Thanks! All done 👍

@Helium314
Copy link
Owner

Thanks 👍

@Helium314 Helium314 merged commit 36debbf into Helium314:modified Nov 1, 2023
@mcliquid mcliquid deleted the shelter_type_quest branch November 1, 2023 10:08
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.

5 participants