search

Helium314 / SCEE

OpenStreetMap surveyor app for experienced OSM contributors
GNU General Public License v3.0
153 stars 12 forks source link

Increase the distance to which the POI can be moved #402

Open deevroman opened 1 year ago

deevroman commented 1 year ago

30 meters is a small distance even for small shopping centers. Sometimes there are POIs that are added to OSM automatically only based on the address, so they are placed randomly in the building. For example, the company RocketData adds to stores in the CIS in this way.

Helium314 commented 1 year ago

It could definitely be implemented, but it's more work than it appears. This is because to display other elements over the whole distance, also the highlighting radius needs to be adjusted. And the highlighting radius can be set for each quest individually, so just changing the default value may not work always. Plus, it needs to be optional because increased highlighting radius affects performance.

Maybe I'll try working on this at some later point... For now I can only recommend to move the element multiple times.

ravenfeld commented 4 months ago

Is this still relevant, as I can move node to within a metre without any problems ? Version 58.1

mnalis commented 4 months ago

I can move node to within a metre without any problems ?

@ravenfeld I think this is not about precision of movement, but the issue is about not being able to move a node by more than 30 meters.

If you try to move a node by > 30 meters, SCEE[^1] will not display e.g. Moved by 7.3m in that dialog, but instead Moved too far. Consider leaving a note and remove OK (checkmark) button.

[^1]: at least SCEE 58.0, I have not tried 58.1 yet; but there is not mention in the changelog so I guess it is still probably the same.

ravenfeld commented 4 months ago

Thank you very much, I misunderstood, I was going for greater precision.

ravenfeld commented 4 months ago

Changing the MAX_MOVE_DISTANCE value slightly would not be enough. I didn't understand where the value for the highlighting radius was, maybe even the same value?

For your information, the download distance is 1 SQKM (MIN_DOWNLOADABLE_AREA_IN_SQKM), so that means you could use 500m at the most, even if I think that's a lot.

@deevroman How much would you need for your use case?

deevroman commented 4 months ago

I think no more than 100-150 meters (size of a shopping center or long building)

p.s. I will note that now I rarely use SC/ee to edit POIs

ravenfeld commented 4 months ago

Thank you for your reply. I'm going to wait for a reply from the maintainer (@Helium314 )now that you're no longer using the app.

Helium314 commented 4 months ago

Changing the MAX_MOVE_DISTANCE value slightly would not be enough. I didn't understand where the value for the highlighting radius was, maybe even the same value?

It's in each quest, with some default value if not provided in the quest.

ravenfeld commented 4 months ago

highlightedElementsRadius I don't see what this has to do with demand. In my case, modifying MAX_MOVE_DISTANCE allows the POI to be moved as requested @deevroman

mnalis commented 4 months ago

In my case, modifying MAX_MOVE_DISTANCE allows the POI to be moved as requested

I believe that moving will work just fine, but note the problem mentioned in https://github.com/Helium314/SCEE/issues/402#issuecomment-1348130302 -- while moving longer distances (unless other changes to the code are made), the map will not display other instances of the POIs on the map that are too far, allowing the user to mistakenly create duplicates (i.e. by moving POI from one location to another location, where there such POI already exists, but it "invisible").

ravenfeld commented 4 months ago

Yes, that's what I found, isn't it ? https://github.com/Helium314/SCEE/issues/402#issuecomment-2205751448 As far as I'm concerned, I have all the data for at least 1 square kilometre.

mnalis commented 4 months ago

As far as I'm concerned, I have all the data for at least 1 square kilometre.

Um, are you sure? I could be wrong, but that constant is not 1 (as would be needed for 1 square kilometer) but 0.1 so I read it as the minimum it will fetch is only 0.1km2 (which is much smaller than 1km2 that you suggest):

https://github.com/Helium314/SCEE/blob/e338dd1e86b85c7acf0cbf344889eb9b0e6faa01/app/src/main/java/de/westnordost/streetcomplete/ApplicationConstants.kt#L11

(the area that is downloaded is actually more complex AFAIK, but we're interested in worst case anyway so it can probably be ignored)

One should note that user might have also panned the map off center, even further reducing the amount of "visible" area on that side - e.g. if you download the data and then pan the map to the right (manually panning or by walking in that direction with auto-centering enabled) then when you want to move element which is now on the center of the screen (which is not the center that was at the time of the download!) you will have less "visible space" or the right side of the map (and more visible space on left side, respectively).

And depending on how much you moved new download may not have yet initiated.

As a result, even that 0.1km2 is not really what you've got to work with in most situations.

Helium314 commented 4 months ago

As far as I'm concerned, I have all the data for at least 1 square kilometre.

No, you don't.

  1. You will have the area of the (zoom 16) tile the element is in, everything else might not be downloaded (consider not having internet connection).
  2. The 1 square kilometre (or 0.1, doesn't matter) is a. only valid for auto-download and b. usually not centered on the element you want to move.

highlightedElementsRadius I don't see what this has to do with demand.

When you move an element, you should see existing elements of that type. This requires them to be both downloaded and highlighted. When you have the world downloaded, and highlight a radius of 10m, you will not see a similar element (potential dupliacate) that is 11m away. So in this case you should not be allowed to move it more than 10m.