Fix skyboxes orientations that were not standard cube (#9184)

Extensions/3D/JsExtension.js

@@ -2401,16 +2401,16 @@ module.exports = {
         .setType('resource')
         .addExtraInfo('image')
         .setLabel(_('Left face (X-)'));
-      properties
-        .getOrCreate('bottomFaceResourceName')
-        .setType('resource')
-        .addExtraInfo('image')
-        .setLabel(_('Bottom face (Y+)'));
       properties
         .getOrCreate('topFaceResourceName')
         .setType('resource')
         .addExtraInfo('image')
-        .setLabel(_('Top face (Y-)'));
+        .setLabel(_('Top face (Y+)'));
+      properties
+        .getOrCreate('bottomFaceResourceName')
+        .setType('resource')
+        .addExtraInfo('image')
+        .setLabel(_('Bottom face (Y-)'));
       properties
         .getOrCreate('frontFaceResourceName')
         .setType('resource')
@@ -2421,6 +2421,23 @@ module.exports = {
         .setType('resource')
         .addExtraInfo('image')
         .setLabel(_('Back face (Z-)'));
+      properties
+        .getOrCreate('top')
+        .setValue('Z+')
+        .setLabel(_('3D world top'))
+        .setType('choice')
+        .addChoice('Z+', _('Z+ (most 3D games)'))
+        .addChoice('Y-', _('Y- (side view 3D games)'))
+        .addChoice(
+          'Legacy',
+          _('Legacy Z+ (deprecated, faces must be rotated by hand)')
+        )
+        .setGroup(_('Orientation'))
+        .setDescription(
+          _(
+            'With Z+ or Y-, the faces are the ones of a usual cube map, as exported by most tools.'
+          )
+        );
       properties
         .getOrCreate('environmentIntensity')
         .setValue('1')

Extensions/3D/Skybox.ts

@@ -2,6 +2,7 @@ namespace gdjs {
   interface SkyboxFilterNetworkSyncData {
     ei?: number;
     bi?: number;
+    t?: string;
   }
   gdjs.PixiFiltersTools.registerFilterCreator(
     'Scene3D::Skybox',
@@ -23,15 +24,31 @@ namespace gdjs {
           _isEnabled: boolean = false;
           _environmentIntensity: float = 1;
           _backgroundIntensity: float = 1;
+          // Skyboxes made before `top` existed have no value for it.
+          _top: string = effectData.stringParameters.top || 'Legacy';
 
           constructor() {
-            this._cubeTexture = target
+            this._cubeTexture = this._getCubeTexture();
+          }
+
+          private _isLegacy(): boolean {
+            return this._top !== 'Z+' && this._top !== 'Y-';
+          }
+          private _getCubeTexture(): THREE.CubeTexture {
+            // Legacy faces follow the axes of the scene: they need the swap on
+            // X done by `getThreeCubeTexture`, which standard faces must not get.
+            const isLegacy = this._isLegacy();
+            return target
               .getRuntimeScene()
               .getGame()
               .getImageManager()
               .getThreeCubeTexture(
-                effectData.stringParameters.rightFaceResourceName,
-                effectData.stringParameters.leftFaceResourceName,
+                isLegacy
+                  ? effectData.stringParameters.rightFaceResourceName
+                  : effectData.stringParameters.leftFaceResourceName,
+                isLegacy
+                  ? effectData.stringParameters.leftFaceResourceName
+                  : effectData.stringParameters.rightFaceResourceName,
                 effectData.stringParameters.topFaceResourceName,
                 effectData.stringParameters.bottomFaceResourceName,
                 effectData.stringParameters.frontFaceResourceName,
@@ -70,8 +87,56 @@ namespace gdjs {
             this._isEnabled = true;
             this._updateEnvironmentIntensity();
             this._updateBackgroundIntensity();
+            this._updateRotation();
             return true;
           }
+          private _setTop(top: string): void {
+            if (this._top === top) {
+              return;
+            }
+            const wasLegacy = this._isLegacy();
+            this._top = top;
+            if (wasLegacy !== this._isLegacy()) {
+              const oldCubeTexture = this._cubeTexture;
+              this._cubeTexture = this._getCubeTexture();
+              const scene = target.get3DRendererObject() as
+                | THREE.Scene
+                | null
+                | undefined;
+              if (scene && this._isEnabled) {
+                scene.background = this._cubeTexture;
+                if (scene.environment === oldCubeTexture) {
+                  scene.environment = this._cubeTexture;
+                }
+              }
+            }
+            this._updateRotation();
+          }
+          private _updateRotation(): void {
+            const scene = target.get3DRendererObject() as
+              | THREE.Scene
+              | null
+              | undefined;
+            if (!scene || !this._isEnabled) {
+              return;
+            }
+            if (this._isLegacy()) {
+              scene.backgroundRotation.set(0, 0, 0);
+            } else {
+              // Standard faces have their top on Y+ of Three.js (Y- of the
+              // scene) and their front on Z+: they are turned to have the
+              // front where a camera with no rotation looks at.
+              scene.backgroundRotation.set(
+                this._top === 'Y-' ? 0 : Math.PI / 2,
+                Math.PI,
+                0
+              );
+            }
+            // Another effect may have set the environment of the scene.
+            if (scene.environment === this._cubeTexture) {
+              scene.environmentRotation.copy(scene.backgroundRotation);
+            }
+          }
           private _updateEnvironmentIntensity(): void {
             const scene = target.get3DRendererObject() as
               | THREE.Scene
@@ -105,6 +170,8 @@ namespace gdjs {
             scene.environment = null;
             scene.environmentIntensity = 1;
             scene.backgroundIntensity = 1;
+            scene.backgroundRotation.set(0, 0, 0);
+            scene.environmentRotation.set(0, 0, 0);
             this._isEnabled = false;
             return true;
           }
@@ -126,7 +193,11 @@ namespace gdjs {
             }
             return 0;
           }
-          updateStringParameter(parameterName: string, value: string): void {}
+          updateStringParameter(parameterName: string, value: string): void {
+            if (parameterName === 'top') {
+              this._setTop(value);
+            }
+          }
           updateColorParameter(parameterName: string, value: number): void {}
           getColorParameter(parameterName: string): number {
             return 0;
@@ -136,6 +207,7 @@ namespace gdjs {
             return {
               ei: this._environmentIntensity,
               bi: this._backgroundIntensity,
+              t: this._top,
             };
           }
           updateFromNetworkSyncData(
@@ -149,6 +221,9 @@ namespace gdjs {
               this._backgroundIntensity = syncData.bi;
               this._updateBackgroundIntensity();
             }
+            if (syncData.t !== undefined) {
+              this._setTop(syncData.t);
+            }
           }
         })();
       }

newIDE/app/src/AssetStore/InstallAsset.js

@@ -10,6 +10,7 @@ import {
   type ExtensionDependency,
 } from '../Utils/GDevelopServices/Extension';
 import newNameGenerator from '../Utils/NewNameGenerator';
+import { markEffectIfLegacySkybox } from '../Utils/LegacySkyboxOrientation';
 import { unserializeFromJSObject } from '../Utils/Serializer';
 import { toNewGdMapStringString } from '../Utils/MapStringString';
 import { getInsertionParentAndPositionFromSelection } from '../Utils/ObjectFolders';
@@ -164,6 +165,7 @@ export const installEffectAsset = ({
     const value = stringParameters[name];
     effect.setStringParameter(name, resourceNewNames[value] || value);
   });
+  markEffectIfLegacySkybox(effect);
   return effect;
 };
 

newIDE/app/src/AssetStore/InstallAsset.spec.js

@@ -49,6 +49,46 @@ const mockFn = (fn: Function): JestMockFn<any, any> => fn;
 
 describe('InstallAsset', () => {
   describe('installEffectAsset', () => {
+    it('marks a skybox without a top as legacy and keeps the top of the others', () => {
+      const { project } = makeTestProject(gd);
+      const effectsContainer = project
+        .insertNewLayout('MyTestLayout', 0)
+        .getLayers()
+        .getLayer('')
+        .getEffects();
+      const effectAsset = (fakeSkyboxAsset.effectAssets || [])[0];
+
+      const legacyEffect = installEffectAsset({
+        project,
+        effectsContainer,
+        asset: fakeSkyboxAsset,
+        effectName: 'LegacySky',
+      });
+      const standardEffect = installEffectAsset({
+        project,
+        effectsContainer,
+        asset: {
+          ...fakeSkyboxAsset,
+          effectAssets: [
+            {
+              ...effectAsset,
+              effect: {
+                ...effectAsset.effect,
+                stringParameters: {
+                  ...effectAsset.effect.stringParameters,
+                  top: 'Z+',
+                },
+              },
+            },
+          ],
+        },
+        effectName: 'StandardSky',
+      });
+
+      expect(legacyEffect.getStringParameter('top')).toBe('Legacy');
+      expect(standardEffect.getStringParameter('top')).toBe('Z+');
+    });
+
     it('installs a skybox as an effect showing its images', () => {
       const { project } = makeTestProject(gd);
       const layout = project.insertNewLayout('MyTestLayout', 0);

newIDE/app/src/EditorFunctions/ChangeScenePropertiesLayersEffectsGroups.spec.js

@@ -379,6 +379,32 @@ describe('change_scene_properties_layers_effects_groups', () => {
     expect(effects.hasEffectNamed('Sky2')).toBe(false);
   });
 
+  it('gives a new layer effect the default values of its parameters', async () => {
+    const effects = testScene
+      .getLayers()
+      .getLayer('')
+      .getEffects();
+
+    const result: EditorFunctionGenericOutput = await editorFunctions.change_scene_properties_layers_effects_groups.launchFunction(
+      {
+        ...makeFakeLaunchFunctionOptionsWithProject(project),
+        args: {
+          scene_name: 'TestScene',
+          changed_layer_effects: [
+            {
+              layer_name: '',
+              effect_name: 'MySepia',
+              effect_type: 'FakeSepia',
+            },
+          ],
+        },
+      }
+    );
+
+    expect(result.success).toBe(true);
+    expect(effects.getEffect('MySepia').getDoubleParameter('opacity')).toBe(1);
+  });
+
   it('renames a layer effect', async () => {
     const effects = testScene
       .getLayers()

newIDE/app/src/EditorFunctions/index.js

@@ -5,6 +5,7 @@ import {
   renameLayoutInProject,
 } from '../Utils/Layout';
 import { mapFor } from '../Utils/MapFor';
+import { setEffectDefaultParameters } from '../EffectsList/EnumerateEffects';
 import { SafeExtractor } from '../Utils/SafeExtractor';
 import {
   serializeToJSObject,
@@ -8161,6 +8162,7 @@ const applyEffectChange = ({
           new_effect_position || 0
         );
         newlyCreatedEffect.setEffectType(effect_type);
+        setEffectDefaultParameters(newlyCreatedEffect, effectMetadata);
       }
     } else if (
       delete_this_effect ||

newIDE/app/src/MainFrame/index.js

@@ -23,6 +23,7 @@ import CloseConfirmDialog from '../UI/CloseConfirmDialog';
 import ProfileDialog from '../Profile/ProfileDialog';
 import PurchaseClaimDialog from '../Profile/PurchaseClaimDialog';
 import Window from '../Utils/Window';
+import { markLegacySkyboxes } from '../Utils/LegacySkyboxOrientation';
 import { showErrorBox } from '../UI/Messages/MessageBox';
 import EditorTabsPane, {
   type EditorTabsPaneCommonProps,
@@ -1425,6 +1426,7 @@ const MainFrame = (props: Props): React.MixedElement => {
       const startTime = Date.now();
       const newProject = gd.ProjectHelper.createNewGDJSProject();
       newProject.unserializeFrom(serializedProject);
+      markLegacySkyboxes(newProject);
       const duration = Date.now() - startTime;
       console.info(`Unserialization took ${duration.toFixed(2)} ms`);
 

newIDE/app/src/Utils/LegacySkyboxOrientation.js

@@ -0,0 +1,38 @@
+// @flow
+import { mapFor } from './MapFor';
+
+/**
+ * A skybox made before the "3D world top" property has faces rotated by hand:
+ * give it the legacy value, as a property without a value shows its default.
+ */
+export const markEffectIfLegacySkybox = (effect: gdEffect) => {
+  if (
+    effect.getEffectType() === 'Scene3D::Skybox' &&
+    !effect.hasStringParameter('top')
+  ) {
+    effect.setStringParameter('top', 'Legacy');
+  }
+};
+
+const markLayersLegacySkyboxes = (layers: gdLayersContainer) => {
+  mapFor(0, layers.getLayersCount(), layerIndex => {
+    const effects = layers.getLayerAt(layerIndex).getEffects();
+    mapFor(0, effects.getEffectsCount(), effectIndex => {
+      markEffectIfLegacySkybox(effects.getEffectAt(effectIndex));
+    });
+  });
+};
+
+export const markLegacySkyboxes = (project: gdProject) => {
+  mapFor(0, project.getLayoutsCount(), i => {
+    markLayersLegacySkyboxes(project.getLayoutAt(i).getLayers());
+  });
+  mapFor(0, project.getEventsFunctionsExtensionsCount(), i => {
+    const eventsBasedObjects = project
+      .getEventsFunctionsExtensionAt(i)
+      .getEventsBasedObjects();
+    mapFor(0, eventsBasedObjects.getCount(), j => {
+      markLayersLegacySkyboxes(eventsBasedObjects.getAt(j).getLayers());
+    });
+  });
+};

newIDE/app/src/Utils/LegacySkyboxOrientation.spec.js

@@ -0,0 +1,30 @@
+// @flow
+import { markLegacySkyboxes } from './LegacySkyboxOrientation';
+import { makeTestProject } from '../fixtures/TestProject';
+
+const gd: libGDevelop = global.gd;
+
+describe('markLegacySkyboxes', () => {
+  it('marks the skyboxes without a top as legacy and keeps the others', () => {
+    const { project, testLayout } = makeTestProject(gd);
+    const effects = testLayout
+      .getLayers()
+      .getLayer('')
+      .getEffects();
+    effects.insertNewEffect('OldSky', 0).setEffectType('Scene3D::Skybox');
+    const newSky = effects.insertNewEffect('NewSky', 1);
+    newSky.setEffectType('Scene3D::Skybox');
+    newSky.setStringParameter('top', 'Y-');
+    effects
+      .insertNewEffect('Light', 2)
+      .setEffectType('Scene3D::HemisphereLight');
+
+    markLegacySkyboxes(project);
+
+    expect(effects.getEffect('OldSky').getStringParameter('top')).toBe(
+      'Legacy'
+    );
+    expect(effects.getEffect('NewSky').getStringParameter('top')).toBe('Y-');
+    expect(effects.getEffect('Light').hasStringParameter('top')).toBe(false);
+  });
+});