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
279 stars 105 forks source link

Add new bulk star/unstar/toggle bookmarks feature. #724

Closed nexushoratio closed 2 months ago

nexushoratio commented 3 months ago

Scroll to an area and use the layer filter to hide certain classes of portals (level, faction, etc). Then use the bulk commands to add/remove portals from the bookmarks.

github-actions[bot] commented 3 months ago

🤖 Pull request artifacts

file commit
IITC_Mobile-test.apk b3be9fef698b316ec09351ea6901a103204035e2
test-0.38.1.20240401.043954.zip b3be9fef698b316ec09351ea6901a103204035e2

See build on website

nexushoratio commented 3 months ago

I first implemented this about 10 years ago.

See https://github.com/iitc-project/ingress-intel-total-conversion/pull/881/files

Also mentioned in #123 in the bookmarks section.

Not the fastest beast. Processes about 100 portals/sec on my machine. So, if one has a large map in the browser (via large monitor, or zooming out in the browser while staying zoomed in with the map), it can take 20s to process 2,000 portals. During that time there is currently no feedback, and, occasionally, the browser, at least Firefox, may give the "the script it slow" message.

McBen commented 3 months ago

What's the use case?

nexushoratio commented 3 months ago

What's the use case?

While all of the above are possible now, when dealing with a view that has hundreds of portals on it (or, more likely, panning around to do the same), clicking a single button is a lot easier than clicking all of the ones in the portals list.

nexushoratio commented 3 months ago

Please take another look.

I created a couple of new functions to handle populating the bookmark object. I would like to eventually have a couple of other places in the file make use of them, effectively replacing/streamlining existing calls to addPortalBookmark(). Hence the *ByGuid() variant, currently unused.

The reviewdog checks are much cleaner now. Unsure if I did something on my end, or if you all tweaked something. If so, thanks!

I'm not well versed in PRs that have updates in it. Do I need to do anything to squash? (going off to read docs now.)

DanielOndiordna commented 3 months ago

Screenshot_20240401-090825_IITC Mobile My Bookmarks add on plugin adds exactly this functionality. And it works fast (instantly). https://softspot.nl/ingress/plugindetails/iitc-plugin-bookmarks-addon

nexushoratio commented 3 months ago

And it works fast (instantly). https://softspot.nl/ingress/plugindetails/iitc-plugin-bookmarks-addon

How does it update the bookmark widget quickly when there are hundreds of bookmarks?

DanielOndiordna commented 3 months ago

How does it update the bookmark widget quickly when there are hundreds of bookmarks?

By creating a new json formatted bookmarks string and using the steps of the import function of the bookmarks plugin. Actually just as fast if you start iitc and all bookmarks are drawn.

nexushoratio commented 3 months ago

Actually just as fast if you start iitc and all bookmarks are drawn.

Yeah, that isn't so fast for me either.

With only 505 portals bookmarked, it took just shy of 24 seconds to execute the line $(".bookmarkList ul li ul").sortable(. (Same after restarting the browser.)

10:35:44.505 entered jquerySortableScript [intel.ingress.com:663:13](https://intel.ingress.com/?ll=37.477991,-122.303625&z=15%20line%202244%20%3E%20injectedScript)
10:35:44.509 finished .bookmarksAnchor [intel.ingress.com:675:13](https://intel.ingress.com/?ll=37.477991,-122.303625&z=15%20line%202244%20%3E%20injectedScript)
10:36:08.298 finished .bookmarksLink [intel.ingress.com:688:13](https://intel.ingress.com/?ll=37.477991,-122.303625&z=15%20line%202244%20%3E%20injectedScript)

This may be getting into a Firefox-on-Linux-using-ViolentMonkey performance issue, and is independent of my code.

nexushoratio commented 2 months ago

Just checking to see if this has any hope of moving forward, or if I should just withdraw it.

xscreach commented 2 months ago

IMHO since there's maintained plugin to handle exactly this and more, we don't need that functionality in core plugins 🤷‍♂️