Vega-strike-engine-source: Drop server support

[royfalk]

A large portion of the code is server code. It complicates things and AFAIK, it doesn't work.
Dropping it (carefully!) would greatly simplify the code base, especially once we remove the resulting unused code.
The world changed a lot since this code was written. We can probably find server infrastructure designed for RT gaming. If we get to the point this is relevant, starting from scratch using such a server would be far easier.
I'm assigning this to Loki, as he's been around the longest and can weigh in, but I'd like this to be a unanimous decision.

[BenjamenMeyer]

I'd suggest we create a branch first (feature_server-support or something) before dropping it from master.

I do agree that removing it would likely simplify stuff so we can get our head around things first, and then come back and tackle it later one.

[Loki1950]

Might be a good idea but my option of the technical details is meaningless due to my lack of C++ experience

[royfalk]

From the forum:
https://forums.vega-strike.org/viewtopic.php?f=1&t=27008&p=145040&hilit=server&sid=1634e43c62b38c9cb466cbf0fd7c1105#p145040
https://forums.vega-strike.org/viewtopic.php?f=1&t=27008&p=145040&hilit=server&sid=1634e43c62b38c9cb466cbf0fd7c1105#p145040

I did a serious review of the network code as part of a general refactoring effort. It (like a lot of stuff) touches everything and everything touches it. Removing it and starting from scratch is probably an order of magnitude easier.
I'm fine with branching it - good idea.
@Loki1950 I asked your opinion as the person active the longest. It's less a programming question and more about impact to the project. I'd hate to discover there are people using this (though I doubt it).
@LifWirser , @pyramid3d , @stephengtuggy , @nabaco - I'd appreciate your input given the major impact this will have on the codebase. I will not move forward with this without consensus.

[Loki1950]

There was a multi-player server setup for testing and it exposed several major bugs IIRC that where not resolved was more 1 on 1 than anything else though that was largely due to the low participation

[nabaco]

@royfalk as discussed in the last meeting, we agreed to remove the server/multiplayer code. So I'm assigning the issue to you for now and you can go on and do it.
Feel free to unassign yourself if you're busy with other things right now.

[stephengtuggy]

I am fully on board with the decision to drop server support. Please, by all means, feel free to do so! :smile:

[royfalk]

I just pushed the code. I managed to compile it but unfortunately I was not able to find the root cause to a segfault in texture creation. @nabaco your assistance is requested and more than welcome. I've reviewed the code and debugged quite extensively and a fresh set of more experienced eyes is required.

[nabaco]

@royfalk I'm compiling your code right now and looking through the changes. There some minor things that might need to be fixed/changed - though I don't think they're the reason for the segfault.

One thing that I have noticed now: star_system_generic.cpp#L910 you need to remove the else, otherwise you get incorrect logic.

[nabaco]

I have caught an error in engine/src/gfx/technique.cpp:622, placed try..catch there to continue, and received stack smashing in engine/src/gfx/vsimage.cpp:138

Just for comparison of result, @royfalk can you run the following command, and post the resultant logs here?

vegastrike > output.txt 2 > error.txt

I hope to continue to investigate this sometime later this week.

[royfalk]

You are absolutely right, though this compiles on my machine. I can't understand how the compiler doesn't scream at it. I assume yours did or did you find it yourself?

[nabaco]

No, it didn't. Maybe there is some warning that might hint about this but I didn't see it. Stack smashing is memory corruption in runtime, due to writing something that is bigger then the allocated memory on the stack. The compiler puts in the code guards (canaries) to catch such things, and tells the program to exit when it finds such a situation

This usually happens when writing into arrays that were allocated on the stack.

[nabaco]

Here is the full stack trace at the moment of the crash.

*** stack smashing detected ***: terminated ==2622== ==2622== Process terminating with default action of signal 6 (SIGABRT): dumping core ==2622== at 0x5908CE5: raise (in /usr/lib/libc-2.31.so) ==2622== by 0x58F2856: abort (in /usr/lib/libc-2.31.so) ==2622== by 0x594C2AF: __libc_message (in /usr/lib/libc-2.31.so) ==2622== by 0x59DC069: __fortify_fail (in /usr/lib/libc-2.31.so) ==2622== by 0x59DC033: __stack_chk_fail (in /usr/lib/libc-2.31.so) ==2622== by 0x30A6D1: VSImage::ReadBMP() (vsimage.cpp:660) ==2622== by 0x30B478: VSImage::ReadImage(VSFileSystem::VSFile*, unsigned char* (*)(int&, int&, unsigned long&, unsigned long&, unsigned char**), bool, VSFileSystem::VSFile*) (vsimage.cpp:138) ==2622== by 0x237A39: Texture::Load(char const*, int, FILTER, TEXTURE_TARGET, TEXTURE_IMAGE_TARGET, unsigned char, int, unsigned char, unsigned char, ADDRESSMODE, Texture*) (aux_texture.cpp:363) ==2622== by 0x23856B: Texture::Texture(char const*, int, FILTER, TEXTURE_TARGET, TEXTURE_IMAGE_TARGET, unsigned char, int, unsigned char, unsigned char, ADDRESSMODE, Texture*) (aux_texture.cpp:276) ==2622== by 0x27CBE6: Mesh::TempGetTexture(MeshXML*, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) const (mesh_gfx.cpp:304) ==2622== by 0x623602: Mesh::PostProcessLoading(MeshXML*, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) (mesh_xml.cpp:1842) ==2622== by 0x65D6EB: Mesh::LoadMeshes(VSFileSystem::VSFile&, TVector<float, double> const&, int, Flightgroup*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) (mesh_bxm.cpp:941)

[nabaco]

It seems that I have found the issue, using Address Sanitizer, though I'm still not sure how to solve it correctly.
See the attached log with the details: error.txt

[nabaco]

@royfalk this is the workaround that I have found so far:
I don't really understand the true root cause, but I managed at least to start the game with this.
Add this before engine/src/gfx/vsimage.cpp#L549

        if(SIZEOF_BITMAPINFOHEADER > sizeof (info)) {
            throw(1);
        }

It seems to me that it happens due to loading a faulty BMP file.

[royfalk]

Unfortunately, neither of these work for me. I'm still getting a seg fault. I've also reverted some modifications I've made to vsimage files and just copied the macros from /networking/const.h and it still crashes.
Since the previous version did not crash even though it was fed invalid/headerless BMP files (white.bmp), I think we need to figure out why it's crashing now. I suspect it will come down to one of two things: 1. something I've changed that looks fine but really isn't. 2. Something removed with the networking code (e.g. a macro) but that has a substitute somewhere else in the code. It still compiles but now the code is different.
I'll take another stab at figuring this out. @nabaco, maybe we should look at the code together using zoom?

[nabaco]

@royfalk Sounds good to me. Either way, push your fresh code to your Github, and I'll have a look at it locally.

[royfalk]

Lovely. Turns out I was running the wrong/stale executable all this time. Game now compiles and runs but ship is unresponsive - won't accelerate or turn.

[stephengtuggy]

@royfalk How frustrating.

[royfalk]

I found it. The game works. I hope to commit tomorrow.