Page MenuHomeWildfire Games

Ingame summary window as dialog
ClosedPublic

Authored by ffffffff on Nov 23 2017, 3:57 PM.

Details

Summary

When in game its realy annoying when open summary screen and its taking the whole screen so the game gets overlapped completely with summary window and the touch to the game gets lost. So as solution having the summary as dialog is much better than any transparency experiment.

Ingame



End summaryscreen still filled completely

Also in replays menu, perhaps having dialog (maybe to big)

Test Plan

discuss

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ffffffff created this revision.Nov 23 2017, 3:57 PM
ffffffff edited the summary of this revision. (Show Details)
ffffffff updated this revision to Diff 4338.Nov 23 2017, 4:00 PM
ffffffff edited the summary of this revision. (Show Details)

fade added

ffffffff edited the summary of this revision. (Show Details)Dec 4 2017, 6:40 PM
ffffffff edited the summary of this revision. (Show Details)
ffffffff retitled this revision from Summary window as dialog to Ingame summary window as dialog.Dec 11 2017, 8:19 PM
ffffffff edited the summary of this revision. (Show Details)
ffffffff edited the summary of this revision. (Show Details)

@temple comment here for ingame summary window as dialog window:)

@temple comment here for ingame summary window as dialog window:)

Seems nice. (What else to say?)

tx.

important.

Having only the bottommost page in fullscreen and every other on top of it as a dialog means that the user will never assume that the topmost page is the bottommost page.
That is a reason that could lead to the patch being considered correct.

binaries/data/mods/public/gui/summary/summary.js
137 ↗(On Diff #4721)

So we can revert rP20111 or should the same approach be used here?

elexis added inline comments.Dec 21 2017, 6:39 PM
binaries/data/mods/public/gui/summary/summary.js
137 ↗(On Diff #4721)

If it's done via JS and if it were in a separate function initGUIWindow, initGUIWindowDecoration or initGUIWindowLocation, it would mean that the reader doesn't have to interpret this small hunk before reading a summary of what this function handles otherwise.

ffffffff added inline comments.Dec 21 2017, 7:05 PM
binaries/data/mods/public/gui/summary/summary.js
137 ↗(On Diff #4721)

lol

all

true

XD

ffffffff updated this revision to Diff 5032.Jan 2 2018, 4:21 PM

Convert summary window to dialog window when wanted.

elexis requested changes to this revision.Jan 2 2018, 8:01 PM

continueButton() shouldn't switch the GUI page if in replay menu but pop it.

binaries/data/mods/public/gui/summary/summary.js
142 ↗(On Diff #5032)

showAsDialogWindow -> dialog
should always be defined for any context, so nuke that isIngame check

call a function setupSummaryWindowStyle(data.dialog) and move all logic there

176 ↗(On Diff #5032)

Move the size from XML to here, or have two empty mock XML objects with the two sizes

sprite = dialog ? "...Dialog" : "...window";

just group everything

This revision now requires changes to proceed.Jan 2 2018, 8:01 PM
ffffffff updated this revision to Diff 5128.Jan 6 2018, 5:37 PM
elexis added a comment.Jan 6 2018, 8:03 PM

Looks good

binaries/data/mods/public/gui/summary/summary.js
140 ↗(On Diff #5128)

After reading the comments in this function, you might find a better place within this function to call the function.

174 ↗(On Diff #5128)

initGUIWindow

465 ↗(On Diff #5128)

Only check for g_GameData.dialog and pass the boolean in all calls

binaries/data/mods/public/gui/summary/summary.xml
27 ↗(On Diff #5128)

Was wondering if we should hide it for good measure. Perhaps it would be better for performance (wanna do a test)?

ffffffff updated this revision to Diff 5139.Jan 6 2018, 8:29 PM

y

binaries/data/mods/public/gui/summary/summary.js
140 ↗(On Diff #5128)

y

174 ↗(On Diff #5128)

y

465 ↗(On Diff #5128)

y

binaries/data/mods/public/gui/summary/summary.xml
27 ↗(On Diff #5128)

n

elexis accepted this revision.Jan 7 2018, 2:07 AM

Actually on first sight it seems wrong to add a dialog property, because it gives the caller the freedom to switch the page with dialog = true or to push the page with dialog = false.
Instead we can get the information wheather it's a dialog or not directly from the GUI Manager like this:

But on second sight it seems ok to have that one of these two freedoms, that is displaying the fullscreen variant when pushing instead of switching the GUI page - as is the case in the current code.

Panels exceed 1024 before and after, I've resigned about that.
It's quite bad that it's rendered beyond the dialog size.

It's not completely obvious whether we want the feature to begin with, since it has the disadvantage that we see less of the summary screen and are distracted by the map visible in the background.
But I accept it because it is only consistent if we use a dialog everywhere a page is stacked.

binaries/data/mods/public/gui/replaymenu/replay_actions.js
124 ↗(On Diff #5139)

In theory it should be opened as a dialog, but we cannot see the background, so the we cannot perceive it as a dialog but only as a messed up window. So keeping the switchpage.

132 ↗(On Diff #5139)

It's a "gui" property

binaries/data/mods/public/gui/summary/summary.js
177 ↗(On Diff #5139)

Unneeded negation

179 ↗(On Diff #5139)

If we add code, we should have a reason to add code.

If it's a dialog I get 80 FPS with the fade shown and 93 FPS without it.
If it's a window I get 100 FPS in both cases.

Since we always want to show it if it's a dialog, the line of code is useless according to this data.

460 ↗(On Diff #5139)

As mentioned, dialog should always be defined.

140 ↗(On Diff #5128)

No, it should have been moved below the // Output globals part.

It's the "input-processing-output" model and your new function is output, not input https://en.wikipedia.org/wiki/IPO_model https://de.wikipedia.org/wiki/EVA-Prinzip

This revision is now accepted and ready to land.Jan 7 2018, 2:07 AM
This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.Jan 7 2018, 2:12 AM
ffffffff added a comment.EditedJan 10 2018, 1:08 AM

maybe same darkness as fade as for dialog overlay with background color 0 0 0 200 alpha darkness https://imgur.com/a/VtRPu and little more gap from 16,24,16,24 to 24,28,24,28 px

na guess from style points its fine we have atm

ffffffff added inline comments.Jan 10 2018, 1:11 AM
ps/trunk/binaries/data/mods/public/gui/summary/summary.js
177

missing line here

Engine.GetGUIObjectByName("fadeImage").hidden = !g_GameData.gui.dialog;
ps/trunk/binaries/data/mods/public/gui/summary/summary.xml
27

for some reason i forgot to set this hidden or !hidden in js