regression in Utility (upgrade rules)

[ihptru]

#4694 is a reason of regression in Utility

From what I've managed to notice so far is that patch adds c0 (or maybe similar) to Range value in map rules.

1. OpenRA.Utility.exe --upgrade-map... does not understand what is "c" char:

System.FormatException: Unknown char: c
  at System.Double.Parse (System.String s, NumberStyles style, IFormatProvider provider) [0x00000] in <filename unknown>:0 
  at System.Single.Parse (System.String s) [0x00000] in <filename unknown>:0 
  at OpenRA.Utility.UpgradeRules.ConvertFloatToRange (System.String& input) [0x00000] in <filename unknown>:0 
  at OpenRA.Utility.UpgradeRules.UpgradeActorRules (Int32 engineVersion, System.Collections.Generic.List`1& nodes, OpenRA.FileFormats.MiniYamlNode parent, Int32 depth) [0x00000] in <filename unknown>:0 
  at OpenRA.Utility.UpgradeRules.UpgradeActorRules (Int32 engineVersion, System.Collections.Generic.List`1& nodes, OpenRA.FileFormats.MiniYamlNode parent, Int32 depth) [0x00000] in <filename unknown>:0 
  at OpenRA.Utility.UpgradeRules.UpgradeActorRules (Int32 engineVersion, System.Collections.Generic.List`1& nodes, OpenRA.FileFormats.MiniYamlNode parent, Int32 depth) [0x00000] in <filename unknown>:0 
  at OpenRA.Utility.UpgradeRules.UpgradeMap (System.String[] args) [0x00000] in <filename unknown>:0 
  at OpenRA.Utility.Program.Main (System.String[] args) [0x00000] in <filename unknown>:0 
[ERROR] FATAL UNHANDLED EXCEPTION: System.FormatException: Unknown char: c
  at System.Double.Parse (System.String s, NumberStyles style, IFormatProvider provider) [0x00000] in <filename unknown>:0 
  at System.Single.Parse (System.String s) [0x00000] in <filename unknown>:0 
  at OpenRA.Utility.UpgradeRules.ConvertFloatToRange (System.String& input) [0x00000] in <filename unknown>:0 
  at OpenRA.Utility.UpgradeRules.UpgradeActorRules (Int32 engineVersion, System.Collections.Generic.List`1& nodes, OpenRA.FileFormats.MiniYamlNode parent, Int32 depth) [0x00000] in <filename unknown>:0 
  at OpenRA.Utility.UpgradeRules.UpgradeActorRules (Int32 engineVersion, System.Collections.Generic.List`1& nodes, OpenRA.FileFormats.MiniYamlNode parent, Int32 depth) [0x00000] in <filename unknown>:0 
  at OpenRA.Utility.UpgradeRules.UpgradeActorRules (Int32 engineVersion, System.Collections.Generic.List`1& nodes, OpenRA.FileFormats.MiniYamlNode parent, Int32 depth) [0x00000] in <filename unknown>:0 
  at OpenRA.Utility.UpgradeRules.UpgradeMap (System.String[] args) [0x00000] in <filename unknown>:0 
  at OpenRA.Utility.Program.Main (System.String[] args) [0x00000] in <filename unknown>:0

[Unit158]

Can we have the offending map?

[cjshmyr]

What I think is happening here (and may not be a regression):

If you --update-map, providing a map that is already up to date by our rule definitions (not sure if date range applies to the Float->Range conversion) which already has the correct values in Range (e.g. Range: 8c512), this happens. It's trying to cast the range (in my example, 8c512) to a float which won't work.

Edit: attempts to convert something that doesn't need further conversion ;)

[ihptru]

for example, distributed with OpenRA client map: http://resource.openra.net/maps/487

[Unit158]

Hamb, that's pretty close. It still should be fixed/not crash

[cjshmyr]

I agree; Utility could check if Range was already in the right format.

[pchote]

IMO it is the responsibility of the person running the utility to make sure they give it the correct date. This is one case where you can programmatically check that they have asked for a bogus conversion, but there are others that will run without error and give bogus results.

[ihptru]

I've tried every possible date, it just does not work and crashes. I think it also does something bogus to maps because after --upgrade-map, Lint.exe also crashes at Range: *c* saying something similar to:

OpenRA.Lint(1,1): Error: Failed with exception: System.InvalidOperationException: FieldLoader: Cannot parse `13c910` into `Speed.System.Int32`

[pchote]

The date is the date of the engine version that the map was written for. If the range already has a c it means that it is already up to date and that you shouldn't be upgrading them further (or should be passing a date from after the change was introduced, when upgrading future changes).

#4636 covers adding a RulesVersion key to the map to avoid confusion like this.