Fix: [AdminPort] #12411 Send Network Welcome Packet to admin port after game creation completed

OpenTTD / OpenTTD

OpenTTD is an open source simulation game based upon Transport Tycoon Deluxe

https://www.openttd.org/

Other

6.06k stars 842 forks source link

Fix: [AdminPort] #12411 Send Network Welcome Packet to admin port after game creation completed #12647

Closed MuxyDuGoulp closed 1 month ago

MuxyDuGoulp commented 1 month ago

Motivation / Problem

ADMIN_PACKET_SERVER_WELCOME is sent too early when a newgame is issued with an external bot already connected through AdminPort.

See #12411.

Description

Move the sending of the packet to a better appropriate place when the game creation is over : NetworkOnGameStart

Closes #12411.

Limitations

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

The bug fix is important enough to be backported? (label: 'backport requested') : No
This PR touches english.txt or translations? Check the guidelines : No
This PR affects the save game format? (label 'savegame upgrade') : No
This PR affects the GS/AI API? (label 'needs review: Script API') : No
- ai_changelog.hpp, game_changelog.hpp need updating.
- The compatibility wrappers (compat_*.nut) need updating.
This PR affects the NewGRF API? (label 'needs review: NewGRF') : No
- newgrf_debug_data.h may need updating.
- PR must be added to API tracker

2TallTyler commented 1 month ago

Thanks for this. Please use the PR template that was prefilled in the form, it's there for a reason. 😉

To link the issue, you can write Fixes #12411.

rubidium42 commented 1 month ago

I think the content of the new function should become part of NetworkOnGameStart.

MuxyDuGoulp commented 1 month ago

where can i find the PR template ?

rubidium42 commented 1 month ago

where can i find the PR template ?

https://github.com/OpenTTD/OpenTTD/blob/master/.github/PULL_REQUEST_TEMPLATE.md?plain=1

Furthermore please rebase instead of merge master into this branch. See https://github.com/OpenTTD/OpenTTD/blob/master/CONTRIBUTING.md for more information. As long as there are no merge conflicts, you do not need to rebase/update your branch for every change that reached master.

MuxyDuGoulp commented 1 month ago

Initial comment updated according to template.

2TallTyler commented 1 month ago

Great, thank you.

Now you will have to squash all your Merge branch 'master' into master commits using an interactive rebase, or our automated checks won't allow this PR to be merged. 🙂

MuxyDuGoulp commented 1 month ago

not sure if all is fine :[

2TallTyler commented 1 month ago

Since this is such a simple code change, you might want to start over with Git. 😉

First, you will need to use the command line interface for GitHub — using the web interface is not powerful enough, and neither was GitHub Desktop last I tried it. If you are not doing this already, here are the instructions to set it up for OpenTTD.

You will need to reset your master branch and then create a branch for this feature.

Fetch the latest version of OpenTTD master branch with git fetch upstream
Reset your master branch with git reset --hard upstream/master
Create a new branch "welcome-packet" for this feature with git checkout -b welcome-packet
Make your code changes and commit normally. You will need to open a new PR, since you'll be merging from a different branch (welcome-packet, as opposed to master).

MuxyDuGoulp commented 1 month ago

made some cleaning made a fork in my GitHub account clone my OpenTTD Repo on my local PC (windows) using Tortoise Git create a branch and swtich to it made the changes launch a full build commit and push from my GitHub account, detected the changes, create a new PR when the PR will be accepted and merged, what should i do with the branch (delete ?) ?