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

Hooks may behave not as expected #442

Open
johnd0e opened this issue Feb 17, 2021 · 4 comments
Open

Hooks may behave not as expected #442

johnd0e opened this issue Feb 17, 2021 · 4 comments
Labels
development general development issue enhancement New feature or request

Comments

@johnd0e
Copy link
Contributor

johnd0e commented Feb 17, 2021

Here I'm going to list misc. unrelated weirdness.

  1. Already described in Permanent portal entities #325 (should be addressed in Permanent portal entities #437)
@johnd0e johnd0e added enhancement New feature or request development general development issue labels Feb 17, 2021
@johnd0e
Copy link
Contributor Author

johnd0e commented Feb 17, 2021

  1. On portal click renderPortalDetails is called thrice:
    • render cached data immediately
    • render response from server (onSuccess )
    • onSuccess also results in reparse entity -> portalDetailLoaded -> renderPortalDetails

One is needless (should be addressed in #437)

@johnd0e
Copy link
Contributor Author

johnd0e commented Feb 17, 2021

  1. portalSelected may be called often
    • on explicit user action (another click on same portal). Pretty obvious case
    • on unobvious internal reasons, e.g.
      • explicit click causes portalSelected, then renderPortalDetails -> portalSelected again
    • it's called every 180 s, perhaps at the end of the render pass (renderPortalDetails)
    • etc.?

@le-jeu
Copy link
Contributor

le-jeu commented Feb 17, 2021

portalSelected is trigger by selectPortal on each renderPortalDetails call.

When renderPortalDetails is called directly by a user click (portal or link/selectPortalByLatLng commonly used by plugins)

  • selectPortal is called
  • if portal details is not fresh in cache, request details
    • on response:
      • call renderPortalDetails
      • call createPortalEntity by the portalDetailLoaded hook, ending with a renderPortalDetails call if the data has changed

Also, renderPortalDetails is called at the end of the render pass (move/zoom/idle refresh)

Reverse call tree of renderPortalDetails

  • window.renderPortalDetails
    • processDeletedGameEntityGuids (unselect)
      • processTileData (never called ?)
      • processRenderQueue
    • endRenderPass (for selected)
      • processRenderQueue ⚠️ 1️⃣
    • getPortalDetails response (for selected) ⚠️ 2️⃣
      • window.renderPortalDetails ⚠️ 2️⃣
    • createPortalEntity (for selected)
      • processGameEntities
        • processTileData (never called ?)
        • processRenderQueue ⚠️ 1️⃣
        • hook mapDataEntityInject callback
      • createPlaceholderPortalEntity
        • createLinkEntity/createFieldEntity
          • processGameEntities...
      • hook portalDetailLoaded (for selected)
        • getPortalDetails response ⚠️ 2️⃣
    • marker click
    • zoomToAndShowPortal
      • link in interface/plugins
    • selectPortalByLatLng
      • link in interface/plugins
    • link in interface/plugins
      • search
      • close portal details (unselect)
      • bookmarks, portals list...

@johnd0e
Copy link
Contributor Author

johnd0e commented Feb 18, 2021

Thoughts:

  1. We should consider that all the code relies on that renderPortalDetails will make portal current.
    When portal in not on map yet - that can be deferred, with unexpected consequences - e.g. we click through several portals, but after a second we see that selection returned to some previous.
  2. Anyway to avoid useless hook calls selectPortal should not be called unconditionally.
    I suppose somewhere (in renderPortalDetails or in selectPortal itself) should be some check if (guid !== selectedPortal) { ....
  3. zoomToAndShowPortal/selectPortalByLatLng should be refactored anyway zoomToAndShowPortal and selectPortalByLatLng #93

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

No branches or pull requests

2 participants