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

[Bug]: Admin packet SERVER_WELCOME can send UINT32_MAX as seed #12411

Closed
MuxyDuGoulp opened this issue Apr 2, 2024 · 5 comments · Fixed by #12672
Closed

[Bug]: Admin packet SERVER_WELCOME can send UINT32_MAX as seed #12411

MuxyDuGoulp opened this issue Apr 2, 2024 · 5 comments · Fixed by #12672
Labels
bug Something isn't working

Comments

@MuxyDuGoulp
Copy link
Contributor

Version of OpenTTD

13.4, 14.0-RC3, g9954187680, windows, linux

Expected result

after a newgame is started, the actual game seed should be sent, not the GENERATE_NEW_SEED. Seems the new seed is updated to late.

Actual result

this is the log of my admin-bot :
regular start of the bot, it get both packets and the game seed :

20240402 121219 S3(1):server_do_packet Admin_Packet_ServerProtocol
20240402 121219 S3(1):server_do_packet Protocol 3
20240402 121219 S3(1):server_do_packet Admin_Packet_Server_Welcome
20240402 121219 S3(1):server_do_packet Welcome name OpenTTD France version 20240331-master-g9954187680 dedicated 1
20240402 121219 S3(1):server_do_packet Welcome gameseed 1012292036 startingyear 730485 landscape 2 map 512*512
20240402 121219 S3(1):server_game_new with gameseed 1012292036

newgame on the console

20240402 121330 S3(1):New game 183, 1012292036
20240402 121330 S3(1):server_game_close 183
20240402 121330 S3(1):server_game_close closing companies from game 183
20240402 121330 S3(1):server_game_close closing clients 1
20240402 121330 S3(1):server_game_close closing game into database 183
...
20240402 121334 S3(1):server_do_packet Admin_Packet_Server_Welcome
20240402 121334 S3(1):server_do_packet Welcome name OpenTTD France version 20240331-master-g9954187680 dedicated 1
20240402 121334 S3(1):server_do_packet Welcome gameseed 4294967295 startingyear 730485 landscape 2 map 512*512
20240402 121334 S3(1):server_game_new with gameseed 4294967295

The game seed received is GENERATE_NEW_SEED value. Dont know what to do with this.

Steps to reproduce

Start a dedicated server with admin port setup properly
connect the bot to the admin-port
receive packet protocol and welcome
launch a newgame on the console or wait for an automatic newgame
check the generation seed in the welcome packet. The value is UINT32_MAX, not the real generation seed.

@MuxyDuGoulp
Copy link
Contributor Author

Did not take the time to check the code for now.

@MuxyDuGoulp
Copy link
Contributor Author

NetworkStart is called BEFORE map generation
openttd.cpp:1072
and Game generation is called later
openttd.cpp:1124

@TrueBrain TrueBrain changed the title [Bug]: AdminPort - Admin_Packet_Server_Welcome is sending bad generationseed (GENERATE_NEW_SEED = UINT32_MAX) after "newgame" (manual or automatic) instead of actual generation seed [Bug]: Admin packet SERVER_WELCOME can send UINT32_MAX as seed Apr 2, 2024
@MuxyDuGoulp
Copy link
Contributor Author

MuxyDuGoulp commented Apr 2, 2024

Here is some tested diff file made on trunk
If there is a better place to send it, let me know.

src/network/network.cpp    | 9 ++++++---
 src/network/network_func.h | 1 +
 src/openttd.cpp            | 5 +++++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/network/network.cpp b/src/network/network.cpp
index 2b0a79ba1..6b2611ec7 100644
--- a/src/network/network.cpp
+++ b/src/network/network.cpp
@@ -925,6 +925,12 @@ static void CheckClientAndServerName()
 	}
 }
 
+void NetworkSendWelcome()
+{
+	/* welcome possibly still connected admins - this can only happen on a dedicated server. */
+	if (_network_dedicated) ServerNetworkAdminSocketHandler::WelcomeAll();
+}
+
 bool NetworkServerStart()
 {
 	if (!_network_available) return false;
@@ -975,9 +981,6 @@ bool NetworkServerStart()
 	/* if the server is dedicated ... add some other script */
 	if (_network_dedicated) IConsoleCmdExec("exec scripts/on_dedicated.scr 0");
 
-	/* welcome possibly still connected admins - this can only happen on a dedicated server. */
-	if (_network_dedicated) ServerNetworkAdminSocketHandler::WelcomeAll();
-
 	return true;
 }
 
diff --git a/src/network/network_func.h b/src/network/network_func.h
index 66b466016..63fcc8fe8 100644
--- a/src/network/network_func.h
+++ b/src/network/network_func.h
@@ -69,6 +69,7 @@ void NetworkServerSendConfigUpdate();
 void NetworkServerUpdateGameInfo();
 void NetworkServerShowStatusToConsole();
 bool NetworkServerStart();
+void NetworkServerSendWelcome();
 void NetworkServerNewCompany(const Company *company, NetworkClientInfo *ci);
 bool NetworkServerChangeClientName(ClientID client_id, const std::string &new_name);
 
diff --git a/src/openttd.cpp b/src/openttd.cpp
index 31b9220bb..3d52fdc52 100644
--- a/src/openttd.cpp
+++ b/src/openttd.cpp
@@ -1229,6 +1229,11 @@ void SwitchToMode(SwitchMode new_mode)
 
 		default: NOT_REACHED();
 	}
+
+	if (_is_network_server) {
+		NetworkServerSendWelcome();
+	}
+
 }

@2TallTyler
Copy link
Member

Thanks for the diagnosis and proposed fix.

If you'd like to propose a change to the code, please open a PR and we'd be happy to take a look. 😄

@MuxyDuGoulp
Copy link
Contributor Author

MuxyDuGoulp commented May 8, 2024

Ok for PR, I couldn't find how to link the PR with the issue :(

MuxyDuGoulp added a commit to MuxyDuGoulp/OpenTTD that referenced this issue May 12, 2024
…after game creation completed

WelcomeAll moved into NetworkOnGameStart

Signed-off-by: Muxy <muxy@goulp.net>
rubidium42 pushed a commit that referenced this issue May 14, 2024
…ame creation completed

WelcomeAll moved into NetworkOnGameStart

Signed-off-by: Muxy <muxy@goulp.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants