Merge lp:~majcherlk/widelands/bug-1243700 into lp:widelands

[Lukasz]

Changes in displaying statistics in order to fix bug 1243700. The time displayed in statistics is rounded to integer: game_time/ticks so everytime number of ticks was not divisible by time game we got incorrect time value. Now game time is always divisible by number of ticks. Another way to solve this bug is to display fractional time but I think the first way is more user friendly. Any suggestions?

[SirVer (sirver)]

Generally I think this is a good solution. A couple of comments inlined.

sorry for taking so long to answer this merge request :/.

review: Needs Fixing

[Lukasz (majcherlk)]

max_x is time to display on graph and how_many_ticks is number of intervals on graph. To get correct data on graph, how_many_ticks has to be a divisor of max_x.
The code tries to find a nice divisor of max_x (by nice I mean some number between 3 and 7 so data on graph are readable). To do it the while loops divide how_many_ticks by prime numbers till we get nice number. I changed the order of loops - now dividing by 5 is first and we get more readable intervals (e.g. without this change for 60min we got intervals: [0,12,24,36,48,60] and now we get [0,10,20,30,40,50,60]).

[SirVer (sirver)]

thanks, I understood that while reading this merge request because I had the context. What I meant is that you should add this documentation to the code, so that future readers of the code still understood your thinking.

Sorry for not being clear - generally when somebody asks for clarification in a code review, it is always best to answer in the code, not the comments.

review: Needs Fixing

[TiborB (tiborb95)]

Looks OK now.

Are you able to merge it to trunk?

review: Approve

[Lukasz (majcherlk)]

> Looks OK now.
>
> Are you able to merge it to trunk?
I don't have rights to merge it to trunk.

[TiborB (tiborb95)]

I was not sure so I asked. I will merge it.

[TiborB (tiborb95)]

Merged. Thanks for contribution

=== modified file 'src/wui/plot_area.cc'
--- src/wui/plot_area.cc	2014-12-11 12:38:10 +0000
+++ src/wui/plot_area.cc	2015-04-23 23:35:21 +0000
@@ -155,27 +155,22 @@
 	UNIT unit = get_suggested_unit(time_ms);
 	max_x = ms_to_unit(unit, time_ms);
 
-	// Find a nice division of max_x
-	if (max_x % 5 == 0) {
-		if (max_x <= 10) {
-			how_many_ticks = 5;
-		} else {
-			how_many_ticks = max_x / 5;
-			while (how_many_ticks > 7 && how_many_ticks % 2 == 0) {
-				how_many_ticks /= 2;
-			}
-			while (how_many_ticks > 7 && how_many_ticks % 3 == 0) {
-				how_many_ticks /= 3;
-			}
-			while (how_many_ticks > 7 && how_many_ticks % 5 == 0) {
-				how_many_ticks /= 5;
-			}
-			while (how_many_ticks > 7 && how_many_ticks % 7 == 0) {
-				how_many_ticks /= 7;
-			}
-		}
-	} else {
-		how_many_ticks = 4;
+	// Try to find a nice division of max_x (some number between 3 and 7 so data on graph are
+	// readable) - to get correct data on graph, how_many_ticks has to be a divisor of max_x. Now
+	// dividing by 5 is first and we get more readable intervals.
+	how_many_ticks = max_x;
+
+	while (how_many_ticks > 10 && how_many_ticks % 5 == 0) {
+		how_many_ticks /= 5;
+	}
+	while (how_many_ticks > 7 && how_many_ticks % 2 == 0) {
+		how_many_ticks /= 2;
+	}
+	while (how_many_ticks > 7 && how_many_ticks % 3 == 0) {
+		how_many_ticks /= 3;
+	}
+	while (how_many_ticks > 7 && how_many_ticks % 7 == 0) {
+		how_many_ticks /= 7;
 	}
 
 	// first, tile the background