Opened 15 months ago

Closed 14 months ago

Last modified 14 months ago

#4187 closed defect (fixed)

[PATCH] Summary screen must show who has actually won / was defeated

Reported by: elexis Owned by: javiergodas
Priority: Must Have Milestone: Alpha 21
Component: UI & Simulation Keywords: simple patch
Cc: Patch:

Description

Sometimes a game ends and it isn't obvious who won. Often some players are defeated and the host quits the game before a winner has been declared officially.

The summary screen should show which players have been defeated and which won.

It would be handy to also capture and display the time of defeat (this information could also be reused when saving games, since the load-game dialog shows which players have been defeated too).

Attachments (7)

WinLosePatch.diff (4.8 KB) - added by javiergodas 14 months ago.
Update (thx elexis for the review)
WinLose_fixed_whitespace.patch (5.3 KB) - added by elexis 14 months ago.
Fixed whitespace issues.
winlose.png (152.8 KB) - added by elexis 14 months ago.
Current state
sample.jpg (194.4 KB) - added by elexis 14 months ago.
WinLosePatchV2.diff (4.6 KB) - added by javiergodas 14 months ago.
WinLosePatchLaurels.diff (4.3 KB) - added by javiergodas 14 months ago.
WinLosePatch_swords.diff (5.5 KB) - added by elexis 14 months ago.
Fixed so much broken whitespace.... Use tabs instead of spaces and don't have space after the last character please! Patch uses swords icons as suggested by niektb.

Download all attachments as: .zip

Change History (26)

comment:1 Changed 14 months ago by javiergodas

Owner: set to javiergodas

comment:2 Changed 14 months ago by elexis

Keywords: patch rfc added
Milestone: BacklogAlpha 21
Summary: Summary screen must show who has actually won / was defeated[PATCH] Summary screen must show who has actually won / was defeated

Thanks for working on this!

  • Please add your name to the programming.json file, so you are mentioned in the credits.
  • Use correct indentation and tabs.
  • Rename playerOutcomet to playerOutcome
  • Use translate("Defeated") and translate("Victory") so that it will be translated into other languages
  • For "active" players, don't display anything (empty tooltip and sprite)
  • unneeded variable outcomeState, you can just reuse playerState

Other than that looks good to me.

Changed 14 months ago by javiergodas

Attachment: WinLosePatch.diff added

Update (thx elexis for the review)

comment:3 Changed 14 months ago by Vladislav Belov

There are still few indentation notes: spaces/tabs and spaces between "if(". And probably better to use "Won" instead of "Victory" as opposition to "Defeated", as in other places.

Last edited 14 months ago by Vladislav Belov (previous) (diff)

Changed 14 months ago by elexis

Fixed whitespace issues.

comment:4 Changed 14 months ago by elexis

The summary screen is alrady too filled with 1024x768, so perhaps we might want to commit the patch above that replaces the colored playerbox with the playerstate-icon.

But just for comparison, can you show us how the summary screen would look like if we had the icons right beside the playercolor box?

comment:5 Changed 14 months ago by javiergodas

I fear I can't cause I tried to place the outcome icons under another header, but no matter how I tried, I couldn't increase the stats colored box width. Came to this idea because of that problem: just replacing the colored box.

But sure I'll try to do that.

Thx vladislav for the whitespace fix, sry for not doing it myself.

comment:6 Changed 14 months ago by elexis

Correct me if I'm wrong, perhaps it's sufficient to

1) add the new lines of code added in the patch above attachment:WinLose_fixed_whitespace.patch 2) not remove the lines removed by that patch?

i.e. don't delete the old object. Just move it a bit to the left or right. Alternatively one could use an image editor to just get a preview of how it might look. I might try that later.

Also you had sent some screenshots in IRC (see also http://irclogs.wildfiregames.com/), can you link them here?

Changed 14 months ago by elexis

Attachment: winlose.png added

Current state

Changed 14 months ago by elexis

Attachment: sample.jpg added

comment:7 Changed 14 months ago by elexis

Current state of the patch: http://trac.wildfiregames.com/raw-attachment/ticket/4187/winlose.png

Was thinking of this (created using image editor): http://trac.wildfiregames.com/raw-attachment/ticket/4187/sample.jpg

I checked with 1024x768 and large names. The playername column doesn't become smaller (on the other hand the units tab panel won't work with 1024 already, so it will need scrolling or a new page). Long names use multiple lines and never collide with anything (rather don't show the complete name).

But to be honest I don't think we will miss that color sprite, it just seems redundant.

comment:8 Changed 14 months ago by Itms

The icons used here are not very consistent (they're not the same size and they don't use the same symbols), plus it's rather unclear that the crossed swords mean victory.

I would rather use ps/trunk/binaries/data/mods/public/art/textures/ui/session/icons/stances/passive.png for losing and ps/trunk/binaries/data/mods/public/art/textures/ui/session/icons/stances/violent.png for winning. The symbol is clear, the icons are coherent, and you have a clear opposition between swords towards the sky or towards the ground.

comment:9 Changed 14 months ago by elexis

And we settled on having both a colorbox and the sword icons. So don't remove the old code, only add new code (and potentially resize).

comment:10 Changed 14 months ago by javiergodas

Hi elexis,

About the size of the icons, I can search something fitting for win/lose icons in OpenGameArt? website.

I'll try to keep the color box, but before that I have to figure out how to expand the player box.

I told you I'ld upload the screenshots myself but thx anyway.

comment:11 Changed 14 months ago by javiergodas

Found this icon for Victory:

http://opengameart.org/content/laurels

comment:12 Changed 14 months ago by javiergodas

Hi guys,

I've been tampering with the size attribute like I knew what I was doing, and as a result (since you decided to keep the color box along with the outcome), I've created some kind of hybrid that you might like.

http://s18.postimg.org/sozwolpl5/Win_Lose_V2.png

New patch is on the attachments section as V2.

Changed 14 months ago by javiergodas

Attachment: WinLosePatchV2.diff added

comment:13 Changed 14 months ago by javiergodas

Could also reuse the laurels png to mix it with champion recruitment slots background so you don't have to look at its types to see it's a champion.

Last edited 14 months ago by javiergodas (previous) (diff)

comment:14 Changed 14 months ago by javiergodas

"A persian anusya with laurels on its recruitment png for example".

Last edited 14 months ago by javiergodas (previous) (diff)

comment:15 Changed 14 months ago by Itms

Hi, I'm sorry but I really don't like at all this mix you propose :/

  • The win/lose icons are ugly when put on top of this solid color.
  • You're mixing two things that have nothing to do with each other. In your screenshot it is completely impossible to understand the color is the player color.
  • The solid color is disgusting, you need to keep the border around.

Can you rather try to do what elexis proposed? The laurels are nicer than the 'aggressive' swords in any case. Keep up the good work!

comment:16 Changed 14 months ago by javiergodas

Tried to do the thing that elexis said since the beggining, but there is a problem with the XML (quite confusing) size attribute that gets the sprites distorted when you try to move them, so either they mix with the colorbox, or they get cut.

Also tried to modify the layout.js, tried everything I saw to increase the player box but the image didn't got any better.

So unfortunately ATM that's the best I can do to make it look good.

Modified the first patch to include the laurels. I know you agreed on keeping the colored box, but the background still does the trick, and as I told, I didn't find a way to keep both things.

http://s21.postimg.org/is0035u8n/Win_Lose_Laurels.png

Changed 14 months ago by javiergodas

Attachment: WinLosePatchLaurels.diff added

Changed 14 months ago by elexis

Attachment: WinLosePatch_swords.diff added

Fixed so much broken whitespace.... Use tabs instead of spaces and don't have space after the last character please! Patch uses swords icons as suggested by niektb.

comment:17 Changed 14 months ago by elexis

http://i.imgur.com/SjOeWi0.png

Background player color opacity increased, so its more visible. Icons as suggested by Itms, niektb not a huge fan of, I have no strong opinion on the icon.

comment:18 Changed 14 months ago by elexis

Resolution: fixed
Status: newclosed

In 18771:

Show an icon with tooltip in the summary screen indicating which player was defeated, has won or is still playing. Patch by javiergodas, reviewed by niektb, fixes #4187.

comment:19 Changed 14 months ago by elexis

Keywords: rfc removed

Thanks for the patch and quick updates :)

Note: See TracTickets for help on using tickets.