Does it work smoothly on Elbrus?

In rP24261#45506, @wraitii wrote:

Would you maybe be OK with something like this (not necessarily that name or operator)

[[nodiscard]] int toto()
{
    return 5;
}

struct with_unused_result {};
template <typename T>
void operator>>(with_unused_result, const T& ref) {}

int main()
{
    with_unused_result{} >> toto();
    return 0;
}

I'd rather not have to wrap the whole function, but I do take your point that it could be vaguely ambiguous (with admittedly weird code).

Looks ok but still doesn't work for some cases, like with_unused_result{} >> func1() && func2_if_func1_succeeded.

In rP24261#45501, @wraitii wrote:

I'd disagree. UNUSED2 is necessary for unused local variables and such. In this instance, (void) ought to be enough, and we were already using it, so there was already a duplication of usage.

In the current implementation - yes, they're slightly different. But in fact we use the UNUSED2 macro for 2 reasons: 1) unused function result 2) hiding unused argument. 1) is solved by UNUSED_RESULT 2) is more a design problem - because it's adds implicit dependency with additional code changes, hence should be avoided.

I raise a concern to not forget to fix it before release. It's not a real concern, but pretty important for a code health.

The idea is good but the implementation is controversial. We already have a macro called UNUSED2 for that cases, so the change introduces a code duplication. I think the name DISCARD is too broad, because it's not clear what to discard: the whole call or the only result. In my opinion the good implementation and naming is in chromium. Also the usage might be unsafe because it requires space and hence all related to macros issues.

In D3142#138536, @Stan wrote:

punjab 2p
bdeska bog
barcania
corsica
median oasis
nearestearn badlands
nile river
The list above are the maps where I think unit color is better. For the rest terrain ambient is IMHO.

Agree, + caspian_sea_2pv2p.

Seems incorrect, you add svn:global-ignore for vs2017, but vs2015 has just svn:ignore. They should have similar types, and I suppose it's svn:ignore, because of immediate children.

That breaks/not fixes MSAA.

That commit breaks crash/exception handling on Windows. So any error happened in the std::thread aren't handled and reported. That must be fixed.

The function doesn't work for compatibility mode, it hides the real system. It's ok for the features detecting, but bad for statistics.

In D3075#137876, @wraitii wrote:

This is essentially what this diff does though. Except there's no "passing" since the server can 'peek' via being in the same process. Doesn't work for dedicated servers but I don't think it's a huge concern.

That sounds a bit unsafe. Since two parallel object creations depend on the global state. When you create NetClient and NetServer you know that they're on the same machine/in the same process. Why do you need to hide it and delay it?

So it does auth correctly on if ps_generate_guid() will be the same for client and server? So it might mean that someone can relatively easily generate it? Why to not generate a GUID with some randomness before and pass it to both?

Further, SM52 removes the ErrorReporter (it comes back in future versions, I believe).

Have you checked it?

Does it add a compilation error on dynamic_cast usage?

In D14#136177, @Kuba386 wrote:

Aren't we already using a thread pool?
We create a few threads during initialization of pathfinder component and they stay running until component is destroyed.
Isn't that what thread pool is?

Kind of, but my point isn't about the local thread pool, but a single thread pool for the whole application.

I think the current design of threading isn't that we should target on. It allocates the amount of path finder threads around to the number of cores. That means that if we want to use threads extensively for other parts (and we already somehow use for some parts) then we might have performance penalties because of context switches and memory consumption.

Fixes transparent objects and macOS.

In D3084#135356, @wraitii wrote:

I think at this point you might consider removing the "real depth" setting entirely, it's pretty much free with refraction and it looks _so_ much better that it's not funny.

I agree, maybe in next patches.

In D2812#135354, @wraitii wrote:

Quick somewhat related question: how difficult would it be to implement rendering on an arbitrary multiple of the game's resolution?

In terms of rendering - not hard.

I think I prefer lighter only for polar sea.

In D3053#134082, @wraitii wrote:

You're right, you actually have to autostart it, too. Which isn't easy to trigger without D11.

How it behaves in atlas?