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

Fix: do not list non-public servers or servers that cannot be reached #40

Merged
merged 1 commit into from Jul 10, 2021

Conversation

TrueBrain
Copy link
Member

No description provided.

@TrueBrain TrueBrain merged commit 0ab492d into OpenTTD:main Jul 10, 2021
@TrueBrain TrueBrain deleted the fix-non-public-servers branch July 10, 2021 12:59
Copy link

@glx22 glx22 left a comment

Choose a reason for hiding this comment

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

I know there are comments, but named constant should be easier

Comment on lines 65 to +66
info["game_type"] = 1 # Public
info["connection_type"] = 2 # Direct-IP
Copy link

Choose a reason for hiding this comment

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

Magic numbers

Copy link
Member Author

Choose a reason for hiding this comment

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

With the comments there, I am fine with this for now. In the end, we need to solve #43 which would allow access to the enums defining this. Similar for the others, ofc :)

A ticket so we don't forget ;)

@@ -124,8 +125,14 @@ async def get_server_info_for_web(self, server_id):
if info_str is None:
return None

info = json.loads(info_str)
if info["game_type"] != 1: # List only GameType.PUBLIC servers.
Copy link

Choose a reason for hiding this comment

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

same here

info = json.loads(info_str)
if info["game_type"] != 1: # List only GameType.PUBLIC servers.
return None
if info["connection_type"] == 1: # Do not list ConnectionType.ISOLATED servers.
Copy link

Choose a reason for hiding this comment

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

and here

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

2 participants