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

Refactor mtaserver.conf.template #3857

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

Fernando-A-Rocha
Copy link
Contributor

@Fernando-A-Rocha Fernando-A-Rocha commented Nov 14, 2024

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 via the installer or zip provided by mtasa.com. Devs compiling MTA:SA 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 gets deleted after use. So, it has zero practical use.

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), the server will receive new settings added by newer server builds, which will be added to the server's mtaserver.conf

@Nico8340

This comment was marked as outdated.

@Fernando-A-Rocha

This comment was marked as outdated.

@Fernando-A-Rocha

This comment was marked as outdated.

@Nico8340

This comment was marked as outdated.

@Fernando-A-Rocha Fernando-A-Rocha changed the title Remove mtaserver.conf.template Refactor mtaserver.conf.template Nov 15, 2024
@Fernando-A-Rocha
Copy link
Contributor Author

Fernando-A-Rocha commented Nov 15, 2024

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.

Server/mods/deathmatch/logic/CMainConfig.cpp Outdated Show resolved Hide resolved
Server/mods/deathmatch/logic/CMainConfig.cpp Outdated Show resolved Hide resolved
@TheNormalnij TheNormalnij added enhancement New feature or request refactor labels Nov 17, 2024
Copy link
Contributor

@Nico8340 Nico8340 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good, although I recommend inserting a comment at the beginning of the template file to let the user know that they shouldn't change it

@Dutchman101
Copy link
Member

Dutchman101 commented Nov 21, 2024

Please resolve conflicts, @Fernando-A-Rocha

Didn't this appear for you?
Untitled

@Fernando-A-Rocha
Copy link
Contributor Author

Fernando-A-Rocha commented Nov 22, 2024

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

@Fernando-A-Rocha
Copy link
Contributor Author

@Nico8340 All good now?

Copy link
Contributor

@Nico8340 Nico8340 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Fernando-A-Rocha
Copy link
Contributor Author

Fernando-A-Rocha commented Nov 22, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants