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

Rename vertex-specific functions in FCesiumFeatureIdAttribute #1574

Merged
merged 7 commits into from
Dec 24, 2024

Conversation

j9liu
Copy link
Contributor

@j9liu j9liu commented Dec 20, 2024

With #1537 the FCesiumFeatureIdAttribute class gets used for both per-vertex and per-instance feature IDs. This makes the "vertex" verbiage in the API inaccurate.

This PR renames those functions so they are more generic. I added Redirects so that Blueprints can seamlessly pick up the new functions. If you'd like, you can test this by:

  • Put the old nodes in the Blueprint of a level. Save, and then exit Unreal.
  • Switch to this branch and open Unreal. Open the level Blueprint -- the nodes should be properly renamed.

Base automatically changed from instance-feature-metadata to main December 20, 2024 21:22
@j9liu j9liu added this to the January 2025 Release milestone Dec 23, 2024
*/
UFUNCTION(
BlueprintCallable,
BlueprintPure,
Category = "Cesium|Features|FeatureIDAttribute")
static int64
GetVertexCount(UPARAM(ref)
const FCesiumFeatureIdAttribute& FeatureIDAttribute);
GetFeatureIDCount(UPARAM(ref)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This might be a bit confusing on second glance -- I don't want this to convey the # of actual unique feature IDs versus. the length of the attribute. Let me tweak this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Settled on GetCount with some additional documentation. Open to other alternatives too.

@j9liu
Copy link
Contributor Author

j9liu commented Dec 23, 2024

@kring do you mind giving this a review?

@kring
Copy link
Member

kring commented Dec 24, 2024

Looks good, thanks @j9liu!

@kring kring merged commit 633b037 into main Dec 24, 2024
16 checks passed
@kring kring deleted the rename-features-getter branch December 24, 2024 04:50
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