Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for new defaults service to CesiumIonClient. #773

Merged
merged 3 commits into from
Dec 14, 2023

Conversation

kring
Copy link
Member

@kring kring commented Dec 4, 2023

This is a PR into #772 so merge that first.

Adds support for Cesium ion's new defaults service which will be used to populate the Quick Add panels. The new service is not available in the public Cesium ion just yet, though.

Comment on lines 90 to 93
struct Defaults {
DefaultAssets defaultAssets{};
std::vector<QuickAddAsset> quickAddAssets{};
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is defaultAssets or the DefaultAssets struct actually used for anything? Can we remove it and rename Defaults -> DefaultAssets? It would make this more intuitive to read in the Unreal Editor code

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this rename would be wide-reaching, so if you agree with the decision I am happy to do the name replacements on the Unreal / Unity side

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't currently plan to use the DefaultAssets for anything in Unreal or Unity, at least, but it's part of ion's /v1/defaults service, so we should include it. Also, as mentioned offline, I think we need to stick to the names that the service documentation uses, so we shouldn't rename Defaults to something else. It's namespaced (CesiumIonClient::Defaults) so hopefully not too bad.

Base automatically changed from fix-api-url to main December 14, 2023 00:13
@kring kring merged commit 05fa364 into main Dec 14, 2023
14 checks passed
@kring kring deleted the cesium-ion-defaults branch December 14, 2023 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants