It could be interesting to add a repair aura for workshop, both to compensate for the fact that mace have to build an extra building compared to other civs, and to give an incentive to capture workshop by civ without workshop.
Currently 2 siege units can be garrisoned inside.
Details
- Reviewers
elexis - Commits
- rP20461: workshop repair aura
Check if you agree on the principle, and if yes, on the values.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 3718 Build 6459: Vulcan Build (Windows) Jenkins Build 6458: Vulcan Build Jenkins Build 6457: arc lint + arc unit
Event Timeline
I thought it was a ranged aura and would have suggested Repairable.RepairTimeRatio then.
But since it's a garrisoned one, sounds ok to me. (Maybe the repair one - if it's supported - would be interesting as well, possibly additionaly?)
400HP / 4HP/s = 100s seems ok too.
Patch should be complete, since war elephants do heal in elephant stables last time I checked.
Commit (I didn't test this, so care ) if the repair thing is disapproved.
binaries/data/mods/public/simulation/data/auras/structures/cart_super_dock_repair.json | ||
---|---|---|
8 | and this (no further occurrences exist) | |
binaries/data/mods/public/simulation/data/auras/structures/workshop_repair.json | ||
10 | Delete this line (extra mile would be adding a warning for auras with icons in a suitable location I didn't find) |
Principle, yes, I agree.
Values, no, not really. Temple heals 20 units at 3 health per second, barracks (with technology) 10 units at 1 health. So maybe allow workshop to heal 5 garrisoned units at, let's say 2 health?
Last time I played temples healed too quickly. At least it seemed cheap to build them near the enemy and jump in and out.
Updated following comments. I'm not really in favor of increasing the garrison capacity to 5 (the structure is supposed to be relatively small, may be 3 if people think that's needed).
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
is this aura json file necessary?
standing at GarrisonHolder component, there is the BuffHeal entry which job is to regen health of garrisoned unit class already.
( while at it, i guess that 1hp / sec for elephants is kinda low ).
( same for ships in the super dock )
That doesn't restrict the 'healing' to siege units does it?
In fact to me it sounds like a good reason to delete BuffHeal.
Auras also have the advantage that we can add tooltips.
Futhermore, currently BuffHeal applies only to Healable units while mechanical ones (like ship and siege) are not Healable but Repairable. Of course, the BuffHeal code could be tweaked, but cleaner to use Auras for repairables.
you are right.
<GarrisonHolder> <Max>5</Max> <EjectHealth>0.1</EjectHealth> <EjectClassesOnDestroy datatype="tokens">Unit</EjectClassesOnDestroy> <List datatype="tokens">Support Infantry Cavalry Ship</List> <BuffHeal>0</BuffHeal> <LoadingRange>2</LoadingRange> </GarrisonHolder>
this piece of code is from the carthaginian super dock and, at this point, i wonder if ships garrisoned by elephants and sieges can garrison the super dock.
In any case, i can't think any valid reason from the practical point of view to let other than mechanical units garrison such buildings.
By preventing any unit other than ships in the dockyard and sieges in the workshop to garrison, would resolve the issue ( this approach perhaps may conflict with Chanakya mauryan hero aura which decreases research time of buildings in which he can't garrison even if there are other buildings not able to being garrisoned like docks, storehouses and farmsteads ).
on the other hand, having a dedicated aura and tooltip would be more modular but wonder if the healing feature would have better visibility if directly shown on the structure tooltip since it isn't a feature unlocked by technology as happens with barracks.
I don't know if those 2 different approaches are relevant from a performance point of view.
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
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