Bug 321028 - Time step is incorrect when switching time flow direction
Summary: Time step is incorrect when switching time flow direction
Status: RESOLVED FIXED
Alias: None
Product: kstars
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal (vote)
Target Milestone: ---
Assignee: Rishab Arora
URL:
Keywords: junior-jobs
Depends on:
Blocks:
 
Reported: 2013-06-11 14:22 UTC by Rishab Arora
Modified: 2013-06-14 18:16 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Proposed patch. (1.76 KB, patch)
2013-06-12 12:52 UTC, Aneesh Dogra
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Rishab Arora 2013-06-11 14:22:12 UTC
The Local Time seems to continue for one unit in the same direction even if you click on the opposite step button.

Reproducible: Always

Steps to Reproduce:
1. In the main toolbar, change the step size to say, 10 minutes to make the effect more prominent
2. Click on "Advance one step Forward in time" twice.
3. Check time
3. Click on "Advance one step Backward in time" once.
4. Check time
Actual Results:  
Local Time increases even after Clicking on "Advance one step Backward in time" once. UT works fine, only LT is affected.
Only happens the first time you switch directions.

Expected Results:  
+10 min for forward
-10 min for backward
Comment 1 Aneesh Dogra 2013-06-11 16:25:39 UTC
Somehow the timeChanged() event is emitted one move later. I don't know the reason though, but if you add 2 emits in SimClock::setUTC, it fixes the issue.
Comment 2 Rishab Arora 2013-06-12 11:31:41 UTC
The issue seems to occur because the slot that updates local time and the slot that updates the info widget are connected to the same signal timeChanged(). And the order of connection is reserved (ideally, that shouldn't matter) and LT is updated just after the info box reads it.
Comment 3 Aneesh Dogra 2013-06-12 12:52:17 UTC
Created attachment 80474 [details]
Proposed patch.

The time in the timebox lags by one step when the user advances one step backward/forward in time. The happens because the timebox updater function and local time updater function are connected with the same signal i.e timeChanged. The order of connection to this signal is reversed. We first connect the timebox updater and then the local time updater. This causes the timebox to be updated before the local time; hence, causing the lag. The patch simply uses the correct order, i.e connects the local time updater to the signal before it connects the timebox. This ensures that local time gets updated before the timebox.
Comment 4 Rishab Arora 2013-06-13 14:53:17 UTC
Git commit 35a3949b217c0d8dcd21695f0eff9f26220073b9 by Rishab Arora, on behalf of Aneesh Dogra.
Committed on 13/06/2013 at 16:42.
Pushed by rishabarora into branch 'master'.

Update the local time before we update the timebox.
Fixes the issue with time displayed being one step behind.
Related: bug 296829
REVIEW: 110973

M  +3    -3    kstars/kstarsinit.cpp

http://commits.kde.org/kstars/35a3949b217c0d8dcd21695f0eff9f26220073b9
Comment 5 Rishab Arora 2013-06-14 17:46:41 UTC
The fix introduced a new bug where the behavior of the sky changed when step size is large.

The log contains the following new entry:
QObject::connect: Cannot connect SimClock::scaleChanged( float ) to (null)::slotClockSlewing()

Reopening the bug for now.
Comment 6 Aneesh Dogra 2013-06-14 17:57:36 UTC
Was a very silly little mistake. Fixed in: https://git.reviewboard.kde.org/r/111027/
Comment 7 Rishab Arora 2013-06-14 18:16:02 UTC
Git commit 782083bc9b7510a5c18e218789e9791332452c0f by Rishab Arora, on behalf of Aneesh Dogra.
Committed on 14/06/2013 at 20:13.
Pushed by rishabarora into branch 'master'.

Fixed regression from earlier fix.
Skymap should be inited before we register any of its methods to a signal.

M  +3    -2    kstars/kstarsinit.cpp

http://commits.kde.org/kstars/782083bc9b7510a5c18e218789e9791332452c0f