Page MenuHomeWildfire Games

Dynamically sizes the dataCounter overlay
Needs ReviewPublic

Authored by ramtzok1 on Fri, Jan 25, 6:05 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Trac Tickets
#5385
Summary

This is a simple fix is for #5385.

Test Plan

Switch to the Bulgarian language and confirm the fps counter is fully shown

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6912
Build 11326: arc lint + arc unit

Event Timeline

ramtzok1 created this revision.Fri, Jan 25, 6:05 PM
Owners added a subscriber: Restricted Owners Package.Fri, Jan 25, 6:05 PM
ramtzok1 edited the summary of this revision. (Show Details)Fri, Jan 25, 6:23 PM
elexis added a subscriber: elexis.Fri, Jan 25, 6:49 PM

This is a code problem, not a string problem. It can happen with other languages too. I think the same problem exists with some playernames in the lag-warnings area too.
The *.po files are downloaded from transifex, so they would be edited there ifanything. But I would recommend to not change it that way.

smiley added a subscriber: smiley.Fri, Jan 25, 7:14 PM

Removing one arbitrary number with another arbitrary number can fix this.

Index: binaries/data/mods/public/gui/common/functions_global_object.js
===================================================================
--- binaries/data/mods/public/gui/common/functions_global_object.js	(revision 21929)
+++ binaries/data/mods/public/gui/common/functions_global_object.js	(working copy)
@@ -17,9 +17,9 @@
 	dataCounter.caption = counters.join("\n") + "\n";
 	dataCounter.hidden = !counters.length;
 	dataCounter.size = sprintf("%(left)s %(top)s %(right)s %(bottom)s", {
-		"left": "100%%-100",
+		"left": "100%-110",
 		"top": "40",
-		"right": "100%%-5",
+		"right": "100%-5",
 		"bottom": 40 + 14 * counters.length
 	});
 }

However, this would also break given a lengthy enough string. The correct solution would be to adapt the sizes based on the width of the longest string just as how it's done for the network warning thingy. Engine.GetTextWidth(font, string) IIRC.

In D1764#71050, @elexis wrote:

This is a code problem, not a string problem. It can happen with other languages too. I think the same problem exists with some playernames in the lag-warnings area too.
The *.po files are downloaded from transifex, so they would be edited there ifanything. But I would recommend to not change it that way.

Do you have any reference to the playernames problem so I can try to understand the linking between them?

I think the same problem exists with some playernames in the lag-warnings area too.

It won't exist there as the width check is done in that specific case.

In D1764#71050, @elexis wrote:

This is a code problem, not a string problem. It can happen with other languages too. I think the same problem exists with some playernames in the lag-warnings area too.
The *.po files are downloaded from transifex, so they would be edited there ifanything. But I would recommend to not change it that way.

I found the script "functions_global_object.js" interesting.
The function updateCounters updates the overlay, I think we need to make an if-statement ask if the text overflows the overlay width then subtract X to "left" field but I can't find a way to do so.
For example, in Bulgarian if I subtract to the "left" field 5, it fixes the problem.

dataCounter.size = sprintf("%(left)s %(top)s %(right)s %(bottom)s", {
		"left": "100%%-105", // Was 100
		"top": "40",
		"right": "100%%-5",
		"bottom": 40 + 14 * counters.length
	});

Maybe try rather adapts the code with the fonction enunciate by smiley.

So the updateCounters function that smiley mentioned uses a fixed width, but the width should change dynamically. I thought this was already the case. Also as he correctly pointed out, Engine.GetTextWidth can be used for that, like getNetworkWarnings() in binaries/data/mods/public/gui/common/network.js does. If you want you can try to download the code and try to implement that (JavaScript changes only, no need to compile). BuildInstructions.

ramtzok1 updated this revision to Diff 7402.Sun, Jan 27, 1:33 PM

How about this fix? I think it works perfectly with any string.

Owners added a subscriber: Restricted Owners Package.Sun, Jan 27, 1:33 PM

Add yourself to gui/credits/text/programming.json (path might be wrong).
And also remove the unnecessary three files from the diff. Only needs the changed JS file.

binaries/data/mods/public/gui/common/functions_global_object.js
18

The width of the longest counter. The second one might be longer.
longestString = counters.reduce((a, b) => a.length > b.length ? a : b)

Also, that negative sign seems useless.

smiley added inline comments.Sun, Jan 27, 1:51 PM
binaries/data/mods/public/gui/common/functions_global_object.js
18

Also, are those %% typos?. I could not find any evidence to suggest otherwise. (Not that I looked).

smiley retitled this revision from Fixes FPS overlay bug in the Bulgarian language to Dynamically sizes the dataCounter overlay.Sun, Jan 27, 2:03 PM
smiley edited the summary of this revision. (Show Details)
smiley edited the test plan for this revision. (Show Details)
elexis added inline comments.Sun, Jan 27, 3:56 PM
binaries/data/mods/public/gui/common/functions_global_object.js
18

a.length > b.length ? a : b

number of characters != text width.
1.2 looks like a cheat (hiding a bug?) Maybe it just needs to be one more pixel wider than the text width to not trigger a linewrap, or what is the issue?

%%

(I was wondering too and not looking either. I only know them from a translated sprintf string where %% encoded %, but not from a sprintf argument.)

smiley added inline comments.Sun, Jan 27, 4:20 PM
binaries/data/mods/public/gui/common/functions_global_object.js
18

Replace with GetTextWidth thingy then.

With very wide strings, that 1.2 would cause issues. I assume its a buffer area, So just add a constant number of pixels.

ramtzok1 marked an inline comment as done.Mon, Jan 28, 11:02 AM
In D1764#71134, @smiley wrote:

Add yourself to gui/credits/text/programming.json (path might be wrong).
And also remove the unnecessary three files from the diff. Only needs the changed JS file.

Will do, I'm pretty new to that kind of source control, sorry for my nooby understanding.

ramtzok1 updated this revision to Diff 7413.Mon, Jan 28, 11:51 AM
ramtzok1 marked an inline comment as done.

Getting the width of the longest text and then adding a constant to make the text fit to the overlay

ramtzok1 added a comment.EditedMon, Jan 28, 12:16 PM
In D1764#71134, @smiley wrote:

Add yourself to gui/credits/text/programming.json (path might be wrong).
And also remove the unnecessary three files from the diff. Only needs the changed JS file.

Sorry for the dumb question but how do I remove these files? I don't understand how to remove these 3 files.

binaries/data/mods/public/gui/common/functions_global_object.js
18

I made that it gets the width of the longest string and then adding a constant number which is 15 (For some reason it works no matter how long the string is, it fits perfectly with the overlay).

(I guess *.exe and *.dll files changed because you compiled. Notice that you don't need to do this as long as you only change .xml / .js)

binaries/data/mods/public/gui/common/functions_global_object.js
18

Perhaps rename a, b to max and current, so that the reader doesn't have to infer.

I think it should be possible to only store the maxWidth instead of maxText, removing two GetTextWidth calls?

reduce consumes two arguments, might be nicer to not rely on the default value (which is the first element of the array) and pass that explicitly (0 in case of computing the width instead of the text).

(I guess that function should also account for different fonts in theory (that would be my mistake when introducing the function). I recall s0600204 has worked on a patch to get the text size of mixed contents: D844. Perhaps the GetTextWidth function can be superseded then.)

Otherwise patch loooks good, thanks!

smiley added inline comments.Fri, Feb 1, 6:46 AM
binaries/data/mods/public/gui/common/functions_global_object.js
15

These lines were intentional for making it easier on the eye.

binaries/data/mods/public/gui/credits/texts/programming.json
252

Alphabetical order

ramtzok1 updated this revision to Diff 7474.Fri, Feb 8, 9:02 PM
ramtzok1 marked an inline comment as done.

Spaced lines and fixed my name in the programmers credits file.