Page MenuHomeWildfire Games

introduce Builder class and hot key
Needs ReviewPublic

Authored by Nescio on Jan 4 2021, 1:23 PM.

Details

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

Currently all units that can build can also gather all resources, and vice versa; those units have the Worker class. While this works for the public mod, there is no compelling reason this will always be the case: units that can build but not gather are perfectly conceivable, as are units that can gather but not build.

This patch introduces a Builder visible class, which may make it clearer for (new) players to understand which units can build. This is also the class that should be used by technologies (currently none) and auras.
Furthermore, it introduces an “idle builder” hot key, which ought to make it easier to find units to build structures, and assigns the semicolon for it, due to its proximity to the period (idle worker) and slash (idle warrior), and because that key is not used yet.

Test Plan

Check for correctness and completeness, verify everything works as intended.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
SeverityLocationCodeMessage
Warningbinaries/data/mods/public/gui/session/input.js:241ESLintBear (no-undef-init)ESLintBear (no-undef-init)
Warningbinaries/data/mods/public/gui/session/input.js:478ESLintBear (default-case)ESLintBear (default-case)
Warningbinaries/data/mods/public/gui/session/input.js:493ESLintBear (operator-linebreak)ESLintBear (operator-linebreak)
Warningbinaries/data/mods/public/gui/session/input.js:505ESLintBear (default-case)ESLintBear (default-case)
Unit
No Unit Test Coverage
Build Status
Buildable 14803
Build 31814: Vulcan BuildJenkins
Build 31813: Vulcan Build (macOS)Jenkins
Build 31812: Vulcan Build (Windows)Jenkins
Build 31811: arc lint + arc unit

Event Timeline

Nescio created this revision.Jan 4 2021, 1:23 PM
Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Jan 4 2021, 1:23 PM
Vulcan added a comment.Jan 4 2021, 1:25 PM

Build failure - The Moirai have given mortals hearts that can endure.

builderr-debug-macos.txt
fatal error: file '/Users/wfg/Jenkins/workspace/macos-differential/build/workspaces/gcc/../../../libraries/source/spidermonkey/include-unix-debug/mozilla/StaticAnalysisFunctions.h' has been modified since the precompiled header 'obj/network_Debug/precompiled.h.gch' was built
note: please rebuild precompiled header 'obj/network_Debug/precompiled.h.gch'
1 error generated.
make[1]: *** [obj/network_Debug/NetMessageSim.o] Error 1
make: *** [network] Error 2

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/2672/display/redirect

Nescio published this revision for review.Jan 4 2021, 4:33 PM

Idea is nice, not sure if one needs to add a default hotkey when it is not needed for vanilla, but no strong opinion.

binaries/data/mods/public/gui/session/hotkeys/misc.xml
55–59

^^

binaries/data/mods/public/simulation/data/auras/units/heroes/pers_hero_xerxes_i.json
7

Affects worker? And not specify that above? Not sure whether that is possible :)

Idea is nice, not sure if one needs to add a default hotkey when it is not needed for vanilla, but no strong opinion.

The hot key is useful, it selects only units that can build, whereas the “idle worker” selects not just workers, but also traders, fishing boats, and citizen cavalry.

Ah, right, totally forgot that ^^'

wraitii requested changes to this revision.Jan 9 2021, 10:10 AM
wraitii added a subscriber: wraitii.

I guess you're planning for this but this calls for a "Gatherer" class and the removal of "Worker".
I guess we should move the visible classes to .json and add a description and then we can have a "Civ-opedia" like linking.

Requesting changes over the typo (see inline).

binaries/data/config/default.cfg
256

I fear these hotkeys are a bit complex, particularly since it seems "idlewarrior" could be "idleunit+militaryonly" or something, but I guess that's out of scope.

binaries/data/mods/public/simulation/data/auras/units/elephant_builder.json
3

Maybe the aura should be renamed then?

binaries/data/mods/public/simulation/data/auras/units/heroes/pers_hero_xerxes_i.json
7

No, it actually works, and acts as and "AND". I reckon this might be behaviour we want to change.

There's a typo, though.

This revision now requires changes to proceed.Jan 9 2021, 10:10 AM
Nescio updated this revision to Diff 15070.Jan 9 2021, 12:41 PM
  • fix typo spotted by @wraitii
  • rename worker elephant aura

Build is green

builderr-debug-macos.txt
ld: warning: text-based stub file /System/Library/Frameworks//CoreAudio.framework/CoreAudio.tbd and library file /System/Library/Frameworks//CoreAudio.framework/CoreAudio are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks//AudioToolbox.framework/AudioToolbox.tbd and library file /System/Library/Frameworks//AudioToolbox.framework/AudioToolbox are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks//ForceFeedback.framework/ForceFeedback.tbd and library file /System/Library/Frameworks//ForceFeedback.framework/ForceFeedback are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks//CoreVideo.framework/CoreVideo.tbd and library file /System/Library/Frameworks//CoreVideo.framework/CoreVideo are out of sync. Falling back to library file for linking.
ld: warning: text-based stu

See https://jenkins.wildfiregames.com/job/macos-differential/2728/display/redirect for more details.

I guess you're planning for this but this calls for a "Gatherer" class and the removal of "Worker".

Perhaps. I agree it would be more consistent and admit I've toyed with the idea more than once. However, it's less straightforward, units are not necessarily able to gather all resources. Female citizens and citizen infantry can, but citizen cavalry can only gather meat, no other resources, so basically both the Citizen and the Worker classes have gatherer functions, as does the FishingBoat.
Moreover, a mod might want to introduce separate units (a butcher, farmer, miner, stone mason, etc.), so it might be better to introduce a class for each resource subtype (e.g. MeatGatherer, GrainGatherer, WoodGatherer). However, displaying that many classes makes things less readable. Maybe we should not make those classes visible and simple display the gather rates instead. Unfortunately those numbers are wrong, gather rates vary by resource subtype, e.g. citizen infantry has:

<ResourceGatherer>
  <Rates>
    <food.fruit>0.5</food.fruit>
    <food.grain>0.25</food.grain>
    <food.meat>1</food.meat>
  </Rates>
</ResourceGatherer>

which is displayed in game as “food: 0.58” (the average). But that's a different discussion.