Connection manager abstraction layer

[erebuswolf]

This pull request is to propose an abstraction layer for the network connection to sit between the upd class and the actual socket calls. The idea is to add dependency injection to the library to both enable easy mocking of the socket calls and to allow easier swapping of other network apis (like steam or console api calls) into GGPO. The change as is doesn't work because it assumes the user will pass a ConnectionManager derived object into the ggposession. I had a change that basically broke the C style bindings for the library and was able to build run and test the program, and everything worked fine. I reverted those changes and this version shows how I thought it could be used. But, I don't have a clear vision for how to provide this functionality to the user while maintaining the C support and current DLL style wrapping for the lib. I'm looking for input on what the best way to provide this as an external interface would be.

It may be that we add explicit struct typing or c style function to the supported networking apis and anyone who wants to fork and add their own support will need to create their own functions that know what parameters the apis require for creating a ConnectionManager.

Hoping for feedback. I tried to match style as much as possible. VS defaulted to tabs and I forgot to change that so I know in the new files I need to swap to spaces. But if I missed some other style or formatting stuff I'll make sure to fix that before submitting a CL that I'm hoping to get in. I'm looking to start a design conversation with this.

[pond3r]

Hey erebuswolf. Thanks for the pull! I think this is great direction to move in (we did something similar at Riot). I have some tactical feedback about file structure I can give later on today, but won't get to take an in-depth look until sometime next week. Thanks!

[erebuswolf]

Hey, just a reminder I'm looking for feedback this week on this cl. I am sure you are busy with the day job, so I didn't want this to fall off the radar.

[pond3r]

Yup, I still want to get to it. I'm still slammed. Soon (tm). Thanks for your patience.

[pond3r]

Hi. Just got a chance to take a look. I think the idea is sound, but you've added a new requirement to use the ggpo in the developer's application. Previously the interface to GGPO was purely via the C api, even though the implementation details were in C++. This made it possible to use GGPO in applications which had C compatibility layer, but no good support for C++ (e.g. Lua).

There are two ways we can go from here: one way implement the connection manager interface as a C dispatch table (kind of like how the GGPO callbacks mechanism works) so that we maintain a pure C API. Another is to just bite the bullet and say "GGPO is a C++11 (or 14, or whatever) API" and drop support for pure C embedding.

That's kind of a big choice. Maybe we can ask in the Discord which direction people would prefer. Let me noodle on it some more.

[erebuswolf]

Sounds good. I'm not in that discord, if you can dm me on twitter I'd love to join it (erebuswolf there as well). Another option is to have a C bindings branch that maintains the ip/port passing interface and hides the connection manager code underneath that. Most of the differences for that branch moving forward would be in the example and main interface/dll definition files rather than the library itself. I don't love that idea tbh, but yeah, I don't see a good way of making it work both ways.

[erebuswolf]

Hey, checking in. Any update on your thoughts on this design change?

[dantarion]

I'm interested in this. I started going down this route in my fork. It would be nice to be able to keep the same C-only API, although it would be nice if there was a way to provide both C++ and C bindings somehow

[erebuswolf]

@dantarion I'm open to suggestions. I'm not super familiar with building DLLs or what designs are possible between the C/C++ bindings. I imagine you could do a massive ifdef and use flags to toggle a C binding vs a C++ binding and just not offer this support in C.

[pond3r]

Hey y'all. Sorry for not chiming in sooner. I am super super super swamped with work and probably won't have time to work on GGPO for a while. When I come back to it doing this sort of thing will be a priority for me, but I just don't have time to devote to it now.

[erebuswolf]

@pond3r Hey thanks for the update. I will leave this request open and failing so anyone who wants to do their own version of it can see this as an example. I'm not blocked on this for my own work. So, I'll just ping this thread every few months and see if you have time again.

[NeatSketch]

Hey, what if you make GGPO a C++ library and create a separate C bindings library for it?

[osor-io]

I hope that a C interface will always be there. Other non-C-family languages have a way easier time using C libraries than C++ ones generally 😄

[erebuswolf]

I have moved this pull to a Unreal Engine 4 plugin version of the codebase. Because git will not let me have multiple forks of this project, I'm closing this pull request. For those of you who want to see the original request when I froze my fork, it is on this branch . There is a way to do this via C style void pointer passing and function pointer passing. If someone wishes to pursue it please do. For my purposes, I don't see the advantage of keeping the project as a DLL and supporting C. So I'm going with supporting it as a UE4 plugin.