iitc-project / ingress-intel-total-conversion

ingress.com/intel total conversion user script with some new features. Should allow easier extension of the intel map.
http://iitc.jonatkins.com/
ISC License
989 stars 552 forks source link

Portal display filtering #962

Open perringaiden opened 9 years ago

perringaiden commented 9 years ago

Disclaimer: I'm a developer but not in JavaScript, so I might be wrong on reading the code.

Request: Plugin-accessible portal display filtering.

I've been looking at the Uniques plugin, to determine how to be able to control whether to show or not show portals based on their capture/visited status. Looking into how the apparent multi-option ability to filter portals based on level and faction, I discovered that its not so multi-option. It appears that it is a fixed array of [level], [faction] per layer, which means its not adjustable by a plugin. Layers are displayed when Level is ticked and Faction is ticked, but its a simple filter.

The modified Uniques plugin that I was given recently attempted to do this filtering by adding the portal to an additional layer. It works in some respects, as you can toggle the layer to make portals disappear, but its definitely not the best solution as it doesn't persist properly. Scrolling the view shows portals that shouldn't appear, and you have to toggle it again to get them removed.

I'd like to request that we adjust the code to provide an accessible filter method that plugins can hook, where each portal runs through the filter hook, and each plugin has the ability to reject the portal. Each level option would return true if the option is on, each faction option would return true if the option is on. Plugins would get the same option to return true and allow portals to be displayed. Any plugin returning true would stop the process and reject the portal.

From what I can see, it would change the way layers are displayed, at least for portals, which might be a fairly hefty change, but it would allow a much greater flexibility than just the current highlighter styles.

Speeddymon commented 9 years ago

I agree this should be a feature of the uniques plugin. Highlighting is great but there are over 18,000 portals in my city and trying to get my Platinum Explorer by zooming in to level 16 and flagging portals so I can see them when zoomed out sucks. Being able to check a box on the layer chooser to hide portals that are no longer unique would be a great addition

michaeldever commented 9 years ago

This is a reasonably straightforward thing to add, I have a working solution here, however, there needs to be some discussion on what happens when a portal is removed from the window I think.

The first issue is that it's not only the uniques plugin that adds markers/layers, for example the portal names and portal levels plugin also add layers to the map. This can be overcome by waiting for the whole map to load and then telling those plugins to remove their layers. At the same time, we can iterate over each portal in the window and remove based upon whether or not its unvisited/visited/captured.

The problem here lie in: what do we do when we remove a portal from the map, for this specific purpose. Do we (1) remove it from the window altogether (the default behaviour of iitc) or do we (2) just remove it from the layers and keep it in the set of window portals (for easily re-adding) it to the map.

  1. This may be the best approach, as it means that the portals list plugin etc. work correctly, however this will require a reload of the map in order to show these portals again.
  2. If we follow this approach, then the portals list will show portals that have no marker displayed on the map, and that plugin will need to be modified to suit.

At this point, I've no idea which is best, any suggestions are useful, otherwise, I think it's best to go with approach (1).

Speeddymon commented 9 years ago

I'm working on some code cleanups to portals list and will be happy to work with you to ensure it works properly with approach 2. I'm fairly new to JavaScript though, so I may have some (a lot of) questions. I feel like making users reload in order to display portals after checking a box in the layer chooser is not the right way to go about it because other plugins don't require it. What about storing the removed portals in an array in your plugin and using that array to unhide them when the layer is re-enabled? On Apr 18, 2015 4:05 PM, "Michael Dever" notifications@github.com wrote:

This is a reasonably straightforward thing to add, I have a working solution here, however, there needs to be some discussion on what happens when a portal is removed from the window I think.

The first issue is that it's not only the uniques plugin that adds markers/layers, for example the portal names and portal levels plugin also add layers to the map. This can be overcome by waiting for the whole map to load and then telling those plugins to remove their layers. At the same time, we can iterate over each portal in the window and remove based upon whether or not its unvisited/visited/captured.

The problem here lie in: what do we do when we remove a portal from the map, for this specific purpose. Do we (1) remove it from the window altogether (the default behaviour of iitc) or do we (2) just remove it from the layers and keep it in the set of window portals (for easily re-adding) it to the map.

  1. This may be the best approach, as it means that the portals list plugin etc. work correctly, however this will require a reload of the map in order to show these portals again.
  2. If we follow this approach, then the portals list will show portals that have no marker displayed on the map, and that plugin will need to be modified to suit.

At this point, I've no idea which is best, any suggestions are useful, otherwise, I think it's best to go with approach (1).

— Reply to this email directly or view it on GitHub https://github.com/jonatkins/ingress-intel-total-conversion/issues/962#issuecomment-94200373 .

michaeldever commented 9 years ago

@Speeddymon The problem is not with unhiding the hidden portals, it is with markers being hidden while the portals list is shown. I think the portals list should only show portals that are displayed within the bounds of the map. If we have hidden them with our modification to uniques, then they should not be displayed within the portals list. This leads to approach (1), but comes with the cost of reloading once the portals have been removed from the windows list of portals in order to get them to display again.

Maybe @jonatkins can weigh in with his opinion on how to address this? Maybe the addition of a 'displayed' attribute to portals?

Speeddymon commented 9 years ago

If you're referring to the portals list plugin, I'm happy to work around the need to reload by reiterating the portals visible within the bounds of the map any time the portals list window is closed and opened again. I believe it should be doing that anyway as it stand. On Apr 18, 2015 4:30 PM, "Michael Dever" notifications@github.com wrote:

@Speeddymon https://github.com/Speeddymon The problem is not with unhiding the hidden portals, it is with markers being hidden while the portals list is shown. I think the portals list should only show portals that are displayed within the bounds of the map. If we have hidden them with our modification to uniques, then they should not be displayed within the portals list. This leads to approach (1), but comes with the cost of reloading once the portals have been removed from the windows list of portals in order to get them to display again.

Maybe @jonatkins https://github.com/jonatkins can weigh in with his opinion on how to address this? Maybe the addition of a 'displayed' attribute to portals?

— Reply to this email directly or view it on GitHub https://github.com/jonatkins/ingress-intel-total-conversion/issues/962#issuecomment-94202740 .

michaeldever commented 9 years ago

Ok, I have somewhat of a fix for this, we can add a listener to each of the links in the portals list. When clicked, that individual portal shall be redrawn if it is hidden.

One more slight issue: if a portal is selected, when the portals are hidden, should that portals marker be hidden too.

I'll finish it out tomorrow morning/afternoon.

Speeddymon commented 9 years ago

What happens when you zoom out on a low level portal that is selected? On Apr 18, 2015 5:07 PM, "Michael Dever" notifications@github.com wrote:

Ok, I have somewhat of a fix for this, we can add a listener to each of the links in the portals list. When clicked, that individual portal shall be redrawn if it is hidden.

One more slight issue: if a portal is selected, when the portals are hidden, should that portals marker be hidden too.

— Reply to this email directly or view it on GitHub https://github.com/jonatkins/ingress-intel-total-conversion/issues/962#issuecomment-94204756 .

michaeldever commented 9 years ago

It stays shown, I've preserved this in my fix, and hit another issue. If we have removed portals from the window and then select another highlighter, they remain hidden. I'll have to add some new hooks to the highlighter plugin and we should be good to go!

Ok, that's all done, on changing highlights, the portals markers are redrawn / removed if changed back to uniques. If the selected portal should be hidden, it is left drawn as it is selected.

Think that's everything??? Apart from adding a means to hide visited/shown... this could just be an option in along with stuff like DrawTools Opts etc? Maybe a little dialog like DrawTools opts?

perringaiden commented 9 years ago

@michaeldever While it might fix the specific issue in Uniques with a custom change, it doesn't address the issue, which is to allow plugins to hide or show portals based on criteria. Another use would be for missions, being able to hide other portals on screen to show just the mission set. Or to have a view that shows only portals below 50% health, or hides portals that have 8 outgoing links. The idea is to have a generic solution, not 'fix uniques'.

michaeldever commented 9 years ago

@perringaiden This is true, however, such a change is coupled to the plugins that have their own layers, such as portal-names and portal-levels. For every portal that a highlighter removes, each of these should also remove the associated item from their layer. As such it doesn't make sense to make this generic, to me at this point anyway, as you then tie this generic function directly to those specific plugins, which everyone may not have.

If & when PR #977 by @Meaglin is added in, making this generic becomes viable, as those plugins with their own layers can then be updated to reflect added/removed portals on the fly.