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 camera#flying property #5993

Closed
wants to merge 4 commits into from
Closed

Conversation

gberaudo
Copy link
Contributor

This read-only property allows the flying information to be easily accessed from everywhere.

This read-only property allows the flying information to be easily accessed
from everywhere.
@cesium-concierge
Copy link

Signed CLA is on file.

@gberaudo, thanks for the pull request! Maintainers, we have a signed CLA from @gberaudo, so you can review this at any time.

⚠️ I noticed that CHANGES.md has not been updated. If this change updates the public API in any way, fixes a bug, or makes any non-trivial update, please add a bullet point to CHANGES.md and comment on this pull request so we know it was updated. For more info, see the Pull Request Guidelines.


I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

Previously, when trying to add a tween with a duration of 0, its complete
function was immediately called, it was not added to the collection and
a fake/empty tween was returned.

This was bringing confusion in the flyTo code where the result of the add
is put into the current_flight variable.

With this change a null is now returned.
@gberaudo
Copy link
Contributor Author

I added 2 more commits to fix issues with the current_flight property not beeing updated in some cases.

@hpinkos
Copy link
Contributor

hpinkos commented Nov 21, 2017

Thanks for the PR @gberaudo! Can you briefly explain why this change is helpful?

@ggetz
Copy link
Contributor

ggetz commented Nov 21, 2017

There are moveStart and moveEnd events, so it's possibly, albeit a little clumsier, to check this already.

@gberaudo
Copy link
Contributor Author

Thanks for the quick reply and advice.

I prevent the user for moving too far away from a sphere. When it is the case, I fly it back to home.
However I do not want that mechanism to triggers when the user is already flying.

You are right @ggetz, these events looks great to me. I will have a try to use them and will reopen the PR if needed.

@gberaudo gberaudo closed this Nov 23, 2017
@gberaudo gberaudo reopened this Nov 24, 2017
@gberaudo
Copy link
Contributor Author

I looked at it @ggetz. Unfortunately I can not use these events because they are too broad, including any movement of the camera, not just flying.

My mechanism triggers when the user moves too far from the sphere using mouse control, not when flying.

This PR simply fixes how Cesium tracks in progress flights, and export the value. There may be better way to do it though, and I am opened to your suggestions.

@cesium-concierge
Copy link

Thanks again for the pull request!

Looks like this pull request hasn't been updated in 30 days since I last commented.

To keep things tidy should this be closed? Perhaps keep the branch and submit an issue?

I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

@hpinkos
Copy link
Contributor

hpinkos commented Jun 25, 2018

Hi @gberaudo, thanks for opening this pull request, but I don't think this is something we want to add to our API. As always, we're happy to look at any PRs you want to submit in the future!

@hpinkos hpinkos closed this Jun 25, 2018
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.

None yet

4 participants