[PATCH] Number type problem with relative templates.

[fatherbushido]

When using relative template, for example

  <Cost>
    <BuildTime>350</BuildTime>
    <Resources>
      <stone op="mul">0.8</stone>
    </Resources>
  </Cost>

in mace_fortess.xml,

We get an (expected) error due to wrong number type :

ERROR: RelaxNGValidator: Validation error: structures/mace_fortress:1: Type nonNegativeInteger doesn't allow value '799.9878'

see L157 of ps/trunk/source/simulation2/system/ParamNode.cpp

// TODO: Support parsing of data types other than fixed; log warnings in other cases

attachment:reproduce.patch:ticket:3908​.

[fatherbushido]

I had just seen that there is a TODO in :​r17386

// TODO: Support parsing of data types other than fixed; log warnings in other cases

[sanderd17]

Isn't this a bit normal? If the code expects an integer, giving 0.8 won't be valid. Note that 0.8 has no finite expansion in binary, so multiplication with a something that should theoretically give an integer might not always result in something integer after all.

If you want to have multiplication with a decimal, the cost itself should be allowed to be a decimal, and the code should handle decimals correctly (rounding in the simulation or in the gui).

[wraitii]

The problem here is mostly that we have been using integers for things that should probably be able to handle decimals (most stats). Integers should be reserved for attributes that really are only supposed to be integers and everything would go haywire (integers as enums - though I can't think of a use case for us), IDs, counts,...

This should therefore be fixed by switching things to handle decimals.

[fatherbushido]

I guess the idea of wraitii and sanderd17 is the right one. cost roundings are yet in the gui so it should be ok

[fatherbushido]

In 18500:

Use decimals instead of integers to allow relative templates for resource costs, as proposed by sanderd17, fixes #3818. Use relative templates following r17706.

[fatherbushido]

Following many discussions on irc, i hesitated to revert r18500. It seems an option can be to allow decimals in templates, but round in the sim. Here is a (not so) draft patch, the AI part must be checked carefully.

[leper]

How about actually fixing that TODO and doing the rounding (or similar) when parsing the XML?

[fatherbushido]

Thanks for your clever input. I'll revert r18500, and i (or another) will try to look at this TODO. It'll be cleaner.

[fatherbushido]

I feel a bit incompetent to fix that TODO in a clean way.

The 2 (clumsy) solutions i saw are:

• distinguish the op like imul (for integers) and fmul (for fixed) for example.
• adding another attribute (like datatype = "int") in xml files.

[fatherbushido]

In 18632:

Reverts r18500 (excepting the buildtime part of iberian and macedonian fortress templates) as #3818 must be fixed in a cleaner way. Fixes a typo. Refs #3818.

[elexis]

Backlogging due to lack of progress.

[wraitii]

​Phab:D268

[fatherbushido]

That one is still a TOHAVE. I am now convinced that we should not try the workaround of doing an imul (in templates) but should use the schema instead as leper said many times.

[elexis]

In r22003 by wraitii:

Add "mul_round" op to template parsing to support multiplying-then-rounding.

This allows using arbitrary 'mul' values with Integer types, instead of having to switch them to Decimal types.
The ParamNode is not aware of validation (thus types) so a better solution is incredibly non-trivial.

Differential Revision: ​https://code.wildfiregames.com/D268

[Imarok]

Move tickets to Simulation as UI & Simulation got some sub components.

[Freagarach]

In 26236:

Ceil the resource costs for insufficient resources.

A controversial change (refs. #3818, #4099, ​Phab:D1438), but good to have in some form.

Patch by: @Langbart
Differential revision: ​https://code.wildfiregames.com/D4332