Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#3570 closed enhancement (fixed)

[PATCH] Change message for failed join to host.

Reported by: scythetwirler Owned by: Stan
Priority: Should Have Milestone: Alpha 21
Component: Network Keywords: patch
Cc: Patch:

Description (last modified by scythetwirler)

"Unknown reason" is hardly actually the case when one cannot join another's hosted game. The message should detail that most likely the host has not configured port forwarding/firewall properly.

Strings are located at binaries/data/mods/public/gui/common/network.js .

Attachments (9)

3570.diff (791 bytes ) - added by Mikita Hradovich 8 years ago.
Updated message for NDR_UNKNOWN
screenshot0001.png (325.0 KB ) - added by Mikita Hradovich 8 years ago.
Dialog with new message
3570.2.diff (820 bytes ) - added by Stan 8 years ago.
Should take into account leper's elexis' and scythetwirler's comments.
3570.2.2.diff (3.5 KB ) - added by Stan 8 years ago.
Do as indicated in the comment above.
3570.3.diff (3.6 KB ) - added by Stan 8 years ago.
Some fixes, thanks elexis for the feedback
3570.4.diff (3.6 KB ) - added by Stan 8 years ago.
Fix a typo
3570.5.diff (3.1 KB ) - added by Stan 8 years ago.
Rebased
3570.6.diff (3.9 KB ) - added by Stan 8 years ago.
Should fix the above.
3570.7.diff (3.9 KB ) - added by Stan 8 years ago.
grammar fix.

Download all attachments as: .zip

Change History (32)

comment:1 by scythetwirler, 8 years ago

Not extremely urgent, but if we're breaking string freeze at the moment, we may as well address this. Push back if necessary.

comment:2 by scythetwirler, 8 years ago

Description: modified (diff)

comment:3 by leper, 8 years ago

Unknown reason is still the best we know. Adding something more about possible causes can't hurt though.

comment:4 by elexis, 8 years ago

It would be really awesome to add a notice about the host probably not having forwarded UDP port 20595 on the router and that the host should look up the router manual.

The lobby is spammed day by day with people who don't know why they can't connect, annoying.

comment:5 by scythetwirler, 8 years ago

Most notably, people tend to blame something on their own side rather than the host's, leading them to leave/uninstall etc.

by Mikita Hradovich, 8 years ago

Attachment: 3570.diff added

Updated message for NDR_UNKNOWN

by Mikita Hradovich, 8 years ago

Attachment: screenshot0001.png added

Dialog with new message

comment:6 by Yves, 8 years ago

Thanks for submitting a patch. For the next time, if you want to get the patch reviewed and committed, please follow the steps described in the wiki: SubmittingPatches This makes sure it shows up on the list of patches waiting for review.

To the patch: In my opinion, the message isn't quite optimal yet in regard to scythetwirler's comment Replying to scythetwirler:

Most notably, people tend to blame something on their own side rather than the host's, leading them to leave/uninstall etc.

This message gets to the client who tries to join a game. However, most likely the problem is actually the host because port forwarding is not configured. I'd say it's quite rare that outgoing connections are blocked on a network. This is usually the case in corporate networks, but people shouldn't be playing games there. In addition, it's quite likely that the connection to the multiplayer lobby wouldn't have worked in the first place if outgoing connections were blocked (you could still be connecting manually, but that's quite rare too). In my opinion, the message should point out that the problem is most likely on the host side (missing port forwarding on the router or a firewall), but still could be on the client side in rare cases.

comment:7 by Stan, 8 years ago

Some other comments from leper in yesterday's IRC conversation

00:14 <@leper> should keep the Unknown Reason part
00:15 <@leper> keep that part, then add possible causes for it
00:16 <@leper> since it could be the wrong ip address, wrong forwarding rules, missing cable, no wifi, ...

comment:8 by leper, 8 years ago

Any updates?

comment:9 by Stan, 8 years ago

Keywords: review patch added
Owner: set to Stan
Status: newassigned
Summary: Change message for failed join to host.[PATCH] Change message for failed join to host.

by Stan, 8 years ago

Attachment: 3570.2.diff added

Should take into account leper's elexis' and scythetwirler's comments.

comment:10 by elexis, 8 years ago

Keywords: review removed

Notice the disconnect event can either happen when failing to join or when being disconnected from a game.

So having the port-forward-hint displayed after being disconnected from a running game is wrong, as the server clearly works.


Implementation: If I understand correctly and there was an explicit disconnect (server shutdown, banned, rejecting join because the game is loading), then the reason byte will be != 0.

If the client can't connect to the server (port not forwarded, server not running, network timeout, no internet) or has been disconnected from the server without being explicity disconnected by the server (network timeout, no internet), this byte will be 0.

We may only give the clients a hint that the server didn't forward the port, if we were not connected to the game previously.

To accomodate that, getDisconnectReason and reportDisconnect must have a boolean argument wasConnected, which must be

  • true when calling getDisconnectReason from messages.js (disconnect from session)
  • true when calling reportDisconnect from gamesetup.js (disconnect from gamesetup)
  • false when calling reportDisconnect from gamesetup_mp.js (could not connect to server)

Not sure about the message itself.

  • open should be replaced with forwarded to be more accurate.
  • I agree with leper that it is a bit weird to mention some possible reasons (port forward, firewall) but don't mention others (no internet connection, network timeout).
  • Having three sentences is also quite long. Maybe we could just list those reasons?

Sorry that I didn't have time to post this earlier.

comment:11 by Stan, 8 years ago

About the first part I think that's for another ticket as the problem here is just supposed to be the message. For the second part I agree with all points, I don't know how to make the dots though.

by Stan, 8 years ago

Attachment: 3570.2.2.diff added

Do as indicated in the comment above.

by Stan, 8 years ago

Attachment: 3570.3.diff added

Some fixes, thanks elexis for the feedback

comment:12 by Stan, 8 years ago

Keywords: review added

by Stan, 8 years ago

Attachment: 3570.4.diff added

Fix a typo

comment:13 by leper, 8 years ago

Milestone: Alpha 19Alpha 20

comment:14 by elexis, 8 years ago

Also notice the text for NDR_UNEXPECTED_SHUTDOWN is actually wrong. It is not an "unexpected shutdown" but as far as I can see it only happens when either the host or the client willfully closes the game. When the client closes the game, we don't report the disconnect reason, so it should be safe to return "The host has ended the match" in getDisconnectReason.

comment:15 by leper, 8 years ago

Keywords: review removed

comment:16 by elexis, 8 years ago

In 17710:

Show a more specific disconnect-message in case the host closed the server. Refs #3264, #3570.

comment:17 by Itms, 8 years ago

Milestone: Alpha 20Alpha 21

comment:18 by elexis, 8 years ago

IIRC the only thing to be changed in the patch uploaded by Stan is the string. It shouldn't claim to know why the disconnect occured, but it could indeed hint that the host perhaps didn't forward UDP port 20595 on the router (as that's by far the most common case).

by Stan, 8 years ago

Attachment: 3570.5.diff added

Rebased

comment:19 by elexis, 8 years ago

Phrasing:

  • Generally -> often
  • port -> UDP port
  • It should first state something true (i.e. "Could not connect to host.") then follow with the speculation. the text seems too long, kill the last sentence and continue the prior one with ... or firewall bla

Logic:

  • Don't nuke the translateWithContext
  • That patch misses the change to reportDisconnect.

by Stan, 8 years ago

Attachment: 3570.6.diff added

Should fix the above.

by Stan, 8 years ago

Attachment: 3570.7.diff added

grammar fix.

comment:20 by Stan, 8 years ago

Keywords: review added

comment:21 by elexis, 8 years ago

Resolution: fixed
Status: assignedclosed

In 18176:

Show a hint when failed to connect to a game stating that it is likely caused by the host not forwarding the UDP port. Patch by Stan, fixes #3570.

comment:22 by elexis, 8 years ago

Keywords: simple review removed

Thanks

Last changes:

  • the "Failed to connect to server" is already contained in the reportDisconnect, so no need to add it to the reason string itself
  • tricky: the dot is added to the reason itself (someone should fix that sometime probably. maybe not now as it would change all translated strings)
  • Failed to connect to server -> Failed to connect to the server.
  • an antivirus-software -> antivirus-software, as software doesn't have a singular
  • some newlines
Last edited 8 years ago by elexis (previous) (diff)

comment:23 by elexis, 8 years ago

Component: UI & SimulationNetwork

(changed component to network)

Note: See TracTickets for help on using tickets.