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

Move logic for real-time mode (SYSTEM_CLOCK) into Clock itself. #4007

Merged
merged 4 commits into from
Jun 10, 2016

Conversation

shunter
Copy link
Contributor

@shunter shunter commented Jun 7, 2016

SYSTEM_CLOCK implies animating forward at a 1x multiplier. Centralize the logic for dealing with coordinating clock settings in Clock itself. When switching to SYSTEM_CLOCK, other values are immediately updated to be consistent. When changing other values, e.g. current time or multiplier, SYSTEM_CLOCK is deactivated.

This fixes some inconsistencies in the UI where the logic was not consistent, due to being scattered around. Specifically, when you dragged the timeline while in realtime mode, you ended up in a hybrid state where the mode was still set to SYSTEM_CLOCK and therefore the label in the UI still said "Today" etc.

SYSTEM_CLOCK implies animating forward at a 1x multiplier.  Centralize the logic for dealing with coordinating clock settings in Clock itself.  When switching to SYSTEM_CLOCK, other values are immediately updated to be consistent.  When changing other values, e.g. current time or multiplier, SYSTEM_CLOCK is deactivated.

This fixes some inconsistencies in the UI where the logic was not consistent, due to being scattered around.  Specifically, when you dragged the timeline while in realtime mode, you ended up in a hybrid state where the mode was still set to SYSTEM_CLOCK and therefore the label in the UI still said "Today" etc.
@mramato
Copy link
Contributor

mramato commented Jun 9, 2016

I'm getting a test failure for Widgets/Viewer/Viewer updates the clock when the data source changes

@shunter
Copy link
Contributor Author

shunter commented Jun 9, 2016

Fixed.

@shunter
Copy link
Contributor Author

shunter commented Jun 9, 2016

One thing I noticed while looking at DataSourceClock is that it blindly copies all values from itself into the Clock instance, but a default-constructed DataSourceClock has undefined values for all settings, which would corrupt the clock I believe. Now fortunately the CzmlDataSource and KmlDataSource make sure to set all the properties no matter what, otherwise it would wreck everything, but should the getValue be smarter about checking for undefined values and not changing the Clock?

@mramato
Copy link
Contributor

mramato commented Jun 10, 2016

Now fortunately the CzmlDataSource and KmlDataSource make sure to set all the properties no matter what, otherwise it would wreck everything, but should the getValue be smarter about checking for undefined values and not changing the Clock?

We certainly shouldn't put the clock in an invalid state. Feel free to open another PR with DataSourceClock changes. That code is really old and used to be specific to CZML, so it probably just evolved outside of its original expected usage.

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

2 participants