diff --git a/CHANGES.md b/CHANGES.md index aa1e3144..ddf05a0b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,14 +4,15 @@ ##### Fixes :wrench: -- Removed Universal Additional Camera Data script from DynamicCamera prefab, as it shows up as a missing script in other render pipelines. +- Removed the "Universal Additional Camera Data" script from DynamicCamera, as it shows up as a missing script in other render pipelines. +- Fixed a bug where adding a `CesiumSubScene` as the child of an existing `CesiumGeoreference` in editor would cause the parent `CesiumGeoreference` to have its coordinates reset to the default. +- Fixed the "DynamicCamera is not nested inside a game object with a CesiumGeoreference" warning when adding a new DynamicCamera in the editor. ### v1.7.1 - 2023-12-14 ##### Fixes :wrench: - Fixed a bug that prevented the default `CesiumIonServer` asset from remembering its token in a clean project. -- Fixed "DynamicCamera is not nested inside a game object with a CesiumGeoreference" warning when adding a new DynamicCamera in the editor. ### v1.7.0 - 2023-12-14 diff --git a/EditorTests.meta b/EditorTests.meta new file mode 100644 index 00000000..589cc141 --- /dev/null +++ b/EditorTests.meta @@ -0,0 +1,8 @@ +fileFormatVersion: 2 +guid: ee37edb25779c4544b705b2954c45f15 +folderAsset: yes +DefaultImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/EditorTests/CesiumEditorTests.asmdef b/EditorTests/CesiumEditorTests.asmdef new file mode 100644 index 00000000..ef321288 --- /dev/null +++ b/EditorTests/CesiumEditorTests.asmdef @@ -0,0 +1,25 @@ +{ + "name": "CesiumEditorTests", + "rootNamespace": "", + "references": [ + "UnityEngine.TestRunner", + "UnityEditor.TestRunner", + "CesiumRuntime", + "Unity.Mathematics" + ], + "includePlatforms": [ + "Editor" + ], + "excludePlatforms": [], + "allowUnsafeCode": false, + "overrideReferences": true, + "precompiledReferences": [ + "nunit.framework.dll" + ], + "autoReferenced": false, + "defineConstraints": [ + "UNITY_INCLUDE_TESTS" + ], + "versionDefines": [], + "noEngineReferences": false +} \ No newline at end of file diff --git a/EditorTests/CesiumEditorTests.asmdef.meta b/EditorTests/CesiumEditorTests.asmdef.meta new file mode 100644 index 00000000..dae21a6d --- /dev/null +++ b/EditorTests/CesiumEditorTests.asmdef.meta @@ -0,0 +1,7 @@ +fileFormatVersion: 2 +guid: 8065b9c90ecacd44ca0906e38b93ca37 +AssemblyDefinitionImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/EditorTests/TestCesiumSubScene.cs b/EditorTests/TestCesiumSubScene.cs new file mode 100644 index 00000000..ccd8f2cb --- /dev/null +++ b/EditorTests/TestCesiumSubScene.cs @@ -0,0 +1,55 @@ +using CesiumForUnity; +using NUnit.Framework; +using System.Collections; +using UnityEngine; +using UnityEngine.TestTools; + +public class TestCesiumSubScene +{ + [UnityTest] + public IEnumerator AddingSubSceneCopiesGeoreferenceCoordinates() + { + GameObject goGeoreference = new GameObject("Georeference"); + CesiumGeoreference georeference = goGeoreference.AddComponent(); + georeference.SetOriginLongitudeLatitudeHeight(-55.0, 55.0, 1000.0); + + GameObject goSubScene = new GameObject("SubScene"); + goSubScene.transform.parent = goGeoreference.transform; + CesiumSubScene subScene = goSubScene.AddComponent(); + + yield return null; + + Assert.AreEqual(-55.0, georeference.longitude); + Assert.AreEqual(55.0, georeference.latitude); + Assert.AreEqual(1000.0, georeference.height); + Assert.AreEqual(georeference.longitude, subScene.longitude); + Assert.AreEqual(georeference.latitude, subScene.latitude); + Assert.AreEqual(georeference.height, subScene.height); + } + + [UnityTest] + public IEnumerator ModifyingSubsceneModifiesParentGeoreference() + { + GameObject goGeoreference = new GameObject("Georeference"); + CesiumGeoreference georeference = goGeoreference.AddComponent(); + georeference.SetOriginLongitudeLatitudeHeight(-55.0, 55.0, 1000.0); + + GameObject goSubScene = new GameObject("SubScene"); + goSubScene.transform.parent = goGeoreference.transform; + CesiumSubScene subScene = goSubScene.AddComponent(); + + yield return null; + + subScene.SetOriginLongitudeLatitudeHeight(-10.0, 10.0, 100.0); + + yield return null; + + Assert.AreEqual(subScene.longitude, -10.0); + Assert.AreEqual(subScene.latitude, 10.0); + Assert.AreEqual(subScene.height, 100.0); + + Assert.AreEqual(georeference.longitude, -10.0); + Assert.AreEqual(georeference.latitude, 10.0); + Assert.AreEqual(georeference.height, 100.0); + } +} \ No newline at end of file diff --git a/EditorTests/TestCesiumSubScene.cs.meta b/EditorTests/TestCesiumSubScene.cs.meta new file mode 100644 index 00000000..0354a560 --- /dev/null +++ b/EditorTests/TestCesiumSubScene.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: b77912debd15cc948b79e3db7086440a +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Runtime/CesiumSubScene.cs b/Runtime/CesiumSubScene.cs index 0bdbe74e..9a6d802e 100644 --- a/Runtime/CesiumSubScene.cs +++ b/Runtime/CesiumSubScene.cs @@ -191,6 +191,21 @@ public double ecefZ [NonSerialized] internal CesiumGeoreference _parentGeoreference; +#if UNITY_EDITOR + // The coordinates of the parent CesiumGeoreference before being changed by `UpdateOrigin`. + // This is used when adding a CesiumSubScene component to a GameObject. Since `OnEnable` + // runs before `Reset`, it will force the georeference to move to the default coordinates + // of CesiumSubScene, which is undesired. But there's no way to modify `OnEnable` to + // distinguish whether the subscene was just added or already existed in the scene. + // Instead, we need to store the georeference's previous values during `OnEnable`, + // so that we can move it back during `Reset`. + [NonSerialized] + private double3 _oldParentCoordinates = double3.zero; + + [NonSerialized] + private CesiumGeoreferenceOriginAuthority _oldParentOriginAuthority; +#endif + /// /// Sets the origin of the coordinate system to particular , , /// coordinates in the Earth-Centered, Earth-Fixed (ECEF) frame. @@ -231,6 +246,17 @@ public void SetOriginLongitudeLatitudeHeight(double longitude, double latitude, this.originAuthority = CesiumGeoreferenceOriginAuthority.LongitudeLatitudeHeight; } + private void CopyParentCoordinates() + { + this._longitude = this._parentGeoreference.longitude; + this._latitude = this._parentGeoreference.latitude; + this._height = this._parentGeoreference.height; + + this._ecefX = this._parentGeoreference.ecefX; + this._ecefY = this._parentGeoreference.ecefY; + this._ecefZ = this._parentGeoreference.ecefZ; + } + private void DetachFromParentIfNeeded() { if (this._parentGeoreference != null) @@ -263,6 +289,7 @@ private void OnValidate() this.UpdateOrigin(); } +#if UNITY_EDITOR /// /// Called by the Editor when the user chooses to "reset" the component. /// The implementation here makes sure the newly-reset values for the serialized @@ -271,7 +298,24 @@ private void OnValidate() private void Reset() { this.UpdateParentReference(); + + // The default coordinates for the CesiumSubScene component should be the coordinates of its parent, if possible. + // This means adding the component as the child of an existing CesiumGeoreference won't reset the parent's coordinates. + if (this._parentGeoreference != null) + { + if (this._oldParentOriginAuthority == CesiumGeoreferenceOriginAuthority.EarthCenteredEarthFixed) + { + this._parentGeoreference.SetOriginEarthCenteredEarthFixed(this._oldParentCoordinates.x, this._oldParentCoordinates.y, this._oldParentCoordinates.z); + } + else + { + this._parentGeoreference.SetOriginLongitudeLatitudeHeight(this._oldParentCoordinates.x, this._oldParentCoordinates.y, this._oldParentCoordinates.z); + } + + this.CopyParentCoordinates(); + } } +#endif private void OnEnable() { @@ -291,6 +335,19 @@ private void OnEnable() scene.gameObject.SetActive(false); } +#if UNITY_EDITOR + // The parent's previous coordinates are saved here in case they have to be reverted in the editor during `Reset` + this._oldParentOriginAuthority = this._parentGeoreference.originAuthority; + if (this._oldParentOriginAuthority == CesiumGeoreferenceOriginAuthority.EarthCenteredEarthFixed) + { + this._oldParentCoordinates = new double3(this._parentGeoreference.ecefX, this._parentGeoreference.ecefY, this._parentGeoreference.ecefZ); + } + else + { + this._oldParentCoordinates = new double3(this._parentGeoreference.longitude, this._parentGeoreference.latitude, this._parentGeoreference.height); + } +#endif + this.UpdateOrigin(); } @@ -310,13 +367,7 @@ private void OnParentChanged() // Update our origin to our parent georef, maintain our origin authority, // and copy both sets of reference coordinates. No need to calculate any of this again - this._longitude = this._parentGeoreference.longitude; - this._latitude = this._parentGeoreference.latitude; - this._height = this._parentGeoreference.height; - - this._ecefX = this._parentGeoreference.ecefX; - this._ecefY = this._parentGeoreference.ecefY; - this._ecefZ = this._parentGeoreference.ecefZ; + this.CopyParentCoordinates(); } private void OnDisable() @@ -386,7 +437,7 @@ public void UpdateOrigin() } } - #if UNITY_EDITOR +#if UNITY_EDITOR private void OnDrawGizmos() { if (this._showActivationRadius) @@ -396,6 +447,6 @@ private void OnDrawGizmos() Gizmos.DrawWireSphere(this.transform.position, (float)this._activationRadius); } } - #endif +#endif } }