Change street component parameters #512

Algorush · 2024-05-03T20:44:58Z

Added the ability to change the parameters of the street component (showGround, showVehicles, showStriping) without reloading the scene. It remains to add globalAnimated. So this PR not ready yet.
Also, now there is no need to rename street to not-street, like streetmix-loader and intersection. I added a check that the scene is loaded from jsonFile and do not run the update function in these components

netlify · 2024-05-03T20:45:40Z

✅ Deploy Preview for 3dstreet-core-builds ready!

Name	Link
🔨 Latest commit	`4fc7a90`
🔍 Latest deploy log	https://app.netlify.com/sites/3dstreet-core-builds/deploys/66354ccc5d63e30008c5295b
😎 Deploy Preview	https://deploy-preview-512--3dstreet-core-builds.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

# Conflicts: # src/lib/aframe-loader-3dtiles-component.js # src/lib/aframe-loader-3dtiles-component.min.js

vincentfretin · 2024-06-25T16:39:48Z

src/index.js

+    this.toggleEntitiesVisibillity(vehicleEntities, showVehicles);
+  },
+  toggleGround: function (showGround) {
+    const groundEntities = Array.from(document.querySelectorAll('.ground-left, .ground-right'));


Array.from is not needed.

Algorush · 2024-06-27T20:45:29Z

Updated this PR to the latest version of the 3dstreet repository. This issue has now been fixed: #673

kfarr

please add a new property to street component such as synchronized true/false

if synchronized is true (it should be true by default at the start) then it will create children entities; after creating children entities for the first time synchronized should be set to false; then use that instead of the global STREET.sourceType

kfarr · 2024-06-28T16:27:57Z

src/components/streetplan-loader.js

@@ -78,6 +78,8 @@ AFRAME.registerComponent('streetplan-loader', {
      return;
    }

+    STREET.sourceType = 'streetplanURL'; // it also could be jsonFile/streetmixURL


@Algorush there will be more than 1 street per scene so I don't understand why this needs to be set ?

Yes, I thought about more than 1 street per scene and was tested with this case also. STREET.sourceType refers to the currently loaded street. And global STREET is not good place for it, yeah...
And yes, maybe its not good idea, but I didn’t find a better option when was thinking about it.

kfarr · 2024-06-28T16:29:34Z

src/index.js

-    // fired once at start and at each subsequent change of a schema value
-    var data = this.data;
+    // do not call the update function when the component is initially loaded and sourceType is jsonFile
+    if (STREET.sourceType === 'jsonFile' && this._initLoad) {


@Algorush we should not be using a global for sourceType that describes this specific entity; multiple streets may be loading from different types of sources on the same scene;

kfarr · 2024-06-28T16:49:02Z

src/index.js

+    const el = this.el;
+
+    // do not call update function while loading scene from JSON file
+    if (STREET.sourceType === 'jsonFile') return;


same feedback, not a global

Algorush · 2024-06-29T18:38:28Z

please add a new property to street component such as synchronized true/false

if synchronized is true (it should be true by default at the start) then it will create children entities; after creating children entities for the first time synchronized should be set to false; then use that instead of the global STREET.sourceType

Ok, it works good, when I added synchronized flag for streetmix-loader and stretplan-loader too. But I dont understand the logic why synchronized should be true by default?

Algorush · 2024-07-01T02:37:24Z

question: should visibility options showGround, showStripping, showVehicles be saved in JSON or should they only be valid during the session?
I have now made that these parameters are updated without reloading the scene. The reason for this was that if the user changed one of these parameters, the entire street would be reloaded and all his changes would be lost.

vincentfretin

synchronized means the entities are recreated each time from streetmix json.
If we don't want the behavior, we set synchronized to false after loading the json the first time.

vincentfretin · 2024-07-01T08:39:37Z

src/index.js

@@ -185,6 +228,8 @@ AFRAME.registerComponent('streetmix-loader', {
          JSON.stringify({ streetmixSegmentsMetric: streetmixSegments })
        );
        el.emit('streetmix-loader-street-loaded');
+        // the streetmix data has been loaded, set the synchronized flag
+        data.synchronized = true;


Don't set it like that, otherwise the change won't be properly exported. Use
this.el.setAttribute('street', 'synchronized', true}

The current code may work on aframe 1.5.0 but I think it won't work on aframe 1.6.0 or master because of internals restructuring, calling getAttribute('street') won't return the modified data object.

Thanks for clarifying. Yes, I have read the documentation why it is better to use setAttribute. But in general it would be better to use an internal variable of the component in these cases, which is saved in JSON, but will not be displayed in the editor. There is no such mechanism in the component implementation, but it could be done using __innerAttrName and internal get/set functions

@Algorush it's ok for this variable to be exposed to the end-user

vincentfretin · 2024-07-01T08:41:43Z

src/index.js

@@ -171,7 +214,7 @@ AFRAME.registerComponent('streetmix-loader', {
          );
        }

-        el.setAttribute('data-layer-name', 'Streetmix • ' + streetmixName);
+        el.setAttribute('data-layer-name', 'Streetmix � ' + streetmixName);


unicode issue?

yes, something strange with my code editor. Will fix in next commit too

vincentfretin · 2024-07-01T08:42:15Z

src/index.js

@@ -86,6 +123,8 @@ AFRAME.registerComponent('street', {
      );
      this.el.append(buildingsEl);
    }
+    // the scene has been loaded, set the synchronized flag
+    this.data.synchronized = true;


use setAttribute

kfarr · 2024-07-01T16:27:18Z

@Algorush sorry for the miscommunication, this is close but the logic is inverted.

Instead of "this.data.synchronized = true;" we would like to have synchronize be the variable and the default is true. If the value of synchronize is true, then please yes do the actual element creation; if it is false, then do not. Effectively this is the opposite logic of what you have implemented.

For the avoidance of doubt, after the objects are created initially, then set synchronize to false.

kfarr · 2024-07-03T16:46:56Z

test

import a streetmix scene
save / reload that 3dstreet scene
ensure that the scene is only loaded once per "synchronize" flag
see if we see props panel
if we are able to use the component properites -- great! that's a bonus

Algorush added 3 commits April 30, 2024 21:36

add STREET.sourceType variable

4f6b0fe

move cyclist models from loud-bicycle-game to cyclists

bfb5a41

toggle street parameters without scene reloading

4fc7a90

Algorush mentioned this pull request May 3, 2024

ability to change street component parameters without scene reloading #511

Closed

Algorush added 2 commits June 21, 2024 16:05

update aframe-loader-3dtiles-component

9e99dfb

Merge branch 'main' of https://github.com/3DStreet/3dstreet

42f3807

# Conflicts: # src/lib/aframe-loader-3dtiles-component.js # src/lib/aframe-loader-3dtiles-component.min.js

Algorush mentioned this pull request Jun 25, 2024

saving and reloading a scene makes streetmix / street component properties not visible #673

Closed

vincentfretin reviewed Jun 25, 2024

View reviewed changes

Algorush added 5 commits June 27, 2024 16:23

Merge branch 'main' into change-street-component-parameters

1148f91

add public, dist, .storybook folders to .eslintignore

9e3f5f1

Replace index.html with version from main branch

a06b3f4

remove comment, change var to const

0098e5e

fix lint error: remove trailing spaces

c24c29a

kfarr reviewed Jun 28, 2024

View reviewed changes

add synchronized flag instead of global STREET.sourceType

ebe4a5b

vincentfretin reviewed Jul 1, 2024

View reviewed changes

Algorush added 2 commits July 1, 2024 14:27

change synchronized to synchronize and revert logic

b859507

dont return after updating showGround, showVehicles, showStriping

44fa8f2

Merge branch 'main' into change-street-component-parameters

d6a28d2

kfarr merged commit 3517f96 into main Jul 3, 2024
1 check passed

kfarr deleted the change-street-component-parameters branch July 3, 2024 17:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change street component parameters #512

Change street component parameters #512

Algorush commented May 3, 2024

netlify bot commented May 3, 2024 •

edited

Loading

vincentfretin Jun 25, 2024

Algorush commented Jun 27, 2024

kfarr left a comment

kfarr Jun 28, 2024

Algorush Jun 28, 2024 •

edited

Loading

kfarr Jun 28, 2024

kfarr Jun 28, 2024

Algorush commented Jun 29, 2024

Algorush commented Jul 1, 2024 •

edited

Loading

vincentfretin left a comment

vincentfretin Jul 1, 2024

vincentfretin Jul 1, 2024

Algorush Jul 1, 2024 •

edited

Loading

kfarr Jul 1, 2024

vincentfretin Jul 1, 2024

Algorush Jul 1, 2024

vincentfretin Jul 1, 2024

kfarr commented Jul 1, 2024 •

edited

Loading

kfarr commented Jul 3, 2024 •

edited

Loading

Change street component parameters #512

Change street component parameters #512

Conversation

Algorush commented May 3, 2024

netlify bot commented May 3, 2024 • edited Loading

✅ Deploy Preview for 3dstreet-core-builds ready!

Choose a reason for hiding this comment

Algorush commented Jun 27, 2024

kfarr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Algorush Jun 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Algorush commented Jun 29, 2024

Algorush commented Jul 1, 2024 • edited Loading

vincentfretin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Algorush Jul 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kfarr commented Jul 1, 2024 • edited Loading

kfarr commented Jul 3, 2024 • edited Loading

netlify bot commented May 3, 2024 •

edited

Loading

Algorush Jun 28, 2024 •

edited

Loading

Algorush commented Jul 1, 2024 •

edited

Loading

Algorush Jul 1, 2024 •

edited

Loading

kfarr commented Jul 1, 2024 •

edited

Loading

kfarr commented Jul 3, 2024 •

edited

Loading