aricneto / TwistyTimer

Twisty Timer is a material design twisty puzzle timer for Android. It uses the TNoodle library to generate scramble sequences for all current official speedsolving puzzles.
GNU General Public License v3.0
241 stars 53 forks source link

All my recent pull requests, plus "THE BIG ONE(tm)", on one branch #45

Closed damo closed 7 years ago

damo commented 8 years ago

I'd be grateful for some feedback on all of this. Reasonably up-to-date screen shots can be seen on issue #37 ("Best AoN" circles on the graphs arrived after those screen shots were taken).

I'll probably just build and install the contents of "my_master" branch for my own use in the meantime.

The main changes are to the management and presentation of the statistics and chart. There are also lots of improvements to stability (lots of crashes fixed), to state restoration (for orientation changes), to start-up time (less database reading and only reading things once), to importing from text (about 50x faster and supports DNFs), and to the disconcerting behaviour of the timer ("0.00" is now displayed while "holding" or if deleted).

There is a separation of the session statistics into "Best" and "Current", which show the best values across all times in the current session and the values for the most recent times up to the last added time. For example, the "Session Best" Ao5 is the best Ao5 for any five consecutive times during the current session; while the "Session Current" Ao5 is the Ao5 of the last five times to be added in the session. The previous behaviour was only showing "Session Current", which made it impossible to tell if the all-time best was set in the current session or not (well, it required a good memory, or writing down values at the start of the session and checking them later).

The "All Time Best" times include times from the current session. This was already the case for the statistics table, but it is now also the case for the graph.

The statistics are loaded in one big read from the database and distributed to all fragments. This is pretty fast (10-50 ms for 1,000 times on an emulator). They are kept up to date as solve times are changed. If a new solve time is added, the statistics (and chart) can be kept up to date without a new database read. The same statistics are used on the timer tab (summary statistics and best/worst detection), list tab (for sharing) and graph tab (for the statistics table). The new data is loaded using the Loader API, which keeps things in sync with the Fragment life-cycles and prevents some crashes that were happening when results arrived after fragments had been destroyed or detached. A "StatisticsCache" class makes it very easy to share "Statistics" and be notified of changes. It also ensures that the fragments can easily handle the cases where the statistics are first loaded either before or after the creation/destruction of the fragments' views. The chart statistics are loaded in a similar manner, but are only maintained by the TimerGraphFragment.

The new "Statistics", "ChartStatistics" and "AverageCalculator" classes provide all of the smarts. They do not store all times in memory, just the most recent "N" times for each AoN. The ChartStatistics store the times in "LineData" for the chart, which is no different from before, but they depend on a separate internal "Statistics" instance to provide the data for the AoN trend lines. The "Statistics" and "AverageCalculator" classes are different, though, as they now hold data in memory. However, the total increased memory use is in the order of a few tens of kilobytes, which should not cause any problems. The memory consumption does not increase after 1,000 solve times have been recorded, as Ao1000 is the highest average-of-N used. (This does not hold for the chart, though, but that is not much different from before, save that it shows more lines, which grow longer with more solve times.)

Great app. Hope you like my tweaks.

Combinacijus commented 8 years ago

Hey can you put apk with your updates on google drive or somewhere?

damo commented 8 years ago

This is an APK for a current debug build (commit 749cc29). However, you cannot install it on a device that already has TwistyTimer installed, as the signing key is not the same. You will have to uninstall TwistyTimer, LOSE ALL YOUR DATA, and then install this APK.

THIS IS AN UPDATE TO THE ORIGINAL APK

TwistyTimer-35e81a9-damo-debug.apk.zip

Combinacijus commented 8 years ago

It crashes when I try to open other tab. (like graph)

aricneto commented 8 years ago

damo, thank you SO much for all the improvements you brought to the app. I absolutely love them. I'll have some of my free time back come the end of the next week and I'll be sure to merge these pull requests!

Sorry for the shoddy coding you had to sift through to implement these features haha :p this was my first Android project ever, but I learned a lot both from it and from your changes. Sorry also for the lack of communication, I'll try to be more active in the future.

Again, I can't thank you enough for all these changes. I like to list everyone who contributed to the project (one way or the other) on the app, so let me know how I can credit you for the update.

Cheers!

damo commented 8 years ago

@aricneto There is still a little bit of work left on that pull request: the translations for some of the strings are missing or just guessed at. Release builds fail to complete as a result. That, plus I haven't really tested it all out in anger yet. Which brings me to...

@Combinacijusx Was that crash on a clean install of the APK? What version of Android are you running? Do you have any logs that you can attach? There are apps that can save the system log to a file, which would make it much easier to find out what is going wrong. Could you attach some logs to a comment here? It's possible that it is not working because there is no initial solve data. I haven't tested that condition for a while, so I'll look into it.

In other news.... I'm working on improving the state restoration after rotations, etc. Before any changes, if the screen is rotated, the timer keeps running with the old orientation, but when it is stopped, the new orientation is applied and the timer resets to zero. The time is actually saved, but the timer screen shows 0.00 instead of the new time. I want to get to a point where the timer will keep running when the screen is rotated, and run in the new orientation, and where the solve and scramble will be preserved through orientations. I'm also trying to make the app remember the puzzle type from the last time it was run. It's looking like another big change. I'm starting out by reducing the number of fields in the fragment classes to a minimum (which is having some knock-on effects). Once that's done, I can figure out what is instance state and what is not.

We'll save any credits until all this is working again! It'll give me an incentive.

damo commented 8 years ago

@Combinacijusx I found the problem (merging issue; my bad). Try this new APK:

TwistyTimer-35e81a9-damo-debug.apk.zip

(I updated my earlier comment with the same link. That other APK never existed...got it?)

damo commented 8 years ago

@aricneto I bumped _mymaster branch to commit 35e819a which fixes the crash in the APK. Just an NPE from something that I overlooked during the merge of all my branches.

Combinacijus commented 8 years ago

@damo Tested your apk. In settings after pressing back key (on phone) it shows white screen and by pressing again goes back how it should do. Changed some setting used back button few times and finally app crashes 😁 And it have laggy feeling (don't know why).

So I'll stay with origal APK. Unless you need testing.

I would like to help more but I'm bad with GitHub (first time) and just recently started learning Android Studio.

damo commented 8 years ago

I appear to have broken a few more things:

I'll fix all these soon.

damo commented 8 years ago

Pushed some new fixes to "my_master". The transitions into and out of the Settings are now working properly, with a single "Back" press to exit and the MainActivity then recreates itself properly. The settings code has been simplified, combining the two activities into one. The MainActivity code has been simplified (particularly in the area of export/import, as the dialog now holds all the details before the activity executes the task). The position of the spinner on the statistics table is fixed, too (needed to use "INVISIBLE" instead of "GONE", to avoid messing up the layout).

Access to the shared preferences is also a bit simpler, using string resource IDs for the keys and a little "Prefs" class to wrap up the read and write operations. I'll extend this to other code still accessing preferences using hard-coded key names.

I just did a quick smoke test, so there are still probably some broken things.

aricneto commented 7 years ago

After a long delay, I'm finally able to work on this thing again. I'll be releasing all of damo's improvements plus some other minor things in a beta test (will be advertised on /r/cubers) to iron out the bugs, and hopefully have a full release soon.

Bugs that I found in this version (I plan to fix these before the beta starts):

I'll also take a look at the other pull requests and implement them when I'm done figuring out these bugs.

Again, thank you so much @damo for all the work you put into this!

aricneto commented 7 years ago

Just noticed you have a branch that is way ahead of this one, so I'll wait for your input before I do anything. I've a lot to learn still hehe.