HomeWildfire Games

Adapt the civinfo page to read from templates
Concern RaisedrP24492

Description

Adapt the civinfo page to read from templates

Instead of loading information about heroes, special buildings, et al., from the
{civ}.json files, load the information desired from templates.

Determination of "specialness" works as follows:

  • Heroes are detected via the Hero class.
  • Special Structures are detected via the SpecialBuilding or Wonder class.
  • Technologies are determined to be "special" when they can be researched by the currently selected civ, but are not open to be researched by every civ.
  • Team Bonuses are any applicable aura that reside in the appropriate folder.

Accepted by: Freagarach
Comments by: Stan

Differential Revision: https://code.wildfiregames.com/D759

Event Timeline

Nescio raised a concern with this commit.Jan 4 2021, 1:59 PM
Nescio added a subscriber: Nescio.

This is conceptually sound and certainly an improvement, at least in theory. However, in practice there are aspects that need to be improved:

  • None of the civ-specific structures actually shows up in the civilization overviews (see D2733 for all currently buildable unique structures).
  • All civs have a wonder and they're all statistically identical, therefore it's unnecessary to include them in the civilization overviews.
  • Heroes display their identity unit tooltips, i.e. bonus attack multiplier, which is not terribly important; their aura descriptions are more important.
  • Should catafalque auras be included?
  • Should both the specific (non-English) and generic (English) names be displayed in the civilization overviews?
  • The right column displays only three hero names, whereas the central columns displays both special technologies and special structures; because there tend to be more of these (see e.g. kush or pers), I'd recommend moving structures to the right column.
  • The four columns have unequal widths; changing that to 30% each would probably look better.
This commit now has outstanding concerns.Jan 4 2021, 1:59 PM

Which point is your concern precisely?

  • None of the civ-specific structures actually shows up in the civilization overviews (see D2733 for all currently buildable unique structures).

That is where your patch was for :)

  • All civs have a wonder and they're all statistically identical, therefore it's unnecessary to include them in the civilization overviews.

This is a matter of preference, IMHO.

  • Heroes display their identity unit tooltips, i.e. bonus attack multiplier, which is not terribly important; their aura descriptions are more important.

Agreed, feel free to improve ;)

  • Should catafalque auras be included?

No (D294).

  • Should both the specific (non-English) and generic (English) names be displayed in the civilization overviews?

Yes.

  • The right column displays only three hero names, whereas the central columns displays both special technologies and special structures; because there tend to be more of these (see e.g. kush or pers), I'd recommend moving structures to the right column.
  • The four columns have unequal widths; changing that to 30% each would probably look better.

Feel free to improve, I guess.

Nescio added a comment.Jan 4 2021, 2:25 PM

Which point is your concern precisely?

The first point is most concerning: prior to this patch civ overview pages listed civ specific structures, now they no longer do. Doing a grep -r SpecialBuilding in the simulation folder returns zero occurrences.

This is a matter of preference, IMHO.

Perhaps, but why should wonders be included? They weren't in the past, and other structures available to all civs aren't included either.

  • The right column displays only three hero names, whereas the central columns displays both special technologies and special structures; because there tend to be more of these (see e.g. kush or pers), I'd recommend moving structures to the right column.

(Formerly, the hero description took quite some space (rP24493#46757).)

Nescio added a comment.Jan 4 2021, 3:23 PM

It did not: the description (what is displayed) is something different than the history (what is hidden in a pop-up). Here's how it looks like in A23:

Ah, my mistake then.

s0600204 added a comment.EditedJan 5 2021, 9:53 AM

The Special Buildings not actually showing up is an oversight on my part - somehow I had missed that you (and bb) removed the SpecialBuilding class back in July. I shall try to work with you to get D2733 done and committed, which should remedy that.

Much of the other suggestions are more about adding content to, or changing the visuals and/or layout of the civinfo page, all of which is out of the intended scope of this commit. The intent was to swap the {civ}.json-sourced strings for ones taken from entity/technology templates, not to change what or how things should be displayed. (Any entries appearing/disappearing was a side-effect.)

However, now that this commit is in I feel it is probably time to seriously consider redesigning the layout of the page (something I've been wanting to do since 2015), which hopefully will satisfy the visuals/layout portion of your raised Concern. Give me a few days to update things locally, and then I should have some screenshots of potential directions a redesign might take.

The first point is the most important and the reason I raised a concern. Points two and five are things that are done differently now than they were prior to this commit. Points three and four are questions, but yes, out of scope. I made a patch addressing points six and seven: D3295, which merely tweaks the current design.
I'm certainly looking forward to your fundamental redesign of the layout!

Nescio removed an auditor: Nescio.Jan 11 2021, 11:09 AM

The problem has been solved by D2733/rP24535, civ-specific structures are again properly displayed in the civilization overviews.

This commit no longer requires audit.Jan 11 2021, 11:09 AM

I'm certainly looking forward to your fundamental redesign of the layout!

🚧 M1 🚧

Nescio raised a concern with this commit.Sun, Feb 14, 3:42 PM

It seems you load the team bonuses directly from the simulation/data/auras/teambonuses/ files. This is incorrect, strictly speaking: which team bonuses (and other auras) a civ has is actually defined in the simulation/templates/special/player/player_{civ}.xml files.

/ps/trunk/binaries/data/mods/public/gui/reference/civinfo/Sections/Subsections/BonusesSubsection.js
42–50

If a civ doesn't have any team bonuses (perfectly possible in a mod), then there is no need to display “Team Bonuses”.

This commit now has outstanding concerns.Sun, Feb 14, 3:42 PM