Skip to content

AddLevelThing quest#6433

Merged
westnordost merged 19 commits into
streetcomplete:masterfrom
paulklie:thing_level
Sep 8, 2025
Merged

AddLevelThing quest#6433
westnordost merged 19 commits into
streetcomplete:masterfrom
paulklie:thing_level

Conversation

@paulklie

@paulklie paulklie commented Aug 9, 2025

Copy link
Copy Markdown
Collaborator

Fixes #6431

@paulklie paulklie marked this pull request as draft August 9, 2025 16:07
@paulklie paulklie marked this pull request as ready for review August 9, 2025 16:17
@westnordost

Copy link
Copy Markdown
Member

It would definitely be necessary to highlight other things, too. E.g. there might be a bench at level 1 and 2 at basically the same location as the one one is asked for. When one switches through the levels, those on 1 and 2 must be visible, otherwise the user will add the wrong level.

@westnordost

Copy link
Copy Markdown
Member

But it is anyway a good idea to also highlight shops, because this is necessary for orientation as the map itself is not helpful for indoor things.

@westnordost

Copy link
Copy Markdown
Member

Regarding the icon, could just be the same icon as for the place level quest, but with an amenity-green background.

@paulklie

Copy link
Copy Markdown
Collaborator Author

Regarding the icon, could just be the same icon as for the place level quest, but with an amenity-green background.

Aye, thats what I added.

@paulklie

Copy link
Copy Markdown
Collaborator Author

It would definitely be necessary to highlight other things, too. E.g. there might be a bench at level 1 and 2 at basically the same location as the one one is asked for

The quest currently does not ask about benches though

@westnordost

Copy link
Copy Markdown
Member

Hmm, I wonder if this could even be put into the AddLevel quest.

The question text would need to be adapted. And things should maybe only be highlighted if the element that is asked for is also a thing.

@paulklie

Copy link
Copy Markdown
Collaborator Author

Hmm, I wonder if this could even be put into the AddLevel quest.

I see a few issues with this:

  1. Icon colour: Things normaly use green and places yellow, which one do we use for the merged quest?
  2. Harder to disable: In busy malls with alot of micromapping, a user might like to disable the "things" quest since this can cause alot of workload. However if it is only a single quest they cannot chose to not disable the (likely to be less intense) places quest.

@westnordost

Copy link
Copy Markdown
Member

Right, I agree, especially on point 2.

@paulklie paulklie requested a review from westnordost August 20, 2025 12:31
@paulklie

Copy link
Copy Markdown
Collaborator Author

What do you think of also asking for things inside of shop=department_store?
These are mostly multi-level in my experience, and often have toilets or similar amenity's.

@paulklie

Copy link
Copy Markdown
Collaborator Author

It would definitely be necessary to highlight other things, too. E.g. there might be a bench at level 1 and 2 at basically the same location as the one one is asked for. When one switches through the levels, those on 1 and 2 must be visible, otherwise the user will add the wrong level.

Done

@westnordost

Copy link
Copy Markdown
Member

What do you think of also asking for things inside of shop=department_store?
These are mostly multi-level in my experience, and often have toilets or similar amenity's.

Other than toilets, I've no idea what one would want to map in a department store. I don't expect department stores to be indoor-mapped for each level.

@paulklie

Copy link
Copy Markdown
Collaborator Author

What do you think of also asking for things inside of shop=department_store?
These are mostly multi-level in my experience, and often have toilets or similar amenity's.

Other than toilets, I've no idea what one would want to map in a department store. I don't expect department stores to be indoor-mapped for each level.

Hmm,I think aeds, first aid kits, phones, artwork, atms could all also be in department stores

@westnordost westnordost left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd find it cleaner if there was one abstract AAddLevelForm and both forms would inherit from that. You'll find this is also consistent with the rest of the codebase (, as I consider inheritance of concrete classes messy).

@paulklie paulklie requested a review from westnordost September 2, 2025 16:55
@paulklie paulklie requested a review from mnalis September 2, 2025 16:55

@westnordost westnordost left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good job! I found a few smallish things.

@riQQ riQQ added the new quest accepted new quest proposal (if marked as blocked, it may require upstream work first) label Sep 5, 2025
@westnordost westnordost merged commit 5fb93f7 into streetcomplete:master Sep 8, 2025
@matkoniecz

Copy link
Copy Markdown
Member

it seems that svg file was not committed:

Execution failed for task ':generateQuestList'.
> Could not find the SVG for icon 'level_thing' (quest AddLevelThing).

@paulklie

paulklie commented Sep 9, 2025

Copy link
Copy Markdown
Collaborator Author

it seems that svg file was not committed:

Execution failed for task ':generateQuestList'.
> Could not find the SVG for icon 'level_thing' (quest AddLevelThing).

I never created a .svg, did not know it was needed. I only modified the .xml from the level quest to change the colour.
Will have a look at creating one though now.

@westnordost

Copy link
Copy Markdown
Member

It's not needed for the app but for that task that helps some people keep the documentation in the OpenStreetMap wiki up-to-date with the app. Also, since the Android drawable resources are somewhat of a proprietary format, it makes sense to keep the vector resources around in a format that can actually be easily edited.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new quest accepted new quest proposal (if marked as blocked, it may require upstream work first)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Quest: On which level number is this thing located?

5 participants