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

Codechange: use NetworkAddress instead of two host/port variables where possible #9137

Merged
merged 4 commits into from Apr 29, 2021

Conversation

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Apr 29, 2021

Motivation / Problem

While working on the network code, I noticed we had last_host and last_port, which is a bit weird. As most other settings are a combination of the two. Working towards STUN support, this also becomes a bit problematic, as last_joined can become a non host/port pair. As says, I set out to collapse last_host/last_port into last_joined.

Sadly, the rabbit hole was a bit deeper.

Description

This PR changes a few things, without functional changes:

  • Remove the write-only variable hostname in the NetworkGameInfo.
  • Use the std::string variant of GetAddressAsString where possible (reducing the amount of times I had to read + 6 + 7 with a bad comment) (basically: deduplication)
  • Use NetworkAddress for host/port pairs where possible. I know, this is a bit ironic, as recently someone changed (cbad518) a few of these from NetworkAddress into host/port, but that is just not working out.
  • Added a few helper functions to also return NetworkAddress instead of host/port to deduplicate code. This is mostly around ParseConnectingString and friends.
  • last_host/last_port was set in various of places and also used in the GUI, while there is a this->server telling the same. Simplified this by only setting the now new last_joined when joining a server or opening a lobby, but otherwise not using it.

There is one functional changes in this PR:

The location connect_to_ip was set was weird. This happened on NetworkAddServer. This was called from two places. 1) when clicking "Add Server" and fill in an IP. The comment suggests it was meant for that. 2) when you open the Network GUI for every server you manually added. This often means connect_to_ip changed to something completely different. I removed this last flow, and it is now only 1)

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')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')
@LordAro LordAro changed the title Codechange: use NetworkAddress instead of two host/port variables where possible Codechange: use NetworkAddress instead of two host/port variables where possible Apr 29, 2021
@TrueBrain TrueBrain merged commit a61696d into OpenTTD:master Apr 29, 2021
12 checks passed
@TrueBrain TrueBrain deleted the network-last-host branch Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants