Permanent portal entities #437

le-jeu · 2021-02-06T18:00:44Z

Address #325

Changes

Extend the CircleMarker class to PortalMarker class following the same idea of #251

Portal entities are only deleted when removed from rendering:

portal deleted according to getEtentities data (does this really occurs?)
no selected and absent from getEntities data (zoom <15, not in view, not a endpoint)

On any other situtation, the portal marker is the same and only receive updates until removed from rendering.

class `L.PortalMarker`

Methods:

setStyle(style) (deprecated, for compatibility with old highlighters)
don't apply a style, but save it for later when calling setMarkerStyle
setMarkerStyle(style)
- compute base portal style according to details and level
- use the given style on top of it
- call the highlighter to update further the style
- if selected, force the color
- apply the style
setSelected(details)
- change selected intern state and call setMarkerStyle()
willUpdate(details)
- given details (from decodeArray.portal), return true if the details are more recent or more detailled than the known details
updateDetails(details)
- apply new details
- maintain permanent data (history)
- handle partial update with identical timestamp (mods, resonators...)
- update options (compatibility)
- update the marker on the map
- if selected, update the side bar
getDetails()
- return portal details
hasFullDetails()
- return true if the marker store mods/resonators/... details
renderDetails()
- call renderPortalDetails (even if unselected) while avoiding a call to selectPortal

Hooks `portalSelected`

Add a new field event that specified the origin of the event.
This is mainly to distinguish a selection that comes from a call to renderPortalDetails and a click/touch event on the marker from the user.

Portal Details Cache

Before

on portalDetail.request, the data is parsed, store, used for render portal details in the side bar if the portal is selected, then the portalDetailLoaded hook trigger the creation of a new portal entity.

portal entity only stores partial data (summary only but full raw)

Now

on portalDetail.request, we create or update a portal entity, this part renders the portal details if needed, store the portal details in cache, run the portalDetailLoaded hook.

portal detail cache is used for incomplete portal entity upon creation

Gain: the portal entity always has the most recent data, details are parsed once, the portal entity is sure to exist for the hook.

Missing: if the portal changed before details in cache expires, the code won't fetch new details unless a direct call to portalDetail.request (is it true before also?) see a587775

Placeholder entities

Placeholder portal entities are introduced for link/field endpoints.
Fields and links have an unused timestamp.
The PR uses the timestamp from link/field to handle portal updates as any other portal entity.

Sidenotes

Portal timestamp change on portal edit (title, location, portal main picture).

deprecated:

window.createMarker
window.setMarkerStyle

From #251

related functions (next candidates for refactoring/deprecation)

window.setPortalIndicators
window.selectPortal
window.portalMarkerScale
window.getMarkerStyleOptions
window.renderPortalDetails

johnd0e · 2021-02-15T07:53:53Z

See also #251

johnd0e · 2021-02-17T10:31:36Z

core/code/portal_marker.js

 }
 function handler_portal_dblclick (e) {
-  e.target.select(true);
+  handler_portal_click(e);


what is so bad for you to capslock ?

click/dblclicl/contexmenu - are separate actions.
Don't call one from another, even if currently it does the job.
Better make separate function for these calls:

window.selectPortal(e.target.options.guid, e.type); window.renderPortalDetails(e.target.options.guid, true)

Also I suspect that we we can refactor code to leave only one of these.

johnd0e · 2021-02-17T11:19:09Z

Please consider this use case: #195
It would be great if the tooltip will be kept open even during internal data updating.

le-jeu · 2021-02-17T11:32:16Z

It is, using standard portal.bindPopup(content).openPopup()

johnd0e · 2021-02-17T11:50:00Z

It is, using standard portal.bindPopup(content).openPopup()

But what will happen when placeholder will be replaced with real entity? What if it will be moved from one faction to another, etc.
(Of course I will test them myself too, when PR will be ready)

le-jeu · 2021-02-17T12:07:01Z

placeholders are not destroyed but updated when we get real data.

Portal entities are only deleted when removed from rendering:

portal deleted according to getEtentities data (does this really occurs?)

no selected and absent from getEntities data (zoom <15, not in view, not an endpoint)

On any other situtation, the portal marker is the same and only receive updates until removed from rendering.

the main feature (permanent portal entities) is ready for testing

the portalSelected related code (renderPortalDetails() and selectPortal()) needs better plumbing but the behavior is saner than before

le-jeu

Link/field timestamp may be higher than the portal timestamp, breaking the logic
Maybe use a timestamp minus decay time...

johnd0e · 2021-02-18T10:31:27Z

core/code/portal_detail_display.js

@@ -9,8 +9,8 @@ window.resetScrollOnNewPortal = function() {
  }
 };

-window.renderPortalDetails = function(guid) {
-  selectPortal(window.portals[guid] ? guid : null);
+window.renderPortalDetails = function(guid, dontSelect) {


Suggested change

window.renderPortalDetails = function(guid, dontSelect) {

window.renderPortalDetails = function(guid) {

I haven't reviewed this PR as a whole, but still believe that here we need slightly different logic:
do not select portal if it is already set as current without additional arguments: #442 (comment)

In case if we will found that it is still required in some special case, then we better add opposite:

Suggested change

window.renderPortalDetails = function(guid, dontSelect) {

window.renderPortalDetails = function(guid, forceSelect) {

Second looks saner.
My though was to not break (unknonwn) plugins that would assume renderPortalDetails triggers a portalSelected event as long as the portal has its own marker.
But I bet plugins relying on this will eventually update to match the needs

But I bet plugins relying on this will eventually update to match the needs

So am I.
Let's try to remove dontSelect, may be taht will be best for all.

le-jeu · 2021-02-19T17:47:19Z

core/code/map_data_render.js

+
+    marker.updateDetails(data);
+
+    window.runHooks('portalAdded', {portal: marker, previousData: previousData});


the hook is misleading now since the marker is the same marker. Only the data changed.
In this case, we are closer to the portalDetailLoaded but with eventually no details (i.e. from core to summary/extended)

the hook is misleading

But that was expected, that's why previousData provided. I suppose we should leave it as it.
May be we should add some property to denote placeholders.

But we can revise that from scratch when implementing #217.

johnd0e · 2021-02-20T12:14:14Z

breaking the logic

Could you point exact places in code? (I haven't studied this PR yet).

le-jeu · 2021-02-20T13:35:07Z

I introduced link/field timestamp as placeholder timestamp instead of the value 0.
The value 0 was set to be sure that any complete data (from getEntities or getPortalDeails) takes priority over placeholder data (that are forged by iitc).

In many cases, the link/field timestamp is more recent that the portal timestamp. The previous rule that portal data is ignored if timestamp is older prevents the marker to update from detailed data (while getPortalDetails are always up to date to my knowledge).

I fix this following this new rule about placeholder (provided the field/link timestamp):

if the placeholder is more recent and has a different team, it takes priority
if the placeholder is older or has the same team, no update
That is, trust the placeholder team as long it's more recent that the known portal data.

Uses cases:

a selected portal with detail loaded, but outside the view, this portal switches team and a link is thrown, the placeholder will update the selected portal data.
intel send a link while it has just been destroyed (but details are correct), the placeholder won't update the portal details

johnd0e · 2021-02-20T17:33:45Z

if the placeholder is more recent and has a different team, it takes priority

And what if that portal had links/fields attached?

P.S.
If we continue this way, than we could parse messages plexts digging for updates...

le-jeu · 2021-02-20T17:44:39Z

And what if that portal had links/fields attached?

I don't understand your question
placeholders come from links/fields, so they do have links/fields attached
that's the point of this rule, if intel says that there is a blue field at T+1 on the portal A, while the data about portal A being green at T is invalidate (and we don't get data on this portal because of request's bounding box)

The current code is also trusting the team of placeholders, whatever the timestamp of the attached field/links.

P.S.
If we continue this way, than we could parse messages plexts digging for updates...

A plugin could do this to update the map before getEntities...

johnd0e · 2021-02-20T17:49:22Z

I don't understand your question

Imaging that we have Green portal with links and fields.
And then we get fresher info where same portal (as placeholder) is already Blue.
In this moment our data become inconsistent, as Blue portal cannot have Green links and fields.
So I wonder what are you proposing for such case.

le-jeu · 2021-02-20T17:57:20Z

I don't understand your question

Imaging that we have Green portal with links and fields.
And then we get fresher info where same portal (as placeholder) is already Blue.
In this moment our data become inconsistent, as Blue portal cannot have Green links and fields.
So I wonder what are you proposing for such case.

Since data is fresher, the portal is updated to blue, the links may stays until the the next getEntities request

you should give a look to portalMarker.willUpdate

apply/adapt set(Marker)Style PR IITC-CE#251 by johnd0e commit 6201837

avoid missing details when getEntities replies that the portal has changed while portal details is still fresh (3min) in cache

This reverts commit 1878ec6.

photo/title

le-jeu changed the title ~~Permanent portal entities~~ [WIP] Permanent portal entities Feb 6, 2021

le-jeu marked this pull request as draft February 6, 2021 22:53

le-jeu force-pushed the update_marker branch 3 times, most recently from 228aa0e to 7e9c1e7 Compare February 14, 2021 18:41

le-jeu force-pushed the update_marker branch 4 times, most recently from d2aec21 to dd0c20b Compare February 17, 2021 10:18

johnd0e mentioned this pull request Feb 17, 2021

Hooks may behave not as expected #442

Open

johnd0e reviewed Feb 17, 2021

View reviewed changes

le-jeu marked this pull request as ready for review February 17, 2021 12:12

le-jeu commented Feb 17, 2021

View reviewed changes

le-jeu force-pushed the update_marker branch from c4560ee to 21faf55 Compare February 17, 2021 18:10

le-jeu changed the title ~~[WIP] Permanent portal entities~~ Permanent portal entities Feb 17, 2021

johnd0e reviewed Feb 18, 2021

View reviewed changes

le-jeu commented Feb 19, 2021

View reviewed changes

le-jeu force-pushed the update_marker branch from 5257ed8 to bb5e120 Compare February 19, 2021 21:48

le-jeu force-pushed the update_marker branch from bb5e120 to 4129d76 Compare February 20, 2021 13:41

le-jeu added 26 commits January 8, 2022 13:53

set marker style once

e329408

apply/adapt set(Marker)Style PR IITC-CE#251 by johnd0e commit 6201837

highlighter may return the applied style

ae46d7a

add event name to portalSelected hook

eae4ed6

remove portalMarker.reset

e8a144f

render portal base on portal marker details

a24f110

add portal health function

68443e8

use portal detail cache on new portal entities

fb4ef1e

remove obsolete data from portal details cache

2bf85b0

avoid missing details when getEntities replies that the portal has changed while portal details is still fresh (3min) in cache

placeholder: use timestamp from link/field

eb9344a

fixup 97ed9a7

bcce300

portalmarker: init select state

b0f9a57

render details on new selected portal entity

9303425

fix: use this._details

c053363

better placeholder handling

78e5ef0

render details: select if forced or not selected

9c0bf28

handle missing history

5e61254

keep portals attached to visible links/fields

91215e2

Revert "keep portals attached to visible links/fields"

0840652

This reverts commit 1878ec6.

dont recreate markers when outside view

3066487

marker: update all data available

1b13892

portal edits change the timestamp

009567c

photo/title

apply to all placeholders

f13ff25

move portal style as statics of L.PortalMarker

3fda197

portal location edit changes the timestamp

38bfdb7

add comment IITC-CE#1

e62410f

fixup! placeholder: use timestamp from link/field

8a4e4cf

le-jeu force-pushed the update_marker branch from f0c0f1a to 8a4e4cf Compare January 8, 2022 13:00

lint

cbd43a1

MysticJay added the core label Jan 21, 2024

modos189 mentioned this pull request Nov 18, 2024

Update PR: Permanent portal entities #783

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Permanent portal entities #437

Permanent portal entities #437

le-jeu commented Feb 6, 2021 •

edited

Loading

johnd0e commented Feb 15, 2021

johnd0e Feb 17, 2021

le-jeu Feb 17, 2021

johnd0e Feb 17, 2021

johnd0e commented Feb 17, 2021

le-jeu commented Feb 17, 2021

johnd0e commented Feb 17, 2021

le-jeu commented Feb 17, 2021

le-jeu left a comment

johnd0e Feb 18, 2021 •

edited

Loading

le-jeu Feb 18, 2021

johnd0e Feb 18, 2021

le-jeu Feb 19, 2021

johnd0e Feb 20, 2021

johnd0e commented Feb 20, 2021

le-jeu commented Feb 20, 2021

johnd0e commented Feb 20, 2021

le-jeu commented Feb 20, 2021 •

edited

Loading

johnd0e commented Feb 20, 2021

le-jeu commented Feb 20, 2021 •

edited

Loading

	window.renderPortalDetails = function(guid, dontSelect) {
	window.renderPortalDetails = function(guid) {


		marker.updateDetails(data);

		window.runHooks('portalAdded', {portal: marker, previousData: previousData});

Permanent portal entities #437

Are you sure you want to change the base?

Permanent portal entities #437

Conversation

le-jeu commented Feb 6, 2021 • edited Loading

Changes

class L.PortalMarker

Hooks portalSelected

Portal Details Cache

Before

Now

Placeholder entities

Sidenotes

deprecated:

related functions (next candidates for refactoring/deprecation)

johnd0e commented Feb 15, 2021

johnd0e Feb 17, 2021

Choose a reason for hiding this comment

le-jeu Feb 17, 2021

Choose a reason for hiding this comment

johnd0e Feb 17, 2021

Choose a reason for hiding this comment

johnd0e commented Feb 17, 2021

le-jeu commented Feb 17, 2021

johnd0e commented Feb 17, 2021

le-jeu commented Feb 17, 2021

le-jeu left a comment

Choose a reason for hiding this comment

johnd0e Feb 18, 2021 • edited Loading

Choose a reason for hiding this comment

le-jeu Feb 18, 2021

Choose a reason for hiding this comment

johnd0e Feb 18, 2021

Choose a reason for hiding this comment

le-jeu Feb 19, 2021

Choose a reason for hiding this comment

johnd0e Feb 20, 2021

Choose a reason for hiding this comment

johnd0e commented Feb 20, 2021

le-jeu commented Feb 20, 2021

johnd0e commented Feb 20, 2021

le-jeu commented Feb 20, 2021 • edited Loading

johnd0e commented Feb 20, 2021

le-jeu commented Feb 20, 2021 • edited Loading

le-jeu commented Feb 6, 2021 •

edited

Loading

class `L.PortalMarker`

Hooks `portalSelected`

johnd0e Feb 18, 2021 •

edited

Loading

le-jeu commented Feb 20, 2021 •

edited

Loading

le-jeu commented Feb 20, 2021 •

edited

Loading