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

New scene function #509

Merged
merged 30 commits into from
Jun 21, 2024
Merged

New scene function #509

merged 30 commits into from
Jun 21, 2024

Conversation

Algorush
Copy link
Collaborator

@Algorush Algorush commented Apr 20, 2024

Added a newScene function to clear the old scene and create an empty scene. Perhaps there is something else to consider in this function, @kfarr ?
In the next commits I will add call of this function to the appropriate places in the code.
#485

Copy link

netlify bot commented Apr 20, 2024

Deploy Preview for 3dstreet-core-builds ready!

Name Link
🔨 Latest commit 81cd211
🔍 Latest deploy log https://app.netlify.com/sites/3dstreet-core-builds/deploys/665a8d32c8dd8600084e8a4a
😎 Deploy Preview https://deploy-preview-509--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.

@Algorush
Copy link
Collaborator Author

Algorush commented Apr 24, 2024

what should be the initial structure of Blank Scene? Like this one?

<a-entity id="street-container" data-layer-name="3D Street Layers" data-layer-show-children>
   <a-entity id="default-street" set-loader-from-hash></a-entity>
</a-entity>
<a-entity id="reference-layers" data-layer-name="Reference Layers" data-layer-show-children></a-entity>
<a-entity id="environment" data-layer-name="Environment" street-environment="preset: day;"></a-entity>

@Algorush
Copy link
Collaborator Author

Algorush commented Apr 25, 2024

I have added newScene funtion as STREET.utils.newScene. I have also added a call to this function in the appropriate scene loading methods, including in the Editor code: 3DStreet/3dstreet-editor#420

@kfarr kfarr requested a review from vincentfretin June 19, 2024 03:49
@kfarr
Copy link
Collaborator

kfarr commented Jun 19, 2024

things this does not reset yet:

  • * currently selected object blue bounding box
  • * long / lat / elevation in geopanel above scene title
  • only show for pro users
  • * camera pose (position and rotation) -- this is probably ok to leave for a later date

image

Copy link
Collaborator

@vincentfretin vincentfretin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While reviewing, I ended up refactoring the buttons to properly use the Button component.

… with sceneEl here once we will have undo/redo
@kfarr
Copy link
Collaborator

kfarr commented Jun 19, 2024

similar to scene title edit modal, when the street-geo component is updated, we should also fire an event such that the react app is listening for this event and 'resets' the geo panel?

@vincentfretin
Copy link
Collaborator

similar to scene title edit modal, when the street-geo component is updated, we should also fire an event such that the react app is listening for this event and 'resets' the geo panel?

@kfarr and I started a GeoContext on @kfarr machine. Almost finished, we just need to reset the context value after calling newScene. We will finish that Friday.

@vincentfretin
Copy link
Collaborator

Also probably put the "New" button behind a flag or just comment it before merging.
Like discussed, just having a new blank scene, without some templates or onboarding is confusing for the user.

@kfarr
Copy link
Collaborator

kfarr commented Jun 19, 2024

@vincentfretin i committed our WIP work and tested, all seems to work with exception of resetting geo stuff as you noted so we can resume that on Fri

@kfarr kfarr merged commit d33bdca into main Jun 21, 2024
1 check passed
@kfarr kfarr deleted the new-scene-function branch June 21, 2024 16:49
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.

3 participants