Improve our existing tag name and discuss clear standards for it

[DeathFishAtEase]

Important

Since this involves historical usage and naming habits, we are opening this PR to collect opinions and discuss possible improvements. The goal is to establish clearer standards for similar cases in the future.

Take the problem that this PR initially hoped to fix as an example:

• In the documentation, Crit.AffectsAbovePercent is used, although it is more standard compared to Crit.AffectAbovePercent ( s ), but what is actually checked is this->Crit_AffectAbovePercent.Read(exINI, pSection, "Crit.AffectAbovePercent");
  However, there is also a Debug Log that uses Affect s:

      if (this->Crit_AffectsAbovePercent > this->Crit_AffectsBelowPercent)
    	  Debug::Log("[Developer warning][%s] Crit.AffectsAbovePercent is bigger than Crit.AffectsBelowPercent, crit will never activate!\n", pSection);

• And, similarly, the warhead logic Customizable Warhead trigger conditions uses the following flags:

  [SOMEWARHEAD]               ; WarheadType
  AffectsBelowPercent=1.0     ; floating point value, percents or absolute
  AffectsAbovePercent=0.0     ; floating point value, percents or absolute

  ---

• Unifying the use of Affect s has the following advantages:

  1. It is better to maintain the same naming rules as similar logic, and this helps avoid spelling errors in INI code;
  2. Using Affects is more standard in expression compared to Affect;

• But if we do this, we need to consider the issue of user INI flag Migration.

  1. Its paired flag Crit.AffectBelowPercent was added as early as DevBuild#27, meaning that since its addition, it has gone through two major versions, 0.3 and 0.4, while Crit.AffectsAbovePercent or the one it should actually use, Crit.AffectBelowPercent, are both new content added in DevBuild#48.
  2. There is a previous handling scheme: when BelowPercent was first introduced, the documentation also used Affect s, but this was corrected as a spelling error after 9 months.

---

The above example issue has been temporarily fixed based on the source code operation method as the standard.

But indeed, we have always lacked specifications for handling such issues—whether naming or minor content INI tag changes such as typos. We need to clarify the handling methods for these types of problems.

Note

Currently, users have expressed two viewpoints: some believe that since it has already been added, it should not be changed; others believe that as a non-standard and inconsistent flag name, it should be unified, especially since we have MigrationUtility, which makes flag migration not troublesome.

In summary, for issues similar to it, please provide your opinions, such as

• Listing other existing similar issues.
• Specifying how migration documents should be written.
• Defining the standards we need to follow.

After sufficient discussion, we can pre-process all similar issues and descriptions of changes through this PR.

---

Currently existing tag replacement plan:

# 1. Standardization and Misspelling - Affect → Affect**s**
- Crit.AffectBelowPercent
+ Crit.AffectsBelowPercent

# 2. Standardization and Misspelling - Affect → Affect**s**
- Crit.AffectAbovePercent
+ Crit.AffectsAbovePercent

# 3.
# Standardization and Misspelling - Affect → Affect**s**
# Keep consistent with Ares num flags - AffectsHouses → AffectsHouse
- DetonateOnAllMapObjects.AffectHouses
+ DetonateOnAllMapObjects.AffectsHouse

# 4. Keep consistent with Ares num flags - AffectTargets → AffectsTarget
- DetonateOnAllMapObjects.AffectTargets
+ DetonateOnAllMapObjects.AffectsTarget

# 5. Keep consistent with Ares num flags - AffectTargets → AffectsTarget
- AffectTargets
+ AffectsTarget

# 6. AutoFire's misleading issue
- AutoFire
- AutoFire.TargetSelf
+ AutoTargetOwnPosition
+ AutoTargetOwnPosition.Self

# 7.
# Standardization and Misspelling - Affected → Affect**s**
# Keep consistent with Ares num flags - AffectsHouses → AffectsHouse
- Convert(N).AffectedHouses
+ Convert(N).AffectsHouse

# 8. Standardization and Misspelling - Affected → AffectsHouse
- LimboKill.Affected
+ LimboKill.AffectsHouse

# 9. Crit.Affects misleading issue
- Crit.Affects
+ Crit.AffectsTarget

# 10. Keep consistent with Ares num flags - AffectsHouses → AffectsHouse
- Crit.AffectsHouses
+ Crit.AffectsHouse

# 11. Keep consistent with Ares num flags - AffectsHouses → AffectsHouse
- RevengeWeapon.AffectsHouses
+ RevengeWeapon.AffectsHouse

# 12. Keep consistent with Ares num flags - AffectsHouses → AffectsHouse
- ReflectDamage.AffectsHouses
+ ReflectDamage.AffectsHouse

# 13. Keep consistent with Ares num flags - AffectsHouses → AffectsHouse
- KillWeapon.AffectsHouses
+ KillWeapon.AffectsHouse

# 13. Keep consistent with Ares num flags - AffectsHouses → AffectsHouse
- KillWeapon.OnFirer.AffectsHouses
+ KillWeapon.OnFirer.AffectsHouse

[Starkku]

This warrants further discussion for under which conditions it is acceptable to change INI syntax when no functionality change is present in this project. There are other INI key names that even more inconsistent or misleading than this one that have not been touched upon f.ex AutoFire.

[Coronia]

there's also a similar case elsewhere: while other tags are all AffectsHouses, it's AffectHouses for the case of DetonateOnAllMapObjects

[NetsuNegi]

I think Affects is better than Affect.
An Affect in a series of Affects can be abrupt and easily mistaken for Affects.

[Metadorius]

My opinion is that the typos in docs should always be corrected ASAP without any preceding discussion because that's a doc mistake, and we also can then discuss unification of how the tags are written (with a mandatory migration script entry).

[DeathFishAtEase]

This warrants further discussion for under which conditions it is acceptable to change INI syntax when no functionality change is present in this project.

Yeah, the standard needs to include the conditions for making this kind of processing.

---

There are other INI key names that even more inconsistent or misleading than this one that have not been touched upon f.ex AutoFire.

A very useful suggestion, misleading is indeed very important. AutoFire is a good example, especially because it is written in [TechnoType] itself, so I have encountered some users who think this can make units more "intelligently" choose weapons or decide when to fire. The former misunderstanding was further deepened after DevBuild#48 added the Multi Weapon feature. And this name does not mention at all that the unit's behavior changes from selecting targets based on ThreatPosed to simply firing at the cell under its feet (or at itself when AutoFire.TargetSelf=yes). So I think it should be named similarly to AlwaysFireAtOwnPosition(.TargetSelf=false) or ForceAttackSelf(.PositionOnly=true). At the very least, it should convey the original design intent: no longer needing to equip a unit with a weapon with a huge Range to maintain fire.

[DeathFishAtEase]

there's also a similar case elsewhere: while other tags are all AffectsHouses, it's AffectHouses for the case of DetonateOnAllMapObjects

Nice, I have already listed it in the plan table at the end of the PR description. Can you list more such issues?

[DeathFishAtEase]

My opinion is that the typos in docs should always be corrected ASAP without any preceding discussion because that's a doc mistake, and we also can then discuss unification of how the tags are written (with a mandatory migration script entry).

The content in the develop branch has been corrected according to the current source code execution method, and Phobos-developers/PhobosSupplementaries#13 has been created for adding associated migration script changes.

[github-actions]

Nightly build for this pull request:

• compiled-dll-7a58d7aa4047f1c1c90c199edba1e9654e0c9edc.zip
  These artifacts will expire in 90 days and will not be available for download after that time.

This comment is automatic and is meant to allow guests to get latest nightly builds for this pull request without registering. It is updated on every successful build.

[mevitar]

Since you're changing it, AffectsHouses= would still be inconsistent with Ares, as the tags there are singular, not plural (like SW.AffectsHouse= or SW.RequiresHouse=, or even SW.AffectsTarget= as opposed to DetonateOnAllMapObjects.AffectTargets= ).
Out of all Ares tags i think only AffectsEnemies= is plural, but that is also a boolean, not an enum (and it stays consistend with YR's AffectsAllies= ).

And don't know if anyone noticed it, but Phobos' AttachEffects also has AffectTargets= (not even AffectsTargets= ), so that too needs to be changed.

[DeathFishAtEase]

Since you're changing it, AffectsHouses= would still be inconsistent with Ares, as the tags there are singular, not plural (like SW.AffectsHouse= or SW.RequiresHouse=, or even SW.AffectsTarget= as opposed to DetonateOnAllMapObjects.AffectTargets= ). Out of all Ares tags i think only AffectsEnemies= is plural, but that is also a boolean, not an enum (and it stays consistend with YR's AffectsAllies= ).

And don't know if anyone noticed it, but Phobos' AttachEffects also has AffectTargets= (not even AffectsTargets= ), so that too needs to be changed.

I checked the issue you mentioned. Apart from AffectsEnemies and AffectsOwner, which are consistent with AffectsAllies, Ares only has three flags containing Affect: SW.AffectsHouse, SW.AffectsTarget, and CellSpread.MaxAffect.

Currently, among Phobos features, only Detonate Warhead on all objects on map uses AffectHouses, while all other Phobos features use AffectsHouses. Similarly, only this feature and Attached Effects use AffectTargets. Therefore, changing Affect to Affects should be reasonable.

Additionally, according to Ares-docs/Availability, it uses SW.RequiredHouses and SW.ForbiddenHouses, not House( s ).
I believe they likely considered maintaining consistency with vanilla INI flag names, as the original game already includes RequiredHouses and ForbiddenHouses, which use House s.

Tip

Among these, SW.AffectsHouse was introduced in Ares 0.2, while SW.RequiredHouses and SW.ForbiddenHouses were introduced in Ares 0.9.

As for Targets, we don’t seem to have much more reference material? However, I think perhaps the more important question is: which is more standard? After all, whether it’s Targets or Houses, they are both followed by a list.

[mevitar]

SW.RequiredHouses= is not an enum, it lists actual house names, just like RequiredHouses=.
My point is that tags like AffectsHouses= use an enumeration which was first introduced by Ares with tags like SW.AffectsHouse=, so it might be best for all Phobos tags that also use enumeration to be consistent with it.

Where actual entries are listed, even YR is inconsistent with itself as it uses plural for one thing, then singular for another (plural for RequiredHouses=, DebrisTypes= or DebrisAnims=, but then singular for Anim= despite it allowing multiple entries).

Still, i think that for listing specific types (animations, units, warheads, specific houses, etc), tags should be using plural, and it's only enumerations that should be using singular.

[DeathFishAtEase]

SW.RequiredHouses= is not an enum, it lists actual house names, just like RequiredHouses=.
My point is that tags like AffectsHouses= use an enumeration which was first introduced by Ares with tags like SW.AffectsHouse=, so it might be best for all Phobos tags that also use enumeration to be consistent with it.

I understand that, from the perspective of where the enum values are introduced, it is indeed appropriate to use House instead of Houses, so it is necessary to change Affect→Affects & Houses→House.

Where actual entries are listed, even YR is inconsistent with itself as it uses plural for one thing, then singular for another (plural for RequiredHouses=, DebrisTypes= or DebrisAnims=, but then singular for Anim= despite it allowing multiple entries).

[Weapon] -> Anim= is an INI flag name whose functionality was expanded in later games but was originally inherited from RA1. Its situation is similar to that of [General] -> BaseUnit=, which did not consider a list before RA2.

[Starkku]

AutoFire should realistically be something like AutoTargetSelf and AutoFire.TargetSelf could be AutoTargetSelf.Snap or whatever. What it does functionality-wise is skip passive target selection checks altogether and always pick its own cell as target, or if the latter key is set to true, the firer itself. It does not strictly speaking concern firing weapons itself at all.

[DeathFishAtEase]

AutoFire should realistically be something like AutoTargetSelf and AutoFire.TargetSelf could be AutoTargetSelf.Snap or whatever.

Uncertain whether AutoTargetSelf.Snap will be misunderstood as immediately detonating on oneself while ignoring the ballistic flight process. How about the flag name example in issuecomment-3617388253?

What it does functionality-wise is skip passive target selection checks altogether and always pick its own cell as target

You mentioned here "skip passive target selection checks altogether", but currently AutoFire is still blocked by CanPassiveAquire=false. Is this a design intention or an oversight?

[Starkku]

You mentioned here "skip passive target selection checks altogether", but currently AutoFire is still blocked by CanPassiveAquire=false. Is this a design intention or an oversight?

Right, I was not very clear. It skips all target-related checks but not whether or not the firer can passively acquire targets. That is still technically intentional per the way it currently works altho potentially misleading.