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

Clearer Documentation on CesiumCameraManager #515

Open
Reag opened this issue Oct 14, 2024 · 1 comment
Open

Clearer Documentation on CesiumCameraManager #515

Reag opened this issue Oct 14, 2024 · 1 comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@Reag
Copy link

Reag commented Oct 14, 2024

First off, thank you for adding the CesiumCameraManager. We've been manually adding this functionality by maintaining our own fork of the repo, which has been nightmarish. The addition of this component is very welcome to my team (and my sanity), and now we can stop attempting to compile the c++ code every update.

I'd like to request a minor documentation change in the component however. I was under the assumption that this manager was a global manager that I could place with my other singletons and bootstrapped Scriptable-Objects. However when I attempted this, the manager would not function. After some experimentation, I discovered that I had to add it directly to the CesiumGeoreference I was displaying and configure it.

While this problem is now solved for me, it might be a good idea to add a bit more documentation to the summary of the CesiumCameraManager to explain how its used! Something akin to

Manages the set of cameras that are used for Cesium3DTileset culling and level-of-detail.
Attach this component to the Cesium CesiumGeoreference.

I believe this small change would prevent future users from falling into the trap I fell into, believing that the component is broken. Once again, thank you for adding this feature, its a lifesaver!

@j9liu j9liu added documentation Improvements or additions to documentation good first issue Good for newcomers labels Nov 15, 2024
@j9liu
Copy link
Contributor

j9liu commented Nov 15, 2024

I'm not sure how we missed this @Reag but thank you for the feedback! That extra detail makes sense, though I might tweak the wording to say:

Must be attached to a game object with a <see cref="CesiumGeoreference"/> component.

If you have the bandwidth, we'd happily welcome a community contribution for this 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants