speculative fix for failed spatial updates in scenecomposer window

neph1 · neph1 · commit 537c4c724263 · 2024-11-26T18:34:55.000+01:00
diff --git a/jme3-core/src/com/jme3/gde/core/assets/ExternalChangeScanner.java b/jme3-core/src/com/jme3/gde/core/assets/ExternalChangeScanner.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003-2012 jMonkeyEngine
+ * Copyright (c) 2003-2024 jMonkeyEngine
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -36,7 +36,6 @@
 import com.jme3.gde.core.scene.SceneApplication;
 import com.jme3.gde.core.sceneexplorer.SceneExplorerTopComponent;
 import com.jme3.gde.core.sceneexplorer.nodes.JmeNode;
-import com.jme3.gde.core.sceneexplorer.nodes.JmeSpatial;
 import com.jme3.gde.core.util.SpatialUtil;
 import com.jme3.gde.core.util.TaggedSpatialFinder;
 import com.jme3.gde.core.util.datatransfer.CopyAnimationDataFromOriginal;
@@ -186,9 +185,7 @@ private void notifyUser() {
     
     private void applyExternalData(final boolean onlyMeshData, 
             final boolean onlyAnimData) {
-        final ProgressHandle handle = ProgressHandle.createHandle("Updating "
-                + "file "
-                + "data");
+        final ProgressHandle handle = ProgressHandle.createHandle("Updating file data");
         handle.start();
         try {
             final Spatial original = loadOriginalSpatial();
@@ -207,13 +204,11 @@ private void applyExternalData(final boolean onlyMeshData,
                 new CopyTransformDataFromOriginal(finder).update(spat, original);
                 new CopyMaterialDataFromOriginal(finder).update(spat, original);
             }
-            // Do a complicated recurse refresh since AbstractSceneExplorerNode:refresh() isn't working
+            
             SwingUtilities.invokeLater(() -> {
                 Node rootNode = SceneExplorerTopComponent.findInstance().getExplorerManager().getRootContext();
-                if (rootNode instanceof JmeNode) {
-                    SceneApplication.getApplication().enqueue((Runnable) () -> {
-                    refreshNamedSpatial((JmeNode) rootNode, spat.getName());
-                    });
+                if (rootNode instanceof JmeNode jmeNode) {
+                    SceneApplication.getApplication().enqueue(new RefreshJmeSpatial(jmeNode, spat.getName()));
                 }
             });
                 
@@ -228,46 +223,15 @@ private void applyExternalData(final boolean onlyMeshData,
         }
     }
     
-    /**
-     * Look for the spatial to update using the name of the asset
-     * @param spatial
-     * @param name 
-     */
-    private void refreshNamedSpatial(JmeSpatial spatial, String name){
-        if(spatial.getName().equals(name)){
-            recurseRefresh(spatial);
-        } else {
-            for(Node s: spatial.getChildren().getNodes()){
-                if(s instanceof JmeSpatial){
-                    refreshNamedSpatial((JmeSpatial) s, name);
-                }
-                
-            }
-        }
-    }
-    
-    /**
-     * Refreshes the spatial and all children
-     * @param spatial 
-     */
-    private void recurseRefresh(JmeSpatial spatial){
-        spatial.refresh(false);
-        for(Node s: spatial.getChildren().getNodes()){
-            if(s instanceof JmeSpatial){
-                recurseRefresh((JmeSpatial) s);
-            }
-        }
-    }
-
     private Spatial loadOriginalSpatial() {
         try {
             final DataObject dobj = DataObject.find(originalObject);
             final AssetData originalAssetData =
                     dobj.getLookup().lookup(AssetData.class);
             if (originalAssetData != null) {
                 final Savable sav = originalAssetData.loadAsset();
-                if (sav instanceof Spatial) {
-                    return (Spatial) sav;
+                if (sav instanceof Spatial spatial) {
+                    return spatial;
                 } else {
                     LOGGER.log(Level.SEVERE, "Trying to load original for {0}"
                                     + " but it is not a Spatial: {1}",
@@ -352,18 +316,22 @@ public void assetDataPropertyChanged(final String property,
         }
     }
 
+    @Override
     public void fileFolderCreated(FileEvent fe) {
     }
 
+    @Override
     public void fileDataCreated(FileEvent fe) {
     }
 
+    @Override
     public void fileChanged(FileEvent fe) {
         LOGGER.log(Level.INFO, "External file {0} for {1} changed!",
                 new Object[]{fe.getFile(), assetDataObject.getName()});
         notifyUser();
     }
 
+    @Override
     public void fileDeleted(FileEvent fe) {
         LOGGER.log(Level.INFO, "External file {0} for {1} deleted!",
                 new Object[]{fe.getFile(), assetDataObject.getName()});
@@ -377,6 +345,7 @@ public void fileDeleted(FileEvent fe) {
         //TODO: add folder listener for when recreated
     }
 
+    @Override
     public void fileRenamed(FileRenameEvent fe) {
         LOGGER.log(Level.INFO, "External file {0} for {1} renamed!",
                 new Object[]{fe.getFile(), assetDataObject.getName()});
@@ -388,6 +357,7 @@ public void fileRenamed(FileRenameEvent fe) {
         }
     }
 
+    @Override
     public void fileAttributeChanged(FileAttributeEvent fe) {
     }
 }
diff --git a/jme3-core/src/com/jme3/gde/core/assets/RefreshJmeSpatial.java b/jme3-core/src/com/jme3/gde/core/assets/RefreshJmeSpatial.java
@@ -0,0 +1,58 @@
+
+package com.jme3.gde.core.assets;
+
+import com.jme3.gde.core.sceneexplorer.nodes.JmeNode;
+import org.openide.nodes.Node;
+
+import com.jme3.gde.core.sceneexplorer.nodes.JmeSpatial;
+
+/**
+ * Work around for refresh not working recursively on JmeSpatial
+ * @author rickard
+ */
+public class RefreshJmeSpatial implements Runnable {
+    
+    private final JmeNode rootNode;
+    private final String spatialName;
+    
+    public RefreshJmeSpatial(JmeNode rootNode, String spatialName) {
+        this.rootNode = rootNode;
+        this.spatialName = spatialName;
+    }
+    
+    @Override
+    public void run() {
+        refreshNamedSpatial(rootNode, spatialName);
+    }
+    /**
+     * Look for the spatial to update using the name of the asset
+     * @param spatial
+     * @param name 
+     */
+    private void refreshNamedSpatial(JmeSpatial spatial, String name){
+        if(spatial.getName().equals(name)){
+            recurseRefresh(spatial);
+        } else {
+            for(Node s: spatial.getChildren().getNodes()){
+                if(s instanceof JmeSpatial jmeSpatial){
+                    refreshNamedSpatial(jmeSpatial, name);
+                }
+                
+            }
+        }
+    }
+    
+    /**
+     * Refreshes the spatial and all children
+     * @param spatial 
+     */
+    private void recurseRefresh(JmeSpatial spatial){
+        spatial.refresh(false);
+        for(Node s: spatial.getChildren().getNodes()){
+            if(s instanceof JmeSpatial jmeSpatial){
+                recurseRefresh(jmeSpatial);
+            }
+        }
+    }
+
+}
diff --git a/jme3-core/src/com/jme3/gde/core/sceneexplorer/Bundle.properties b/jme3-core/src/com/jme3/gde/core/sceneexplorer/Bundle.properties
@@ -1,4 +1,4 @@
 CTL_SceneExplorerAction=SceneExplorer
 CTL_SceneExplorerTopComponent=SceneExplorer
 HINT_SceneExplorerTopComponent=The SceneExplorer provides an Overview over the SceneGraph of your Scene.
-SceneExplorerTopComponent.jButton1.text=update
+SceneExplorerTopComponent.jButton1.text=Update
diff --git a/jme3-core/src/com/jme3/gde/core/sceneexplorer/SceneExplorerTopComponent.java b/jme3-core/src/com/jme3/gde/core/sceneexplorer/SceneExplorerTopComponent.java
@@ -1,5 +1,5 @@
 /*
- *  Copyright (c) 2009-2010 jMonkeyEngine
+ *  Copyright (c) 2009-2024 jMonkeyEngine
  *  All rights reserved.
  * 
  *  Redistribution and use in source and binary forms, with or without
@@ -31,6 +31,7 @@
  */
 package com.jme3.gde.core.sceneexplorer;
 
+import com.jme3.gde.core.assets.RefreshJmeSpatial;
 import com.jme3.gde.core.icons.IconList;
 import com.jme3.gde.core.scene.PreviewRequest;
 import com.jme3.gde.core.scene.SceneApplication;
@@ -48,6 +49,7 @@
 import java.util.Map;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.swing.SwingUtilities;
 import org.netbeans.api.settings.ConvertAsProperties;
 import org.openide.actions.CopyAction;
 import org.openide.actions.CutAction;
@@ -80,8 +82,9 @@ public final class SceneExplorerTopComponent extends TopComponent implements Exp
 //    private final Result<AbstractSceneExplorerNode> nodeSelectionResult;
     private AbstractSceneExplorerNode selectedSpatial;
     private AbstractSceneExplorerNode lastSelected;
-    private Map<String, MaterialChangeProvider> materialChangeProviders = new HashMap<String, MaterialChangeProvider>();
-    private Map<String, List<MaterialChangeListener>> materialChangeListeners = new HashMap<String, List<MaterialChangeListener>>();
+    private final Map<String, MaterialChangeProvider> materialChangeProviders = new HashMap<>();
+    private final Map<String, List<MaterialChangeListener>> materialChangeListeners = new HashMap<>();
+    private transient ExplorerManager explorerManager = new ExplorerManager();
 
     public SceneExplorerTopComponent() {
         initComponents();
@@ -90,8 +93,6 @@ public SceneExplorerTopComponent() {
         setToolTipText(NbBundle.getMessage(SceneExplorerTopComponent.class, "HINT_SceneExplorerTopComponent"));
         setIcon(IconList.jmeLogo.getImage());
         associateLookup(ExplorerUtils.createLookup(explorerManager, getActionMap()));
-//        nodeSelectionResult = Utilities.actionsGlobalContext().lookupResult(AbstractSceneExplorerNode.class);
-//        nodeSelectionResult.addLookupListener(this);
     }
 
     private void initActions() {
@@ -151,7 +152,15 @@ private void jButton1ActionPerformed(java.awt.event.ActionEvent evt) {//GEN-FIRS
         if (selectedSpatial == null) {
             return;
         }
-        selectedSpatial.refresh(false);
+        SwingUtilities.invokeLater(() -> {
+            Node rootNode = SceneExplorerTopComponent.findInstance().getExplorerManager().getRootContext();
+            if (rootNode instanceof JmeNode jmeNode) {
+                SceneApplication.getApplication().enqueue(new RefreshJmeSpatial(jmeNode, selectedSpatial.getName()));
+            } else {
+                selectedSpatial.refresh(false);
+            }
+        });
+        
     }//GEN-LAST:event_jButton1ActionPerformed
     // Variables declaration - do not modify//GEN-BEGIN:variables
     private javax.swing.JScrollPane explorerScrollPane;
@@ -164,6 +173,7 @@ private void jButton1ActionPerformed(java.awt.event.ActionEvent evt) {//GEN-FIRS
      * only, i.e. deserialization routines; otherwise you could get a
      * non-deserialized instance. To obtain the singleton instance, use
      * {@link #findInstance}.
+     * @return 
      */
     public static synchronized SceneExplorerTopComponent getDefault() {
         if (instance == null) {
@@ -175,6 +185,7 @@ public static synchronized SceneExplorerTopComponent getDefault() {
     /**
      * Obtain the SceneExplorerTopComponent instance. Never call
      * {@link #getDefault} directly!
+     * @return 
      */
     public static synchronized SceneExplorerTopComponent findInstance() {
         TopComponent win = WindowManager.getDefault().findTopComponent(PREFERRED_ID);
@@ -183,8 +194,8 @@ public static synchronized SceneExplorerTopComponent findInstance() {
                     "Cannot find " + PREFERRED_ID + " component. It will not be located properly in the window system.");
             return getDefault();
         }
-        if (win instanceof SceneExplorerTopComponent) {
-            return (SceneExplorerTopComponent) win;
+        if (win instanceof SceneExplorerTopComponent sceneExplorerTopComponent) {
+            return sceneExplorerTopComponent;
         }
         logger.warning(
                 "There seem to be multiple components with the '" + PREFERRED_ID
@@ -233,8 +244,8 @@ Object readProperties(java.util.Properties p) {
     }
 
     private void readPropertiesImpl(java.util.Properties p) {
-        String version = p.getProperty("version");
         // TODO read your settings according to their version
+        
     }
 
     @Override
@@ -246,7 +257,6 @@ protected String preferredID() {
     public UndoRedo getUndoRedo() {
         return Lookup.getDefault().lookup(UndoRedo.class);
     }
-    private transient ExplorerManager explorerManager = new ExplorerManager();
 
     @Override
     public ExplorerManager getExplorerManager() {
@@ -266,26 +276,15 @@ public void setSelectedNode(AbstractSceneExplorerNode node) {
                 explorerManager.setSelectedNodes(new Node[]{});
 //                setActivatedNodes(new Node[]{});
             }
-        } catch (Exception ex) {
+        } catch (PropertyVetoException ex) {
             Exceptions.printStackTrace(ex);
         }
     }
 
-//    public void resultChanged(LookupEvent ev) {
-//        Collection collection = nodeSelectionResult.allInstances();
-//        for (Iterator it = collection.iterator(); it.hasNext();) {
-//            Object object = it.next();
-//            if (object instanceof AbstractSceneExplorerNode) {
-//                return;
-//            }
-//        }
-//        selectedSpatial = null;
-//    }
     @Override
     public void sceneOpened(SceneRequest request) {
         final JmeNode node = request.getJmeNode();
-        for (Iterator it = materialChangeProviders.values().iterator(); it.hasNext();) {
-            MaterialChangeProvider provider = (MaterialChangeProvider) it.next();
+        for (MaterialChangeProvider provider : materialChangeProviders.values()) {
             provider.clearMaterialChangeListeners();
         }
         if (node != null) {
@@ -339,7 +338,7 @@ public void addMaterialChangeListener(MaterialChangeListener listener) {
             logger.log(Level.FINE, "New material listener for : {0}", listener.getKey());
             List<MaterialChangeListener> listeners = materialChangeListeners.get(listener.getKey());
             if (listeners == null) {
-                listeners = new ArrayList<MaterialChangeListener>();
+                listeners = new ArrayList<>();
                 materialChangeListeners.put(listener.getKey(), listeners);
             }
             listeners.add(listener);
@@ -383,7 +382,7 @@ public void swapMaterialChangeListener(MaterialChangeListener listener, String o
             //  assert newKey.equals(listener.getKey());
             List<MaterialChangeListener> listeners = materialChangeListeners.get(newKey);
             if (listeners == null) {
-                listeners = new ArrayList<MaterialChangeListener>();
+                listeners = new ArrayList<>();
                 materialChangeListeners.put(newKey, listeners);
             }
             listeners.add(listener);
@@ -397,6 +396,7 @@ public void swapMaterialChangeListener(MaterialChangeListener listener, String o
 
     /**
      * Terrain has a LOD control that requires the camera to function.
+     * @param jmeRootNode
      */
     protected void setTerrainLodCamera(JmeNode jmeRootNode) {
         Camera camera = SceneApplication.getApplication().getCamera();
