Oh yeah this is almost a duplicate of that, in fact. I've posted some of my comments there, I'll change this bit here to be about upgrading wall turrets into proper towers then after that diff goes in.

I failed to realised that this existed, but I fully agree with this and duplicate it somewhat in D3466.

This looks OK to me, but haven't tested, make sure to check it still works after the changes (and forgot about const, sorry)

Hm, indeed, I was wrong in that we need the object anyways for melee for attack effects.

After discussing with FeldFeld, only do the HP change for A24.

I don't think the added object is a huge problem, but I agree that this currently seems like change for change's sake.

I had a thought on this, basically 'rounding for humans':

• As an upper limit, we never really need more than 2 significant digits for the decimal places: 0.25 is better than 0.246
• However, we probably want 2 significant digits at least: 0.25 is also better than 0.3 or 0.2 (see gather rates).
• But we're not scientists and we don't care about 'insignificant' significant digits: 0.1 is generally better than 0.10, a fortiori 1 is better than 1.00
  • This is debatable to an extent, and we might prefer 1.0 over 1 or 1.00 if all other related numbers have decimal places, just for visual coherence.
• We probably don't want to round integers: 12455 over 12000, even though it's also 2 significant digits.
• But we probably no longer care about decimals at that point: 124 over 124.2
  • I would say above 10, only 1 decimal place at most instead of 2, and above 100, none: 1.23 over 1.2, 12.6 over 12.57.
• Multiples of .25 are probably readable enough compared to rounding to the upper/lower digit, e.g. 3.75 over 3.8 even in a "1 digit" context. Likewise, 24.25 over 24.3
  • Also debatable

In D3464#152961, @Freagarach wrote:

Why 10 instead of 8?

I think we should probably change that as part of a more extensive stance rewrite ;)

This writes 1 as 1.00 but I think it's actually OK for readability because all numbers use the same decimals.

In D3460#152895, @Freagarach wrote:

Why 2 and not 1?

Slight formatting tweaks before commit.

Make the password not print to the logs. The length doesn't matter since it's hashed to a constant length (or 0 if there is no password, but that's also not secret information).

Fix Angen's remarks.

In D3456#152814, @Stan wrote:

Is there a way to test what broke and forces the revert of this?

In D3411#152810, @Stan wrote:

A24 or A25?

In D3446#152808, @Stan wrote:

A24 or A25?

In D3326#152805, @asterix wrote:

Any news here?

Confirmed & complete

I'm not a fan of several of these, to be honest.

I can see a few artefacts with mines with 0 bias, but 0.0005 is probably bigger than it needs to be for medium/high/very high shadow settings.
Perhaps we should just scale the bias with the shadow setting, from like 0.001 (very low) to 0.0001 (very high)

Fix tests.

In D3455#152730, @Angen wrote:

I meant idle timer may not run yet

I'm not entirely sure where units are supposed to check their surroundings in CHEERING, tbh.

See https://trac.wildfiregames.com/ticket/5968

Congrats on your first patch :)
A few general remarks:

• Not everything is specified, but we have some general conventions here: https://trac.wildfiregames.com/wiki/Coding_Conventions
  • We write comments capitalised + dotted
• from reading your forum posts, I believe you're used to 'older' C++, this has no real impact here but we're using C++17
• we don't really do the "author: some comment" thing, though I guess you might intend those for review only. In that case, you can also leave inline comments on Phabricator.
• you appear to have a few whitespace issues with the patch.

You need to rebase this one m8 :p

See https://trac.wildfiregames.com/ticket/5945#comment:10 & corresponding replay (works on rP24757 to rP24767)

Tested to work and this seems reasonable, and fixes Seleucids.

Height difference (the one with the added height is obvious).

I actually renamed the C++ to "GetHeightAtFixed", which makes more sense.

Cleanup StreamItem control flow per Stan

logs/ folder under binaries/

Just FYI, but you might already now: on Mac OS, "compatibility" is always GL 2.1, and "core" is whatever is supported, generally 4.x something. Might be worth adding that as a comment / ifdef somewhere

Inlines

Really fix svn_revision & do #5955

Thnks for fixing