Separated gestalt from NUI core code

[BenjaminAmos]

Description

This pull request attempts to move all gestalt-using code out of the NUI core libraries (nui-input, nui-reflect and nui). It does this for additional portability to non-gestalt platforms, as well as to make it easier to support both gestalt-5 and gestalt-7 within the same branch. This should hopefully make it easier to maintain NUI, as it effectively combines the 1.0 and 2.0 branches together. This does introduce some breaking changes, however their effect on UI code should be minimal. This will require some changes in implementing engine code though.

The new nui-gestalt5 and nui-gestalt7 libraries contain the previously gestalt-specific code but I have not changed the packages at the moment for compatibility reasons. They are essentially copies of each other but for their respective gestalt versions. As the asset classes are not typically changed as frequenly as the UI widgets, keeping the nui-gestalt5 and nui-gestalt7 in sync should be mostly possible as a temporary measure.

I am not entirely sure about the ClassLibrary changes made here but I felt using potentially gestalt-specific patterns e.g. module urns as class look-up identifiers, was not suitably generic.

This is a potential solution to #43 and an alternative to #46. This does not preserve any unique 2.x commits though, as these changes are based on the considerably more up-to-date 1.x branch. I am not aware of any functional changes present in 2.x are not present in 1.x.

Summary of Changes

• Removed all @API annotations from nui, nui-input and nui-reflect
• Removed package-info.java files not containing package-level documentation
• UISkin no longer inherits from Asset and no longer depends on UISkinData
• UISkinData had been moved into nui-gestalt5/nui-gestalt7 for compatibility. It is now just a wrapper around a UISkin
• UISkinAsset from nui-gestalt5/nui-gestalt7 is an asset containing a UISkin instance
• Moved all gestalt-dependant classes into both nui-gestalt5 and nui-gestalt7
• Refactored the ClassLibrary interface to be less gestalt-specific

Migration

Removed @API annotations

In order for modules to have access to NUI packages after these changes, you will need to add the relevant packages to the API package whitelist.

String[] NUI_PACKAGES = new String[]{
    "org.terasology.input",
    "org.terasology.input.device",
    "org.terasology.input.device.nulldevices",
    "org.terasology.nui",
    "org.terasology.nui.asset",
    "org.terasology.nui.asset.font",
    "org.terasology.nui.canvas",
    "org.terasology.nui.databinding",
    "org.terasology.nui.events",
    "org.terasology.nui.itemRendering",
    "org.terasology.nui.layouts",
    "org.terasology.nui.layouts.miglayout",
    "org.terasology.nui.layouts.relative",
    "org.terasology.nui.properties",
    "org.terasology.nui.reflection",
    "org.terasology.nui.skin",
    "org.terasology.nui.translate",
    "org.terasology.nui.util",
    "org.terasology.nui.widgets",
    "org.terasology.nui.widgets.treeView",
    "org.terasology.nui.widgets.types",
    "org.terasology.nui.widgets.types.builtin",
    "org.terasology.nui.widgets.types.builtin.object",
    "org.terasology.nui.widgets.types.builtin.util",
    "org.terasology.nui.widgets.types.math",
    "org.terasology.reflection", // TODO: Is this safe?
    "org.terasology.reflection.metadata", // TODO: Is this safe?
    "org.terasology.reflection.reflect", // TODO: Is this safe?
    "org.terasology.reflection.copy", // TODO: Is this safe?
};

for (String package : NUI_PACKAGES) {
    permissionFactory.getBasePermissionSet().addAPIPackage(package);
}

Moved gestalt-dependant classes into nui-gestalt5/nui-gestalt7

If using gestalt 5, then you should add a dependency on the nui-gestalt5 package e.g.

implementation 'org.terasology.nui:nui-gestalt5'

If using gestalt 7, then you should add a dependency on the nui-gestalt7 package e.g.

implementation 'org.terasology.nui:nui-gestalt7'

Refactored ClassLibrary to be less gestalt-specific

In most cases, if you were using AbstractClassLibrary, you should now use ModuleClassLibrary instead. ModuleClassLibrary is a re-named version of the former AbstractClassLibrary base class. If you used DefaultClassLibrary, then you should now use DefaultModuleClassLibrary instead. In the base ClassLibrary interface, all ResourceUrn parameters have been replaced with String parameters. This also applies to ClassMetadata. ResourceUrn method overloads
have been moved into ModuleClassLibrary.

UISkin no longer inherits from Asset and no longer depends on UISkinData

Whenever you were previously using UISkin as an asset type, you now use UISkinAsset instead e.g.

-  assetTypeManager.registerCoreAssetType(UISkin.class,
-        UISkin::new, "skins");
+  assetTypeManager.registerCoreAssetType(UISkinAsset.class,
+        UISkinAsset::new, "skins");

To obtain a UISkin instance from a UISkinAsset, call the UISKin.getSkin() method.

Notes

• As an example of how this might affect existing UI code, you can see the changes I made to make this work with Terasology on my nui-gestalt-separation branch, specifically BenjaminAmos/Terasology@36a8fb6. No changes were required in module code, apart from WordlyTooltip, which was not using the standard Assets.getSkin method.

[keturn]

Big fan overall! It enables Terasology to decouple these dependencies while upgrading, and it seems like it's a good thing for NUI too, as the API annotations weren't exactly for NUI's benefit.

Refactored ClassLibrary to be less gestalt-specific

I am not entirely sure about the ClassLibrary changes made here but I felt using potentially gestalt-specific patterns e.g. module urns as class look-up identifiers, was not suitably generic.

That touches on a subject I've been unclear about when reading over Terasology code of varying ages. There was SimpleUri which was replaced by ResourceUrn?

If, for NUI's use case, it just needs an identifier and it doesn't need to parse it in to parts, or compose a new one from parts, or have specific comparison rules, sticking with plain old String sounds fine.

Suggestion: Rename the parameters from uri to something like id or name in that case? Easy enough to do for method parameters, I guess if uri appears as part of public method names or fields then that gets in to API changes.

Removed API annotations

In order for modules to have access to NUI packages after these changes, you will need to add the relevant packages to the API package whitelist.

If I understand correctly, this is the motivating force behind #27. To make it easier to make rules about things under the org.terasology.nui prefix and make it unambigious if things like org.terasology.reflections are from NUI or gestalt or Terasology, etc.

(Note this is very much not saying that #27 should be combined in to this PR! Having them separate helps. I bring it up to make sure we're on the same page about next steps and to note that we don't expect to need to maintain that list beyond the transition.)

Are there any public classes in NUI you expect a module should not have access to? e.g. access to arbitrary filesystem paths, loading resources from HTTP URLs, etc.

[BenjaminAmos]

I've just pushed a commit that should rename the uri parameters to id and urn respectively, depending on how they're used (ModuleClassLibrary converts the ids to a ResourceUrn internally). There was a public ClassMetadata#getUri() method, so I deprecated it and created a replacement ClassMetadata#getId() instead, which should mostly preserve API compatibility.

In terms of the areas of NUI that modules's shouldn't have access to, The NUI core libraries don't do any network/filesystem access directly that I'm aware of. The nui-gestalt packages use gestalt, so they might use the filesystem indirectly. Almost all of NUI was covered by @API annotations before, so it's actually easier to list which packages weren't covered by @API annotations:

• org.terasology.nui.widgets.types
• org.terasology.nui.reflection (different to org.terasology.reflection)
• org.terasology.nui.canvas
• org.terasology.input.device.[Except InputDevice, KeyboardDevice and MouseDevice]
• org.terasology.reflection.[Except MappedContainer and TypeInfo]
• org.terasology.reflection.copy
• org.terasology.reflection.reflect
• org.terasology.reflection.reflect.internal

There's an updated list of packages needed to get Terasology working with these changes here, although it's essentially everything in nui apart from the packages listed above. If #27 and gestalt 7 can remove the need for the whitelist afterwards though, then that's even better.

[keturn]

I'm interested in moving forward with this quickly so we can keep up the momentum that's going in to Tersasology-on-gestalt-v7.

I haven't seen anything in this PR that's raised red flags for me, but I hesitate to be the one to give it the "1 approving review" go-ahead because (a) a lot of the diff is from code moved from one place to another, and I'm not sure if any of it needs special attention, and (b) I don't have the experience with either NUI's implementation or usage to guess what the problems might be.

The other question I have is: after we do approve this, how do we release the result?
As 1.5.0? As 3.0?

If NUI 2.0 users will be migrating to this, it doesn't make much sense to call it 1.5.

[DarkWeird]

Looks good for me.

Maybe you count major version to v3? :D