Page MenuHomeWildfire Games

Count Trained Cattle as resource and not as unit (And another cattle fix)
ClosedPublic

Authored by Imarok on Nov 21 2017, 3:42 PM.

Details

Summary

Adds cattle as "Livestock trained" to Resources. Don't count them to the total trained units.
That doesn't fit in min res, but the resource panel didn't fit before in min res...

Also fix the used/gathered food for cattle.

refs rP19584 / D494

Test Plan

Trained Sheeps and goats should appear in the summary under resources -> livestock and not be counted to total units.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Imarok created this revision.Nov 21 2017, 3:42 PM
Stan added a subscriber: Stan.Nov 21 2017, 4:45 PM
Stan added inline comments.
binaries/data/mods/public/gui/summary/counters.js
249 ↗(On Diff #4304)

Is that an extra space between return and the statement ?

Vulcan added a subscriber: Vulcan.Nov 21 2017, 5:20 PM

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

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Relax-NG validity error : Extra element props in interleave
/public/art/actors/structures/carthaginians/stable_elephant.xml:0: Relax-NG validity error : Element variant failed to validate content
/public/art/actors/structures/carthaginians/stable_elephant.xml:0: Relax-NG validity error : Element group has extra content: variant
Relax-NG validity error : Extra element group in interleave
/public/art/actors/structures/carthaginians/stable_elephant.xml:0: Relax-NG validity error : Element actor failed to validate content
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/gui/summary/counters.js
| 249| »   return··playerState.sequences.unitsTrained["Domestic"][index];
|    | [NORMAL] JSHintBear:
|    | ['Domestic'] is better written in dot notation.
Imarok added inline comments.Nov 21 2017, 6:04 PM
binaries/data/mods/public/gui/summary/counters.js
249 ↗(On Diff #4304)

Ah, yeah, thanks.

Imarok updated this revision to Diff 4308.Nov 21 2017, 6:21 PM

Remove double space

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

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/gui/summary/counters.js
| 249| »   return·playerState.sequences.unitsTrained["Domestic"][index];
|    | [NORMAL] JSHintBear:
|    | ['Domestic'] is better written in dot notation.
temple added a subscriber: temple.Nov 21 2017, 8:34 PM

It might be nice to also adjust the food gathered/used to take into account livestock. Instead of each trained unit counting as +50 food used, have them count as -50 food gathered. Then the food totals would be comparable between players who use corrals and those who don't (currently they're meaningless), and we wouldn't have to make an adjustment for livestock when calculating the economy score.

elexis added a subscriber: elexis.Nov 22 2017, 12:06 PM

Good, right idea to address the issue.
(Making things worse for 1024x786? If so, we don't care about making it worse?)

binaries/data/mods/public/gui/summary/counters.js
249 ↗(On Diff #4304)

.Domestic

binaries/data/mods/public/gui/summary/layout.js
121 ↗(On Diff #4308)

The word training fits for military units. Breed seems more accurate for domestic animals.
Not sure if the resources or units panel is preferable.

Imarok marked 2 inline comments as done.Nov 26 2017, 1:26 PM
In D1052#41622, @temple wrote:

It might be nice to also adjust the food gathered/used to take into account livestock. Instead of each trained unit counting as +50 food used, have them count as -50 food gathered. Then the food totals would be comparable between players who use corrals and those who don't (currently they're meaningless), and we wouldn't have to make an adjustment for livestock when calculating the economy score.

Good idea. Sounds correct.

In D1052#41678, @elexis wrote:

(Making things worse for 1024x786? If so, we don't care about making it worse?)

I don't like to make it worse for 1024x768, but there is no choice, so I just hope for horizontal scrolling...

binaries/data/mods/public/gui/summary/layout.js
121 ↗(On Diff #4308)

I've chosen resource panel, so that it is clear livestock isn't added to the unit totals.

Imarok updated this revision to Diff 4392.Nov 26 2017, 2:36 PM
Imarok marked an inline comment as done.

Fixed said above

Imarok retitled this revision from Count Trained Cattle as resource and not as unit to Count Trained Cattle as resource and not as unit (And another cattle fix).Nov 26 2017, 2:41 PM
Imarok edited the summary of this revision. (Show Details)

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

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Executing section Default...
Executing section Source...
Executing section JS...
temple accepted this revision.Nov 26 2017, 5:35 PM

We keep track of the vegetarian food gathered, so I think the vegetarian ratio is still correct. It can get weird if we train some sheep but don't gather them, since that counts as negative food gathered, so the ratio could be over 100% or even negative in the very early game. But I don't think that's anything to worry about.

In D1052#42448, @Imarok wrote:

I don't like to make it worse for 1024x768, but there is no choice, so I just hope for horizontal scrolling...

That would be nice.

This revision is now accepted and ready to land.Nov 26 2017, 5:35 PM
This revision was automatically updated to reflect the committed changes.
elexis edited the summary of this revision. (Show Details)Sun, Oct 20, 3:14 PM