SCEE icon indicating copy to clipboard operation
SCEE copied to clipboard

Ask if benches have armrests (StreetComplete #5664)

Open rYR79435 opened this issue 4 months ago • 12 comments

Can we add the rejected armrest request?

rYR79435 avatar Aug 09 '25 15:08 rYR79435

Can we add the rejected armrest request?

I'd say Yes... If someone is interested in doing it, it would be mostly a copy/paste of existing quest "Does this bench have a backrest". Instructions are here.

I'm willing to help extensively to anyone willing to give it a try (the exercise would also help to clarify the documentation if instructions there are unclear).

No programming experience is required, nor any special programs - one can just use web browser and do some clicking on GitHub website, copy/pasting and changing few strings. Are you perhaps interested in giving it a try, @rYR79435 ?

mnalis avatar Aug 09 '25 19:08 mnalis

if OP isn't willing/interested, maybe I could find some time to have a crack at this 👀 have (also) been mapping the armrests via direct tag editing for the time being, slightly tedious that way

edge case to account for: benches with only one armrest - I guess as per the OSM wiki article it'll (ideally?) be a case of armrest=yes and wheelchair=yes for that option

newplaylist47 avatar Aug 13 '25 19:08 newplaylist47

I was gonna do it eventually, if no one had done it by then, but I don't have too much time lately.. Feel free to do it 😄

Regarding the edge case, the Armrest article itself list a bench with only one armrest as armrest=yes without any wheelchair.. so no edge case I guess.

rYR79435 avatar Aug 13 '25 19:08 rYR79435

Can we add the https://github.com/streetcomplete/StreetComplete/issues/5664?

Sure, would be fine.

Note that I decided to pause development for a while, as I had some issues merging upstream and want to wait until the waiting large changes to SC are done (at least MapLibre Compose). So PR may sit around for some time.

Helium314 avatar Aug 15 '25 16:08 Helium314

if OP isn't willing/interested, maybe I could find some time to have a crack at this 👀 have (also) been mapping the armrests via direct tag editing for the time being, slightly tedious that way

@newplaylist47 OK, it's yours! Let me know if you get stuck or needs help with anything, or if anything is unclear in instructions for contributing a new quests !

edge case to account for: benches with only one armrest - I guess as per the OSM wiki article it'll (ideally?) be a case of armrest=yes and wheelchair=yes for that option

Yes, I would just ignore wheelchairs in that new armchair quest. If SCEE mapper wants to map which benches are wheelchair accessible, they can change element selection for green current wheelchair quest....

- leisure=dog_park
+ (leisure=dog_park or amenity=bench)

mnalis avatar Aug 16 '25 04:08 mnalis

@Helium314 fair enough - no rush!

re wheelchair accessibility: went for just duplicating the backrest quest for this for the time being then, stubbornly via the web interface - can tweak later on if there's any demand for any additional options

@mnalis some things to point out re the new quest instructions:

  • "visit https://github.com/streetcomplete/StreetComplete and press the "fork" button on the top right" - pretty sure it's SCEE we should be forking in this case? :p though should be obvious for most, but just to be clear
  • all the links to main-folder files/folders are broken currently (maybe you're aware of this, maybe not)
    • I'm presuming the "app/src/main/java/" path in the links should be "app/src/androidMain/kotlin/" now?
    • the XML file linked in the "Locating a quest" section seems to be here now?
  • "Add the quest to the list of active ones" section - the example-step link is a little outdated (numberless), but this is more a StreetComplete issue because of it being their demo repo; might also wanna mention something about the SCEE-specific "quests added in SCEE" part in QuestsModule.kt in the instructions

anyway - linking my fork's bench armrest quest files and modified QuestsModule file (lines 37 and 615), for looking at

newplaylist47 avatar Aug 16 '25 15:08 newplaylist47

I'm presuming the "app/src/main/java/" path in the links should be "app/src/androidMain/kotlin/" now?

It's currently split into app/src/androidMain/ and app/src/commonMain/. There's certainly a lot that will move from android to common after merging upstream next time.

Helium314 avatar Aug 18 '25 15:08 Helium314

@mnalis some things to point out re the new quest instructions:

Thanks @newplaylist47, I'll keep it in mind to update those docs after we merge upstream where a lot things changed. It is great that you're going over them as new pair of eyes will catch all those details which people already familiar with SCEE code will overlook!

  • "visit https://github.com/streetcomplete/StreetComplete and press the "fork" button on the top right" - pretty sure it's SCEE we should be forking in this case? :p though should be obvious for most, but just to be clear

Yeah, it would be useful... But changing all the URLs will make any changes in the future harder to merge. Maybe we should just add a preamble saying something like "of course change all https://github.com/streetcomplete/StreetComplete to https://github.com/Helium313/SCEE" ? That should be much easier to manage. 🤷‍♂️

  • all the links to main-folder files/folders are broken currently (maybe you're aware of this, maybe not)
  • I'm presuming the "app/src/main/java/" path in the links should be "app/src/androidMain/kotlin/" now?

Thanks, known issues. We need to wait with merges, see e.g. https://github.com/Helium314/SCEE/pull/823#issuecomment-3116846255

  • the XML file linked in the "Locating a quest" section seems to be here now?

yeah, wherever that values/strings.xml ends up being - it is being at the moment changed. But there should always be only one strings.xml in values/ directory at any point in time.

Relatedly -- for adding any new SCEE-specific strings (i.e. strings which are not used in regular StreetComplete, which is basically all new strings if you are creating SCEE-only Quest), one should use values/strings_ee.xml instead. We should add that to docs too!

  • "Add the quest to the list of active ones" section - the example-step link is a little outdated (numberless), but this is more a StreetComplete issue because of it being their demo repo; might also wanna mention something about the SCEE-specific "quests added in SCEE" part in QuestsModule.kt in the instructions

Thanks, something should be done about instructions for that too! but that demo repo is problematic as you note. Perhaps also note that issue in the SCEE preamble until we have better solution...

anyway - linking my fork's bench armrest quest files and modified QuestsModule file (lines 37 and 615), for looking at

Thanks, feel free to create a (draft) PR against SCEE modified branch, it should be much easier to review that way, and we'll automatically see all the changes and any new updates


You'll also need to define English strings you've created (in values/strings_ee.xml file) e.g. quest_bench_armrest_title

Then, you can click on GitHub to see if it manages to compile the .apk

if not, link the failing GitHub action here, and we can look in errors to find out what was wrong and how to fix it.

And if it compiles, congrats!! Then you'll have an debug version of .apk which you can download (you need to be logged-in to GitHub for download link to work) and install on your phone (it installs separately alongside regular SCEE, so no worries about them mixing up), and test (answering quest and clicking undo button to see what was tagged) if it works as you intended

mnalis avatar Aug 19 '25 04:08 mnalis

understandable re the StreetComplete-merge stuff, a preamble seems like it'd be an easier way to go by things indeed

I was wondering about the strings xml file, thank you for clarifying! will get around to all that once I get some time to sit down with this. On Tuesday, August 19th, 2025 at 07:07, Matija Nalis @.***> wrote:

mnalis left a comment (Helium314/SCEE#834)

@.***(https://github.com/mnalis) some things to point out re the new quest instructions:

Thanks @.***(https://github.com/newplaylist47), I'll keep it in mind to update those docs after we merge upstream where a lot things changed. It is great that you're going over them as new pair of eyes will catch all those details which people already familiar with SCEE code will overlook!

  • "visit https://github.com/streetcomplete/StreetComplete and press the "fork" button on the top right" - pretty sure it's SCEE we should be forking in this case? :p though should be obvious for most, but just to be clear

Yeah, it would be useful... But changing all the URLs will make any changes in the future harder to merge. Maybe we should just add a preamble saying something like "of course change all https://github.com/streetcomplete/StreetComplete to https://github.com/Helium313/SCEE" ? That should be much easier to manage. 🤷‍♂️

  • all the links to main-folder files/folders are broken currently (maybe you're aware of this, maybe not)
  • I'm presuming the "app/src/main/java/" path in the links should be "app/src/androidMain/kotlin/" now?

Thanks, known issues. We need to wait with merges, see e.g. #823 (comment)

  • the XML file linked in the "Locating a quest" section seems to be here now?

yeah, wherever that values/strings.xml ends up being - it is being at the moment changed. But there should always be only one strings.xml in values/ directory at any point in time.

Relatedly -- for adding any new SCEE-specific strings (i.e. strings which are not used in regular StreetComplete, which is basically all new strings if you are creating SCEE-only Quest), one should use values/strings_ee.xml instead. We should add that to docs too!

  • "Add the quest to the list of active ones" section - the example-step link is a little outdated (numberless), but this is more a StreetComplete issue because of it being their demo repo; might also wanna mention something about the SCEE-specific "quests added in SCEE" part in QuestsModule.kt in the instructions

Thanks, something should be done about instructions for that too! but that demo repo is problematic as you note. Perhaps also note that issue in the SCEE preamble until we have better solution...

anyway - linking my fork's bench armrest quest files and modified QuestsModule file (lines 37 and 615), for looking at

Thanks, feel free to create a (draft) PR against SCEE modified branch, it should be much easier to review that way, and we'll automatically see all the changes and any new updates


You'll also need to define English strings you've created (in values/strings_ee.xml file) e.g. quest_bench_armrest_title

Then, you can click on GitHub to see if it manages to compile the .apk

if not, link the failing GitHub action here, and we can look in errors to find out what was wrong and how to fix it.

And if it compiles, congrats!! Then you'll have an debug version of .apk which you can download (you need to be logged-in to GitHub for download link to work) and install on your phone (it installs separately alongside regular SCEE, so no worries about them mixing up), and test (answering quest and clicking undo button to see what was tagged) if it works as you intended

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

newplaylist47 avatar Aug 19 '25 06:08 newplaylist47

okay - finally had some time to sit down with this properly. the debug APK thing worked perfectly, started a draft pull request.

added the quest string to the 'new quests' bit, couldn't figure out a better place for it for now (though maybe could've put it under the bench materials part and word its comment-heading more generally)

newplaylist47 avatar Sep 06 '25 10:09 newplaylist47

Thanks for your contribution ❤️ !

Note that we're currently in major code restructuring so new releases of SCEE are delayed, but it looks good to me! You can use debug APK to contribute just fine, until new release happens...


Do we have someone good with editing SVG to be able to modify icon to look slightly differently (i.e. to differentiate armrest quest from backrest)? It does not have to be an art masterpiece, but just to be different so it's not confusing.

mnalis avatar Sep 10 '25 00:09 mnalis

let this sit around for too long admittedly haha - I don't have a proper svg editor at hand and a bit too late in the night to want to make sense of the online editors atm, so a png mockup will do for now, but maybe something like this? couldn't figure out a way to make the backrest and armrest coexist well without any clashing

Image

newplaylist47 avatar Oct 19 '25 23:10 newplaylist47