Don't log masterserver registration errors in the lobby

[ASpoonPlaysGames]

Testing:

The error saying "reached maximum ms registration attempts" i think is fine to show? The main purpose of this PR is to not flood the console with errors that aren't really errors, thus preventing various tickets or misleading users.

[ASpoonPlaysGames]

Im aware that this code isnt fantastic, but I've been waiting too long for the fabled MS rewrite that I'm just gonna PR a stopgap solution

[F1F7Y]

I've been waiting too long for the fabled MS rewrite

https://github.com/F1F7Y/NorthstarLauncher/blob/primedev/primedev/networksystem/atlas.cpp 👉 👈

[pg9182]

Looks good and works overall.

[uniboi]

Reached max ms server registration attempts should be a warning imo. If it's an error it's still misleading for users.

[ASpoonPlaysGames]

Reached max ms server registration attempts should be a warning imo. If it's an error it's still misleading for users.

I can do this if needed, would like a second opinion though if possible.
@pg9182 ?

[ASpoonPlaysGames]

Side note: format check is so stupid sometimes:

bool shouldLogError =
	!strstr(pServerPresence->m_MapName, "mp_lobby")
	&& strstr(pServerPresence->m_PlaylistName, "private_match")
	&& !IsDedicatedServer();

is apparently bad, and should become

bool shouldLogError = !strstr(pServerPresence->m_MapName, "mp_lobby") &&
					  strstr(pServerPresence->m_PlaylistName, "private_match") && !IsDedicatedServer();

it seriously wants me to use 5 tabs + 2 spaces to indent that (relative to the start of the line)

[pg9182]

I can do this if needed, would like a second opinion though if possible.

Yes, warning for non-dedi; error for dedi (in fact, should consider making another PR to exit if it fails unless a convar is set to disable it -- then version gates could update automatically with docker always-pull too).

[GeckoEidechse]

Yes, warning for non-dedi; error for dedi (in fact, should consider making another PR to exit if it fails unless a convar is set to disable it -- then version gates could update automatically with docker always-pull too).

We already established in testing that with auto restart and alway-pull in docker compose will, the container dying and restarting does not actually pull a new image. Only explicit restart docker compose down && docker compose up actually restarts it.

[Alystrasz]

I tested this PR with the game client, and saw no errors while in lobby.
But while launching a dedicated server, I had the following output:

Since it's a dedicated server, I would expect the error messages to be displayed, but they do not appear.
Is this expected behaviour?

[ASpoonPlaysGames]

Since it's a dedicated server, I would expect the error messages to be displayed, but they do not appear. Is this expected behaviour?

Nah, seemingly that's just me a month ago apparently not knowing how boolean operations work, will fix later

[GeckoEidechse]

If someone wants to host a listen server but forgot to set up port forwarding, do they still have a way to see the error message like "Unable to reach gameserver"?

So that in the rare case someone is actually trying to host a listen server but failing they actually have a way of seeing what's wrong.

[ASpoonPlaysGames]

If someone wants to host a listen server but forgot to set up port forwarding, do they still have a way to see the error message like "Unable to reach gameserver"?

They would already see it, since a listen server would be in private match. This PR stops it specifically in the normal lobby.

[GeckoEidechse]

They would already see it, since a listen server would be in private match. This PR stops it specifically in the normal lobby.

I'm aware that it doesn't show in normal lobby. What I'm wondering is what happens if the player doesn't go to private match before the 4 registration attempts but only afterwards. AFAIK there's no explicit re-registration attempt when we enter private match so they wouldn't see the error message. Or am I missing something? ^^

[ASpoonPlaysGames]

What I'm wondering is what happens if the player doesn't go to private match before the 4 registration attempts but only afterwards. AFAIK there's no explicit re-registration attempt when we enter private match so they wouldn't see the error message. Or am I missing something? ^^

Yeah that's probably accurate, haven't tested though, will maybe test this case later

[GeckoEidechse]

Aight. Otherwise I'd suggest re-trying master server registration in case of failure on private match but that would probably be a separate PR ^^

[Alystrasz]

Code logic looks good.
Registration error messages aren't displayed on client anymore (while still displayed on dedicated).
Reached max ms server registration attempts message is now displayed as a warning on client (still as an error on dedicated).

[Jan200101]

Alystrasz tested and reviewed the code, marking as ready to be merged

[GeckoEidechse]

Merging based on reviews