Also include techs.

In D2630#111403, @user1 wrote:

In D2630#111281, @Dunedan wrote:

I can't apply the patch to current SVN. Am I doing something wrong or is it incomplete?

Ok. I'm working on this now. I can't apply the patch either.

Meh,,,
Needs to be elaborated.

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

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

• Rebased.
• Some other changes.

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

• Bump CCmpPos.-year.
• Correct comment in MapReader.cpp.
• return false in FOLLOWING.enter.

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

• Rebased.
• Fix the crash on initGarriosn.

In D1957#111616, @Freagarach wrote:

Sorry for removing them then.
IMHO An entry on a review list is like a question: "Would you like to review this, please?" But since that is not the case here I reckoned it is not fair to add them. My mistake then.

Sorry for removing them then.
IMHO An entry on a review list is like a question: "Would you like to review this, please?" But since that is not the case here I reckoned it is not fair to add them. My mistake then.

In D1957#111609, @Freagarach wrote:

Still not high on my priority list. The summary shows what it should be so it is not even close to reviewable.
I think the "properties" was - for now - a big enough leap forward :)

Looking good :)

• Tasking a unit to chase a unit which later dies lets the former keep running to the last known position.
• Otherwise no strange/different behaviour is observed.

Just one notion, perhaps/probably out of scope for this patch. When a unit chases another on its own and the target dies the former will stop in its tracks.

Still not high on my priority list. The summary shows what it should be so it is not even close to reviewable.
I think the "properties" was - for now - a big enough leap forward :)

Yep, concern fixed :)
Thanks for the reminder.

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.

• Rebased.
• Use StatusEffects.

Tests will fail, still some stuff to do.

Needs rework,
caption change event is triggering rebuild, so solution would be possibly override.

In D2596#111589, @Nescio wrote:

Does it make sense for heavy cavalry to move as fast as light cavalry?

A horse is a horse. :)

Does it make sense for heavy cavalry to move as fast as light cavalry?

A horse is a horse. :)

Does it make sense for heavy cavalry to move as fast as light cavalry?

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

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

rebased

This should be updated.

Warning was fixed in rP23531

Thanks for the review and commit @Angen :)

Comparison to number is done correctly.

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.

cancel CHASING timer

warnings

What could be done is to have %(statusName)s: %(tooltip)s, %(effects)s, %(interval)s, %(duration)s %(stackability)s defined in template.
But duration and interval can be changed through aura or technology, what makes 4 options defined in template and 4 ifs in code what will make it easier to read.
Downside is 4 strings for every template.

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

better approach

Requested replay.

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

Some more work, it properly snaps and is buildable now.

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

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

Fix inlines.

I suppose the value is cumulative

So far the value of my acceptance ^^

(I didn't check whether the message moving makes sense, or test it in any way, or anything else)
IsVisiblyGarrisoned does loop for each entity checked, perhaps that can be optimized, I didn't check (code only handled for garrisoned entities, so its not as grave as it could be, but there also was some 10s freeze when a ship was destroyed some releases ago, and there could be cases where a player deletes/renames/ownershipchanges 100+ entities with 10+ entities garrisoned each). (Also didn't check whether sending visible entities in the messages instead of having the message handlers call into the function is more performant.)

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

Simplify code by returning true when there are no allowed classes specified for a VGP. The check is handled by perfom garrison.

One cannot visibly garrison a unit that cannot be garrisoned non visibly
The fallback is needed for things like walls

Why is the check needed for walls if thats the case?

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

One cannot visibly garrison a unit that cannot be garrisoned non visibly. (Added a test for that)

Is the fallback redundant and thus useless (same as always allowed if not specified)?

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

Fix whitespace noticed by linter.

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

Else the classes specified in the List tag will be used.

That works too

Don't hesitate to ask questions on IRC :)

Fallback to what is allowed to garrison, fix comment, and tests.

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

In D2650#111452, @norsnor wrote:

Cool!

I found some other minors I would submit after commit (’m still confused by svn&git)