Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1149/display/redirect

I’ve left a few comments, but I must say I love these changes overall.

As for +3.0 vs +3, I have no strong preference, provided it is done consistently. The gui displays armour with one decimal, though:

Most auras are of the global or range type; formulating those is straightforward. However, there are a few other aura types, which are more problematic:

"type": "formation":
units/heroes/athen_hero_iphicrates_1.json

"type": "garrison":
units/heroes/maur_hero_chanakya_1.json
units/heroes/iber_hero_caros_1.json
units/heroes/athen_hero_themistocles_1.json
units/heroes/hero_garrison.json

"type": "garrisonedUnits":
structures/cart_super_dock_repair.json
structures/wall_garrisoned.json
structures/workshop_repair.json

"type": "player":
units/catafalques/sele_catafalque_2.json
structures/wonder_pop_1.json
structures/wonder_pop_2.json
teambonuses/ptol_player_teambonus.json
teambonuses/mace_player_teambonus.json

So perhaps it would be best to settle upon a generic scheme for those and then apply it?

[EDIT]: Furthermore, this should probably be rebased because of edits in the intervening months.
Also, shouldn't classes be capitalized? (My mistake, I believe.)

So perhaps it would be best to settle upon a generic scheme for those and then apply it?

Any ideas?

Looking at the April 1 version:
formation type: [class] in his formation [change] [attributes]
garrison type: [class] he is garrison in [change] [attributes]
garrisoned type: Garrisoned [class] [change] [attributes]

improvement suggestions are welcome

still applies :)

Things to be decided upon before I'll update this patch:

• +1 or +1.0 armour?
• should classes be capitalized or not?
• formation, garrison, and garrisoned aura description format.

In D1808#87832, @Nescio wrote:

Things to be decided upon before I'll update this patch:

• +1 or +1.0 armour?
• should classes be capitalized or not?

• I think we should avoid trailing zeroes on user-visible text for now. I would not mind adding extra zeroes for values where a maximum number of decimals has been chosen, but even if we decide to go that path for certain values I would do so in a separate patch.
• I would postpone this decision as well. The current style guide says capitalized (full disclosure: I wrote that), but I would not enforce that as part of this patch. I would leave the capitalization as it currently is in the aura descriptions, and have a separate, later patch to enforce either capitalized or non-capitalized style in every non-flavor user-visible string.

In D1808#87832, @Nescio wrote:

• formation, garrison, and garrisoned aura description format.

I think you already nailed garrisoned type: Garrisoned [class] [change] [attributes]

As for the other two, I have 2 suggestions for formation and 1 for garrison:
formation type: Formation [class] [change] [attributes] or Accompanying [class] [change] [attributes]
garrison type: Hosting [class] [change] [attributes]

should classes be capitalized or not?

I remember that this already was practice to capitalize them and remember changing things to account for that - Spearman, Infantry. You may find some evidence for that in the files? Wondering why they're not standardized already.
The reason for that was mostly that one didn't mean the common noun but the proper noun (And I recall that these words were also used in the related discussions) - for example Boudicca is female but not Female.
I find D464 where common nouns were proponed for consistency apparently. But I recall sycthetwirler changing things for proper nouns in one of the Changelogs pages.
Those classes are identity classes, so in the tooltip they also appear as Spearman etc., so it seems like one is the right way and the other is the consistent way?

I think we should avoid trailing zeroes on user-visible text for now.

Agreed. In that case the GUI should probably be edited to not add any trailing zeroes for e.g. armour either (in a different patch, of course).

should classes be capitalized or not?

I would postpone this decision as well.

I don't think postponing that decision is a good idea. Everytime a string is updated, the entire string has to be re-translated again for all languages, which means that changing capitalization later doubles the work-load for our translators. Besides, the purpose of this patch is to improve consistency, and a policy to keep it uncapitalized in some data files while demanding it to be capitalized elsewhere doesn't seem very consistent.

I just had a quick look at some technologies (because those are the data files closest to auras) and it seems there is no class capitalization consistency there either. E.g. rP15713 introduced five years ago
simulation/data/technologies/armor_cav_01.json with:
"tooltip": "Equip your cavalry mounts with armor. All Cavalry +1 Hack and Pierce armor level.",
but also:
speed_cavalry_01.json with:
"tooltip": "+10% cavalry walk speed.",

The aura description formats should be clear and unambiguous.

garrisoned type: Garrisoned [class] [change] [attributes]

Agreed.

garrison type: Hosting [class] [change] [attributes]

E.g. “Hosting warship +10% health.” Would the uninformed player understand that means the hero is to be garrisoned?

formation type: Formation [class] [change] [attributes] or Accompanying [class] [change] [attributes]

E.g. “Formation spearmen +10% health.” Wouldn't that imply all spearmen in any formation, rather than only spearmen in the formation containing the aura-entity (e.g. hero)?
E.g. “Accompanying spearmen +10% health.” Again, how is the player supposed to know that means only spearmen in the hero's formation?

[EDIT]: How about “His formation's [class] [change] [attributes]”?

In D1808#87907, @Nescio wrote:

garrison type: Hosting [class] [change] [attributes]

E.g. “Hosting warship +10% health.” Would the uninformed player understand that means the hero is to be garrisoned?

formation type: Formation [class] [change] [attributes] or Accompanying [class] [change] [attributes]

E.g. “Formation spearmen +10% health.” Wouldn't that imply all spearmen in any formation, rather than only spearmen in the formation containing the aura-entity (e.g. hero)?
E.g. “Accompanying spearmen +10% health.” Again, how is the player supposed to know that means only spearmen in the hero's formation?

I agree with all your points. Those are just the best things I could think so far that are reasonably short.

[EDIT]: How about “His formation's [class] [change] [attributes]”?

In addition to the need to handle His/Her, which I would try to avoid, this option would have the same problem as e.g. “Same formation [class] [change] [attributes]”: it’s possible to think that any units using the same type of formation as the hero get the benefit.

Maybe what we need first is to have two completely different words to refer to (1) formation as in a way a group of units can organize themselves and (2) formation as in a specific group of units. For example, if we used ‘group’ for the latter, “Same group [class] [change] [attributes]” may work.

Before I can update this, we still need to find a proper format for the formation and garrison aura types.

Maybe what we need first is to have two completely different words to refer to (1) formation as in a way a group of units can organize themselves and (2) formation as in a specific group of units. For example, if we used ‘group’ for the latter, “Same group [class] [change] [attributes]” may work.

A group is an arbitrary selection of entities, which can be defined by e.g. Ctrl+1 and selected by pressing 1 or clicking on the group panel at the left of the screen.

In D1808#90733, @Nescio wrote:

Before I can update this, we still need to find a proper format for the formation and garrison aura types.

Maybe what we need first is to have two completely different words to refer to (1) formation as in a way a group of units can organize themselves and (2) formation as in a specific group of units. For example, if we used ‘group’ for the latter, “Same group [class] [change] [attributes]” may work.

A group is an arbitrary selection of entities, which can be defined by e.g. Ctrl+1 and selected by pressing 1 or clicking on the group panel at the left of the screen.

OK, so group is not an option. We could use “formation type” or “type of formation” for 1, and hope that people won’t ocnfuse it with “formation” (2). But I would really prefer to have a different word for either 1 or 2.

What about squad, squadron, battalion, brigade, party, company, or regiment?

What about squad, squadron, battalion, brigade, party, company, or regiment?

Those have the same problem as "formation" and potentially add more confusion.

[Note to myself]: according to simulation/components/Player.js, the affectedPlayers tags have the following meanings:

• Ally: player is our ally
• ExclusiveAlly: player is our ally excluding ourself
• MutualAlly: player is our ally, and we are its ally
• ExclusiveMutualAlly: player is our ally, and we are its ally, excluding ourself
• Enemy: player is our enemy

In D1808#91032, @Nescio wrote:

What about squad, squadron, battalion, brigade, party, company, or regiment?

Those have the same problem as "formation" and potentially add more confusion.

The main problem I see with ‘formation’ is that it can mean both (1) the way units group (e.g. Battle Line, Phalanx, Flanks) and (2) the group they form itself.

I believe these words can only mean 2, so I think keeping ‘formation’ for 1 and choosing one of these words for 2 makes sense.

So, let’s say we choose squad. You could say something like: “Select some units and choose a formation to have those units form a squad using the selected formation. Some heroes provide bonuses for units in their squad.”

How about “When in a formation, Spearmen ...”?

Also, we still need something for the garrison type.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/486/display/redirect

Let’s go with your original proposal, ‘Soldiers in his formation’, and revisit this topic if we find a better alternative in the future.

That's actually from the current string. I didn't change it, because I couldn't think of something better.

Same, I think we can go with your initial proposal for now (garrison type: [class] he is garrison in [change] [attributes]; garrisoned type: Garrisoned [class] [change] [attributes]), and hopefully revisit this in the future if a better proposal comes to light.

I thought we already had agreement on the garrisoned format (see D1806)?
As for the problematic garrison auras, I changed it to “When garrisoned, the Structure has ...”, staying closer to the original hero_garrison.json string. It can (and should) be improved in a later patch, I agree.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/487/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/522/display/redirect

@Freagarach May I merge?

In D1808#93264, @Gallaecio wrote:

@Freagarach May I merge?

Yeah, not happy about the sele_hero_seleucus_victor.json but I trust that to be fixed seperately :D

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/33/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/542/display/redirect

In D1808#93280, @Nescio wrote:

Now includes extra non-changing modification line, as requested by @Freagarach.

I actually assumed that would be fixed in D2000, as you mentioned. But this sounds good to me!

Yes, D2000 would have been the proper place, but I don't know if and when that one will be reviewed and committed.

In D1808#93316, @Nescio wrote:

Yes, D2000 would have been the proper place, but I don't know if and when that one will be reviewed and committed.

Your chances just went up ;) I don't mind where it's done, nor do I have strong objections, I just gave my opinion on the matter ;)
Doesn't probably get said often, but I, for what it is worth, definitely very much like the cleanup/standardisation you do!

I don't mind where it's done, nor do I have strong objections, I just gave my opinion on the matter ;)

That's perfectly fine, and @Gallaecio specifically asked you, so you can request changes or accept, if you like.

Doesn't probably get said often, but I, for what it is worth, definitely very much like the cleanup/standardisation you do!

Thanks. The public repository is full of inconsistencies, cleaning up things makes life easier for future additions and mods in the long run (even if it might break things occassionally).

Reason for my hesitance: the diff title only talks about aura descriptions, while there is actually a modif-change (yes also one class change). This *could* make it hard to track down possibly.

Well, I only included it here because of your earlier remark, but I could exclude it again, if you think that's better. I don't mind either way, it could be done in a minute.

In D1808#93493, @Nescio wrote:

Reason for my hesitance: the diff title only talks about aura descriptions, while there is actually a modif-change (yes also one class change). This *could* make it hard to track down possibly.

Well, I only included it here because of your earlier remark, but I could exclude it again, if you think that's better. I don't mind either way, it could be done in a minute.

For me you don't have to do it :) As said, I agree, but I'm usually trying to play the devil's advocate to create a full picture. I hope you don't mind...

As said, I agree, but I'm usually trying to play the devil's advocate to create a full picture. I hope you don't mind...

No, of course not, please be critical!

In D1808#93495, @Nescio wrote:

No, of course not, please be critical!

Yeah, but I do not know how the message gets conveyed ;)

Thank you, I'm glad this one is finally committed!