Page MenuHomeWildfire Games

Display wonders alongside heroes and relics
Needs ReviewPublic

Authored by Nescio on Sat, Jul 6, 4:56 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

This patch adds the wonder to the entities shown with separate panels on top, as is already the case for heroes and relics. In ordinary games, a wonder is at least as crucial as a hero, therefore it makes sense

Also remove one pixel of empty space from each side around the entity panels (the icons themselves remain the same size), to allow displaying 20 icons instead of 19 (20×48=960 instead of 19×50=950), because if heroes+relics=19, then building a wonder means you need to display one more, heroes+relics+wonders=20. Besides, 19 is an odd number.

See also https://wildfiregames.com/forum/index.php?/topic/22779-0abc-mod/page/12/&tab=comments#comment-379310

Test Plan

Launch a game, advance to city phase, train a hero, build a wonder, and check they're both displayed as they ought to be.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 8223
Build 13405: Vulcan BuildJenkins
Build 13404: arc lint + arc unit

Event Timeline

Nescio created this revision.Sat, Jul 6, 4:56 PM
Owners added a subscriber: Restricted Owners Package.Sat, Jul 6, 4:56 PM
Vulcan added a comment.Sat, Jul 6, 4:57 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/Player.js
| 379| »   for·(var·type·in·amounts)
|    | [NORMAL] JSHintBear:
|    | 'type' is already defined.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 392| 392| 				// Players see colors depending on diplomacy
| 393| 393| 				g_DisplayedPlayerColors[i] =
| 394| 394| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 395|    |-					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
|    | 395|+						g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 396| 396| 					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 397| 397| 					getDiplomacyColor("enemy");
| 398| 398| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 393| 393| 				g_DisplayedPlayerColors[i] =
| 394| 394| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 395| 395| 					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 396|    |-					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
|    | 396|+							g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 397| 397| 					getDiplomacyColor("enemy");
| 398| 398| 
| 399| 399| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 5.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 394| 394| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 395| 395| 					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 396| 396| 					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 397|    |-					getDiplomacyColor("enemy");
|    | 397|+								getDiplomacyColor("enemy");
| 398| 398| 
| 399| 399| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
| 400| 400| 	}
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 4 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
| 644| 644| 					"civ": setStringTags(g_CivData[g_Players[g_ViewedPlayer].civ].Name, { "font": "sans-bold-stroke-14" }),
| 645| 645| 					"hotkey_civinfo": colorizeHotkey("%(hotkey)s", "civinfo"),
| 646| 646| 					"hotkey_structree": colorizeHotkey("%(hotkey)s", "structree")
| 647|    |-			});
|    | 647|+				});
| 648| 648| 	}
| 649| 649| 
| 650| 650| 	// Following gaia can be interesting on scripted maps
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1194|1194| 
|1195|1195| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1196|1196| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1197|    |-		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|    |1197|+			"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1198|1198| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1199|1199| 	});
|1200|1200| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1195|1195| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1196|1196| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1197|1197| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1198|    |-		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|    |1198|+			"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1199|1199| 	});
|1200|1200| 
|1201|1201| 	let resCodes = g_ResourceData.GetCodes();
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 1.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1196|1196| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1197|1197| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1198|1198| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1199|    |-	});
|    |1199|+		});
|1200|1200| 
|1201|1201| 	let resCodes = g_ResourceData.GetCodes();
|1202|1202| 	for (let r = 0; r < resCodes.length; ++r)
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js
|1679|1679| 	for (let rct of resourcesCounterTypes)
|1680|1680| 		for (let rt of resourcesTypes)
|1681|1681| 			reportObject[rt + rct.substr(9)] = playerStatistics[rct][rt];
|1682|    |-			// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|    |1682|+	// eg. rt = food rct.substr = Gathered rct = resourcesGathered
|1683|1683| 
|1684|1684| 	reportObject.vegetarianFoodGathered = playerStatistics.resourcesGathered.vegetarianFood;
|1685|1685| 	for (let type of unitsClasses)

binaries/data/mods/public/gui/session/session.js
|1055| »   let·getPanelEntNameTooltip·=·panelEntState·=>·"[font=\"sans-bold-16\"]"·+·template.name.specific·+·"[/font]";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'panelEntState' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1130| »   »   button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1131| »   »   button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1132| »   »   button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1928/display/redirect

Freagarach added inline comments.
binaries/data/mods/public/gui/session/session.js
180

I would say that Wonder ought to be before Relic, because in some games you've got a lot of relics ;)

Opinions on the desired order of these entities are welcome!

  • hero, relic, wonder
  • hero, wonder, relic
  • relic, hero, wonder
  • relic, wonder, hero
  • wonder, hero, relic
  • wonder, relic, hero

(Entity panels are placed from top left corner to the right, which means the last item is closer to the centre of the screen than the first.)

binaries/data/mods/public/gui/session/session.js
180

In an ordinary (e.g. conquest) game one could also have several wonders (by capturing them from other players).
That said, I myself have no particular preference as to the order of these preferences.

Freagarach added inline comments.Sun, Jul 7, 11:47 AM
binaries/data/mods/public/gui/session/session.js
180

My reasoning was: the entities which you *can* generally have the least of, the most to the left (not considering the exact amount of relics and that you can theoretically capture an unlimited number of wonders) and ordering further.
But that said, I have no preference as well.

FeXoR added a subscriber: FeXoR.Sun, Jul 7, 12:13 PM

To be able to have access to more items (hero, relic and wonder icons) the way to go is IMO to make the "sidebar" scrollable if it extends the height of the available area on the screen.
That would also allow mods have any number of heroes.
Fiddling with pixels rarely really fixes space issues.

Nescio added a comment.EditedSun, Jul 7, 12:27 PM

To be able to have access to more items (hero, relic and wonder icons) the way to go is IMO to make the "sidebar" scrollable if it extends the height of the available area on the screen.
That would also allow mods have any number of heroes.
Fiddling with pixels rarely really fixes space issues.

Currently the numbers of items displayed are hardcoded, both in js and xml.
Ideally a new row of entity panels would start when the current one reaches the right end of the screen. Unfortunately my understanding of the gui is rather limited; I have no difficulty with the xml files, but the js code is a different cup of tea.
De-hardcoding displayed item numbers (e.g. 10 researches on right, 24 structures in selection panel, etc.) is outside the scope of this patch, though.

FeXoR added a comment.Sun, Jul 7, 1:20 PM
In D2051#85354, @Nescio wrote:

Currently the numbers of items displayed are hardcoded, both in js and xml.

Sad.

Ideally a new row of entity panels would start when the current one reaches the right end of the screen.

That would potentially fill the entire screen with icons so I object!

Unfortunately my understanding of the gui is rather limited; I have no difficulty with the xml files, but the js code is a different cup of tea.

No problem, take your time ;)

De-hardcoding displayed item numbers (e.g. 10 researches on right, 24 structures in selection panel, etc.) is outside the scope of this patch, though.

Alright. I'm just pointing out that the hardcoded number of icons means (at least) some might not be visible and how a solution could be achieved.

I agree with you that a wonder is an essential entity. Still we have to check it this leads to bugs (including non-visible entities in the list that are "supposed" to be visible).

Nescio added a comment.Sun, Jul 7, 1:25 PM

Still we have to check it this leads to bugs (including non-visible entities in the list that are "supposed" to be visible).

What do you mean exactly? template_structures_wonder.xml is the only file with the "Wonder" class.

FeXoR added a comment.EditedSun, Jul 7, 1:35 PM

Well, e.g. how many wonders can be build per player? (I don't say there's a good reason to build more than one but if e.g. a map has 2 wonders for one player, what's then? That last one wouldn't be a real argument for me to object the patch - still try to think of cases that might lead to breakage ;) )

And conceptually: If an ally has a wonder shouldn't that be shown as well? Can that be done if and only if the player itself doesn't have a wonder on it's own so the hardcoded limit won't be exceeded?

Nescio added a comment.Sun, Jul 7, 1:48 PM

Players can't train more than one hero nor build more than one wonder (see simulation/templates/special/player/player.xml). Although wonders could be captured in theory, in practice I don't expect any player to get more than twenty.

And conceptually: If an ally has a wonder shouldn't that be shown as well?

Why? Catafalques and heroes of other players aren't currently shown either, not even in regicide games. Only entities owned by the player are displayed.

Fundamentally, this patch doesn't alter the status quo—if something is broken with this patch, it's also broken without—all it does is add the wonder to the entities with a panel below top bar.

elexis added a subscriber: elexis.Sun, Jul 7, 2:36 PM

I thought there was a ticket for this, but I couldn't find one.
I'm sure it was discussed before, perhaps it was on #0ad-dev or a related revision proposal on Phabricator.

2017-04/2017-04-08-QuakeNet-#0ad-dev.log:03:44 < Sandarac> elexis: I think it would be a good idea to have wonder ents in the panel
2017-04/2017-04-08-QuakeNet-#0ad-dev.log:05:18 < Sandarac> but shouldn't Wonders be shown in that panel for the sake of consistency?

I suppose only few people write a script to download irclogs, but more people would use the web-search UI for the chatlogs.

Disclaimer: I didn't read the logs, don't remember what my former self said on the topic, new-self might disagree.

But this sentence still seems valid:

05:19 < elexis> well I had posted questions in that proposal, i.e. whether we should make that more generic, for example allowing the endgamemanager to pass the classes

http://irclogs.wildfiregames.com/2017-04/2017-04-08-QuakeNet-%230ad-dev.log

One can also find the related revision proposals on phabricator.

FeXoR added a comment.Sun, Jul 7, 2:43 PM

Well, that depends on the outcome of your lobby survey I guess :p

However, I agree that showing the wonder would be consistent.

@why show allied wonders: Because they can be game deciding for the player.
I don't demand to have that, I am asking if that would make sense/be helpful for the player because I definitely think so (the hero on the other side ... well, 0 A.D. is not Warcraft III).

Nescio added a comment.Sun, Jul 7, 2:50 PM

Suppose you haven't scouted the entire map. Wouldn't it be weird if you nonetheless could see the health of the wonders of other players, while you don't even know their location?
To me, the gui is to display things the player has access to, not reveal information they don't have.

FeXoR added a comment.Sun, Jul 7, 3:11 PM

Winning unexpectedly by an allied wonder one didn't know existed also sounds weird to me :p

If the GUI isn's used for vital information (and what can be more vital that winning or losing the game?) there's something wrong IMO. :D

But I guess showing only owned entities there is at least consistent with the current state.

Angen added a subscriber: Angen.Sun, Jul 7, 3:12 PM

I thought that if someone builds/captures wonder message is broadcasted that player has one and countdown starts.

FeXoR added a comment.Sun, Jul 7, 4:59 PM

Good point, @Angen ! :D

Nescio added a comment.EditedSun, Jul 7, 5:09 PM

Indeed, victory timers are something else and are already shown separately to all players (top centre, below faction emblem).

To avoid further confusion, what entity panels do is the following:

  • looking at it shows the health bar of these entities
  • hovering the cursor to the entity panel shows a tooltip with the entity's exact stats
  • clicking on it will select the entity
  • double-clicking on it will select the entity and bring the player immediately to its current location

This patch does not change anything of that. All it does is add the wonder to the entities with top panels.

Also, in observer mode, only entities of the selected player are displayed, but if no player is selected, then all entities of all players are shown:


Which means that in ordinary games there could be as many as 16 (eight players, one wonder and one hero each) in observer mode. However, capture-the-relic games can have as many as 14 catafalques, which could mean 30 entities, theoretically, i.e. more than 20.
On the other hand, this is already present without this patch: 8 heroes plus 14 relics is 22 entities, i.e. more than 19.
In practice, I don't expect that particular situation to happen. The question is whether the gui should indeed be able to display as many as 30 entity panels, and if so, how that could be practically achieved.