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

New generic other answer: "At slightly wrong position" #3502

Closed
westnordost opened this issue Nov 12, 2021 · 23 comments
Closed

New generic other answer: "At slightly wrong position" #3502

westnordost opened this issue Nov 12, 2021 · 23 comments

Comments

@westnordost
Copy link
Member

westnordost commented Nov 12, 2021

Use case
I am being asked a question like:

  • bench backrest
  • hydrant type
  • hydrant diameter
  • whether X is still here
  • if a picnic table is covered
  • anything about a bicycle parking
  • etc. ... any small POI feature

and I can see that it is at a slightly wrong position by comparing my GPS trace and the location displayed on the map. I could leave a note, but it would be quicker if I could just move the node by a little within this app.

Proposed Solution
Technically similar as for split way, but much easier and much less code. Estimated code cost:

  • 200 LOC for UI (MoveNodeFragment.kt)
  • 10 LOC for the serializable data model (MoveNodeTo.kt)
  • 20 LOC for the (MoveNodeAction.kt) + 40 LOC test code
  • 40 LOC for adaptions in the quest controller and edits dao

So, about 300 LOC.

Opinions
Does this make sense? Is this worth the cost?

@peternewman
Copy link
Collaborator

Yes please! I often open notes like this. It would also stop the note blocking the further quests.

You'd need to implement #2376 too.

You'd need to either not allow it for nodes like AEDs on the side of a wall, or have some more complicated system to move them, compared to a standalone bin or bench.

It's also probably more accurate to just do it yourself onsite, especially if the satellite stuff was enabled (even if there was an offset you could do them both relatively.

Does this make sense? Is this worth the cost?

While it seems somewhat trivial if a bin and bench are the wrong way round, the whole point of a map is accuracy so improving that feels like it should be encouraged to me.

@mnalis
Copy link
Member

mnalis commented Nov 12, 2021

I personally would love it, as it would seem easier (and not requiring data network) than launching Vespucci at that coordinates (or leaving notes and post-processing later)

However, I am little worried that average SC user might move nodes wrongly (as they might not be aware of exact definition of GPS Dilution of Precision, what 68% confidence actually means, and why approximations that SC shows in form of transparent circle are often wrong and by how much, and why that can't really be fixed).
I.e. average SC user might think their GPS precision is good and move nodes around according to that, while their position was in fact more wrong then they realized.

I would still love it, but perhaps hide it behind Expert mode checkbox in settings #471? Or show stern warning on each use (like for answering quests far away from current GPS location)?

@FloEdelmann
Copy link
Member

See also #2586.

@westnordost
Copy link
Member Author

You have a point, @mnalis and let's see if we can address the issues why #2586 was closed:

You'd need to implement #2376 too.

I think a scale bar would not help a lot. Anyway, maybe instead of a scale bar, when moving the node, the distance one is about to move the node could be shown as a message on screen.

You'd need to either not allow it for nodes like AEDs on the side of a wall

This will be difficult. At the point the quest form is open, the quest form doesn't know the geometry of the surrounding, not even with #3480. It is known when the move-action is being executed (applying the change locally, uploading the change) however. So maybe AEDs, ATMs etc. that are a vertex on a way could be unglued from that way in that case. I.e. create a new node in that case and remove all tags from the old node.

moving still may be incorrect. Maybe shop has address tagged? Maybe at location B bin is already mapped? To make such edit you need to see full data :(

  1. the option should only be present for small point features, like park benches, bins, hydrants, trees etc. For other things, a note should be left instead
  2. With Show nearby related POIs #3480 merged, the user will actually be able to see the "full data", or at least the part that matters for the question. So, the user knows his own (approximate) location, sees a scale (see above), sees a (simplified) map and I think this is as good as it gets. If he instead left a photo note, I doubt that a desktop user would be able to pinpoint the corrected location more precisely.

average SC user might think their GPS precision is good and move nodes around according to that, while their position was in fact more wrong then they realized.

Just like with the dialog that pops up when splitting a way, a dialog could pop up for moving a node and this dialog includes mentioning that the user's current location precision is only an estimate that is accurate to X meters at 68% confidence.

@smichel17
Copy link
Member

smichel17 commented Nov 12, 2021

worried that average SC user might move nodes wrongly

When I edit on openstreetmap.org, and I go to upload my changes, iD presents a checkbox,

  • I would like someone to review my edits.

I'm not sure exactly what its effect is; would it be useful for SC to do something similar? E.g. put the moved object in its own changeset, and flag it as asking for review.

Aside: this might also be a good idea for the answer which removes a node, or other potentially-destructive edits (I don't know if there are any others at the moment; removing keys on a vacant shop used to be, before #3278).

@smichel17
Copy link
Member

I think a scale bar would not help a lot. Anyway, maybe instead of a scale bar, when moving the node, the distance one is about to move the node could be shown as a message on screen.

I can imagine something like a scale bar being useful. However, I agree that such an indicator should only be shown when moving the node. This would address the screen space concern from #2376 (comment), since it is only shown when needed.

I would want to experiment with what the indicator should look like, but I'm imagining something like a bullseye or crosshairs. Not necessarily this exact pattern, but something like:

Compared to showing a number that says how far the object has been moved, this lets you quickly tell the distance to multiple objects — both the original location and others — without having to rotate the screen.

@matkoniecz
Copy link
Member

matkoniecz commented Nov 12, 2021

iD presents a checkbox,
I would like someone to review my edits.
I'm not sure exactly what its effect is

approximately none, it adds tag on changeset but number of people reviewing specifically such changeset is low

also, it is not really possible to remotely review accuracy of shop moving except extreme cases. SC edit should be OK without need for external help here (but it is not necessary to be 100% accurate).

You'd need to implement #2376 too.

Why? At least I cannot really distinguish 15 from 20 or 120 from 150 meters in real life, so scale bar would not help at all.

@matkoniecz
Copy link
Member

matkoniecz commented Nov 12, 2021

but it would be quicker if I could just move the node by a little within this app.

How would you deal with objects attached to ways?

For example ATM as vertex of building way, bump that is within road line, pole that is within power line etc.

I can imagine cases where

  • node should be detached
  • node should be detached moved and reattached
  • node should be moved while being within line
  • node should be moved while being within line and other nearby nodes should be deleted
  • node should become detached from one line and become embedded in another
  • node should be detached from line, new one should be placed at the same position (or tags copied to new node)
  • node and entire line should be moved

I.e. create a new node in that case and remove all tags from the old node.

That will fail at least for gates, traffic calming objects, power poles and entrances.

@westnordost
Copy link
Member Author

How would you deal with objects attached to ways?

Well let's say it is always unglued, i.e. clear all tags on old node, create new node at new position.

For which elements wouldn't that make sense? Maybe power poles, speed bumps, gates etc. - then maybe for those, don't show this option. I think one would usually not want to move power poles, speed bumps etc.

@matkoniecz
Copy link
Member

I think one would usually not want to move power poles, speed bumps etc.

these can be also misplaced or appear to be misplaced

@westnordost
Copy link
Member Author

Maybe so.

@smichel17
Copy link
Member

At least I cannot really distinguish 15 from 20 or 120 from 150 meters in real life, so scale bar would not help at all.

I agree, but a scale would help distinguish which of two distances is longer, which can be hard to do visually when they are not perfectly aligned.

@peternewman
Copy link
Collaborator

I think a scale bar would not help a lot. Anyway, maybe instead of a scale bar, when moving the node, the distance one is about to move the node could be shown as a message on screen.

Why? At least I cannot really distinguish 15 from 20 or 120 from 150 meters in real life, so scale bar would not help at all.

Because as I mentioned in #2376 without one if there is a lack of other reference (e.g. bench and bin on a straight path, I can't tell if I've moved it 2 or 20m down the path, as I don't know a comparable distance onscreen. Showing the distance I've moved it would help a bit, but if it only tells me after it's moved that's a bit clunky. Stopping it being moved more than e.g. 50m/throwing a warning if you do would probably be wise too.

Also I'm ~2m (as is a bench) and my paces are ~1m so 15 vs 20 is easy if I want to make the effort, 120 to 150 takes a bit more walking. Obviously there's more error on longer distances.

I can imagine something like a scale bar being useful. However, I agree that such an indicator should only be shown when moving the node.

TBH as mentioned in #2376 being able to see it would also help for existence, is it that thing, or another one not on the map.

2. With [Show nearby related POIs #3480](https://github.com/streetcomplete/StreetComplete/pull/3480) merged, the user will actually be able to see the "full data", or at least the part that matters for the question.

Won't #3480 just show me the other bins when I'm moving one? I need to see the bench too, to ensure I move it the other side of it.

That will fail at least for gates, traffic calming objects, power poles and entrances.

And AEDs and ATMs and some post boxes and anything else that's in a wall.

When you're moving it, will it know at that point it was on a line? Could it snap to nearby ones to ensure it stayed attached?

@westnordost
Copy link
Member Author

westnordost commented Nov 12, 2021

Regarding the UI, this would be my idea. The blue arrow always points from original point to center of screen:

image

To measure distances to compare, you can just move the screen in any direction to measure "other" distances.

@smichel17
Copy link
Member

smichel17 commented Nov 12, 2021

To answer the original question,

Does this make sense? Is this worth the cost?

Yes and yes, but I don't think the benefit for the effort is as high as #124, #1913, or #3123
edit: although I do think the required refactoring, to give the quest form access to nearby map data, is good benefit for effort.

@westnordost
Copy link
Member Author

Won't #3480 just show me the other bins when I'm moving one? I need to see the bench too, to ensure I move it the other side of it.

Yes, it just shows the other benches.

@westnordost
Copy link
Member Author

And AEDs and ATMs and some post boxes and anything else that's in a wall.

When you're moving it, will it know at that point it was on a line? Could it snap to nearby ones to ensure it stayed attached?

Only because something is "iin a wall" doesn't mean that it is incorrect to have the feature not as a vertex of a way. I.e. for an atm in a wall, I think it is just as fine if the atm is a free floating node that appears just in front of the wall. Same for AEDs. If the node was at a wrong position before, it is an improvement at least.

@peternewman
Copy link
Collaborator

Won't #3480 just show me the other bins when I'm moving one? I need to see the bench too, to ensure I move it the other side of it.

Yes, it just shows the other benches.

We can't make the other known amenities appear as we're dragging it around can we, or would that be too laggy? I find stuff often wants to be placed relative to other bits, e.g. it's actually between the post box and the cycle rack.

@westnordost
Copy link
Member Author

True, but not really. This would require for each type of thing moved to have a custom filter what exactly to show. This would explode the effort estimate.

@smichel17
Copy link
Member

smichel17 commented Nov 12, 2021

Could you share the filters with #3480?

@mnalis
Copy link
Member

mnalis commented Nov 12, 2021

How would you deal with objects attached to ways?

Well let's say it is always unglued, i.e. clear all tags on old node, create new node at new position.

Hmmm, as I general rule, I quite dislike when stuff like that happens, as it makes it much more difficult to find out what went wrong, as the history of node is lost. I'd really much prefer SC always keeps the node history.

For which elements wouldn't that make sense? Maybe power poles, speed bumps, gates etc. - then maybe for those, don't show this option. I think one would usually not want to move power poles, speed bumps etc.

Or maybe, just don't show the option for tagged nodes that are part of the ways or relations? They should be in minority so gain of allowing relocating them is smaller, but the chance of messing up stuff there is quite a lot higher.

Alternatively....

Alternatively, if we're dead set on also allowing to move tagged nodes which are part of ways/relations, we should only whitelist nodes which contain only tags we've whitelisted (ie. not just checking whether the node has amenity=wastebasket, but checking that ALL tags it have are whitelisted -- otherwise it might happen same node has both amenity=wastebasket and power=pole, and we'd mess up power transmission network etc.). But that looks like an overkill to me, so perhaps just skipping such connected nodes is best - at least for initial version.

@westnordost
Copy link
Member Author

I'd really much prefer SC always keeps the node history.

This is possible, but would involve adding the new node to the way in place of the old node. And also, updating all the relations the node was part of etc. It would be considerably more difficult.

Or maybe, just don't show the option for tagged nodes that are part of the ways or relations?

Not possible. An individual node doesn't know if it is part of a way or relation.

With all the points raised here, I fear implementing this action would just involve opening another can of worms (of feature wishes):

  • special behavior if the node is part of a way or relation
  • and/or mantaining a list of whitelisted tags for moving
  • showing other elements in context
  • etc

So at this point, I think this is not worth the (my) time anymore.

@westnordost westnordost added the needs PR accepted suggestion, but only if a contributor implements it label Nov 18, 2021
@RubenKelevra
Copy link
Contributor

RubenKelevra commented Nov 19, 2021

@peternewman wrote:

However, I am little worried that average SC user might move nodes wrongly

Hm yeah, I see your point and a possible solutions:

FIXME nodes

Adding a new point for the POI (without the tags) with just a FIXME telling other users which POI it is related to.

Advantages:

  • The SC user will be recognised in the history of the new point.
  • This would be a 1 second operation in JOSM to melt both points into one, without fiddling with notes, closing them, copying a lat/lot etc.
  • This would work with nodes inside a way, like a vending machine, and also with areas, like a shop tagged on the building outlines.
  • There's no conflict with existing data, so uploading is possible even if the original feature gets modified.

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

No branches or pull requests

8 participants