In D1698#72244, @Stan wrote:

It's still using the 2017 toolset so yes.

In D1745#72226, @elexis wrote:

So you're asking for AddIcons instead of AddIcon.

Actually no, though AddIcons is better than AddIcon. I think these functions are dangerous currently.

In D1698#72240, @Stan wrote:

the goal is to drop support eventually and move on to 2015 2017

It's true, but we didn't drop it yet.

In D1698#72235, @Stan wrote:

Errors for debug build are here https://trac.wildfiregames.com/ticket/4460

Ok, it should be linked then (I added).

In D1698#72233, @Stan wrote:

Updating Fcollada is not that important. Nevertheless though our debug build is broken on windows because of it and the game doesn't work with Fcollada compiled with vs2015 instant default.

So I thought we might as well take all the fixes from the other forks over the internet.

Do we have to update our fcollada? Especially if it has useful changes.

In D1745#72176, @elexis wrote:

So the numbers show us that displaying 250 new textures means that 250 more textures are processed, but that's not surprising, is it?

Of course.

Currently few of my comments should be fixed anyway, so I add a "Request changes". But also I want to discuss about a method to pass the ping event to the C++.

What's about wraitii suggestion:

If not, I would recommend passing an enum VFS::SORTED or something instead of true or false since C++ doesn't have named arguments

It makes sense to use a name. It'd be easier to read such code.

As we discussed XML is a pretty specific case and it may have some problems. But I can suggest to add a more common parameter, like an ignore mask. Example:

... --ignore-mask="*test.js|*.xmb"

But it also requires a motivation.

I would ask you add a context to your patch.

The patch looks good for me. It only needs to fix some details.

In D1745#70233, @elexis wrote:

I get the impression that you two didn't read the patch and spent maybe 2 minutes on your reply.

I disagree with that.

In D1763#71069, @smiley wrote:

(Re: ssize_t to size_t
I thought that this was discouraged since it can introduce subtle bugs.
Google's C++ style guide also mentions something like this)

Fixes elexis and another other another Stan notes.

Fixes other another Stan notes.

Fixes another Stan notes.

Fixes Stan notes.

In D1763#70987, @Stan wrote:

Which of the two implementation was the fastest ? :)

@gentz thank you for the patch!

I tested the patch, it works and looks good for me. In any case it's the improvement. If someone wants a more flexible header, the one can create a ticket or ask for a patch.

In D1746#70770, @elexis wrote:

ctrl+f currentness.

Which word or sentence?

In D1746#70762, @elexis wrote:

If 1/5 of the things I post in a revision proposal are repetitions of something that I already repeated about 3 or 4 times and still not being understood, that's pretty strange for me. Perhaps I misphrased something?

1/5 of RAM.

In D1746#70734, @Stan wrote:

If you significantly increase the size of the bundle like by 500MB to add that file, we will have to find workarounds for the windows installer as it has a size limit (IIRC 2GB) bigger than that it cannot handle. This is I believe one of the reason we don't have the Asian Fonts as part of the main bundle.

If 1/5 of the game is one file that will be outdated each next month and that used only for a small feature, it's pretty strange for me.

In D1676#70736, @Stan wrote:

Could you guys have a look at this. It's starting to be a really wanted feature.

How many elements are sorted for a worst case?

In D1573#66460, @elexis wrote:

And we don't know, was it really initialised.

Shouldn't there also be a callback function on failure then, used when globals are initialized by a config value?

Yes, it could be. If it's needed for someone.

Fix notes.

From https://dev.maxmind.com/geoip/geoip2/geolite2/:

The GeoLite2 Country and City databases are updated on the first Tuesday of each month. The GeoLite2 ASN database is updated every Tuesday.

It means that our packed IP list will be outdated for the next month after each release.

I made a performance test for comparison: P149.

In D1576#70586, @wraitii wrote:

No I just meant an infinite ray can't be contained inside a sphere, so rather its origin is inside the sphere. Otherwise I'm OK with this patch.

Ah, I got it. Yes, you're absolutely right. The origin is inside the sphere, not the ray.

How many space the list of only countries costs?

In D1576#69189, @wraitii wrote:

Does it really make sense to talk about 'inside' for infinite rays?

I guess the bug you are fixing is when the ray's origin is inside the sphere.

Yes. Because it won't be handled as intersection currently.

• s/header/year/
• add basic C++ headers
• a little bit of polishing

In D1746#70505, @elexis wrote:

In D1746#70504, @vladislavbelov wrote:

In D1746#70502, @elexis wrote:

Where is the problem? It just displays nothing or "?" then. It can be the case for any column and any row. It displays something if there is data, otherwise it doesn't.

So the status of proxy is useless, because it does guarantee anything.

I don't understand why you say that. If there is a 1 in the database for that field, then it's a VPN. If it's a VPN and people want to rule out anonymous users / smurfs, it's very useful to them.
If there was any entry in the GeoLite2 database that was useless, it wouldn't be in the database.

Because a VPN service can use a random IP, you can't predict which IP is proxy (because of botnet, etc). Some people use custom VPN (dedicated server with a simple tunnel). So I suppose the GeoLite2 can detect only popular VPN for usual users.

In D1746#70502, @elexis wrote:

Where is the problem? It just displays nothing or "?" then. It can be the case for any column and any row. It displays something if there is data, otherwise it doesn't.

So the status of proxy is useless, because it does guarantee anything.

In D1746#70499, @elexis wrote:

Dunno, it's in the database and it can be displayed. The database is incomplete here and there with some entries and that should be caught already. Just a matter of displaying it in JS.

The problem is that the database can not be complete.

In D1717#70498, @elexis wrote:

https://code.wildfiregames.com/D1717#68827 (also that wall of text about committing things)

@elexis Do you have any objection that prevents to commit the patch?

*VPN detection:* Notice the Blocks file contains which clients are using proxies / VPN networks.

It's not possible for all proxies/VPN.

I think we should restrict the number of pings, because I'm sure there will be many spammer on servers.

I didn't take a look at the JS code.

In D1745#70225, @wraitii wrote:

While that is correct I think it would be reasonably easy to adapt and thus can probably be merged regardless - I'll defer to your opinion but it doesn't seem like we'll use this feature _intensively_.

We shouldn't use such way of dynamic loading until we have atlases for textures. Because currently the AddIcon calling for each sprite means the slow cycle: (init) read data from drive/memory > create a small texture on GPU > fill the texture, (draw) bind the texture > draw the texture. All this makes the RAM and VRAM pretty fragmented.

In rP22010#32170, @elexis wrote:

Is it intended that the Vulcan marks diffs with a wrong year in the header as "build failed"? It seems more like a lint warning.

Edit: Seems to be the case that it warns only, I thought I had seen it in a revision proposal.

Could you attach a link to the build?

Also check that:

• A comment starts with a capital letter.
• No space before brace in static_cast<...>(.