#2936 closed enhancement (fixed)
[PATCH] Relative Templates
Reported by: | scythetwirler | Owned by: | scythetwirler |
---|---|---|---|
Priority: | Must Have | Milestone: | Alpha 20 |
Component: | Core engine | Keywords: | patch |
Cc: | Patch: |
Description (last modified by )
Currently, we [could] have templates for units such as
Generic War Dog
<Armour> <Hack>1</Hack> <Pierce>1</Pierce> <Crush>1</Crush> </Armour>
Ultra-Super Powered War Dog
<Armour> <Hack>11</Hack> <Pierce>8</Pierce> <Crush>9</Crush> </Armour>
It'd be a lot easier to make slight deviations if we had relative templates that perform an operation upon the stats of the parent. That is:
Ultra-Super Powered War Dog
<Armour> <Hack op="add">10</Hack> <Pierce op="add">7</Pierce> <Crush op="add">8</Crush> </Armour>
In the case that we need to change the stats of the entire class of units, we would have to only change the parent, but not every template that deviates.
Supporting the basic operations (+, -, x, ÷) would help greatly imo.
Also, files
- tools/templatesanalyzer/unitTables.py
- tools/entity/Entity.pm
will need to be updated.
Attachments (8)
Change History (36)
comment:1 by , 10 years ago
Description: | modified (diff) |
---|
comment:2 by , 10 years ago
Description: | modified (diff) |
---|
comment:3 by , 10 years ago
follow-up: 5 comment:4 by , 10 years ago
Wasn't it already possible to use <Add> and <Multiply>? (Dunno for sure)
comment:5 by , 10 years ago
Replying to niektb:
Wasn't it already possible to use <Add> and <Multiply>? (Dunno for sure)
I've seen this in Auras but I'm not sure whether it's possible in templates.
by , 9 years ago
Attachment: | relativetemplates.patch added |
---|
leper's patch with scythetwirler's fill-in-the-blanks.
comment:6 by , 9 years ago
Keywords: | review patch added |
---|---|
Milestone: | Backlog → Alpha 18 |
comment:7 by , 9 years ago
TODO: Update tools/templatesanalyzer/unitTables.py tools/entity/Entity.pm
by , 9 years ago
Attachment: | relativetemplates-0adbalance.patch added |
---|
comment:8 by , 9 years ago
Description: | modified (diff) |
---|
by , 9 years ago
Attachment: | TemplateAnalyzerUpdate.patch added |
---|
Should work but needs testing with patch (tested with current templates only)
comment:9 by , 9 years ago
wraitii: Could you include the full license (not that long for MIT license) and change the copyright line to WFG?
comment:10 by , 9 years ago
Oh, wraitii, forgot to mention one thing: the case of the "true" string in line 133 and line 205 (with your patch applied) is inconsistent, making ranged units appear to have 0 attack.
comment:11 by , 9 years ago
Description: | modified (diff) |
---|
comment:12 by , 9 years ago
Summary: | Relative Templates → [PATCH] Relative Templates |
---|
comment:13 by , 9 years ago
Priority: | Should Have → Must Have |
---|---|
Summary: | [PATCH] Relative Templates → [PATCH/WIP] Relative Templates |
Updating tools/entity/Entity.pm is the only remaining thing needed for this patch.
comment:14 by , 9 years ago
Milestone: | Alpha 18 → Alpha 19 |
---|
Doesn't seem like this will get done by A18 (nor the templates that will piggyback off it it).
by , 9 years ago
Attachment: | TemplateAnalyzerUpdate_fixed_typo_license.patch added |
---|
Fixes the typo and license in wraitii's TemplateAnalyzerUpdate.patch. To prove that the patch is identical otherwise, do a diff against his patch.
comment:15 by , 9 years ago
The only difference in the output of the TemplateAnalyzer with and without the patch I uploaded, is that the attack damage of the ranged units are displayed properly; i.e. the TemplateAnalyzer still works with wraitii's patch and and works even better with the typofix in the updated patch.
comment:16 by , 9 years ago
I find the PositiveDecimalAttribute a bit strange. On one hand, it's not easy to know what the difference with PositiveDecimal is (at least not once this patch gets forgotton). On the other hand, it's not intuitive why the other names don't exist for attributes.
The cleanest solution for this would be to forget about encoding sizes in attributes, and always try to encode them in XML node values (like is done in most components). So the Footprint, Obstruction and VisualActor component would need a few bits of code rewritten (and also things that depend on those, like some AI code perhaps).
Secondly, you don't seem to add an "op" attribute to native data types (things encoded with <data type=""> in the component schemas). This is also needed for consistency IMO. In this case, I think it would be better to define names for native types too. So the schemas of those native types can be changed to <ref name=""> and have an "op" attribute.
Apart from those things, good patch.
by , 9 years ago
Attachment: | relative_templates.diff added |
---|
Combined patch, still missing entity.pm fix
comment:17 by , 9 years ago
Above there's a combined patch, where I also added the plain decimal type as a ref, so "op" can be used on that. Integers aren't added as such, since I thought div would be a bit hard for integers (and maybe we need to support decimals in case we want operations anyway).
The PositiveDecimalAttribute doesn't exist anymore, but the C++ components that needed it just use the native definition (it's not that much longer).
The "op" attribute now has only 4 possible values (instead of free-text previously). This should make the error messages nicer, as they should happen on the validation level rather than on the parsing level.
The templateanalyzer is also included, with MIT headers. But it's hard to see what's wrong with entity.pm and this patch, as any code that uses entity.pm already seems to have other issues.
As another remark, do we need "sub" and "div"? "Div" is dangerous for division by zero, and "sub" can make a number negative when we expect it to be positive (XML validation won't help a lot here). While if we support only "add" and "mul", then all cases are clear and we can even support integers:
- int + int = int
- int * int = int
- pos + pos = pos
- pos * pos = pos
If we want division, the template should support decimal values, and be able to cope with it. If we want subtraction, the template should support negative values, and be able to cope with it.
comment:18 by , 9 years ago
Updated version including comment above. I didn't fix the TODO because it is not needed for fixing Entity.pm. Thanks in advance to Ben for working on updating that script! :)
by , 9 years ago
Attachment: | relative_templates_r16964.patch added |
---|
by , 9 years ago
Attachment: | Entity.pm-relative-templates.patch added |
---|
comment:19 by , 9 years ago
Attached patch for Entity.pm, it seems to work for simple cases, but I haven't verified if it produces equivalent results when there's a sequence of operators due to inheritance.
To use entvalidate.pl
, you have to dump the schema first, which involves running the game with the -dumpSchema
option (you'll need -mod=public
, too). Also checkrefs.pl
was broken but has been fixed in SVN.
comment:20 by , 9 years ago
Awesome, thanks for working on it :D scythetwirler, do you have some time to review that? Would you want to have the templates updated for A19 or do you want to commit that after the release?
comment:21 by , 9 years ago
Milestone: | Alpha 19 → Alpha 20 |
---|
comment:22 by , 8 years ago
Looks good to me but I'd like the approval of another team member before committing. (though I haven't taken a look at the perl scripts)
comment:23 by , 8 years ago
s/TODO/TODO:/
apart from that it looks good, given that my original idea wasn't nuts. So I guess you want someone else to take a look.
by , 8 years ago
Attachment: | RelativeTemplates.patch added |
---|
comment:25 by , 8 years ago
Includes the entity.pm fix (which seemed to work from testing) and a new fix for the template analyzer.
comment:26 by , 8 years ago
Summary: | [PATCH/WIP] Relative Templates → [PATCH] Relative Templates |
---|
comment:28 by , 8 years ago
Keywords: | review removed |
---|
#2620 is somewhat related.