Edits to both guides, add deprecation section

Documentation/api-design-guide.md

@@ -2,7 +2,7 @@
 
 Cesium for Unreal aims to unlock geospatial capabilities in Unreal Engine, while also enabling developers to take advantage of in-engine features. As a result, people who use the plugin can often come with existing Unreal knowledge.
 
-In order to make Cesium's integration as seamless as possible, we aim to mirror the paradigms and APIs of the engine where we can.
+In order to make Cesium's integration as seamless as possible, we aim to mirror the paradigms and APIs of the engine where we can. This guide provides best practices for how we do that.
 
 ## Table of Contents
 
@@ -100,40 +100,118 @@ As a result, `BlueprintReadWrite` is rarely used for properties in Cesium for Un
 
 We have settled on the following standards for properties that require post-change logic:
 
-- Declare in the `private:` section of the class. This prevents the property from being get or set directly from outside the class in C++ (which is important, because there is no mechanism like `PostEditChangeProperty` or `BlueprintSetter` available in code).
-- Add `Meta = (AllowPrivateAccess)`.
-- Add `BlueprintGetter` and `BlueprintSetter` functions.
-- Overide the `PostEditChangeProperty` method to be notified of changes to the property in the Editor.
+1. Declare properties as `private` in the class. This prevents the property from being get or set directly from outside the class in C++ (which is important, because there is no mechanism like `PostEditChangeProperty` or `BlueprintSetter` available in code).
+2. Add `Meta = (AllowPrivateAccess)` to the `UPROPERTY`.
+3. Add `BlueprintGetter` and `BlueprintSetter` functions.
+4. Override the `PostEditChangeProperty` method on the class. This allows it to be notified of changes to the property in the Editor.
 
-Perhaps there is a rare-case property where no action is necessary after setting a property from C++ (and it's unlikely to be needed in the future). In this case, the property can be declared in the `public:` section. Such a property is not likely to need a `PostEditChangeProperty` or `BlueprintSetter` either.
+There may be a rare case where no action is necessary after setting a certain property from C++. In this case—and as long as this behavior is unlikely to change—the property can be declared in the `public:` section. Such a property is not likely to need `PostEditChangeProperty` or `BlueprintSetter` either.
 
 #### Details Panel
 
+Properties should be organized or modified in the Details panel such that they provide an intuitive user experience. There are many modifiers that facilitate this, also listed in the cheat sheet.
+
+In general, be mindful of how properties are organized, and in what order. By default, properties will appear in the order that they are listed in the C++ code. A good principle is to start with properties that are fundamental to the functionality, then cascade into more advanced settings.
+
+Properties should be organized into sensible categories, or as much as is reasonably possible, using the `Category` modifier. They should also be grouped together in C++ to reinforce this.
+
+```c++
+
+```
+
+Often times, a `UObject` will have various properties that apply depending on the state of other properties. In this case, the `Meta=EditCondition` modifier is should be used for visual clarity. The dependent properties should be listed below the one that controls whether they're active.
+
+Typically, such settings should depend on either a boolean setting or an enum setting. For instance, in `Cesium3DTileset`, the `Source` property affects whether the URL property can be edited. The `Enable Culling` flag
+
 ### Cheat Sheet
 
 For convenience, here is a cheat sheet of some of the most relevant modifiers for each category.
 
 #### `UPROPERTY`
 
-| Name | How (and when) to use |
-| ---- | --------------------- |
-| `VisibleAnywhere` | Property is read-only in the Details panel. For editable properties, use `EditAnywhere`. Don't use this for variables that shouldn't be visible to the user (e.g., implementation details, internal state management). |
-| `EditAnywhere` | Property is editable in the Details panel. Use `VisibleAnywhere` for read-only variables. |
-| `BlueprintReadOnly` | Property is accessible in Blueprints but read-only. |
-| `BlueprintReadWrite` | Property is editable from Blueprints. Use when the "set" logic is simple enough that nothing additional must happen after the property is set. If additional logic is required, use `BlueprintSetter` instead. |
+| Name | What | When to Use |
+| ---- | ----- | --------------- |
+| `VisibleAnywhere` | Property is read-only in the Details panel. | Use for read-only variables. Don't use this for variables that shouldn't be visible to the user (e.g., implementation details, internal state management). |
+| `EditAnywhere` | Property is editable in the Details panel. | Use for properties that should be user-editable through the Editor UI. |
+| `BlueprintReadOnly` | Property is accessible in Blueprints but read-only. | Self-explanatory. | 
+| `BlueprintReadWrite` | Property is editable from Blueprints. | Use when the "set" logic is simple enough that nothing additional must happen after the property is set. If additional logic is required, use `BlueprintSetter` instead. |
+| `BlueprintGetter` | Property uses the specified function to get the value. | Use whenever you have to use `BlueprintSetter`. | 
+| `BlueprintSetter` | Property uses the specified function to set the value. | Use whenever you have to do additional work after setting a property, e.g., recomputing the object's internal state. |
 
-## Blueprints
+#### `UFUNCTION`
 
-Blueprints are a visual scripting option in Unreal Engine that many users use over C++ code. Thus, part of API design includes creating sensible Blueprints for less code-savvy users.
+| Name | What | When to Use |
+| ---- | ----- | --------------- |
+| `BlueprintPure` | Function will be executed in a Blueprint graph without affect the owning object in any way. | Use for static Blueprints functions. |
+| `meta = (ReturnDisplayName = "")` | Function output on the Blueprint node will be labeled with the specified name. | Use in general. |
 
-### Integrate with Existing Blueprints
+## Blueprints 
 
-Try to defer to Unreal Engine's naming schemes. For example, texture coordinates are referred to as "UV", so any parameters that represent texture coordinates should also be called UV.
+Blueprints are a visual scripting option in Unreal Engine that many users use over C++ code. Thus, part of the API design in Cesium for Unreal includes creating sensible Blueprints for less code-savvy users.
+
+### Copy Parameter Names
+
+Try to defer to Unreal Engine's naming schemes for existing Blueprint functions and parameters. For example, texture coordinates in Blueprints are often referred to as "UV". Cesium for Unreal tries to match this by naming its own texture coordinate parameters as "UV".
 
 ![](Images/matchUnrealNaming.png)
 
-## Materials
+## Deprecation and Backwards Compatibility
+
+Ideally, APIs should not change frequently so that users aren't left confused and frustrated as they upgrade versions. But changes do happen from time to time. Thankfully, there are several measures in Unreal Engine to help prevent user frustration from API changes.
+
+### Macros and Modifiers
+
+First, read this short and sweet [overview](https://squareys.de/blog/ue4-deprecating-symbols/) by Jonathan Hale that explains how to deprecate anything in Unreal Engine. This section expands briefly on some points, but most of it is already covered.
+
+- Use the `DeprecationMessage` should succinctly inform the user of the deprecation and redirect them to its replacement, if applicable.
+
+```c++
+  UFUNCTION(
+      Meta =
+          (DeprecatedFunction,
+           DeprecationMessage =
+               "CesiumMetadataPrimitive is deprecated. Get the associated property texture indices from CesiumPrimitiveMetadata instead."))
+  static const TArray<FString>
+  GetFeatureTextureNames(UPARAM(ref)
+                             const FCesiumMetadataPrimitive& MetadataPrimitive);
+```
+
+- If a `struct` or `class` is deprecated, prefer to use the `UE_DEPRECATED` macro in a forward declaration of the class before it is actually defined. For example:
+
+```c++
+// Forward declare the class with the UE_DEPRECATED macro.
+struct UE_DEPRECATED(
+    5.0,
+    "FCesiumMetadataPrimitive is deprecated. Instead, use FCesiumPrimitiveFeatures and FCesiumPrimitiveMetadata to retrieve feature IDs and metadata from a glTF primitive.")
+    FCesiumMetadataPrimitive;
+
+// Actual definition below.
+USTRUCT(BlueprintType)
+struct CESIUMRUNTIME_API FCesiumMetadataPrimitive { ... }
+```
+
+- For backwards compatibility, sometimes you'll need to keep references to the deprecated classes or functions in C++ code. If this is the case, then be sure to wrap the relevant lines in `PRAGMA_DISABLE_DEPRECATION_WARNINGS` and `PRAGMA_ENABLE_DEPRECATION_WARNINGS`. This will reduce the spam in the Unreal logs that can otherwise occur during the build process.
+
+```c++
+struct LoadPrimitiveResult { 
+  // List of properties here...
+
+  PRAGMA_DISABLE_DEPRECATION_WARNINGS
+  // For backwards compatibility with CesiumEncodedMetadataComponent.
+  FCesiumMetadataPrimitive Metadata_DEPRECATED{};
+  PRAGMA_ENABLE_DEPRECATION_WARNINGS
+
+  // Other properties here...
+};
+```
+
+### Core Redirects
+
+> [Official Documentation](https://dev.epicgames.com/documentation/en-us/unreal-engine/core-redirects-in-unreal-engine).
+
+Whether you're renaming a class, function, property, or even an enum, it is possible to link between the old and new names to preserve existing Blueprint scripts. This is done using the `[CoreRedirects]` item in `Config/Engine.ini`.
+
+One note: ensure that any entries are on the same line, otherwise they will not be parsed correctly.
 
-## Deprecation
+### Versioning
 
-## Backwards Compatibility

Documentation/style-guide.md

@@ -20,21 +20,19 @@ This guide will explicitly link to sections in the above resources when relevant
 - [Types](#types)
 - [File Organization](#file-organization)
 
-(TODO)
-
 ## Naming
 
 First, check out the [Naming Conventions](https://dev.epicgames.com/documentation/en-us/unreal-engine/epic-cplusplus-coding-standard-for-unreal-engine#namingconventions) section in the Unreal Engine coding standard. Then, read below.
 
-### Prefixes
+### Unreal Prefixes
 
-- Follow Unreal's conventions for naming new structs or classes, e.g, prefixing structs with `F` or actor subclasses with `A`. If these prefixes are absent, the Unreal Header Tool will likely complain during the build process, and subsequently fail to compile your code.
+You will have to Unreal's conventions for naming new structs or classes, e.g, prefixing structs with `F` or actor subclasses with `A`. If these prefixes are absent, the Unreal Header Tool will likely complain during the build process, and subsequently fail to compile your code.
 
-- Although the names should be prefixed in the code, the files containing them should *not* follow this rule. For example, `ACesium3DTileset` is defined in `Cesium3DTileset.h` and implemented in `Cesium3DTileset.cpp`, and *not* `ACesium3DTileset.h` or `ACesium3DTileset.cpp`.
+Although the names should be prefixed in code, the files containing them should *not* follow this rule. For example, `ACesium3DTileset` is defined in `Cesium3DTileset.h` and implemented in `Cesium3DTileset.cpp`, and *not* `ACesium3DTileset.h` or `ACesium3DTileset.cpp`.
 
-### Public API
+### Public vs. Private
 
-Functions and fields in the public API should be written in `PascalCase`. However, use `lowerCamelCase` for any private members or anonymous namespace functions.
+Functions and fields in the public API should be written in `PascalCase`. However, prefer to use `lowerCamelCase` for any private members or anonymous namespace functions.
 
 ```c++
 USTRUCT()
@@ -49,7 +47,19 @@ struct CesiumStruct {
 }
 ```
 
-Additionally, preface every `struct` or `class` in the public API with the word "Cesium". This helps to clearly separate our API from other elements in Unreal, and allows users to more easily search for our components in Unreal.
+When any pointers are used in the public API—whether as public fields or as parameters for functions—omit the `p` prefix. For instance:
+
+```c++
+// Don't do this.
+UCesiumEllipsoid* pEllipsoid;
+void SetEllipsoid(UCesiumEllipsoid* pNewEllipsoid)
+
+// Do this.
+UCesiumEllipsoid* Ellipsoid;
+void SetEllipsoid(UCesiumEllipsoid* NewEllipsoid)
+```
+
+Finally, every `struct` or `class` in the public API should be prefaced with the word "Cesium". This helps to clearly separate our API from other elements in Unreal, and allows users to more easily search for our components in Unreal.
 
 ```c++
 // Don't do this.
@@ -59,12 +69,13 @@ public AGeoreference : public AActor {...}
 public ACesiumGeoreference : public AActor {...}
 ```
 
-### Function and Variable Names
+Note that the above is not enforced for classes used in private implementations. For example, users won't ever interact directly with the `LoadModelResult` or `UnrealAssetAccessor` classes, so there is no need to add a "Cesium" prefix.
 
-Functions and variables should be given names that are descriptive yet succinct. This is important for public-facing API, but even private code should aspire to this standard.
+### Clarity
 
-When in doubt, err on the side of being overly explicit.
-Try to be precise about scope and meaning.
+Functions and variables should be assigned names that are descriptive yet succinct. This is important for public-facing API, but even private code should aspire to this standard.
+
+**When in doubt, err on the side of being overly explicit.** Aim to be precise about scope and meaning.
 
 For example, consider the context of multiple frames of reference in Cesium for Unreal. The plugin has to operate in multiple coordinate systems: Unreal's coordinate system, Earth-Centered Earth-Fixed coordinates, and cartographic coordinates (longitude, latitude, height). Whenever something deals with position or orientation, it is important to distinguish what space it is operating in.
 
@@ -107,19 +118,39 @@ For reference, here is what the counterpart for Unreal coordinate system might l
 void MoveToUnrealPosition(FVector UnrealPosition) {...}
 ```
 
-## Units
+Finally, in the same vein, avoid abbreviations unless they are commonly accepted. This improves readability, such that code is understandable to new developers from a first glance. Additionally, consider developers for whom English is not their first language; those arbitrary abbreviations can be even more confusing.
 
-Although Unreal uses centimeter units, Cesium for Unreal uses meters, which is standard across our runtimes. To avoid confusion, it is best to make remarks about the expected units of a function in its documentation. Be consistent with scale.
+In general, it is good practice for code to read close to plain English. It does not have to be *verbose* English, but it should be succinctly and sufficiently understandable from a glance.
 
-Let's use the function from the previous example. Unreal's coordinate system uses centimeters. We don't want to randomly change the scale.
+``` c++
 
-```c++
-// Confusing!
+// You might know what this means from a glance, but it can be jarring for newcomers.
+static FQuat CalcRotAtLoc(FVector UELoc) {...}
 
-/**
-* Moves the object to the given position in Unreal's coordinate system (in meters).
-*/
-void MoveToUnrealPosition(FVector UnrealPositionInMeters) {...}
+// Better!
+static FQuat CalculateRotationAtLocation(FVector UnrealLocation) {...}
+```
+
+### Units
+
+Ensure that units stay consistent across the API to avoid confusion as values are passed. In particular:
+
+- Although Unreal uses centimeter units, Cesium for Unreal uses meters, which is standard across our runtimes.
+- For cartographic positions, longitude and latitude are expressed in degrees. Height above the WGS84 ellipsoid is expressed in meters.
+
+It's encouraged to leave documentation about the expected units and/or frame of reference for a function and its parameters. This comment in `CesiumGeoreference.h` is a good example:
+
+```c++
+  /**
+   * Transforms a position in Unreal coordinates into longitude in degrees (x),
+   * latitude in degrees (y), and height above the ellipsoid in meters (z). The
+   * position should generally not be an Unreal _world_ position, but rather a
+   * position expressed in some parent Actor's reference frame as defined by its
+   * transform. This way, the chain of Unreal transforms places and orients the
+   * "globe" in the Unreal world.
+   */
+  FVector TransformUnrealPositionToLongitudeLatitudeHeight(
+      const FVector& UnrealPosition) const;
 ```
 
 ## Types
@@ -138,18 +169,6 @@ Generally, the above pointers are used whenever you instantiate something that i
 
 Unreal also provides `TUniquePtr` and `TSharedPtr` definitions, equivalent to their `std` counterparts. For consistency, Cesium for Unreal prefers using `TUniquePtr` and `TSharedPtr`. This is especially important when the type `T` is a `UObject`.
 
-When any pointers are used in the public API—whether as public fields or as parameters for functions—omit the `p` prefix. For instance:
-
-```c++
-// Don't do this.
-UCesiumEllipsoid* pEllipsoid;
-void SetEllipsoid(UCesiumEllipsoid* pNewEllipsoid)
-
-// Do this.
-UCesiumEllipsoid* Ellipsoid;
-void SetEllipsoid(UCesiumEllipsoid* NewEllipsoid)
-```
-
 ### Math
 
 Unreal contains various vector and matrix classes that are functionally similar to the classes in `glm`, e.g., `FVector` for `glm::dvec3`, `FMatrix` for `glm::dmat4`.
@@ -164,7 +183,6 @@ This is especially recommended for matrix math. Previously, `FMatrix` made stron
 >
 > (This may no longer be true, but it has ultimately proven safer to use `glm::dmat4` over Unreal's built-in matrix.)
 
-
 ## File Organization
 
 Cesium for Unreal's source code (located in the `Source` folder) is divided into `Runtime` and `Editor`. The `Editor` folder contains elements that are used within the Unreal Editor only, e.g., UI components or functionality. These will not be included in a packaged build, so don't place anything here that is otherwise required for the application to run!