Skip to content
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

Complete Feature of Online Servers List (Server+Client+Manager) & Compatibility Fix for NitroxServer on Linux #581

Closed
wants to merge 10 commits into from

Conversation

DKingAlpha
Copy link
Contributor

@DKingAlpha DKingAlpha commented Jan 5, 2019

1. Compatibility for NitroxServer on Linux

This fixed the bug of nitroxserver on linux that used hardcoded backslash in path \\
It would fail to load cells on linux.

fix proof:
https://imgur.com/a/QTvA4DE

a wiki page of NitroxServer Setup on Linux will be added by shalix later.

2. Complete feature of Online Servers Menu

Server List is fetched from http://nitrox.qaq.link/server.
Server owner can choose to broadcast their server to http://nitrox.qaq.link/server
A Nitrox Server Manager is listening at nitrox.qaq.link to handle pings and serve lists

Final

3. Fixed a spot of Log spam in EntityManager.cs

64c974b

1. Provide a menu listing online servers
2. Fixed Log spam in EntityManager.cs
@DKingAlpha DKingAlpha changed the title Fix compatibility for NitroxServer on Linux Compatibility for NitroxServer on Linux and Feature of Online Servers List Jan 6, 2019
better compare for ServerInfo
fix another hardcoded backslash
…in:port so failed to connect a server in that format

2. Refactor the code that interacts with serverStr
Copy link
Member

@Sunrunner37 Sunrunner37 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also weigh in on this PR: #574 Specifically the running in the background piece?

@DKingAlpha
Copy link
Contributor Author

I have been in touch with Dilicor about that topic.

I would prefer using screen rather than nohup, screen worries about no background compatibility, but I will help that part.

@deejcoder
Copy link

I think we should try something along the lines of pm2, or supervisor, as these run applications in the background rather than foreground.

@DKingAlpha
Copy link
Contributor Author

Hi, ServerInfo looks better now.

…ver manager.

Sync with server manager when player number changed.

Catch Server Exit Event, gracefully exit.
@DKingAlpha
Copy link
Contributor Author

DKingAlpha commented Jan 7, 2019

  1. Catch Server Exit Event, gracefully exit.
  2. NitroxServer will be able to broadcast its open information to a central server manager. This feature serves for OnlineServerList feature in NitroxClient.

Server Manager is under developing. Will finish in today.

My code always tries to keep compatibility with dual-stack IP. that made the code a bit more complex.

@DKingAlpha
Copy link
Contributor Author

complete Online Server features finished.

Final

Client side and Server side is in this PR

Server Manager code is at DKingCN/NitroxServerManager as simple as a single script

@DKingAlpha DKingAlpha changed the title Compatibility for NitroxServer on Linux and Feature of Online Servers List Complete Feature of Online Servers List (Server+Client+Manager) & Compatibility Fix for NitroxServer on Linux Jan 7, 2019
@Sunrunner37
Copy link
Member

A few comments:

  • Please break up PRs by using different GIT branches. That will limit the amount of churn on a PR and act as an isolated unit to review/merge.
  • Ensure to action previous PR comments so we can merge.
  • Removing the entity log line is just masking the problem. We should diagnose why this issue is occuring.
  • As mentioned on discord, I already have a listing server in the works. Although I definitely appreciate the contribution.

@DKingAlpha
Copy link
Contributor Author

  1. The mess of PR is caused more like a historical reason. Feature didnt came as a whole part and Fixes are too small to split to another PR. Force push didnt made the PR cleaner, so I will close this one later and sort the update and create a new one.
  2. I thought I finished the problem you mentioned in comments. Did I missed some?
  3. You are right, masking the log is just making it worse.
  4. I have done most of the feature before you could reply on discord, so I had to finish and share it as an option.

@DKingAlpha DKingAlpha closed this Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants