Found marker lost on live map after opening popup #1851

Closed
Lineflyer opened this Issue Jun 29, 2012 · 17 comments

Projects

None yet

4 participants

@Lineflyer
c:geo member

With version 2012.06.29-NB-fd3eda7 I found this (but it is probably present longer time):

  • Don't hide found caches
  • Open live map (Strategy: Fast)
  • Click on a found cache loaded by live map (Type: ??, Marked found, Unreliable)
  • Popup opens but does not report it as "Found"
  • Close popup
  • The map now displays the correct cache type but the found marker (smiley) is no longer shown

The popup from live map on geocaching.com (where we retrieve the info for our popup) is not indicating whether the cache is found by the user. This means c:geo can not get new information on found-status and thus should not overwrite the found-info in case the popup is opened.

@Lineflyer
c:geo member

This is just to confirm that this issue is still reproducible in NB 2013.02.25.
And this is surely on of the data merging issues.

@marco-dev

Again a merging problem. The main problem is we call geocacheNewLoaded.gatherMissingFrom(geocacheOldStored). We try to guess what we can transfer from old to new, but we are not really sure. Furthermore a dev could use it the other way, geocacheOldStored.gatherFrom(geocacheNewLoaded) without problems. As far as we don't have full information and control there is nothing we can do about.

First we have different reliable information, according to the source (GC.com, OC.de) and type (fast mode, nearby search, popup, full refresh/stored). Say we have GC.com with nerby search amount of reliable information. If you merge this with the full refresh/stored, only the reliable data should be overwritten. We need a timestamp to know which reliable information is the newest.

In this case the new information loaded from net (popup) is gathering missing data from the geocache in CacheCache (might be stored for offline or fast mode result temporary stored). The new information is taken as more reliable. If the popup info doesn't contain found flag reliable it is taken from stored.

We have to refactor a lot...

  • timestamp to know which geocache instance is most reliable
  • flag for every attribute to know if it was set

Have to search the web what is preferred.

@marco-dev

Another problem: which timestamp should the merged geocache object get? The oldest or the newest? Or do we have to store timestamp for every attribute. I would vote against.

@Lineflyer
c:geo member

These sound like the general approach..which is not bad for sure.
Right now the tests show that merging seems to work quite good in all areas except this special case of popup information.
The popup itseld (or the corresponding request to gc.com) does not return any information about the found status AFAIK, so for this special case it shoul dbe sufficient to just do not overwrite the found attribute when returning from the popup. The only valuable information is D/T and cache-type.

@marco-dev

Yes there are everywhere this special cases.

I remember I fixed a problem with live map because found was not shown as found. Only after opening popup the found flag was right. 3 months later I got the same problem back because someone fixed it back due to a reason.

What I want to say is that changing it will always lead to other parts not working correct.

For found-flag we have the special case that it will seldom be reset at the provider (maybe an owner looks through logbook and removes all not signed it...). So found can be set if one of the two data objects have found set to true.

@marco-dev

Another approach to solve merging:

  • add a meta object to geocache object
  • meta object holds information about what is reliable
  • every kind of source provides an own meta object

e.g.:
GcNearbySearchGeocacheMetaInf
GcGeocacheMetaInf
GcGeocacheMetaInf
OcNearbySearchGeocacheMetaInf
OcGeocacheMetaInf

The names , should be changed to the kind of request done.
Geocache object can delegate request about reliability to the meta object.

gcPopupGeocacheMetaInf.isFoundReliable()
gcPopupGeocacheMetaInf.isCacheTypeReliable()

We would be able to reliable merge.

@samueltardieu
c:geo member

This looks like patching: this would be a good idea if you had no control over the existing structure. Here, we can change the fields and their meanings at will.

For example, take the note. We could represent it with a special value (e.g., null) to indicate its unavailability, while an empty note (when we know that there is no note) would be an empty string.

Some fields, such as the number of stars, should always be overriden with the online version.

For mutable fields, such as the note, we should keep the previous versions around, so that we can do a three-way merge and detect whether the different string we get from the web site is a new user input or if it is an old one that has since been erased on the device.

(as a side-note, for people not liking null, it is possible to use unique references instead of null such as new String(), but it will probably translate to NULL in the database anyway)

@marco-dev

@samueltardieu
It's something I also thought about. Then everyone creating a geocache object would have to be aware of this, never to set data not reliable. We would need to change e.g. found to Boolean object instead of basic type to know if it is reliable. Every place calling isFound needs to be changed to getFound() and handle NULL values.

@samueltardieu
c:geo member

Not necessarily. For example, isFound() can be defined as returning true only if we know that the user has found the cache. If found is a Boolean, then a null value would return false. In the case of isFound(), it would make perfect sense.

The problem is not "reliable" vs. "not reliable", it is "known" vs. "unknown". The only unreliable information we can get are coordinates coming from the live map, and those may be handled as a special case with a reliable flag. But all the other pieces of data are either known or unknown.

@marco-dev

Okay, so reliable is the wrong word for it.

We would change found attribute from basic to object and merge with direct variable access or should we provide isFound and getFound, the last returning the object?

@rsudev

As we need the presence information only during merge, a special accessor might not be necessary.

@rsudev

BTW, the found information could also be classified as possibly unreliable, as the status we get from the live map (through png parsing) is not 100% sure, but that is something we can safely leave aside IMO

@marco-dev

That is the other problem: We are not really sure what we know in some places. At icon parsing it depends on the zoom level and overlaying of caches.

@marco-dev

But if we only set found state to true but not to false in this case, merging will be fine.

@marco-dev marco-dev was assigned Jul 19, 2013
@marco-dev

Okay, so for this issue I will change Geocache.found to Boolean and examine all calls to setter.

@marco-dev

Working on this issue.

@marco-dev

PR sent

@marco-dev marco-dev added a commit to marco-dev/c-geo-opensource that referenced this issue Jul 27, 2013
@marco-dev marco-dev fix #1851 - merge popup information 0165f89
@marco-dev marco-dev closed this in 579436c Jul 28, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment