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

Add first version to load JSON from URL #304

Merged
merged 5 commits into from
Jun 26, 2023
Merged

Add first version to load JSON from URL #304

merged 5 commits into from
Jun 26, 2023

Conversation

Algorush
Copy link
Collaborator

Now it works with files from localhost, but didnt tested with remote files yet

@netlify
Copy link

netlify bot commented Jun 20, 2023

Deploy Preview for 3dstreet-core-builds ready!

Name Link
🔨 Latest commit 93c7507
🔍 Latest deploy log https://app.netlify.com/sites/3dstreet-core-builds/deploys/6499227735fa6b0008375fd2
😎 Deploy Preview https://deploy-preview-304--3dstreet-core-builds.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@kfarr
Copy link
Collaborator

kfarr commented Jun 23, 2023

Hi @Algorush I'm curious how you went about testing this? Here is what I did:

Testing method for local:
http://localhost:7001/#/examples/json/crash-scene.json

Testing method for remote:
https://deploy-preview-304--3dstreet-core-builds.netlify.app/#/examples/json/crash-scene.json

Issue with both local and remote:

  • look at dev console and see scrolling messages of street loader initializing
  • infinite loop loading a street scene pegs CPU and eventually browser runs out of memory resulting in crash
  • It appears set-loader-from-hash is running init repeatedly. Might be related to using async with a-frame lifecycle method, i have not seen this before.
  • Instead, suggest to try move away from async and instead go back to an old fashioned synchronous (blocking) call similar to the call to fetch streetmix street json (here). Yes this might be bad because it might freeze UI as compared to async, but this is loading the scene once and the user has to wait during this time anyway.

Architecture brainstorm for future:

  • Create a save-load-json component intended to be added to the scene entity.
  • In viewer mode detect url hash and load the scene from json via this component
  • In editor mode use this component for save and load functions

@Algorush
Copy link
Collaborator Author

Algorush commented Jun 23, 2023

Hi @kfarr . Sorry, I forgot to say about this issue with repeatedly set-loader-from-hash init. This happens when the set-loader-from-hash component is in json. It calls the add scene function from JSON with the set-loader-from-hash component on the a-entity#default-street element. And when the component is initialized, the process is repeated and looped.
In this PR I modified the json-utils.js code to save JSON without the set-loader-from-hash component. And if using the new version of JSON, then there is no looping. Probably it was necessary to change the version of json-utils. Also yesterday I was working on options to not initialize this component twice, but have not finished and uploaded this code yet.
I understood about async/await. Okay, I'll use a blocking call

@Algorush
Copy link
Collaborator Author

Algorush commented Jun 23, 2023

Maybe also such a test case? That is, downloading from a third-party source. Or not at this stage?
http://localhost:7001/#https://deploy-preview-304--3dstreet-core-builds.netlify.app/examples/json/crash-scene.json

@kfarr
Copy link
Collaborator

kfarr commented Jun 23, 2023

@Algorush thanks for the updates, it sounds like you're aware of these issues and I'll wait to test again until you do some more commits.

In the test case you provided yes it makes sense in theory, but the local page is loaded with http protocol so I don't think javascript can access https URLs and I think netlify forces https which means the json probably won't load.

@Algorush
Copy link
Collaborator Author

I've added checks in two places to see if the component set-loader-from-hash is inside the JSON. In one place after downloading the file via XHR. And in json-utils I also added a check in case the component is loaded from the editor mode with the 'load JSON' button

@Algorush
Copy link
Collaborator Author

@Algorush thanks for the updates, it sounds like you're aware of these issues and I'll wait to test again until you do some more commits.

In the test case you provided yes it makes sense in theory, but the local page is loaded with http protocol so I don't think javascript can access https URLs and I think netlify forces https which means the json probably won't load.

that is, do not consider the case of downloading a file from a third-party source yet?

@kfarr kfarr linked an issue Jun 25, 2023 that may be closed by this pull request
@kfarr
Copy link
Collaborator

kfarr commented Jun 26, 2023

Thanks I've started testing:

  • test opening / changing / saving / re-opening local files
  • confirming that streetmix urls still work as expected
  • test remote files

for later / separate tickets:

@kfarr kfarr merged commit 842b62a into main Jun 26, 2023
@kfarr kfarr deleted the json-from-url branch June 26, 2023 06:15
@kfarr kfarr restored the json-from-url branch June 26, 2023 15:23
@kfarr kfarr deleted the json-from-url branch June 26, 2023 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support loading a 3dstreet json file from URL hash
2 participants