Refactor mtaserver.conf.template
Context
The mtaserver.conf.template file is a copy of mtaserver.conf without the <module> and <resource> nodes, that devs have been maintaining manually. Default config already comes in mtaserver.conf.
Without this PR, the script looks for mtaserver.conf.template, and if it finds it, it will scan for missing config nodes in the existing mtaserver.conf config, and then after it's done with the changes, it deletes the .template file. So it's only ever used once.
The problem is that real MTA servers never get their hands on the .template file, so it is never used.
- mtaserver.conf.template file is not shipped with the server files via the installer or zip provided by mtasa.com.
- This file is only present in the source code, which devs compiling mtasa manually will result in this file being moved to the build server
- Devs compiling mtasa also don't need this template ever, because the default config is already in the mtaserver.conf file...
- Also, the note in the beginning of the mtaserver.conf.template file is false
<!-- This file is compiled into the server binary.This file is never compiled in the server binary, it even gets deleted after use by the script.
So, mtaserver.conf.template has had zero practical use over the last years.
What I changed
- Deleted mtaserver.conf.template from files
- Edited buildaction
install_data.luato automatically duplicatemtaserver.conf=>mtaserver.conf.template - Also updated
compose_files.luato do this - Added
mtaserver.conf.templateto be included by the Installer (nightly.nsi) - Refactored AddMissingSettings():
- Will not delete
mtaserver.conf.templateafter running - Removed hungarian notation
- Made the function not add missing resource/module nodes
- Will not delete
Why?
- We no longer need to duplicate every new/changed config setting to
mtaserver.conf.template. - By shipping mtaserver.conf.template with the installer (overwrites existing mtaserver.conf.template of the server), servers owners will receive updates to that file, so their server will receive new settings added by newer server builds, which will then be automatically inserted in the server's mtaserver.conf
I think it's useful, and you just shed light on it with this pull request. Although by default an end user does not have the template file, if someone runs into problems and asks for help, he does not have to lose the changes he has already made, we just send him the template and he can use it to correct the main config file.
If you want to make improvements in this regard, I recommend that instead of removing it, end users should have this file by default, never delete it after repairing, and have a comment in the template file saying that it is not worth changing it because of unexpected behavior.
So yes, the meaning and main purpose of the pull request would completely change, but I see this as a more viable improvement than the complete removal of a fundamentally useful but unfinished feature.
If you want to make improvements in this regard, I recommend that instead of removing it, end users should have this file by default, never delete it after repairing, and have a comment in the template file saying that it is not worth changing it because of unexpected behavior.
Keeping the template file might be useful for saving the server owner in case they deleted an important config node, but it will not introduce new config nodes that were added in newer MTA server builds, unless this template file is updated, which it is currently not by any of the installers/zips.
Although by default an end user does not have the template file, if someone runs into problems and asks for help, he does not have to lose the changes he has already made, we just send him the template and he can use it to correct the main config file.
Chances of anyone receiving a template file to fix their config are slim to none, this is not a good solution.
In my opinion, the default config should be embedded in the server binary, which is kept up to date, because every MTA server updates their exe.
Then, we won't need any .template file, the settings can be loaded from that hidden resource to be compared with the current config, and performa any necessary changes (add missing settings).
I think this approach is ideal ^^
In my opinion, the default config should be embedded in the server binary, which is kept up to date, because every MTA server updates their exe.
Then, we won't need any .template file, the settings can be loaded from that hidden resource to be compared with the current config, and performa any necessary changes (add missing settings).
I think this approach is ideal ^^
Why not use the template file if you already have existing logic for it? I think it's unnecessary to embed the entire config in the code and use it for every reinstallation, instead this should be done from one file, which is this template file
Why not use the template file if you already have existing logic for it? I think it's unnecessary to embed the entire config in the code and use it for every reinstallation, instead this should be done from one file, which is this template file
Hmm, check out what I did.
Please resolve conflicts, @Fernando-A-Rocha
Didn't this appear for you?
@Dutchman101 Merge conflict resolved. I will apply Nico's suggestion now.
@Nico8340 All good now?
Future Work
I believe that later we should refactor the config system removing the need to have 2 separate files local.conf and editor.conf (as well as editor_acl.xml) for the configuration of Host-Game and Map-Editor servers. It's bad practice that we are duplicating all config values to those files, while in reality all that is happening is some values changing when these servers are started (like startup resources and server name etc). This could be hardcoded in C++, and for Host-Game, the values of the last used preferred settings can be cached to core client config.
I volunteer to do this refactoring if you guys agree.
->feedback, @Dutchman101
Ready
@ project maintainers
merge conflict fixed again. this is ready. any opinions?
Ready
Feedback?
Thanks, you followed through with everything and were concise about what and why.
I'm sorry but this PR will have to be temporarily reverted.. it broke build because of a buildserver-local NSIS script issue that i tried to resolve there but was unable to. It will be re-introduced later by @botder if/when he gets to it, he knows his way around the buildscript better than me and i left him notes.
Reverted in 5c3b988
@Fernando-A-Rocha