cmangos / issues

This repository is used as a centralized point for all issues regarding CMaNGOS.
179 stars 47 forks source link

🚀 [Feature Request] Distinct file names by expansion #2628

Open Muehe opened 2 years ago

Muehe commented 2 years ago

Feature Details

Right now each core just creates files called mangosd, mangosd.conf, etc. regardless of expansion. This prevents them from being installed together in the same location.

The proposed solution is adding the expansion to the file name, e.g mangosd-classic, mangosd-classic.conf, etc. so all three server can be installed in the same location.

This might be less then ideal for realmd as it could technically support all three expansions at the same time, but that's a different issue.

Renaming of extractor files might rely on the outcome of #1964

Some related discussion here.

Visualizations

|
| - mangosd-classic
| - mangosd-classic.conf
| - mangosd-tbc
| - mangosd-tbc.conf
| - mangosd-wotlk
| - mangosd-wotlk.conf
| - ...

Benefits and Purpose

All three expansions installed in the same location.

cyberium commented 2 years ago

Hello Thank for this request.

I am not for this change. Main reason is we should keep as much as possible core sync In case of cherry picking a commit to another core the file should have same name.

Starting from that i see no good way to have different name for mangosd.conf.dist.in Of course we can set installer rule to rename that file during install process to something like mangos-tbc.conf But thats not how its supposed to be and will not be intuitive for manual installation. Maybe its good enough ?

For binaries name the final name is less important and can be changed more easly. However my preference was for something like server, worldserver, gameserver.... than something with the expension in it like mangos-tbc.

Muehe commented 2 years ago

Of course we can set installer rule to rename that file during install process to something like mangos-tbc.conf But thats not how its supposed to be and will not be intuitive for manual installation. Maybe its good enough ?

Oh yeah, I wasn't quite clear on that, changing the name on install is good enough. Basically the aim is to make it simpler to make install into the same location. This might not even affect config files as suggested here:

Conflicts with config files can be prevented by defining a sysconfdir per flavor -DCMAKE_INSTALL_SYSCONFDIR=/etc/cmangos-tbc while conflicts with binaries can be prevented the same way as long as CMAKE_INSTALL_PREFIX points to something like /usr/local/cmangos-tbc.

I don't have a final approach to handle this yet, and am open to suggestions. Just leaving this problem for package maintainers to deal with is also fine. The issue is just about inquiring if there is opposition to changing file names at install time (through cmake or some other method).

The only problem I see with renaming the config files would be that the core wouldn't look for them by default due to the different name, but that might have to be revised anyway for #1166

insunaa commented 2 years ago

well you could always handle the file names in a macro that gets evaluated at compile-time and in code always just refer to the macro instead of the filename. that way cherry-picking should not make a difference

killerwife commented 2 years ago

My suggestion is you would configure CMANGOS_BINARY_SERVER_NAME during the package creation for a linux repo push, which is already exposed and you would replace the hardcoded mangosd.conf.dist with CMANGOS_BINARY_SERVER_NAME + .conf.dist and in core you would replace "mangosd.conf" strings with macro like "CONFIG_NAME" which would resolve to mangosd.conf by default. This way, we can ACTUALLY reduce diff across cores and you also get what you want. Also alternative to CMANGOS_BINARY_SERVER_NAME + .conf.dist is also adding a new variable which is by default built like that and adding the value of that variable to the preprocessor with add_compile_definitions which would contain "mangosd.conf" by default. This way I could get behind convincing cyberium to merge it because it would actually clean it up rather than make a mess. (the only difference would be three lines at the start of the big cmakelists.txt which are already different in each repo)

@Muehe

Muehe commented 2 years ago

Thank you both for your input, I'll link here when and if I come up with a PR.

celguar commented 2 years ago

Doesn't -config start paremeter work for selecting a different config file?

Muehe commented 2 years ago

It does (you can also substitute with -c btw).

This issue is only about defaults on Linux in the context of #1961