-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
First attempt at networking (pretty ugly) #26
Conversation
This reverts commit 037609e.
This reverts commit 49cbddb.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a solid start on the networking interface. I added some comments and suggestions throughout.
I think the main thing to lock in on is the style of the interface and how to simultaneously make it flexible and modular. In this case, I think the interface should be treated like any other interface on the server side, but internal to the interface it should process the data to send to the client and collect the results. I don't think that main or world should need to do anything special to make this work; most of the networking should be able to be setup in the constructor or related helper functions.
The more challenging piece will be to make sure multiple network interfaces can run simultaneously, but let's get one going smoothly before putting too much effort into that direction.
The main changes for now are the basic cleanup on the interface so that it can work with existing worlds that other teams are making, and removing some files that were accidentally added to pull request.
Good job with naming, style, and building on a modular API. Initial tests look good!
|
||
std::cout << "Server IP Address: " << sf::IpAddress::getLocalAddress() << std::endl; | ||
|
||
//Establish an initial connection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can establishing the connection for in the network interface constructor? Then it should be possible to set it up with multiple interfaces without them stepping on each other's toes (I think...) It will also keep everything more modular.
tests/build/.gitignore
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this change; it looks like it changes from ignoring all files in tests/build/
to only ignoring files that begin with a dash?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly not sure how I ended up doing this, seems like I must've accidently brushed a key when I was working on tests.
(CMake and gitignore fixes and adding them to the PR)
Due to various issues we had with developer operating systems and libraries, we were not able to make something that is very cohesive or well put together, but something that works at a very low level.