Be sure to not register again asset with same url, create a-assets if missing (#824)

vincentfretin · web-flow · commit a34aa27aad1e · 2025-11-10T09:54:13.000-08:00
* Be sure to not register again asset with same url, create a-assets if missing

* Show error when id is already taken by another asset or entity

* Show 'Texture already loaded' similar to the other errors

* Use Name instead of id in error message
diff --git a/src/components/modals/ModalTextures.js b/src/components/modals/ModalTextures.js
@@ -4,32 +4,12 @@ import { faSearch } from '@fortawesome/free-solid-svg-icons';
 import { AwesomeIcon } from '../AwesomeIcon';
 import Events from '../../lib/Events';
 import Modal from './Modal';
-import { insertNewAsset } from '../../lib/assetsUtils';
-
-function getFilename(url, converted = false) {
-  var filename = url.split('/').pop();
-  if (converted) {
-    filename = getValidId(filename);
-  }
-  return filename;
-}
-
-function isValidId(id) {
-  // The correct re should include : and . but A-frame seems to fail while accessing them
-  var re = /^[A-Za-z]+[\w-]*$/;
-  return re.test(id);
-}
-
-function getValidId(name) {
-  // info.name.replace(/\.[^/.]+$/, '').replace(/\s+/g, '')
-  return name
-    .split('.')
-    .shift()
-    .replace(/\s/, '-')
-    .replace(/^\d+\s*/, '')
-    .replace(/[\W]/, '')
-    .toLowerCase();
-}
+import {
+  getFilename,
+  getIdFromUrl,
+  insertNewAsset,
+  isValidId
+} from '../../lib/assetsUtils';
 
 export default class ModalTextures extends React.Component {
   static propTypes = {
@@ -158,15 +138,20 @@ export default class ModalTextures extends React.Component {
     var self = this;
     function onImageLoaded(img) {
       var src = self.preview.current.src;
+      var name = getFilename(src, true);
+      var existingAssetId = getIdFromUrl(src);
+      if (existingAssetId) {
+        name = existingAssetId;
+      }
       self.setState({
         preview: {
           width: self.preview.current.naturalWidth,
           height: self.preview.current.naturalHeight,
           src: src,
           id: '',
-          name: getFilename(src, true),
+          name: name,
           filename: getFilename(src),
-          type: 'new',
+          type: existingAssetId ? 'asset' : 'new',
           loaded: true,
           value: 'url(' + src + ')'
         }
@@ -179,12 +164,6 @@ export default class ModalTextures extends React.Component {
     this.imageName.current.focus();
   };
 
-  onNameKeyUp = (event) => {
-    if (event.keyCode === 13 && this.isValidAsset()) {
-      this.addNewAsset();
-    }
-  };
-
   onNameChanged = (event) => {
     var state = this.state.preview;
     state.name = event.target.value;
@@ -193,6 +172,7 @@ export default class ModalTextures extends React.Component {
 
   toggleNewDialog = () => {
     this.setState({ addNewDialogOpened: !this.state.addNewDialogOpened });
+    this.clear();
   };
 
   clear() {
@@ -216,23 +196,19 @@ export default class ModalTextures extends React.Component {
     this.setState({ newUrl: e.target.value });
   };
 
-  isValidAsset() {
-    let validUrl = isValidId(this.state.preview.name);
-    let validAsset = this.state.preview.loaded && validUrl;
-    return validAsset;
-  }
-
   addNewAsset = () => {
-    var self = this;
+    if (this.state.preview.type === 'asset') {
+      return;
+    }
+
     insertNewAsset(
       'img',
       this.state.preview.name,
       this.state.preview.src,
-      true,
-      function () {
-        self.generateFromAssets();
-        self.setState({ addNewDialogOpened: false });
-        self.clear();
+      () => {
+        this.generateFromAssets();
+        this.setState({ addNewDialogOpened: false });
+        this.clear();
       }
     );
   };
@@ -244,15 +220,20 @@ export default class ModalTextures extends React.Component {
   renderRegistryImages() {
     var self = this;
     let selectSample = function (image) {
+      let name = getFilename(image.name, true);
+      const existingAssetId = getIdFromUrl(image.src);
+      if (existingAssetId) {
+        name = existingAssetId;
+      }
       self.setState({
         preview: {
           width: image.width,
           height: image.height,
           src: image.src,
           id: '',
-          name: getFilename(image.name, true),
+          name: name,
           filename: getFilename(image.src),
-          type: 'registry',
+          type: existingAssetId ? 'asset' : 'registry',
           loaded: true,
           value: 'url(' + image.src + ')'
         }
@@ -291,8 +272,14 @@ export default class ModalTextures extends React.Component {
     let isOpen = this.state.isOpen;
     let preview = this.state.preview;
 
-    let validUrl = isValidId(this.state.preview.name);
-    let validAsset = this.isValidAsset();
+    let validId = isValidId(this.state.preview.name);
+    let assetIdTaken =
+      validId && !!document.getElementById(this.state.preview.name);
+    let validAsset =
+      this.state.preview.loaded &&
+      validId &&
+      !assetIdTaken &&
+      this.state.preview.type !== 'asset';
 
     let addNewAssetButton = this.state.addNewDialogOpened
       ? 'BACK'
@@ -343,13 +330,32 @@ export default class ModalTextures extends React.Component {
               <input
                 ref={this.imageName}
                 className={
-                  this.state.preview.name.length > 0 && !validUrl ? 'error' : ''
+                  this.state.preview.name.length > 0 &&
+                  (!validId || assetIdTaken)
+                    ? 'error'
+                    : ''
                 }
+                readOnly={preview.type === 'asset'}
                 type="text"
                 value={this.state.preview.name}
                 onChange={this.onNameChanged}
-                onKeyUp={this.onNameKeyUp}
+                onKeyUp={(event) => {
+                  if (event.keyCode === 13 && validAsset) {
+                    this.addNewAsset();
+                  }
+                }}
               />
+              {preview.type !== 'asset' && assetIdTaken && (
+                <div className="iderror">
+                  Name already taken by another asset or entity
+                </div>
+              )}
+              {this.state.preview.name.length > 0 && !validId && (
+                <div className="iderror">Name is not valid</div>
+              )}
+              {preview.type === 'asset' && (
+                <div className="iderror">Texture already loaded</div>
+              )}
               <img
                 ref={this.preview}
                 width="155px"
diff --git a/src/components/widgets/TextureWidget.js b/src/components/widgets/TextureWidget.js
@@ -1,66 +1,7 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import Events from '../../lib/Events';
-
-function getUrlFromId(assetId) {
-  return (
-    assetId.length > 1 &&
-    document.querySelector(assetId) &&
-    document.querySelector(assetId).getAttribute('src')
-  );
-}
-
-function GetFilename(url) {
-  if (url) {
-    var m = url.toString().match(/.*\/(.+?)\./);
-    if (m && m.length > 1) {
-      return m[1];
-    }
-  }
-  return '';
-}
-
-function insertNewAsset(type, id, src) {
-  var element = null;
-  switch (type) {
-    case 'img':
-      {
-        element = document.createElement('img');
-        element.id = id;
-        element.src = src;
-      }
-      break;
-  }
-  if (element) {
-    document.getElementsByTagName('a-assets')[0].appendChild(element);
-  }
-}
-
-function insertOrGetImageAsset(src) {
-  var id = GetFilename(src);
-  // Search for already loaded asset by src
-  var element = document.querySelector("a-assets > img[src='" + src + "']");
-
-  if (element) {
-    id = element.id;
-  } else {
-    // Check if first char of the ID is a number (Non a valid ID)
-    // In that case a 'i' preffix will be added
-    if (!isNaN(parseInt(id[0], 10))) {
-      id = 'i' + id;
-    }
-    if (document.getElementById(id)) {
-      var i = 1;
-      while (document.getElementById(id + '_' + i)) {
-        i++;
-      }
-      id += '_' + i;
-    }
-    insertNewAsset('img', id, src);
-  }
-
-  return id;
-}
+import { getUrlFromId } from '../../lib/assetsUtils';
 
 export default class TextureWidget extends React.Component {
   static propTypes = {
@@ -197,12 +138,8 @@ export default class TextureWidget extends React.Component {
       if (!image) {
         return;
       }
-      var value = image.value;
-      if (image.type !== 'asset') {
-        var assetId = insertOrGetImageAsset(image.src);
-        value = '#' + assetId;
-      }
 
+      var value = image.value;
       if (this.props.onChange) {
         this.props.onChange(this.props.name, value);
       }
diff --git a/src/lib/assetsUtils.js b/src/lib/assetsUtils.js
@@ -1,20 +1,49 @@
-export function insertNewAsset(
-  type,
-  id,
-  src,
-  anonymousCrossOrigin,
-  onLoadedCallback
-) {
-  var element = null;
+export function getUrlFromId(assetId) {
+  return (
+    assetId.length > 1 &&
+    document.querySelector(assetId) &&
+    document.querySelector(assetId).getAttribute('src')
+  );
+}
+
+export function getIdFromUrl(url) {
+  return document.querySelector("a-assets > [src='" + url + "']")?.id;
+}
+
+export function getFilename(url, converted = false) {
+  var filename = url.split('/').pop();
+  if (converted) {
+    filename = getValidId(filename);
+  }
+  return filename;
+}
+
+export function isValidId(id) {
+  // The correct re should include : and . but A-frame seems to fail while accessing them
+  var re = /^[A-Za-z]+[\w-]*$/;
+  return re.test(id);
+}
+
+export function getValidId(name) {
+  // info.name.replace(/\.[^/.]+$/, '').replace(/\s+/g, '')
+  return name
+    .split('.')
+    .shift()
+    .replace(/\s/, '-')
+    .replace(/^\d+\s*/, '')
+    .replace(/[\W]/, '')
+    .toLowerCase();
+}
+
+export function insertNewAsset(type, id, src, onLoadedCallback = undefined) {
+  let element;
   switch (type) {
     case 'img':
       {
         element = document.createElement('img');
         element.id = id;
         element.src = src;
-        if (anonymousCrossOrigin) {
-          element.crossOrigin = 'anonymous';
-        }
+        element.crossOrigin = 'anonymous';
       }
       break;
   }
@@ -25,6 +54,17 @@ export function insertNewAsset(
         onLoadedCallback();
       }
     };
-    document.getElementsByTagName('a-assets')[0].appendChild(element);
+
+    let assetsEl = document.querySelector('a-assets');
+    if (!assetsEl) {
+      assetsEl = document.createElement('a-assets');
+      var sceneEl = document.querySelector('a-scene');
+      if (!sceneEl) {
+        throw new Error('No a-scene element found to append a-assets to');
+      }
+      sceneEl.appendChild(assetsEl);
+    }
+
+    assetsEl.appendChild(element);
   }
 }
diff --git a/src/style/textureModal.styl b/src/style/textureModal.styl
@@ -94,6 +94,13 @@
   margin 8px 0
   width 144px
 
+.preview .iderror
+  background #fff
+  color #a00
+  margin-bottom 8px
+  padding 3px 5px
+  width 148px
+
 .preview button
   width 155px
 
