Changesets refactor #1329

westnordost · 2019-02-06T20:33:03Z

5 steps:

refactor the quest upload to be more easily extendable and more easily testable. (I am slowly losing the overview at 500+ LOC)
in the future, treat (substantial) changes to the geometry as unsolvable conflict and drop quest answer. For example:
- shop node has been moved hundreds of meters (shop moved or node was re-purposed by a mapper)
- road end(s) have been extended or merged with another road-piece
- ... (will need to think about further what changes on the geometry are still ok)
optionally release the changes done so far
implement the ability to split up a way at a certain position, including propagating the change to possible relations automatically
persisting split ways for offline usage (+undo?), interweave with quest system
Add UI for all or some way-based quests to cut a way into two pieces when choosing he other answer option "It differs along the way..."

)

# Conflicts: # app/build.gradle

westnordost · 2019-06-18T09:59:40Z

implement the ability to split up a way at a certain position, including propagating the change to possible relations automatically

all that work done already
tests for updating relations
special logic for splitting up closed ways (require 2 split points and merge ends)
tests for that special logic

app/src/main/java/de/westnordost/streetcomplete/data/osm/split/SplitWayUpload.kt

…Type

app/src/main/java/de/westnordost/streetcomplete/data/osm/split/SplitWayUpload.kt

app/src/main/java/de/westnordost/streetcomplete/data/osm/split/SplitWayAtPosition.kt

matkoniecz · 2019-07-05T21:24:06Z

One thing that seems to be missing - on splitting some closed ways one needs to create a multipolygon (on splitting say natural=water closed way).

Maybe it can be assumed that it will not happen in StreetComplete and is not worth implementing but may be worth noting it in comments that it is supposed to not happen.

westnordost · 2019-07-05T22:07:29Z

Multipolygon? Not sure what you mean. A closed way is just a closed way, if you split it up, you get two unclosed ways. That is the behavior of JOSM.

matkoniecz · 2019-07-05T22:12:52Z

Yes, but

there is closed way with natural=water
way is split in two natural=water ways

is incorrect (water area will disappear), and following is necessary:

there is closed way with natural=water
way is split in two natural=water ways
multipolygon relation is created, with this two ways as members
tags are removed from ways and added to relation

westnordost · 2019-07-05T22:22:50Z

Probably not a multipolygon but two separated closed ways. But anyway, splitting a way is quite a different operation from splitting an area. The way is just 1-dimensional, so it can be split in a point. The area is 2-dimensional, so the split itself is a line, or a polyline.

In OSM, there exists no data structure for an area. I wonder, does JOSM have such a function, splitting a closed way (=area) along a (poly)line?

I can imagine use cases in StreetComplete where such a function would be helpful, for example if one wants to split up a building, a farmland or so. I'll keep this in mind, but for now, this won't be part of the implementation. A simple split of a way is already complicated enough, also UI wise. Imagine the user would need to draw a polyline (on a smartphone screen) to split an area with.

westnordost · 2019-07-25T10:39:54Z

I'll probably be back from my "cave" this weekend, pushing a huge refactor of the whole upload system:

persisting split way uploads, integrating it into the upload service
pulling apart the different functionalities into separate classes that are also separately tested. For example, one class to upload a single quest answer change, one class to manage changesets, one class to update the diff-result of an upload, one class to coordinate the upload of all quest answers
not letting the undo osm quests inherit functionality from osm quests as certain fields (such as quest status, last update time etc) are irrelevant for it

westnordost · 2019-08-26T17:21:57Z

Second problem: I reopened the app and the star counter is zero again. (still not authorized)

OK this sounds like a bug. Will investigate. I also received feedback in the German forum, so probably a beta-Release needs to be postponed (it was initially planned to be this Wednesday).

I understand the problems. But the question is not solved. Offline User is probably confused why the question is not raised again after split.

Yes, they might be. I was hoping that the dialog shown before the split-way screen explains this good enough. Do you or anyone else have a suggestion how to make the wording clearer so that the user is not surprised why he can't answer the question anymore (until next upload)?

If I split a small part from a long way, it would be the best if the longest (as in node count) part would get the osm way id and the stub gets a new id from the server.

Yes, this is the case.

@HolgerJeromin Are you interested to test the cases I wrote about above from A-Z(, or some of them)?

westnordost · 2019-08-26T20:10:16Z

Second problem: I reopened the app and the star counter is zero again. (still not authorized)

Ok fixed

# Conflicts: # app/src/main/res/values/strings.xml # app/src/test/java/de/westnordost/streetcomplete/data/osm/download/OsmQuestDownloadTest.java

app/src/main/res/values/strings.xml

matkoniecz · 2019-08-29T19:09:29Z

app/src/main/res/values/strings.xml

+    <string name="quest_generic_answer_differs_along_the_way">"Differs along the way…"</string>
+    <string name="quest_split_way_description">If it differs along the way, the first step is to split up the way. After that, the quest can be answered for each split part separately.\nNote that the quests for the split parts may appear mach later.\nSplit it now?</string>
+    <string name="quest_split_way_tutorial">Tap on the way to split it at the point(s) where the answer differs. Try to be as precise as possible, you can zoom in as usual.</string>
+    <string name="quest_split_way_too_imprecise">"Please zoom in further"</string>


This appears also at the maximum possible zoom.

Hm. The message appears if you tapped multiple nodes at once with your finger. If the nodes are absurdedly close to each other, then it wouldn't be possible to split up the way.

I'd love to just set the maximum zoom further up. But The default maximum zoom in tangram-es is 20.5 and it cannot be turned up further (and only starting at the new tangram-es version that we do not use).

At zoom 20.5, the diameter of the tap of the finger is about 3 meters in our latitude and about 5 on the equator. So, that is about still 4 zoom levels away from achieving the OSM 7-decimals-precision.

So the best choice here is to instead of requiring to further zoom in at already that high of a zoom level, just choose the node that is closest to the center of the tap position.

Will work on it. Thank you!

Implemented it. Would you mind testing it again at the position where you discovered it?

@matkoniecz

Co-Authored-By: Mateusz Konieczny <[email protected]>

# Conflicts: # app/src/main/res/values/strings.xml

westnordost · 2019-09-01T15:15:29Z

While testing, I discovered two more problems:

on creation/commenting of each note, the app calls the upload-photos API, even if there are no photos to upload.
the (split way) upload does not correctly update the local copy of the uploaded way with the inserted nodes: The inserted nodes are still referred to with negative ids after the upload. This destroys any subsequent attempt to upload new versions of the way (i.e. answering further questions)

So, the release will be postponed further.

matkoniecz · 2019-09-01T15:55:45Z

Is the second error manifesting as de.westnordost.osmapi.common.errors.OsmBadUserInputException: Bad Request (400) - Placeholder node not found for reference -1 in way 713795301 crashes? Or is it a potential separate issue?

westnordost · 2019-09-01T18:01:30Z

Yes

HolgerJeromin · 2019-09-01T19:24:08Z

Yes, they might be. I was hoping that the dialog shown before the split-way screen explains this good enough. Do you or anyone else have a suggestion how to make the wording clearer so that the user is not surprised why he can't answer the question anymore (until next upload)?

The first dialog is forgotten. Another Toast after splitting "The quest you selected will be available after upload" would have helped me.

westnordost · 2019-09-01T21:27:06Z

Solution still WIP...

westnordost · 2019-09-04T20:36:15Z

Tests listed here complete. Further tests done:

split way which is member of multiple different relations
split way which is a member of an unknown relation
unsolvable conflict during split way upload

westnordost added 8 commits February 10, 2019 19:37

Merge branch 'master' into changesets-refactor

0736d29

Merge branch 'master' into changesets-refactor

35f0065

wip split way at...

9a423e0

more null-safety

c555503

slightly different wording for traffic signals button quest (fixes #1353

98d01db

)

update dependencies

8f34703

Merge branch 'master' into changesets-refactor

5da9898

# Conflicts: # app/build.gradle

work on split way upload

f2111a0

westnordost added 3 commits June 22, 2019 13:44

special handling for circular ways

1e8eeb5

add more tests

b63a4b6

add tests and fixes

d16aea1

matkoniecz reviewed Jun 24, 2019

View reviewed changes

app/src/main/java/de/westnordost/streetcomplete/data/osm/split/SplitWayUpload.kt Outdated Show resolved Hide resolved

westnordost added 3 commits June 25, 2019 00:00

treat role "from" and "to" with "via" the same regardless of relation…

666757a

…Type

solve the modification aware stuff in osmapi

490e529

update to new osmapi version

ed937a4

matkoniecz reviewed Jun 29, 2019

View reviewed changes

app/src/main/java/de/westnordost/streetcomplete/data/osm/split/SplitWayUpload.kt Outdated Show resolved Hide resolved

westnordost added 2 commits June 29, 2019 18:40

fix create split position at 18th meridian

c8d75d5

Merge branch 'master' into changesets-refactor

ef9044b

matkoniecz mentioned this pull request Jul 5, 2019

UX testing results #1470

Closed

matkoniecz reviewed Jul 5, 2019

View reviewed changes

app/src/main/java/de/westnordost/streetcomplete/data/osm/split/SplitWayAtPosition.kt Outdated Show resolved Hide resolved

westnordost added 2 commits July 7, 2019 19:03

Merge branch 'master' into changesets-refactor

4317c3a

work in progress upload refactor

319881d

westnordost added 2 commits July 25, 2019 23:54

work in progress upload refactor

8ed1f0b

Merge branch 'master' into changesets-refactor

9f34334

fix unsynced split way seemingly gone after reopening app

6df9cff

westnordost added 7 commits August 27, 2019 23:02

fix correctly splitting restriction relations with multiple via ways

908d982

do not show option to split way for closed roundabouts

e87c0a7

fix form for landscape layout

4c6c71e

enhance text

59a8c71

Merge branch 'master' into changesets-refactor

a6272c8

# Conflicts: # app/src/main/res/values/strings.xml # app/src/test/java/de/westnordost/streetcomplete/data/osm/download/OsmQuestDownloadTest.java

update feature names

4154b74

add confirmation dialog if there are many splits

1baafe2

matkoniecz reviewed Aug 29, 2019

View reviewed changes

app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved

matkoniecz reviewed Aug 29, 2019

View reviewed changes

westnordost and others added 4 commits August 29, 2019 21:11

Update app/src/main/res/values/strings.xml

578aef3

Co-Authored-By: Mateusz Konieczny <[email protected]>

do not require user to zoom in further if it is not possible

12f639d

Merge branch 'master' into changesets-refactor

1b58a84

# Conflicts: # app/src/main/res/values/strings.xml

only activate photos attached to note if there are any

2184be7

westnordost added 2 commits September 1, 2019 21:45

allow Element.copy to copy with null-tags

e5fe906

correctly update elements after split

c79bd52

westnordost added 2 commits September 4, 2019 21:44

correctly update elements after split (2)

00d471f

QuestGiver shall only create quests that are currently visible

1cb30d7

fix tests

50d3823

westnordost merged commit 637343c into master Sep 4, 2019

westnordost deleted the changesets-refactor branch October 20, 2019 12:35

matkoniecz mentioned this pull request Jul 6, 2020

'Tiny line with length 0.0072m' (and importing Kraków center in general) a-b-street/abstreet#130

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changesets refactor #1329

Changesets refactor #1329

westnordost commented Feb 6, 2019 •

edited

Loading

westnordost commented Jun 18, 2019 •

edited

Loading

matkoniecz commented Jul 5, 2019

westnordost commented Jul 5, 2019

matkoniecz commented Jul 5, 2019

westnordost commented Jul 5, 2019

westnordost commented Jul 25, 2019

westnordost commented Aug 26, 2019 •

edited

Loading

westnordost commented Aug 26, 2019

matkoniecz Aug 29, 2019

westnordost Aug 29, 2019

westnordost Aug 29, 2019

westnordost Aug 29, 2019

westnordost Sep 1, 2019

westnordost commented Sep 1, 2019 •

edited

Loading

matkoniecz commented Sep 1, 2019

westnordost commented Sep 1, 2019

HolgerJeromin commented Sep 1, 2019 •

edited

Loading

westnordost commented Sep 1, 2019

westnordost commented Sep 4, 2019 •

edited

Loading

Changesets refactor #1329

Changesets refactor #1329

Conversation

westnordost commented Feb 6, 2019 • edited Loading

westnordost commented Jun 18, 2019 • edited Loading

matkoniecz commented Jul 5, 2019

westnordost commented Jul 5, 2019

matkoniecz commented Jul 5, 2019

westnordost commented Jul 5, 2019

westnordost commented Jul 25, 2019

westnordost commented Aug 26, 2019 • edited Loading

westnordost commented Aug 26, 2019

matkoniecz Aug 29, 2019

Choose a reason for hiding this comment

westnordost Aug 29, 2019

Choose a reason for hiding this comment

westnordost Aug 29, 2019

Choose a reason for hiding this comment

westnordost Aug 29, 2019

Choose a reason for hiding this comment

westnordost Sep 1, 2019

Choose a reason for hiding this comment

westnordost commented Sep 1, 2019 • edited Loading

matkoniecz commented Sep 1, 2019

westnordost commented Sep 1, 2019

HolgerJeromin commented Sep 1, 2019 • edited Loading

westnordost commented Sep 1, 2019

westnordost commented Sep 4, 2019 • edited Loading

westnordost commented Feb 6, 2019 •

edited

Loading

westnordost commented Jun 18, 2019 •

edited

Loading

westnordost commented Aug 26, 2019 •

edited

Loading

westnordost commented Sep 1, 2019 •

edited

Loading

HolgerJeromin commented Sep 1, 2019 •

edited

Loading

westnordost commented Sep 4, 2019 •

edited

Loading