Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#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 scythetwirler)

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)

relativetemplates.patch (8.3 KB ) - added by scythetwirler 9 years ago.
leper's patch with scythetwirler's fill-in-the-blanks.
relativetemplates-0adbalance.patch (8.3 KB ) - added by scythetwirler 9 years ago.
TemplateAnalyzerUpdate.patch (5.9 KB ) - added by wraitii 9 years ago.
Should work but needs testing with patch (tested with current templates only)
TemplateAnalyzerUpdate_fixed_typo_license.patch (7.6 KB ) - added by elexis 9 years ago.
Fixes the typo and license in wraitii's TemplateAnalyzerUpdate.patch​. To prove that the patch is identical otherwise, do a diff against his patch.
relative_templates.diff (17.7 KB ) - added by sanderd17 9 years ago.
Combined patch, still missing entity.pm fix
relative_templates_r16964.patch (15.5 KB ) - added by Itms 9 years ago.
Entity.pm-relative-templates.patch (755 bytes ) - added by historic_bruno 9 years ago.
RelativeTemplates.patch (16.0 KB ) - added by wraitii 8 years ago.

Download all attachments as: .zip

Change History (36)

comment:1 by scythetwirler, 9 years ago

Description: modified (diff)

comment:2 by scythetwirler, 9 years ago

Description: modified (diff)

comment:3 by scythetwirler, 9 years ago

#2620 is somewhat related.

comment:4 by Niek, 9 years ago

Wasn't it already possible to use <Add> and <Multiply>? (Dunno for sure)

in reply to:  4 comment:5 by scythetwirler, 9 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 scythetwirler, 9 years ago

Attachment: relativetemplates.patch added

leper's patch with scythetwirler's fill-in-the-blanks.

comment:6 by scythetwirler, 9 years ago

Keywords: review patch added
Milestone: BacklogAlpha 18

comment:7 by scythetwirler, 9 years ago

TODO: Update tools/templatesanalyzer/unitTables.py tools/entity/Entity.pm

by scythetwirler, 9 years ago

comment:8 by scythetwirler, 9 years ago

Description: modified (diff)

by wraitii, 9 years ago

Should work but needs testing with patch (tested with current templates only)

comment:9 by leper, 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 scythetwirler, 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 scythetwirler, 9 years ago

Description: modified (diff)

comment:12 by historic_bruno, 9 years ago

Summary: Relative Templates[PATCH] Relative Templates

comment:13 by scythetwirler, 9 years ago

Priority: Should HaveMust 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 scythetwirler, 9 years ago

Milestone: Alpha 18Alpha 19

Doesn't seem like this will get done by A18 (nor the templates that will piggyback off it it).

by elexis, 9 years ago

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 elexis, 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.

Last edited 9 years ago by elexis (previous) (diff)

comment:16 by sanderd17, 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 sanderd17, 9 years ago

Attachment: relative_templates.diff added

Combined patch, still missing entity.pm fix

comment:17 by sanderd17, 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 Itms, 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! :)

Last edited 9 years ago by Itms (previous) (diff)

by Itms, 9 years ago

by historic_bruno, 9 years ago

comment:19 by historic_bruno, 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 Itms, 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 Itms, 9 years ago

Milestone: Alpha 19Alpha 20

comment:22 by scythetwirler, 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 leper, 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.

comment:24 by wraitii, 8 years ago

I like it too.

by wraitii, 8 years ago

Attachment: RelativeTemplates.patch added

comment:25 by wraitii, 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 scythetwirler, 8 years ago

Summary: [PATCH/WIP] Relative Templates[PATCH] Relative Templates

comment:27 by scythetwirler, 8 years ago

Owner: set to scythetwirler
Resolution: fixed
Status: newclosed

In 17386:

Implements relative templates. Fixes #2936. Thanks to leper, wraitii, historicbruno and everyone else that helped.

comment:28 by scythetwirler, 8 years ago

Keywords: review removed
Note: See TracTickets for help on using tickets.