mtasa-blue icon indicating copy to clipboard operation
mtasa-blue copied to clipboard

Refactor mtaserver.conf.template

Open Fernando-A-Rocha opened this issue 1 year ago • 12 comments

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.lua to automatically duplicate mtaserver.conf => mtaserver.conf.template
  • Also updated compose_files.lua to do this
  • Added mtaserver.conf.template to be included by the Installer (nightly.nsi)
  • Refactored AddMissingSettings():
    • Will not delete mtaserver.conf.template after running
    • Removed hungarian notation
    • Made the function not add missing resource/module nodes

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

Fernando-A-Rocha avatar Nov 14 '24 23:11 Fernando-A-Rocha

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.

Nico8345 avatar Nov 14 '24 23:11 Nico8345

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.

Fernando-A-Rocha avatar Nov 14 '24 23:11 Fernando-A-Rocha

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 ^^

Fernando-A-Rocha avatar Nov 14 '24 23:11 Fernando-A-Rocha

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

Nico8345 avatar Nov 15 '24 02:11 Nico8345

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.

Fernando-A-Rocha avatar Nov 15 '24 11:11 Fernando-A-Rocha

Please resolve conflicts, @Fernando-A-Rocha

Didn't this appear for you? Untitled

Dutchman101 avatar Nov 21 '24 23:11 Dutchman101

@Dutchman101 Merge conflict resolved. I will apply Nico's suggestion now.

Fernando-A-Rocha avatar Nov 22 '24 10:11 Fernando-A-Rocha

@Nico8340 All good now?

Fernando-A-Rocha avatar Nov 22 '24 11:11 Fernando-A-Rocha

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

Fernando-A-Rocha avatar Nov 22 '24 16:11 Fernando-A-Rocha

Ready

Fernando-A-Rocha avatar Jan 05 '25 15:01 Fernando-A-Rocha

@ project maintainers

merge conflict fixed again. this is ready. any opinions?

Fernando-A-Rocha avatar Mar 07 '25 11:03 Fernando-A-Rocha

Ready

Fernando-A-Rocha avatar May 11 '25 15:05 Fernando-A-Rocha

Feedback?

Fernando-A-Rocha avatar Jul 31 '25 09:07 Fernando-A-Rocha

Thanks, you followed through with everything and were concise about what and why.

Dutchman101 avatar Aug 17 '25 21:08 Dutchman101

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

Dutchman101 avatar Aug 21 '25 17:08 Dutchman101