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 environment node support #300

Merged
merged 3 commits into from
Jul 6, 2023
Merged

add environment node support #300

merged 3 commits into from
Jul 6, 2023

Conversation

Algorush
Copy link
Collaborator

I renamed branch but github added changes to the old one. I hope it's okay.
To support loading the envirinment and other nodes, I need to make changes to 3dstreet-editor. Here is the link to the PR with the changes: 3DStreet/3dstreet-editor#195

@netlify
Copy link

netlify bot commented Jun 15, 2023

Deploy Preview for 3dstreet-core-builds ready!

Name Link
🔨 Latest commit 58f110e
🔍 Latest deploy log https://app.netlify.com/sites/3dstreet-core-builds/deploys/64a30f6968395b000858fe70
😎 Deploy Preview https://deploy-preview-300--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 configuration.

@kfarr
Copy link
Collaborator

kfarr commented Jun 16, 2023

Note to self the testing process is similar to: #287 (comment)

@kfarr
Copy link
Collaborator

kfarr commented Jun 20, 2023

Hey @Algorush I tested this and it doesn't quite work yet, but I think I made some mistake in not explaining the use case very well. I will try to explain in more detail, here are the desired steps:

  • Open up a 3dstreet scene, such as http://localhost:3333/#https://streetmix.net/kfarr/3/example-street (using the testing method linked above)
  • Select the environment layer (entity), and then scroll down to the street-environment component
  • Change the preset value from day to night
  • Save as JSON
  • Inspect JSON file on local machine in text editor, expect to see environment entity with street-environment component preset attribute set to night
  • Open JSON from 3dstreet and see a night scene
  • Instead, what happens with the current version of this PR is that the scene gets brighter -- I think that is because it is adding the lights from the JSON file to the existing lights added by default in the scene

I think there are a few issues here:

  • the street-environment component is not listed as a component value to be saved in the json save utils. That should be added, that should be part of this ticket.
  • the street-environment component does not support dynamically changing from night into day. It should. This could be a separate ticket. (It is a common problem with a-frame components that they work when initialized once in a scene but need extra work to allow dynamic updating) Here is a separate ticket for this: street-environment component should allow setting after load #303
  • we should not save any of the children of the environment id entity (the Environment layer). They should instead be controlled by the street-environment component of the environment id entity. That will probably also need your help to update in json save utils as part of this ticket.

@Algorush
Copy link
Collaborator Author

Algorush commented Jun 20, 2023

Thanks for the detailed explanation of how saving the environment should work. I will change it.
In this PR I've made the environment save, taking into account the case if the user will change the light entities directly

@kfarr kfarr merged commit 1f20a49 into main Jul 6, 2023
7 checks passed
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.

None yet

2 participants