moved the entry VisibleClasses Soldier from template_unit_hero.xml to the inheriting templates which are actually Soldier in order to allow an eventual non Soldier Hero.
Diff Detail
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Event Timeline
Considering that we already had multiple issues with the hero healer requiring to copy things over and then forgetting half of them, this approach sounds like it should be welcomed.
I rather have some additional disabled="" for the hero healer than copies.
Can't we use something like
<VisibleClasses datatype="tokens">Infantry -Soldier</VisibleClasses>
I don't know if the code supports removing classes like it does for units.
datatype="tokens" is the answer to your question.
(And that's indeed the thing elexis wants)
(edit: and even if I am more for adding things than for removing things, it seems the proper solution)
(edit2: still don't know why that template is not fixed 6 months later :p)
Perhaps also template_unit_hero, template_unit_hero_infanty, template_unit_hero_cavalry, template_unit_hero_healer (Yes, cunobelin is a fun case).
Correct that it's better to add things than to remove it. The important thing to me is to avoid the duplication issues (being required to copy parts over means someone will and did forget to address the same change in all copies).
I would recommend Grugnas to print on paper (sorry for the trees)
https://trac.wildfiregames.com/browser/ps/trunk/binaries/data/mods/public/simulation/templates/template_unit.xml
https://trac.wildfiregames.com/browser/ps/trunk/binaries/data/mods/public/simulation/templates/units/maur_hero_chanakya.xml
https://trac.wildfiregames.com/browser/ps/trunk/binaries/data/mods/public/simulation/templates/template_unit_support_healer.xml
https://trac.wildfiregames.com/browser/ps/trunk/binaries/data/mods/public/simulation/templates/template_unit_hero.xml and inherited ones.
Then take a pen and do the stuff.
Retitling the diff too (as non heroes soldier are already allowed). (It can be merge with the other diff imo -> Make the Mauryan healer hero inherit from generic hero templates...)
template_unit_hero_healer is cleaner but perhaps useless while we only have 1 (or 2).
Then we could just commit that. (refs d225)
i didn't get joke?
Retitling the diff too (as non heroes soldier are already allowed). (It can be merge with the other diff imo -> Make the Mauryan healer hero inherit from generic hero templates...)
Since the template_unit_hero has the visible class Soldier, it made me think that Heroes are soldiers by default and if a modder wants to create a non hero soldier he should explicitly remove the Soldier class which is in my opinion inelegant, unnecessary (it is the same thing of adding the Soldier Class) and perhaps not immediate.
template_unit_hero_healer is cleaner but perhaps useless while we only have 1 (or 2).
i agree with this (cunobelin isn't a healer at all).
I could rename this diff "clean Heroes sounds inheritance" creating a dipendency with D808 for the elephant hero and simply modify the chanakya diff D747 in order to remove the Soldier class from its template. would that be ok?
I dont see Soldier used anywhere in the code besides for the AI (and that part of the code seems irrelevant).
So the 0AD-brand Soldier can be considered equal to the common soldier and a soldier is a non-unique unit.
The current heroes don't have the Soldier tag, so why would we add one.
Edit: Didn't search JSON files, but Soldier is still what we casually understand as a soldier, so still don't see the point of adding the tag.
I don't understand. They have currently I think (Soldier, not CitizenSoldier) (btw it's more or less (Infantry || Cavalry) or something like that). It's indeed also often used in techs and auras iirc.
The current heroes don't have the Soldier tag, so why would we add one.
They have currently
You are right. I ran a grep on template_unit_hero_* instead of template_unit_hero*.
(Edit: I just answered because Grugnas sent me a PM asking whether the Soldier tag should be added anywhere. I should either completely review it or shut up)