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
Remove: ENABLE_NETWORK switch #7390
Conversation
This switch has been a pain for years. Often disabling broke compilation, as no developer compiles OpenTTD without, neither do any of our official binaries. Additionaly, it has grown so hugely in our codebase, that it clearly shows that the current solution was a poor one. 350+ instances of "#ifdef ENABLE_NETWORK" were in the code, of which only ~30 in the networking code itself. The rest were all around the code to do the right thing, from GUI to NewGRF. A more proper solution would be to stub all the functions, and make sure the rest of the code can simply assume network is available. This was also partially done, and most variables were correct if networking was disabled. Despite that, often the #ifdefs were still used. With the recent removal of DOS, there is also no platform anymore which we support where networking isn't working out-of-the-box. All in all, it is time to remove the ENABLE_NETWORK switch. No replacement is planned, but if you feel we really need this option, we welcome any Pull Request which implements this in a way that doesn't crawl through the code like this diff shows we used to.
@@ -220,7 +220,7 @@ bool HandleBootstrap() | |||
if (BlitterFactory::GetCurrentBlitter()->GetScreenDepth() == 0) goto failure; | |||
|
|||
/* If there is no network or no freetype, then there is nothing we can do. Go straight to failure. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment may need an update ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially, I was thinking the same. But look a line lower. It is still correct :)
var = network.client_name | ||
type = SLE_STRB | ||
flags = SLF_NOT_IN_SAVE | SLF_NO_NETWORK_SYNC | ||
def = NULL | ||
proc = UpdateClientName | ||
cat = SC_BASIC | ||
|
||
[SDTC_STR] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think this shouldn't've been removed...
…d to disappear from settings.
…d to disappear from settings.
This switch has been a pain for years. Often disabling broke
compilation, as no developer compiles OpenTTD without, neither do
any of our official binaries.
Additionaly, it has grown so hugely in our codebase, that it
clearly shows that the current solution was a poor one. 350+
instances of "#ifdef ENABLE_NETWORK" were in the code, of which
only ~30 in the networking code itself. The rest were all around
the code to do the right thing, from GUI to NewGRF.
A more proper solution would be to stub all the functions, and
make sure the rest of the code can simply assume network is
available. This was also partially done, and most variables were
correct if networking was disabled. Despite that, often the #ifdefs
were still used.
With the recent removal of DOS, there is also no platform anymore
which we support where networking isn't working out-of-the-box.
All in all, it is time to remove the ENABLE_NETWORK switch. No
replacement is planned, but if you feel we really need this option,
we welcome any Pull Request which implements this in a way that
doesn't crawl through the code like this diff shows we used to.