Opened 8 years ago
Last modified 7 years ago
#4237 closed defect
[PATCH] Diplomacy window should show player's defeated/active states. — at Version 16
Reported by: | javiergodas | Owned by: | javiergodas |
---|---|---|---|
Priority: | Should Have | Milestone: | Alpha 22 |
Component: | UI & Simulation | Keywords: | patch |
Cc: | Patch: |
Description (last modified by )
Sometimes while playing in multiplayer, some opponents or allies might disconnect in middle of the battle, and if you get distracted, you may miss the notification saying that they will be back or they surrendered. So you might keep playing thinking that your ally could be alive when he surrendered, or either quit thinking that your ally gave up. Also, when you want to end the game, and you see no enemy units, you always ask your mates who's left. It would be nice to see which ones are dead so you can refinate your search. This is a continuation of ticket #4187.
Change History (25)
comment:1 by , 8 years ago
Description: | modified (diff) |
---|
comment:2 by , 8 years ago
Description: | modified (diff) |
---|
comment:3 by , 8 years ago
comment:5 by , 8 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:6 by , 8 years ago
Milestone: | Backlog → Alpha 22 |
---|---|
Status: | assigned → new |
by , 8 years ago
Attachment: | DiplomacyWinLosePatch.diff added |
---|
by , 8 years ago
Attachment: | DiplomacyWinLosePatch.2.diff added |
---|
by , 8 years ago
Attachment: | DiplomacyWinLosePatch.png added |
---|
by , 8 years ago
Attachment: | DiplomacyWinLosePatch.3.diff added |
---|
by , 8 years ago
Attachment: | DiplomacyWinLosePatch.4.diff added |
---|
by , 8 years ago
Attachment: | DiplomacyWinLosePatch.5.diff added |
---|
comment:8 by , 8 years ago
Keywords: | rfc patch added |
---|
comment:9 by , 8 years ago
Keywords: | review added; rfc removed |
---|---|
Priority: | Must Have → Should Have |
Didn't test yet, but code looks correct now
comment:10 by , 7 years ago
Seems to work correctly. I'm not sure the icons are the most obvious way to do this though.
comment:11 by , 7 years ago
The patch looks good, but there are two "style issues":
- You should use jsdoc comments for the new function in gui/commons/gamedescription.js (like how its done for the other functions in this file).
- binaries/data/mods/public/gui/session/menu.js: L340 has a trailing whitespace
Furthermore the patch needs to be rebased.
When those are fixed, I'll commit it ;)
by , 7 years ago
Attachment: | DiplomacyWinLosePatchFinal.diff added |
---|
by , 7 years ago
Attachment: | DiplomacyWinLosePatchFinal.2.diff added |
---|
comment:13 by , 7 years ago
please rebase your patch (update your svn or git and create a diff again)
comment:14 by , 7 years ago
Keywords: | simple removed |
---|
- Additionally to the proposed feature, we could display the defeated/won icon in the lobby / replay menu list.
In order to accomplish that, we would need a new
sprite
andstyle
definition, so that we can add the image also for texts. For example, after having added an icon with that name togui/common/setup.xml
, thetranslateWithContext("playerstate", "defeated")
string could be removed and instead[icon="icon_defeated"]
would be inserted at the beginning of the string. Unfortunately we wouldn't be able to set a tooltip, so not sure if it's the better choice after all. (Furthermore to avoid duplication of the defeated icon mapping, we wouldn't use animage
for the player outcome, but a string as well that would be set to[icon="icon_defeated"]
. (I repeat in advance: Don't implement this until we decided on this proposal).
source/gui/
bug: The tooltip in the diplomacy screen is visible for all players except the first. The first item will always display thetooltip
property defined in the XML (here empty string). Weirdly this bug isn't reproducible with thetooltip
of theimage
in the player assignment menu of thegamesetup
which also usesrepeat
.
- The patch passed the long strings test wiki:Implementation_of_Internationalization_and_Localization#LongStringsLocale
- The icon is given 25px width (
diplomacyPlayerOutcome
), butdiplomacyPlayerName
starts at 20px.
- Perhaps the icon would look better if the size is increased a bit.
- The tooltip should use the
sessionToolTipBold
style
- Trailing whitespace in
gamedescription.js
- The
<script file="gui/common/gamedescription.js"/>
call should be near the othergui/common
calls, in alphabetical order (unless explicitly needed). Removing proposed comment as we only add the comments if we want to show that a weird loading order is needed, which is the case forlayout.js
, but not forgamedescription.js
.
by , 7 years ago
Attachment: | DiplomacyWinLosePatch_EvenMoreFinalThanBefore.diff added |
---|
comment:16 by , 7 years ago
Description: | modified (diff) |
---|---|
Summary: | Diplomacy window should show player's defeated/active states. → [PATCH] Diplomacy window should show player's defeated/active states. |
Isn't the ghost="true"
missed in the diplomacy_window.xml:46
?
subset of #3787?