Fixes #154:Timer doesn't stops in rapidfire #168
Conversation
| @Override | ||
| protected void onPause() { | ||
| counter.cancel(); | ||
| counter= null; |
There was a problem hiding this comment.
Apply proper spacing between the instance and assignment operator. Here and at other places too.
There was a problem hiding this comment.
ok @chhavip Thank you.I will take care of this from the next time. :)
| @Override | ||
| protected void onResume() { | ||
| if(counter==null){ | ||
| counter= new RapidFireTimeCounter(millisLeft,1000); |
There was a problem hiding this comment.
Creating a new timer every time is not the best way to go. Utilize .start and .cancel in onPause and onResume and work with a single timer. Also I dont believe there is a need to set the timer to null while cancelling it.
There was a problem hiding this comment.
@chhavip CountDownTimer once cancelled can't be started again from the same point using the same object. We have to follow this approach only (Cancelling and then creating new Timer which will be given initial time as the time that was remaining i.e millisLeft) :)
As in the mentioned link:
http://stackoverflow.com/questions/15443219/how-to-pause-and-resume-a-timertask-timer?rq=1
There was a problem hiding this comment.
Also @chhavip I have provided the null check in onResume() because when game starts for first time , both onCreate() and onResume() are called before onStart() of activity => If I don't provide null check, it will create 2 timers - one in onCreate() and other in onResume()
also
in onPause() I have to set timer as null because in onResume() I am providing null check, if I won't set null it won't execute the code in onResume(). :) Thanks
There was a problem hiding this comment.
I do not have a problem with the null check. The timer does not need to set to null and reinitialised is all I am saying. You can cancel it in onPause and then in onResume set the time from sharedPreffs and start it again.
There was a problem hiding this comment.
@HimanshuS01 As mentioned,could you come up with another solution?
There was a problem hiding this comment.
sorry for replying late. I was stuck in my exams . Will update you soon with another solution :-)
There was a problem hiding this comment.
@HimanshuS01 Since, other PRs sent are being merged you may get some conflicts, please remember to resolve them. 👍
|
@yatna Thank You for reviewing it. I will resolve the following conflicts. |
HimanshuS01
force-pushed
the
TimerIssue
branch
from
a4f2259 to
01cd547
Compare
|
PR belongs to code before infrastructure changes, closing this PR. Issue will be |
Fixes #154
Changes
Fix - When app goes in background, the timer should be cancelled in onPause() and when user comes back in onResume(), the new timer should be creating starting from same point (Keeping a variable millisLeft for it)
@chhavip @yatna @MeepyMay Please review my PR.Thanks. :)