I agree with the intention, I don't care very much about periods, I like @bb's suggestion, I'd suggest you commit this whenever you feel like it :)

Tested and functional, I would agree with Freagarach that perhaps only showing 5 and having bigger arrows is easier to click though. Keep showing 6 in the regular case.

The interpolation of the rotation looks fairly bad (it's kind of jerk-y).

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

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

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

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

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

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

@wraitii, any opinions on this?

I would agree with this, IIRC the long repeat-time was to do some "pretend charging" but it's never been a great look.

A simple "worst-case" perf test would be running Combat Demo Huge and moving some units to attack.

I have personally always disliked our promotion system. It gives no real choice to the player, and it stems from a weird vision of historical perspective where people would be more or less specialised towards combat, but this is completely inaccurate anyways -> the warfare division was generally based on social class, not on 'experience'.

I disagree with the consistency argument -> this is a civilisation bonus, not a technology. It effectively is a way to modify all templates for hellenistic civilisations without having to actually modify the templates (since that's annoying to maintain).

Ah, thanks, I had missed that bevhaiour as I wasn't testing large enough formations.
It's fixed, and the units in formations no longer move. If you remove the formation, they will go to the held position (and clump) though - but I don't think we should really fix that.

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

• Wonders are excluded from build time and health technologies and civilization bonuses.
• Small Pyramid cost to 300 stone + 100 metal.
• Large Pyramid cost to 450 stone + 150 metal.

Rebase

Are there other structures (e.g. corral, pyramids, Stonehenge) that should get this same vision change?

Currently build time and health are correlated; if a technology increases a structure's health, its build time goes up, and if it decreases its build time, so does its health. I like consistency, and don't see why an exception should be made here.
Besides, it's not exceptional for a “civ bonus” to be actually a malus; cf. spart_population.json.
Other things that could be done is postpone this civ bonus (e.g. insert a phase_town requirement) or replace it entirely and create new civbonus(es).

The confusing thing is that <Heal/Rate> is actually a time. It ought to be renamed to <RepeatTime> (cf. <Attack>) or <Interval> (cf. <ResourceTrickle>).
Regardless, as @Feldfeld already pointed out, these technologies are currently correct, and this patch messes it up. If you want to remove this or any other patch from the review queue, please click “Add Action...” → “Abandon Revision”.

Whatever you think is fair.

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

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

Minor style fixes

Translate and use setStringTags

In D2840#121790, @Feldfeld wrote:

The values are correct in fact. Multiplying the reload time by 0.8 corresponds to a 25% heal rate increase ( 1 / 0.8 = 1.25) Just like if you multiply the reload time by 0 you won't have a 100% increase but an infinite one instead

Sure, it really affects the early game.
I agree with the patch. it helps in balancing and it also seems to make more sense as a "bonus".
@Nescio has a good point about repair rate.

The values are correct in fact. Multiplying the reload time by 0.8 corresponds to a 25% heal rate increase ( 1 / 0.8 = 1.25) Just like if you multiply the reload time by 0 you won't have a 100% increase but an infinite one instead

In D2835#121773, @Nescio wrote:

Or remove the food and make it metal only?

In D2835#121773, @Nescio wrote:

Or remove the food and make it metal only?

In D2839#121775, @Nescio wrote:

The reason the build time increases is to keep the health/second repair speed unchanged. I disagree with removing it. Other technologies and civ bonuses do this as well. See e.g. cart_walls.json, celt_structures.json, maur_walls.json and buildtime_walls_rubble.json, health_walls_geometric_masonry.json, persian_architecture.json.

All the other technologies/bonuses have a justification gameplay (balance) wise for their changes. As an example, celt_structures.json is a strong economic bonus and sacrificing building health is still quite good for it. For the technologies, since the build time change isn't applied before the technology is researched that makes it a good bonus. (And even for buildings that are built after the technologies are researched, there is no economic snowball so this remain quite good).
I would like to say that keeping the build time / building health ratio is not important here but that's just me I guess

If the economy is the problem, how about replacing Structure with Civic?

The reason the build time increases is to keep the health/second repair speed unchanged. I disagree with removing it. Other technologies and civ bonuses do this as well. See e.g. cart_walls.json, celt_structures.json, maur_walls.json and buildtime_walls_rubble.json, health_walls_geometric_masonry.json, persian_architecture.json.

Or remove the food and make it metal only?

This could be okay values, but what about 500 food and 500 metal ? For me it would make sense that this tech cost more metal. If that's too much maybe 400 food 400 metal ?

@Feldfeld you opnion?

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

• changed territory decay to 1, per @Angen

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

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

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

GetTemplates is now const and removed unused include

I think 1 or 2 can be enough, just to not have it for free.

create square formation of 35 units. Set them to defensive stance. After arriving to position some will move to the Centre of formation.

This looks good, I will test it later! ??

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

Fix roundtrip issue.

I think this feature can in some cases mess up ai planning but it is similar if entity would upgrade or get deleted. as far as it will not be used to remove crutial things it ought to be ok (like removing or adding Siege class to units in the battle not sure how that would play out)

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

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

Switch to a filter instead of tabs because that's likely a better interface. I'd like to add a text field to filter hotkeys further.

In D2838#121714, @Itms wrote:

Additionally I am fine with believing that SM uses UTF16 internally but what is the source of this affirmation? Is it still true with recent versions of SM?

I've found no documentation beyond code comments.
See JSAPI.h:4294

* Flat strings and interned strings are always null-terminated, so
* JS_FlattenString can be used to get a null-terminated string.
*
* Additionally, string characters are stored as either Latin1Char (8-bit)
* or char16_t (16-bit). Clients can use JS_StringHasLatin1Chars and can then
* call either the Latin1* or TwoByte* functions. Some functions like
* JS_CopyStringChars and JS_GetStringCharAt accept both Latin1 and TwoByte
* strings.

and CharacterEncoding.h

/*
 * By default, all C/C++ 1-byte-per-character strings passed into the JSAPI
 * are treated as ISO/IEC 8859-1, also known as Latin-1. That is, each
 * byte is treated as a 2-byte character, and there is no way to pass in a
 * string containing characters beyond U+00FF.
 */

Hence my understanding above that text is always stored "wide" in Spidermonkey, even in the "latin1" encoding.

This looks good to me apart from the roundtrip change. Additionally I am fine with believing that SM uses UTF16 internally but what is the source of this affirmation? Is it still true with recent versions of SM?

Since there seems to be some confusion.

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

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

In D2764#121465, @Freagarach wrote:

Formation behaviour ought to be in UnitAI, one can disband formation temporarily when close to gathering point, perhaps?

@Angen Any chance you could review this again? I think we're good now.

I'm leaving this one up for further comments for now.

The current values for this patch seem good.

Yea, there is an autobuild in the repository, so everything that gets committed will be build. Not every patch will get a build. Though I will try to get a build for this revision from somewhere.

I use svn on windows, did "apply patch". I think build is usually done automatically when updating so I don't know how to do it.

Did you rebuild the game?

I have errors trying this patch