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

Expanding functionallity of findPortalGuidByPositionE6 #418

Open
MysticJay opened this issue Dec 23, 2020 · 13 comments
Open

Expanding functionallity of findPortalGuidByPositionE6 #418

MysticJay opened this issue Dec 23, 2020 · 13 comments
Labels
enhancement New feature or request plugin

Comments

@MysticJay
Copy link
Contributor

effected plugin: CACHE-PORTALS-ON-MAP

The function findPortalGuidByPositionE6 is converting an E6-Position in to a guid.
This is only possible for portals in the current viewport for selected or at zoom level "all portals", as portal details need be loaded.
The plugin CACHE-PORTALS-ON-MAP caches the portal details, but does not allow the function findPortalGuidByPositionE6 to benefit from that information.

The code to add the functionality is ready and available from PR#389 (which was reverted)

The discussion is about how to inject the code into the existing findPortalGuidByPositionE6

The proposed code uses code injection by calling the original function.
The alternative would be a hook, but that slows down the processing as all hooked functions would need to be executed without the possibility to stop execution if the guid is found.
A third alternative would be to overwrite the core function. But this would cause changes to the core function never to get active, when CACHE-PORTALS-ON-MAP is loaded.

OFFLE, a 3rd-party plugin, is also providing caching functionality for portal data. OFFLE has also been changed to use code injection to extend the core function findPortalGuidByPositionE6.

@MysticJay
Copy link
Contributor Author

UNIQUES040 #416 would greatly profit from extending findPortalGuidByPositionE6

@MysticJay MysticJay added enhancement New feature or request plugin labels Dec 24, 2020
@MysticJay
Copy link
Contributor Author

earlier comments from @johnd0e
#388 (comment)

@MysticJay
Copy link
Contributor Author

@MysticJay There is one other option:
cachePortals is a very small plugin. we could even add the code to the core and avoid all the discussion.

@johnd0e
Copy link
Contributor

johnd0e commented Dec 24, 2020

The plugin CACHE-PORTALS-ON-MAP caches the portal details

I have some small but rather important clarifications about that's plugin caching implementation:

  • only explicitly clicked portals are cached
  • the cache is reset on every intel page reload

So my conclusion: the plugin is not so useful as someone would imagine.

UNIQUES040 #416 would greatly profit from extending findPortalGuidByPositionE6

Not really, because that cache is tooo small.

cachePortals is a very small plugin. we could even add the code to the core and avoid all the discussion.

That's why answer is "No way". It is just pointless.
But we can provide special api instead, to be able to benefit from any external plugin.

OFFLE, a 3rd-party plugin, is also providing caching functionality for portal data.

And also many others plugins. That's why I'd prefer some clean api solution for all them.

@johnd0e
Copy link
Contributor

johnd0e commented Dec 24, 2020

The alternative would be a hook, but that slows down the processing (1) as all hooked functions would need to be executed without the possibility to stop execution if the guid is found(2).

  1. Baseless without real measurements
  2. It depends on implementation.
    Yes, current hook loop is uninterruptable, but listener itself can check: if data is already available then do nothing and just exit.

@johnd0e
Copy link
Contributor

johnd0e commented Dec 24, 2020

UNIQUES040 #416 would greatly profit from extending findPortalGuidByPositionE6

And one more clarification: as currently implemented, portal guid MUST be in cache already, otherwise that data is just lost.
Again my conclusion: considering that cache is extremely small (as for now), this code is much less effective than someone would imagine.

The more effective strategy would be save those unresolved latlngs for later checking.

@McBen
Copy link
Contributor

McBen commented Dec 24, 2020

is https://github.com/IITC-CE/ingress-intel-total-conversion/blob/master/core/code/data_cache.js not already doing the same thing?

with the exception that the data isn't rendered and not stored for 12hours - but with improved memory managment.

@MysticJay
Copy link
Contributor Author

@johnd0e From the discussion I agree. Cache portals on map seems less usefull in resolving guid.

uniques is already remembering unresolved portals missedLatLngs

OFFLE saves guids even on reload, so for the time we can leave ir with that.

@johnd0e
Copy link
Contributor

johnd0e commented Dec 24, 2020

is https://github.com/IITC-CE/ingress-intel-total-conversion/blob/master/core/code/data_cache.js not already doing the same thing?

Well, window.DataCache is merely general-purpose class, that can be used for anything.
In fact, it is instantiated as window.MapDataRequest (not related to subject at all), and cache of window.portalDetail.

window.portalDetail is not very useful too, because it stores only portals for those we have full details (results of window.portalDetail.request).

But indeed, it looks like for cache-portals-on-map we can reuse window.portalDetail.get instead of reimplementing one more cache instance.

Edit:
Perhaps it also possible to use window.DataCache for pushPortalGuidPositionCache/findPortalGuidByPositionE6 (requires some refactoring though).

@MysticJay
Copy link
Contributor Author

@johnd0e The problem is not getting something out of the cache if guid is know, but to find the guid when you only have LatLonE6 from the COMM.
Also we do not need the full details.

@johnd0e
Copy link
Contributor

johnd0e commented Dec 24, 2020

@MysticJay Obviously. But my message was not about it.

@johnd0e
Copy link
Contributor

johnd0e commented Mar 11, 2021

Should this issue stay opened?

@MysticJay
Copy link
Contributor Author

@johnd0e this is implemented in Offle afaik
and cache and uniques were also involved.
leave open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request plugin
Projects
None yet
Development

No branches or pull requests

3 participants