Opened 4 years ago

Closed 2 years ago

Last modified 3 months ago

#3948 closed defect (fixed)

[PATCH] Sheep in summary screen

Reported by: elexis Owned by: Imarok
Priority: Should Have Milestone: Alpha 23
Component: UI & Simulation Keywords: patch
Cc: Patch: Phab:D1052

Description (last modified by Imarok)

The total unit number in the summary screen includes animals, so it is not possible to find out how many females or humans a player owns.

Since everyone looks at the total number of units to estimate the strength of an economy of a player, animals should not be included there.

They could be listed in a another column. This will fit 1280x1024. For 1024x768 the current amount of columns already doesn't fit, so adding one more (with less relevant stats like this one) might be okay, especially since there will be scrolling sometime.

Attachments (1)

3948-sheep-summary-screen.patch (7.2 KB) - added by Jon Baer 4 years ago.

Download all attachments as: .zip

Change History (20)

Changed 4 years ago by Jon Baer

comment:1 Changed 4 years ago by Jon Baer

Summary: Sheep in summary screen[PATCH] Sheep in summary screen

comment:2 Changed 4 years ago by elexis

Keywords: patch review added
Milestone: BacklogAlpha 21

Don't forget to add yourself to programming.json and update the ticket with the review keyword an milestone.

comment:3 Changed 4 years ago by fatherbushido

  • In counters.js, you can remove the { } and the else.
  • It's a bit strange to have "trained/killed/lost" for animals. What do you want in killed and lost ?
  • Moreover (it depends on 2) perhaps you should use Domestic instead of Animal.

Thx for working on that.

(NB: for who wants to test it, apply the patch with patch -p1)

comment:4 Changed 4 years ago by sanderd17

If animals get their own category, they could be included in the total IMO.

It should be doable for a player to estimate the total number of humans when you have the overall total, and the number of animals.

It's rather strange to have a total column that isn't the total of all categories listed IMO.

So either the entire stat count should only consider units (and not have a column for sheep), sheep would belong more in the "resources" pane in that case. Or there can be a column for sheep, and then it should be included in the total too.

comment:5 Changed 4 years ago by fatherbushido

Indeed it makes more sense to add them in "resources" pane.

comment:6 in reply to:  4 Changed 4 years ago by elexis

Replying to sanderd17:

If animals get their own category, they could be included in the total IMO. It's rather strange to have a total column that isn't the total of all categories listed IMO.

Agree.

Replying to fatherbushido:

Indeed it makes more sense to add them in "resources" pane.

Disagree, animals are not resources.

comment:7 Changed 4 years ago by Jon Baer

Actually this patch needs more work as it misses the War Dog. If I understand this correctly there are 2 "Animals" in this case, the ones which are trained to be part of a unit (ie the War Dog, Elephants) and ones which are trained for resource (Sheep, Pigs, etc) and in one case the totals would make sense since they would be lost/killed, the other should really be counted w/ food. This is my first attempt @ a patch so bear with me but I understand the issue, that really a unit total shouldn't count those sheep which were "trained".

comment:8 Changed 4 years ago by fatherbushido

(Neither War Dog nor the Elephants (champs, archers or support) are Animal)

Last edited 4 years ago by fatherbushido (previous) (diff)

comment:9 in reply to:  8 Changed 4 years ago by elexis

Replying to fatherbushido:

(Neither War Dog nor the Elephants (champs or archers) are Animal)

...nor should they become that. The summary screen should only show huntable/economic animals produced by the player, as in those that have the Animal tag only.

comment:10 Changed 4 years ago by Jon Baer

Understood, just realized I should have included screenshots for those who don't have time to apply the patch. What I could do for now is keep the total deduction of the sheep but move the stat to Misc tab(?), this is a good ticket because it gives me familiarity w/ both the GUI and statistics.

On the Units tab - if you raise War Dogs they show up in the total but not in the breakdown, such as the corral animals.

I'm trying to think of what a good stat(s) the corral could give, like % of food which came from domestic raising, etc. For now can give just a total number raised I think.

One question I have re: development is at which stages does the translate(string) get done? I read the document on i18n but are the translations usually submitted w/ the functionality patch as well?

comment:11 Changed 4 years ago by fatherbushido

Keywords: review removed

@jonbaer: You did the translation stuff with translate in layout.js. I will push the patch out of the review queue waiting the next version. Thx for keeping working on it.

comment:12 Changed 4 years ago by causative

IMO these "Animals" should be displayed on the summary screen as "Livestock" to eliminate any confusion with war dogs or worker elephants.

comment:13 in reply to:  8 Changed 4 years ago by elexis

Also as pointed out by causative, calculateEconomyScore should at least get a comment that it takes the vegetarianFood into account twice.

Furthermore it were nice if the resource supply of sheep were listed in the tech tree as well.

comment:14 Changed 3 years ago by elexis

Milestone: Alpha 21Backlog

comment:15 Changed 2 years ago by Imarok

Description: modified (diff)
Owner: set to Imarok
Status: newassigned

Plan is to add sheeps, goats etc. as Livestock to the resource panel and don't count it to the total units.

Last edited 2 years ago by Imarok (previous) (diff)

comment:16 Changed 2 years ago by Imarok

Patch: Phab:D1052

comment:17 Changed 2 years ago by Imarok

Resolution: fixed
Status: assignedclosed

In 20543:

Count Trained Cattle as resource and not as unit. Also only count the net amount of gathered/used food for cattle.

Reviewed by: temple
Fixes: #3948
Differential Revision: https://code.wildfiregames.com/D1052

comment:18 Changed 2 years ago by elexis

Keywords: simple removed
Milestone: BacklogAlpha 23

Thanks

Last edited 2 years ago by elexis (previous) (diff)

comment:19 Changed 3 months ago by elexis

In 23087:

Refactor session lobby bot client code to use object orientation, refs #5387.

Adapt rating score for exploration score and add comment to keep calculateExplorationScore and calculateScoreTotal in sync following rP14752 (following the formula in rP12914 and rP14098 being otherwise in sync with the summary screen measures), refs #686.
Adapt rating score for captured entities and add comment to keep calculateMilitaryScore in sync following rP18395, refs #3216, rP16550.
Adapt rating score for trade and vegetarian food and add comment to keep calculateEconomyScore in sync following rP19584/D494 and rP20543/D1052, refs #3948.
Remove ceasefire sending from the client from rP16624 since the bot doesn't record it and since ceasefire time is expected to be 0 after a 1v1 was decided, refs #2749.
Resolve column value computation fragmentation and remove unneeded intermediary object encoding following rP14703, refs #686.
Remove hardcoding/duplication of unit and structure classes and send domesticUnitsTrained too, refs rP23086/D2384.
Move session code including the two globals to a separate folder so as to maximize separation of and ease distribution without lobby code.
Migrate useless ugly trailing commas from rP14098, to be removed in a patch modifying the bot code.

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

Note: See TracTickets for help on using tickets.