Page MenuHomeWildfire Games

Do not hardcode screen ratio in the main menu.
Needs ReviewPublic

Authored by Stan on Nov 24 2018, 4:41 PM.

Details

Reviewers
elexis
bb
Trac Tickets
#5353
Summary

Agree

Test Plan

Check if it doesn't break anything.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 6464
Build 10704: Vulcan BuildJenkins

Event Timeline

Stan created this revision.Nov 24 2018, 4:41 PM
Vulcan added a subscriber: Vulcan.Nov 24 2018, 4:42 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/797/

smiley added a subscriber: smiley.Nov 25 2018, 10:33 AM
smiley added inline comments.
binaries/data/mods/public/gui/pregame/mainmenu.js
93

(Anyone else feel this is a pretty ugly way to show the splashscreen?)

Stan marked 2 inline comments as done.Nov 25 2018, 10:41 AM
Stan added inline comments.
binaries/data/mods/public/gui/pregame/mainmenu.js
93

Feel free to submit a patch :)

Stan can you describe how this number is used? It is not clear to me from a quick read and quick test (and why 1 is better than 1.7777?)

binaries/data/mods/public/gui/pregame/mainmenu.js
93

/me raises hand
(If it isn't just 10 lines of code I'd have proposed to move it to a separate file, also md5sum #4399)
(Reminds me that I also have a mainmenu.xml rewrite somewhere moving that stuff to this file)

Stan marked an inline comment as done.Nov 25 2018, 10:53 AM

Basically this number is multiplied to whatever the height is. So in case it's 1080 you get 1920 if it's 1050 you get 1866 instead of 1680 (Because it's supposed to be a 16:10 ratio) if you give it 768 (for 1024x768 which is the minimun resolution we support) you get 1366. That is not a 4:3 ratio.

elexis added a comment.EditedNov 25 2018, 11:13 AM
In D1680#66674, @Stan wrote:

Basically this number is multiplied to whatever the height is. So in case it's 1080 you get 1920 if it's 1050 you get 1866 instead of 1680 (Because it's supposed to be a 16:10 ratio) if you give it 768 (for 1024x768 which is the minimun resolution we support) you get 1366. That is not a 4:3 ratio.

I understand that it's a multiplication, but what is the purpose of the number?

Edit: rP10042 philip var w = h*16/9; // width of background image

Stan added a comment.Nov 25 2018, 11:32 AM

It's only used to compute the offset of the background. it's passed as an argument to the background to offset it.

Each background defines for each of it's sprites this:

{
    "offset": (time, width) => 0.02 * width * Math.cos(0.05 * time),
    "sprite": "background-kush1-1",
    "tiling": true,
},

Not to mention the computation is really innefficient because it computes it one time per layer.

Just mere guessing, but doesn't that hardcode the ratio of the image rather than the ratio of the screen and tries to make sure that the image is moved into all directions with equal speed?

Stan added a comment.Nov 25 2018, 11:57 AM

The screen height is dependent on the window size.

Did you test it on really wide monitors/windows?

Stan added a comment.Nov 25 2018, 12:22 PM

I tried it by stretching the window to give it an 21/9 ratio.

Stan added a comment.May 6 2019, 1:02 AM

@bb, @vladislavbelov Anything else you want me to fix ?

The problem with the current code is that at no time we recover the real size of the backgrounds.
Depending on the window size the tiling backgrounds are more or less fully displayed.

Stan added a comment.May 20 2019, 11:20 AM

The problem with the current code is that at no time we recover the real size of the backgrounds.
Depending on the window size the tiling backgrounds are more or less fully displayed.

Well it shouldn't matter as long as they re-size accordingly, should it ?

Test is better than words.

Sets a window size of 1280x1024 by example and uses the Kushite background.
At the end of the scroll to the right, the right side of the city is still not displayed.