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.

Sign up for GitHub

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

expand disconnect reasons #815

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

expand disconnect reasons #815

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0c1149e
expand disconnect reasons
NotAFile Mar 25, 2019
a662ebe
fix case value
NotAFile Mar 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions Sources/Client/NetClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -752,14 +752,12 @@ namespace spades {
std::string NetClient::DisconnectReasonString(enet_uint32 num) {
switch (num) {
case 1: return _Tr("NetClient", "You are banned from this server.");
case 2:
// FIXME: this number seems to be used when per-IP connection limit was
// exceeded.
// we need to check other usages
return _Tr("NetClient", "You were kicked from this server.");
case 2: return _Tr("NetClient", "You have too many connections to this server.");
Copy link
Owner

@yvt yvt May 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

The word “you” might be inappropriate here because it’s not necessarily “you” who has the connections (e.g., other people behind a NAT).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had similar concerns but I was unsure how to phrase it instead. "Your IP" might be correct at the moment, but:

  • people, in general, are not that familiar with where their IP comes from, especially in larger networks and with the increased prevalence of CGNATs
  • this is a technical detail and might change in the future

Perhaps "Your location" or "Your network" is a good solution?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like "Your network's public address has too many connections to this server" would be the best solution, even though some users may not understand it, I believe it's best to deliver the most accurate message.

Maybe a text below with additional info could be added including the common reasons this might be happening.

case 3: return _Tr("NetClient", "Incompatible client protocol version.");
case 4: return _Tr("NetClient", "Server full");
case 5: return _Tr("NetClient", "Server was shut down");
case 10: return _Tr("NetClient", "You were kicked from this server.");
case 20: return _Tr("NetClient", "Invalid name");
default: return _Tr("NetClient", "Unknown Reason");
}
}
Expand Down