style: Apply consistent formatting to all numerical values

[Stubbjax]

This change applies consistent formatting to all numerical values (integers, floats and percentages) across all INI files.

Note: Formatting is based on the how the data is parsed by the code, and not based on common or expected usage. For example, FXList → ParticleSystem → InitialDelay is expected in milliseconds, but its value is read via scanReal and therefore it is formatted as a float.

Before

IntegerField = 100.0
FloatField = 100
PercentageField = 100

After

IntegerField = 100
FloatField = 100.0
PercentageField = 100.0%

[xezon]

Does the percent give any additional information to the reader?

Since percentage is not bound to 0 to 100, it appears to be a useless information.

[Stubbjax]

It tells them that a value of 100 is the default.

[xezon]

Is that so? I expect it depends on context like any other. For VolumeShift I would expect a default of 0%.

[Stubbjax]

Sure, it would be a default value of 0% for the VolumeShift, but the default would be 100% for the Volume that it is shifting.

Do you suggest removing all % symbols?

[xezon]

Looking at impl, what it says is "this value is multiplied by 100"

Real INI::scanPercentToReal(const char* token)
{
	return scanReal(token) / 100.0f;
}

So while 100% would typically be 1, it is 100 in this case.

[Stubbjax]

What do you mean? That is just a standard % conversion? 100% is effectively a value of 1 as expected?

[xezon]

I suspect one source of error here could be if someone put a space between the number and the % sign. Then 1 token becomes 2 tokens.

It would be good to add a INI validate tool to the CI so that every INI change is checked against a number of rules.

[xezon]

There are 3,264 percent (%) symbols in INI files, and this change adds another about 4,000.

[Stubbjax]

Someone could do the same with a number and mistakenly change a Field = 100 into a Field = 10 0; there are such fields with <int> <int> so it wouldn't look out of the ordinary. The game would likely crash in such a case, regardless.

Is the ~4,000 % additions a concern or just a fun fact?

[xezon]

I am a bit sceptical about the % sign, but I can understand its intention. It would have been good if simply all relevant variable names had "Percent" in their name. The percent symbol does add a bit of clutter to the value.

Up to you if you want to go ahead with it.

[xezon]

Side note: It is strange that VolumeShift takes % and PitchShift does not.

[Stubbjax]

Perhaps, but this is how the game parses the data. I suppose pitch is not typically limited to a range.

[xezon]

There are 3,264 percent (%) symbols in INI files, and this change adds another about 4,000.

[Stubbjax]

I am a bit sceptical about the % sign, but I can understand its intention.

Would you prefer to remove the % signs?

[xezon]

Right now, I am 50 50 on it and have no clear preference.

[Stubbjax]

Right now, I am 50 50 on it and have no clear preference.

You mean you're 50% on it? 😛

What specifically are you sceptical about? The readability of 100 vs 100.0% or 100% vs 100.0%? The greater file size from the additional % characters (ignoring the thousands of added .0 to standardise floats)?

There are plenty of instances where a % sign aids readability, such as:

Behavior = CountermeasuresBehavior Example
  EvasionRate = 30.0% ; Percent format suggests that 30.0% of missiles are diverted
End

Behavior = JetAIUpdate Example
  OutOfAmmoDamagePerSecond = 10.0% ; Percent format suggests that the jet loses 10.0% of its max HP per second
End

Armor Example
  Armor = CRUSH 200.0% ; Percent format suggests that CRUSH damage is multiplied by 2
End

My original intention was to only standardise fields that alternated between formats. For example, in the retail data, AmericaJetRaptor has a BuildTime of 20 while AmericaVehicleChinook has a BuildTime of 10.0. As the BuildTime is parsed using scanReal (and 97% of BuildTime fields use a decimal format), it made sense to format all BuildTime fields to the same decimal format.

However, rather than just fixing the inconsistencies and then dealing with the anticipated "why not fix them all?" feedback, I opted to just fix them all in the first place. It makes sense to indiscriminately apply the same standard across all data to avoid manually auditing every field as well as eliminating any potential sources of confusion over format inconsistencies. I can apply the change as originally intended if desired.

I can also drop the commit (068e105) that adds missing % signs if it seems unnecessary. But then we'll still end up with inconsistencies such as the MinSampleVolume in AudioSettings.ini.

[xezon]

It is up to you if you want to go ahead with it. I am 50% for and 50% against it.

[Stubbjax]

It is up to you if you want to go ahead with it. I am 50% for and 50% against it.

Do you have any strong reasons for or against the change? It seems like the additional visual clutter entirely comprises your 50% against?

[Skyaero42]

It is up to you if you want to go ahead with it. I am 50% for and 50% against it.

Do you have any strong reasons for or against the change? It seems like the additional visual clutter entirely comprises your 50% against?

I would be in favour of the percentage sign. I looked up other games (e.g. Stellaris) and they use fractions rather than percentages all over - which makes a lot of sense as you know the number is used how it is shown. For percentage the modder needs to know the value gets divided by 100. I think adding the percentage sign is a good way of visualizing this.

Ideally we would use fractions everywhere, but that is a step too far atm I think.

[Skyaero42]

Agreed

[xezon]

Do you have any strong reasons for or against the change? It seems like the additional visual clutter entirely comprises your 50% against?

Reasons against:

1. Adds visual clutter
2. Adds larger error surface when space is added between number and percent sign
3. Adds more percent signs than already exist in INI files
4. "Percent" is already explicitly implied by some of the field names
5. Some percent fields have inconsistent styles such as PitchShift vs VolumeShift

[Stubbjax]

Reasons against:

1. Adds visual clutter
2. Adds larger error surface when space is added between number and percent sign
3. Adds more percent signs than already exist in INI files
4. "Percent" is already explicitly implied by some of the field names
5. Some percent fields have inconsistent styles such as PitchShift vs VolumeShift

Counterarguments:

1. Improves visual clarity
2. Error concerns are already present with existing % signs and not known to be an issue
   • In addition, existing values are often adjusted or copied rather than written from scratch
   • Consistent INI design language conveys that spaces are value separators
   • Superfluous tokens are ignored anyway, so Armor = DEFAULT 100.0 % is technically valid
   • When applicable, any such errors would also cause the parser to throw an error at startup and be quickly caught
3. Whether symbols are added or removed is not an argument in itself
   • One could argue that such additions could help reinforce existing INI design language
4. The percent status of some fields is not explicitly implied, such as volume and armour modifiers
   • It's arguably harder to read "Percent" from within a field name than a % symbol at the end of a value
5. Digital volume is almost always defined in a [0%, 100%] range (where 100% == 0.0dB) whereas pitch is not (e.g. [-12, +12] semitones, [-1200, +1200] cents)