Don't save load_mod_* = false lines in world.mt

[appgurueu]

Fixes #11423.

How to test

Look at world.mt after configuring a world.

[SmallJoker]

I'd like to know what server owners think about such change. I can imagine that it's easier for them to enable the mod if the engine already scans and lists the available mod names within the configuration file.

[grorp]

1. You forgot to update the comment a few lines above in mod_configuration.cpp
2. (Why is this duplicated :[ )

[appgurueu]

I'd like to know what server owners think about such change. I can imagine that it's easier for them to enable the mod if the engine already scans and lists the available mod names within the configuration file.

So far I've only seen consensus that #11423 should be implemented, and I think there has been plenty of time for server owners to voice their opinion on that issue. (And as mruncreative points out, a long list of mods can make editing the file quite annoying.)

In either case I think the problems pointed out in that issue outweigh the supposed potential server owner convenience. If server owners want to ls | sort their mods folder, they can do just that.

[mruncreative]

Nothing easier than listing valid mod names. Literally described it to Bing Copilot and it wrote working scripts after 2 revisions. And they only list valid mods and not empty folders. It's public domain by default because it's machine generated. Just copypaste it wherever.

#!/bin/ruby
require 'find'
def get_mod_name(mod_path)
	mod_conf_path = File.join(mod_path, 'mod.conf')
	if File.file?(mod_conf_path)
		File.foreach(mod_conf_path) do |line|
			key, value = line.strip.split('=', 2)
			return value.strip if key.strip == 'name'
		end
	end
	File.basename(mod_path)
end
def valid_mod?(mod_path)
	File.file?(File.join(mod_path, 'mod.conf')) || File.file?(File.join(mod_path, 'init.lua'))
end
def list_mods(directory = '.')
	mods = []
	begin
		Dir.foreach(directory) do |entry|
			next if entry == '.' || entry == '..'
			mod_path = File.join(directory, entry)
			if File.directory?(mod_path)
				if valid_mod?(mod_path)
					mods << get_mod_name(mod_path)
				elsif File.file?(File.join(mod_path, 'modpack.conf'))
					#~ mods << "[Modpack] #{entry}"
					mods.concat(list_mods(mod_path))
				end
			end
		end
	rescue Errno::ENOENT
		puts "The directory '#{directory}' does not exist."
	rescue StandardError => e
		puts "An error occurred: #{e}"
	end
	mods
end
mods = list_mods
mods.each { |mod| puts mod }

#!/bin/python3
import os
def get_mod_name(mod_path):
	mod_conf_path = os.path.join(mod_path, "mod.conf")
	if os.path.isfile(mod_conf_path):
		with open(mod_conf_path, 'r') as file:
			for line in file:
				if "=" in line:
					key, value = line.strip().split('=', 1)
					if key.strip() == "name":
						return value.strip()
	return os.path.basename(mod_path)

def is_valid_mod(mod_path):
	return os.path.isfile(os.path.join(mod_path, "mod.conf")) or os.path.isfile(os.path.join(mod_path, "init.lua"))
def list_mods(directory="."):
	mods = []
	try:
		entries = os.listdir(directory)
		for entry in entries:
			mod_path = os.path.join(directory, entry)
			if os.path.isdir(mod_path):
				if is_valid_mod(mod_path):
					mods.append(get_mod_name(mod_path))
				elif os.path.isfile(os.path.join(mod_path, "modpack.conf")):
					# mods.append(f"[Modpack] {entry}")
					mods.extend(list_mods(mod_path))
	except FileNotFoundError:
		print(f"The directory '{directory}' does not exist.")
	except Exception as e:
		print(f"An error occurred: {e}")
	return mods

mods = list_mods()
for mod in mods:
	print(mod)

[wsor4035]

I'd like to know what server owners think about such change. I can imagine that it's easier for them to enable the mod if the engine already scans and lists the available mod names within the configuration file.

i know a fair few that use worldmods, cant speak for all however. i have this pr to the group of server owners

[Warr1024]

I have a workaround involving replacing the world.mt file each server startup. The fact that MT defecates all these load_mod_*=false lines all over the file, making it unreadable and more difficult to source control, is a big factor in that. Not needing the workaround would be nice.

[GreenXenith]

There is value for servers in particular for Luanti to populate the world.mt with available mods so I dont have to manually write all the entries (can just s/false/true), but that seems like it should be an opt-in commandline switch.

[mruncreative]

I don't think there's a value in that. Why do you have to manually write the entries? Do I have to write you a TUI mod manager you can use through ssh to babysit you? And no you can not just " (can just s/false/true)" because true isn't a valid value for load_mod_*. It has to be the mod path and you don't know the mod path for sure without guessing it or looking it up.

[GreenXenith]

I don't think there's a value in that. Why do you have to manually write the entries?

Perhaps you have never operated a headless server before, but you may notice that there is no client in which to enable mods, and writing external tools is an excess of effort when a system already exists.

Do I have to write you a TUI mod manager you can use through ssh to babysit you?

I am disappointed that you feel the need to be unkind, but wish you the best of luck.

And no you can not just " (can just s/false/true)" because true isn't a valid value for load_mod_*. It has to be the mod path and you don't know the mod path for sure without guessing it or looking it up.

For those of you who are new here, a boolean value is the original method by which mods were enabled, and is still supported.

[mruncreative]

For those of you who are new here, a boolean value is the original method by which mods were enabled, and is still supported.

I even suggested saving true instead in #15757 but was told the mod path is needed for modpacks, so I made the wrong assumption. Sorry.
Modpacks have a lot of quirks in general in my opinion. mesecons_doors (which is optional) depending on doors makes Mesecons listed as only compatible with games that provide doors for example. (I'll fix that by making it an optional dependency.)

However you are wrong about them being available mods. They can be mod names but they can also simply be folder names of empty folders or otherwise invalid mods that can not be enabled and then actually work as expected.

[Warr1024]

...it should be an opt-in commandline switch.

Yeah, if this feature is to exist at all, it should be user-togglable (opt-in > opt-out), and the sanest default would be to have Luanti NOT modify the world.mt file if it already exists. Really ideally this should be "don't write the file" rather than the potential "replace it with a file with identical contents", which is what it looked like the PR might have been trying to do.

Having to manually work around this using active external scripts in the 99.99% case that I'm NOT setting up a server for the first time or trying to add mods is a serious pain and it makes things like automatic updates for a server very error-prone.

[NathanSalapat]

I'd like to know what server owners think about such change. I can imagine that it's easier for them to enable the mod if the engine already scans and lists the available mod names within the configuration file.

I usually put all my mods into either a game or worldmods directory, so it wouldn't make that big a change, and as I don't run headless anymore it would be even less of an issue. However when I first started running a server in a docker container having all the available mods listed out was super helpful as I didn't have to try and guess what the mod names were to enable them.

[rubenwardy]

I'd say it's probably worth merging this and encouraging the creation of CLI tools or the use of worldmods. I would be against adding a setting

[Bastrabun]

For our server we use the mods folder for permanent mods and the worldmods folder for temporary or experimental ones. The "permanent" mods come from a master repo with submodules.

I occasionally need to quickly disable mods when they cause issues, that's why I'd like the "false entries not to vanish. If they did, I'd have to re-add them.

Also, when something causes issues and it is not known which mod is responsible, I want to quickly disable and enable some of them, it might be inconvenient to have to keep track of all of them if the "false" entries vanish. (Maybe I should add the config to git)

If there is really something to be gained from removing them, then so be it. Otherwise I'd vote for "keep"

[sfan5]

I think the least obstructive way would be to set entries that exist to false if needed but not add any new ones.

[rubenwardy]

Yeah, makes sense for false not to be removed. The only thing we want to change is adding false entries when not necessary

[mruncreative]

I'm against keeping false entries. Are all old players supposed to clean that stuff they never agreed to out of their world files manually?
I already posted two scripts that obtain a list of all valid mods. With those you can easily build whatever mod togglers/managers you need in less time than you spend talking about how this is supposedly useful.

[appgurueu]

I occasionally need to quickly disable mods when they cause issues, that's why I'd like the "false entries not to vanish. If they did, I'd have to re-add them.

Also, when something causes issues and it is not known which mod is responsible, I want to quickly disable and enable some of them, it might be inconvenient to have to keep track of all of them if the "false" entries vanish.

Your use case should already be covered by commenting out lines. Comments are preserved. (I just tested to be sure.)

[sfan5]

Are all old players supposed to clean that stuff they never agreed to out of their world files manually?

If it's important enough to them, sure.

[Bastrabun]

I think the least obstructive way would be to set entries that exist to false if needed but not add any new ones.

I found it rather convenient that I didn't have to look up each technical name of the mod and handcraft the entry when adding new mods. When I added new mods, I ran the server, looked at the bottom of the file and set to true what I wanted to add.

If there's a viable alternative, then I don't see why my use cases are important enough not to clean up the mechanic.

[mruncreative]

I wrote a script to populate the world.mt with disabled mods. You can run this instead of starting up your server to see which mods there are and easily enable them the same way you did before. (except it only adds valid mods) I don't think this should be part of Luanti.

#!/bin/ruby
require 'find'
def get_mod_name(mod_path)
	mod_conf_path = File.join(mod_path, 'mod.conf')
	if File.file?(mod_conf_path)
		File.foreach(mod_conf_path) do |line|
			key, value = line.strip.split('=', 2)
			return value.strip if key.strip == 'name'
		end
	end
	File.basename(mod_path)
end
def valid_mod?(mod_path)
	File.file?(File.join(mod_path, 'mod.conf')) || File.file?(File.join(mod_path, 'init.lua'))
end
def list_mods(directory = File.join(Dir.home,'.minetest/mods'))
	mods = []
	begin
		Dir.foreach(directory) do |entry|
			next if entry == '.' || entry == '..'
			mod_path = File.join(directory, entry)
			if File.directory?(mod_path)
				if valid_mod?(mod_path)
					mods << get_mod_name(mod_path)
				elsif File.file?(File.join(mod_path, 'modpack.conf'))
					#~ mods << "[Modpack] #{entry}"
					mods.concat(list_mods(mod_path))
				end
			end
		end
	rescue Errno::ENOENT
		puts "The directory '#{directory}' does not exist."
	rescue StandardError => e
		puts "An error occurred: #{e}"
	end
	mods
end

modsHash = list_mods.each_with_object({}) do |item, result|
  result[item] = false
end

config_file_path = 'world.mt'
config = {}

File.foreach(config_file_path) do |line|
	line.strip!
	next if line.empty?

	key, value = line.split('=', 2)
	value.strip!
	key.strip!
	if key.start_with?("load_mod_") then
		if value != "false" then
			modsHash[key.sub(/^load_mod_/, '')] = true
			config[key] = value
		end
	else
		config[key] = value
	end
end

modsHash.each do |key, value|
	if not value then
		config["load_mod_" << key] = "false"
	end
end

File.open(config_file_path, 'w') do |file|
	config.each do |key, value|
		file.puts "#{key}=#{value}"
	end
end

puts "Configuration saved to #{config_file_path}."

[mruncreative]

[appgurueu]

Not worth a setting.

I would prefer not to complicate the logic further, such as "keeping old load_mod_* = false" entries. I don't see much of a point in that. It gives sort of a "worst of both worlds" solution where all you get is that old false entries from some point in time are kept. Thus these entries are neither representative of available mods, nor do people get the benefits of not having these entries for old Luanti worlds, they'd have to clean up manually.

I think this PR is, overall, an improvement as-is: It closes a long-standing issue which has modest support and so far no very vocal critics.

It might be a very minor annoyance for some experienced technical users; but due to the option of comments, which are preserved, this isn't problematic; not to mention that they should be able to write their own helpers as they see fit. ¹

Such a trivial improvement isn't worth this much discussion.

Footnotes

1. I think the "list mods" use case should maybe get something like a luanti --mods list CLI API, but I won't implement that. ↩

[mruncreative]

I already posted scripts that list mods and a script that adds disabled mods to world.mt in this issue. They cover those use cases.
Instead we should improve Luanti itself and not make it list empty folders as mods in the main menu for example. That would be a lot more useful. If it doesn't have an init.lua or modpack.conf it should be ignored.