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

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

Closed
wants to merge 9 commits into from

Conversation

MuxyDuGoulp
Copy link
Contributor

@MuxyDuGoulp MuxyDuGoulp commented May 8, 2024

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

@2TallTyler
Copy link
Member

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
Copy link
Contributor

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

…er game creation completed

Welcome moved into OnGameStart
@rubidium42 rubidium42 changed the title Fix: [AdminPort] #12411 Send Network Welcome Packet to admin port after game creation completed Fix: [AdminPort] #12411 Send Network Welcome Packet to admin port after game creation completed May 8, 2024
@MuxyDuGoulp
Copy link
Contributor Author

where can i find the PR template ?

@rubidium42
Copy link
Contributor

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
Copy link
Contributor Author

Initial comment updated according to template.

@2TallTyler
Copy link
Member

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
Copy link
Contributor Author

MuxyDuGoulp commented May 10, 2024

not sure if all is fine :[

@2TallTyler
Copy link
Member

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.

  1. Fetch the latest version of OpenTTD master branch with git fetch upstream
  2. Reset your master branch with git reset --hard upstream/master
  3. Create a new branch "welcome-packet" for this feature with git checkout -b welcome-packet
  4. 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 MuxyDuGoulp closed this by deleting the head repository May 12, 2024
@MuxyDuGoulp
Copy link
Contributor Author

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 ?) ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Admin packet SERVER_WELCOME can send UINT32_MAX as seed
3 participants