IITC-CE / ingress-intel-total-conversion

intel.ingress.com total conversion user script with some new features. Should allow easier extension of the intel map.
https://iitc.app
ISC License
285 stars 110 forks source link

zoomToAndShowPortal and selectPortalByLatLng #93

Open johnd0e opened 5 years ago

johnd0e commented 5 years ago

https://github.com/IITC-CE/ingress-intel-total-conversion/blob/d77562b91a5b1e0bf170b4257eb33316a0b9daa3/code/utils_misc.js#L255-L263

https://github.com/IITC-CE/ingress-intel-total-conversion/blob/d77562b91a5b1e0bf170b4257eb33316a0b9daa3/code/utils_misc.js#L265-L284

Both functions are similar in action, and differs only by input parameters. So if both guid/latlng are available then use zoomToAndShowPortal, but if we have only latlng then use selectPortalByLatLng.

Issues with both functions:

  1. Map center set to latlng.

    • As for me better use map.panInside.
    • In some circumstances better just render details not changing position.
  2. Map zoom set to 17. In most cases I'd prefer to keep current zoom if possible.

    I suppose this is done in order to load portal details (which can be unavailable at the moment), and long time ago 17 was the value when portals of all levels get loaded.

    • Now all portals load at zoom 15.
    • For zoomToAndShowPortal we can use portalDetail.request
  3. Functions are not consistent:

    • selectPortalByLatLng set position/zoom only when portals details is not available yet.
    • zoomToAndShowPortal set position/zoom unconditionally.
johnd0e commented 5 years ago

So now it's possible to improve the functions, and get better user experience. But because of mentioned inconsistency it would be better to replace these 2 functions with some new universal function, such as:

function selectPortal(guid, latlng, needSetView)

Note: May be renderDatails action is also better if optional.

Obviously we can't just throw away old functions, so we keep them for compatibility. IMHO better place such legacy functions in separate file compat.js or legacy.js.

Edit: consider also using these:

johnd0e commented 5 years ago

zoomToAndShowPortal:

selectPortalByLatLng

angel93ayora commented 5 years ago

As a plugin dev. i've missed some function to load portal without moving the map, and to load portal without rendering details.

johnd0e commented 5 years ago

@angel93ayora How exactly are you going to use this function?

angel93ayora commented 5 years ago

It will be useful in cases where you dont need/want to move the map or to load the details on the sidebar. Example: while drawing you want to check the status (if it is captured) of a portal or maybe its outgoing links

Example 2: you want to load only one portal without requesting all portals of an area

johnd0e commented 5 years ago

https://github.com/IITC-CE/ingress-intel-total-conversion/blob/d77562b91a5b1e0bf170b4257eb33316a0b9daa3/plugins/show-linked-portals.user.js#L132-L134

Should be used carefully, avoiding mass requests.

angel93ayora commented 5 years ago

ok, its already done i have to update a plugin :)

johnd0e commented 5 years ago

ok, its already done

Well, that always was there). But now it's more convenient to use.

BTW, what is your plugin?

angel93ayora commented 5 years ago

it's an opsec ENL plugin, if you are ENL i can share it with you