OutpostUniverse / NetFixServer

0 stars 0 forks source link

Merge NetFixClient and NetFixServer repos #29

Open DanRStevens opened 4 years ago

DanRStevens commented 4 years ago

I was thinking, there's some sharing of code between NetFixClient and NetFixServer due to the shared network packets they must both be able to interpret.

It might make sense to merge NetFixClient and NetFixServer into the same repository/solution, but kept as separate projects in separate folders. That might help facilitate working on the shared portions.

It may also make sense to split the shared portion into a static library project.

Of course, the server is designed to be cross platform, while the client is Windows only. Merging the repositories might not be the most sensible when it comes to CI builds that only update one of the projects.

Brett208 commented 4 years ago

I think merging the repos and allowing for code sharing is the right call.

For automated builds, ifs both the server and client projects are part of the same Visual Studio solution, appveyor should build them both, producing the server executable and the client DLL. Just like in op2ext when we have a main project, library code, and a unit test application that all get compiled.

Which platform is the primary for the current server? I was assuming a Linux build, but wasn't sure.

Once we have some shared code, building a static library or not depends a bit on unit tests. We could statically compile the shared project and then perform unit tests. Or we could try to unit test the shared code through the server and client projects. I'm fine either way. It may depend on if the client and server are actually suitable to unit test on their own without a static library. If only a small amount of code is being shared, maybe a full static library and project files are overkill and less flexible. Although, we could figure that out post merging the repositories as well.

This might be a good time to rename the project too. Having a NetFix and NetHelper is confusing. Also, I think describing the fact that this project is really adding a lobby or hub to the game for multiplayer match discovery would be appropriate in the title.

Also, major feature improvements to NetFix may require changes to both the server and client code. Merging the repositories would allow discussing those changes in one project instead of across 2 projects.

DanRStevens commented 4 years ago

Alright, sounds like we should merge then.

I was originally thinking of just calling the repo NetFix. The name was long established. You do make an interesting point though about confusion with the new NetHelper project, and that NetFix has expanded to include a lobby. I think at one point I had tried referring to it as something like OPU.net. I'd not sure I entirely like the name though. Do you have any suggestions?

Brett208 commented 4 years ago

Ehh, I don't have any good suggestions. In light of that, I'm happy if we just use NetFix and punt to the future as opposed to waiting for a good name to show up. Being an established name is a good point on it own.

Do you have a merge timeline on when you want me to stop adding new branches and cleanup existing branches?

DanRStevens commented 4 years ago

No timeline. We can do this whenever things settle down, or when this becomes the blocking issue.

Brett208 commented 4 years ago

Once the 2 branches I just pushed are adjudicated, it is probably a good time to go ahead with the move into the new repository. I would probably have either wandered into Packet.h next or stood up google test. Both those efforts are probably best waiting until post move though.

Brett208 commented 4 years ago

I am putting work on #46 and #32 on hold until the merge is done. Unless you think it will be a while, and then I might start taking a crack at them.

DanRStevens commented 4 years ago

Hmm, #46 is already merged. Did you mean #48?

Actually, we might be better off trying to close off issues first, and then merge later. Things like GitHub Issues won't automatically be ported from one repository to the other.


It should be easy enough to merge histories. We'll probably want to update the folder structure first though, so things stay separated in subfolders after the merge. I don't want to be trying to separate things after a merge.

We'll probably want to update the project structure and makefile first to account for a new project layout. Additionally, the NetFixServer project uses an older makefile, which will be harder to adapt than the newer one used in NetFixClient.

Since NetFixClient uses submodules already, and has the updated makefile, it might be easier to merge NetFixServer into the NetFixClient repository, rather than the other way around.

Brett208 commented 4 years ago

That was a typo on part, you are correct that I meant 48.

It looks like it is easy to transfer issues between repositories as long as the repository owner is the same.

https://help.github.com/en/github/managing-your-work-on-github/transferring-an-issue-to-another-repository

DanRStevens commented 4 years ago

Ahh, thank you for that. All this time it's been right there, and I've just never noticed it.

DanRStevens commented 4 years ago

I hit a bit of a snag with this. The updated makefile used by NetFixClient uses GNU Make extensions, which are not part of Posix Make. The MacOS builds use Posix Make.

The NetFixServer currently has a MacOS build. If it uses an updated makefile, as I was planning, it would no longer work on MacOS. At least not with the native toolset. I would need to install GNU Make, or rework the generic portion of the makefile to be Posix compliant.

Not too sure how to proceed at present.

Brett208 commented 4 years ago

What are the odds it will ever be installed on a Macintosh? Maybe we can just drop support?

DanRStevens commented 4 years ago

The slightly annoying thing about this, is the code would still build, it's just the makefile that doesn't work.

It may be reasonable to just drop support. We could potentially also keep the old makefile, move it into the project folder, and then just delegate to it from the main one. That doesn't seem super clean though, particularly since they're used in slightly different ways.

I suppose I kind of want to get the generic makefile portion working for Posix Make, since then it could be used for projects like NAS2D and OPHD, which do currently have a MacOS build. Though that might be something to defer on.

Another option might be to update the MacOS environment to brew install GNU Make.

I think for the moment I kind of want to try closing out a few of the other issues first so I have a bit of time to think about this. In particular #31 and #32. It looks like #2 might be best to leave until after the merge.

Brett208 commented 4 years ago

It sounds like you are taking a crack at #31. If you are not, let me know and I'll attempt the refactor.

DanRStevens commented 4 years ago

Nope, I haven't touched #31 at all. Have at it.