Add a client side mod dialog #16001

ProunceDev · 2025-04-10T19:03:55Z

Using this dialog you have the ability to enable or disable client mods without manually editing the file.

Add compact, short information about your PR for easier understanding:

Goal of the PR
Give players the ability to select csms without manually editing files.
How does the PR work?
It adds a button to the content tab which opens a dialog with a list of csms and the option to enable or
disable them.
Does it resolve any reported issue?
I don't believe so.
Does this relate to a goal in the roadmap?
Yes, I believe its part of the ui improvements category
If not a bug fix, why is this PR needed? What usecases does it solve?
Many times in the past I've had people attempting to use my CSMs unable because they can't find the file to
manually enable mods.

To do

This PR is a Ready for Review.

How to test

Simply compile this PR, go to the main menu, open the content tab, and click the Client side mods button

Using this dialog you have the ability to enable or disable client mods without manually editing the file.

builtin/mainmenu/dlg_csm.lua

appgurueu · 2025-04-10T22:44:06Z

Note that the future of CSMs is somewhat uncertain (they have been perpetually experimental since their introduction and have not seen any significant updates in recent years), so I'd be wary of spending lots of time on them.

Personally I'm not very interested in advancing them at the moment, especially given that SSCSM will be designed independently from the ground up.

ProunceDev · 2025-04-11T13:52:48Z

Note that the future of CSMs is somewhat uncertain (they have been perpetually experimental since their introduction and have not seen any significant updates in recent years), so I'd be wary of spending lots of time on them.

Personally I'm not very interested in advancing them at the moment, especially given that SSCSM will be designed independently from the ground up.

Since they exist in the game, a simple button like this shouldn't be an issue imo, we can always remove it easily later, but its already been quite a few years with the csms in this middle state, if yall aint gonna finish them then atleast let us make them slightly more useable

ProunceDev · 2025-04-17T04:16:34Z

Any updates on this?

SmallJoker

I like the concept. Let's unify some code to reduce long-term maintenance efforts and we're good to go. Even though the fate of CSM's is uncertain, I believe this is a helpful dialogue for the days to come.

SmallJoker · 2025-04-19T08:51:07Z

builtin/mainmenu/content/pkgmgr.lua

+	for i=1,#clientmods,1 do
+		clientmods[i].type = "mod"
+		clientmods[i].loc = "global"
+		clientmods[i].is_clientside = true
+		retval[#retval + 1] = clientmods[i]
+	end


Tables are references, thus this shorter version is possible:

Suggested change

for i=1,#clientmods,1 do

clientmods[i].type = "mod"

clientmods[i].loc = "global"

clientmods[i].is_clientside = true

retval[#retval + 1] = clientmods[i]

end

for _, mod in ipairs(clientmods) do

mod.type = "mod"

mod.loc = "global"

mod.is_clientside = true

end

retval = clientmods

The retval variable becomes unnecessary, does it not?

Yes, by how it's currently used, it's a duplicate.

SmallJoker · 2025-04-19T08:53:26Z

builtin/mainmenu/content/pkgmgr.lua

+			if element ~= nil then
+				element.enabled = value ~= "false" and value ~= "nil" and value
+			else
+				core.log("info", "Clientmod: " .. key .. " " .. dump(value) .. " but not found")


Suggested change

core.log("info", "Clientmod: " .. key .. " " .. dump(value) .. " but not found")

core.log("info", "Client Mod: Cannot find " .. key .. ", " .. dump(value))

SmallJoker · 2025-04-19T08:54:23Z

builtin/mainmenu/content/pkgmgr.lua

+	local filename = modpath ..
+				DIR_DELIM .. "mods.conf"
+
+	local conffile = Settings(filename)


Suggested change

local filename = modpath ..

DIR_DELIM .. "mods.conf"

local conffile = Settings(filename)

local conffile = Settings(modpath .. DIR_DELIM .. "mods.conf")

SmallJoker · 2025-04-19T08:55:45Z

builtin/mainmenu/dlg_csm.lua

+--This program is free software; you can redistribute it and/or modify
+--it under the terms of the GNU Lesser General Public License as published by
+--the Free Software Foundation; either version 2.1 of the License, or
+--(at your option) any later version.
+--
+--This program is distributed in the hope that it will be useful,
+--but WITHOUT ANY WARRANTY; without even the implied warranty of
+--MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--GNU Lesser General Public License for more details.
+--
+--You should have received a copy of the GNU Lesser General Public License along
+--with this program; if not, write to the Free Software Foundation, Inc.,
+--51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.


Suggestion: you may use the short SPDX-License-Identifier notation instead to avoid this boilerplate. Ctrl+F in the other code for examples.

SmallJoker · 2025-04-19T08:59:25Z

builtin/mainmenu/dlg_csm.lua

+	end
+
+	if fields.btn_csm_mgr_mp_enable ~= nil or fields.btn_csm_mgr_mp_disable ~= nil then
+		pkgmgr.enable_mod({data = {list = packages, selected_mod = tab_selected_pkg}}, fields.btn_csm_mgr_mp_enable ~= nil)


Lines too long. Please keep them below 95 characters.
https://docs.luanti.org/for-engine-devs/code-style-guidelines/#dont-let-things-get-too-large

SmallJoker · 2025-04-19T09:01:36Z

builtin/mainmenu/dlg_csm.lua

+		return true
+	end
+
+	if fields["btn_csm_mgr_rename_modpack"] ~= nil then


Suggested change

if fields["btn_csm_mgr_rename_modpack"] ~= nil then

if fields.btn_csm_mgr_rename_modpack then

The same for the other occurrences. It makes the code look more consistent. Currently you're mixing two different styles.

SmallJoker · 2025-04-19T09:04:03Z

builtin/mainmenu/dlg_csm.lua

+				if modname_valid(mod.name) then
+					conffile:set("load_mod_" .. mod.name,
+						mod.enabled and "true" or "false")
+				elseif mod.enabled then
+					gamedata.errormessage = fgettext_ne("Failed to enable clientmod" ..
+						" \"$1\" as it contains disallowed characters. " ..
+						"Only characters [a-z0-9_] are allowed.",
+						mod.name)
+				end
+				mods["load_mod_" .. mod.name] = nil


Duplicate of dlg_config_world.lua. Please re-use existing code. This dialogue already mentions that it's about Client Mods, thus the error message (Failed to enable mod [...] (that is used there) should be good enough.

Example:

function pkgmgr.foobar_insert_my_mods(mods_ret, packages, conffile, cb_may_add) for _, mod in ipairs(packages) do if cb_may_add(mod) then -- same code here end end end -- for CSM: pkgmgr.foobar_insert_my_mods(mods, packages_raw, conffile, function(mod) return mod.is_clientside end -- for Server Mods: (dlg_config_world.lua) pkgmgr.foobar_insert_my_mods(mods, packages_raw, conffile, function(mod) return not mod.is_modpack and not mod.is_game_content end

appgurueu · 2025-04-19T13:31:52Z

builtin/mainmenu/content/pkgmgr.lua

+	local conffile = Settings(filename)
+
+	for key,value in pairs(conffile:to_table()) do
+		if key:sub(1, 9) == "load_mod_" then


You can do

local mod_name = key:match("^load_mod_(.+)") if mod_name then ... end

appgurueu · 2025-04-19T13:33:26Z

builtin/mainmenu/content/pkgmgr.lua

+		if key:sub(1, 9) == "load_mod_" then
+			key = key:sub(10)
+			local element = nil
+			for i=1,#retval,1 do


Bad style. Just do something like

Suggested change

for i=1,#retval,1 do

for _, client_mod in ipairs(client_mods) do

and give element a better name (such as found_client_mod or whatever).

though really i feel you should have client mods indexed in a table by name. then you wouldn't need a linear search here - the code would be cleaner.

appgurueu · 2025-04-19T13:33:50Z

builtin/mainmenu/content/pkgmgr.lua

+			local element = nil
+			for i=1,#retval,1 do
+				if retval[i].name == key and
+					not retval[i].is_modpack then


this can go on the same line. if you do need continuation lines, indent them by two tabs so they don't blend in with the next line.

appgurueu · 2025-04-19T13:34:02Z

builtin/mainmenu/content/pkgmgr.lua

+					break
+				end
+			end
+			if element ~= nil then


Suggested change

if element ~= nil then

if element then

appgurueu · 2025-04-19T13:36:20Z

builtin/mainmenu/content/pkgmgr.lua

+				end
+			end
+			if element ~= nil then
+				element.enabled = value ~= "false" and value ~= "nil" and value


Suggested change

element.enabled = value ~= "false" and value ~= "nil" and value

element.enabled = value ~= "false" and value ~= "nil"

i don't see and value adding any value, it just creates type confusion: the value is a string and thus necessarily truthy.

appgurueu · 2025-04-19T13:37:50Z

builtin/mainmenu/dlg_csm.lua

+		local conffile = Settings(filename)
+		local mods = conffile:to_table()
+
+		for i = 1, #packages_raw do


Suggested change

for i = 1, #packages_raw do

for _, mod in ipairs(packages_raw) do

but given the name of packages_raw, wouldn't pkg be a better name (it looks like this also includes things like modpacks)?

appgurueu · 2025-04-19T13:38:42Z

builtin/mainmenu/dlg_csm.lua

+
+		-- Remove mods that are not present anymore
+		for key in pairs(mods) do
+			if key:sub(1, 9) == "load_mod_" then


Suggested change

if key:sub(1, 9) == "load_mod_" then

if key:find("^load_mod_") then

appgurueu · 2025-04-19T13:39:05Z

builtin/mainmenu/dlg_csm.lua

+		end
+	end
+
+	if tab_selected_pkg == nil then


Suggested change

if tab_selected_pkg == nil then

tab_selected_pkg = tab_selected_pkg or 1

then delete the rest of the if

appgurueu · 2025-04-19T13:40:30Z

builtin/mainmenu/dlg_csm.lua

+
+	if selected_pkg ~= nil then
+		if selected_pkg.type == "mod" then
+			if selected_pkg.is_clientside then


unnecessary if pyramid. just do if selected_pkg and selected_pkg.type == "mod" and selected_pkg.is_clientside then (wrap as needed).

appgurueu · 2025-04-19T13:41:16Z

builtin/mainmenu/dlg_csm.lua

+	return table.concat(fs, "")
+end
+
+--------------------------------------------------------------------------------


Suggested change

--------------------------------------------------------------------------------

let's not do these anymore, same below.

appgurueu · 2025-04-19T13:41:56Z

builtin/mainmenu/dlg_csm.lua

+		return true
+	end
+
+	if fields.btn_csm_mgr_mp_enable ~= nil or fields.btn_csm_mgr_mp_disable ~= nil then


Suggested change

if fields.btn_csm_mgr_mp_enable ~= nil or fields.btn_csm_mgr_mp_disable ~= nil then

if fields.btn_csm_mgr_mp_enable or fields.btn_csm_mgr_mp_disable then

appgurueu · 2025-04-19T13:42:29Z

builtin/mainmenu/dlg_csm.lua

+		return true
+	end
+
+	if fields.btn_csm_mgr_enable_mod ~= nil or fields.btn_csm_mgr_disable_mod ~= nil then


Suggested change

if fields.btn_csm_mgr_enable_mod ~= nil or fields.btn_csm_mgr_disable_mod ~= nil then

if fields.btn_csm_mgr_enable_mod or fields.btn_csm_mgr_disable_mod then

grorp · 2025-04-26T18:59:33Z

The button should not be shown unless the user has already enabled CSMs in the settings.
Ideally, the dialog would also mention the experimental state of CSMs.

Previously: #10039

Also, the new button's location results in the "Browse online content" button becoming too small for its label - but that doesn't matter much as long as its hidden by default.

ProunceDev added 2 commits April 10, 2025 13:56

Add a client side mod dialog

9d056c4

Using this dialog you have the ability to enable or disable client mods without manually editing the file.

Fix double paste and whitespace

b076f03

sfence reviewed Apr 10, 2025

View reviewed changes

builtin/mainmenu/dlg_csm.lua Outdated Show resolved Hide resolved

Fix Minetest instead of Luanti in copyright

fe5a90b

wsor4035 added @ Mainmenu @ Client Script API labels Apr 10, 2025

appgurueu mentioned this pull request Apr 11, 2025

Clarify fate of CSMs #16005

Open

SmallJoker reviewed Apr 19, 2025

View reviewed changes

SmallJoker added the Feature ✨ PRs that add or enhance a feature label Apr 19, 2025

appgurueu reviewed Apr 19, 2025

View reviewed changes

sfan5 added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Apr 26, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a client side mod dialog #16001

Add a client side mod dialog #16001

ProunceDev commented Apr 10, 2025

appgurueu commented Apr 10, 2025

ProunceDev commented Apr 11, 2025

ProunceDev commented Apr 17, 2025

SmallJoker left a comment

SmallJoker Apr 19, 2025

appgurueu Apr 19, 2025

SmallJoker Apr 20, 2025

SmallJoker Apr 19, 2025

SmallJoker Apr 19, 2025

SmallJoker Apr 19, 2025

SmallJoker Apr 19, 2025

SmallJoker Apr 19, 2025

SmallJoker Apr 19, 2025 •

edited

Loading

appgurueu Apr 19, 2025

appgurueu Apr 19, 2025

appgurueu Apr 19, 2025

appgurueu Apr 19, 2025

appgurueu Apr 19, 2025

appgurueu Apr 19, 2025

appgurueu Apr 19, 2025

appgurueu Apr 19, 2025 •

edited

Loading

appgurueu Apr 19, 2025

appgurueu Apr 19, 2025

appgurueu Apr 19, 2025

appgurueu Apr 19, 2025

grorp commented Apr 26, 2025

	core.log("info", "Clientmod: " .. key .. " " .. dump(value) .. " but not found")
	core.log("info", "Client Mod: Cannot find " .. key .. ", " .. dump(value))

	if fields["btn_csm_mgr_rename_modpack"] ~= nil then
	if fields.btn_csm_mgr_rename_modpack then

	for i=1,#retval,1 do
	for _, client_mod in ipairs(client_mods) do

	element.enabled = value ~= "false" and value ~= "nil" and value
	element.enabled = value ~= "false" and value ~= "nil"

	for i = 1, #packages_raw do
	for _, mod in ipairs(packages_raw) do

	if key:sub(1, 9) == "load_mod_" then
	if key:find("^load_mod_") then

	if tab_selected_pkg == nil then
	tab_selected_pkg = tab_selected_pkg or 1

	if fields.btn_csm_mgr_mp_enable ~= nil or fields.btn_csm_mgr_mp_disable ~= nil then
	if fields.btn_csm_mgr_mp_enable or fields.btn_csm_mgr_mp_disable then

	if fields.btn_csm_mgr_enable_mod ~= nil or fields.btn_csm_mgr_disable_mod ~= nil then
	if fields.btn_csm_mgr_enable_mod or fields.btn_csm_mgr_disable_mod then

Add a client side mod dialog #16001

Are you sure you want to change the base?

Add a client side mod dialog #16001

Conversation

ProunceDev commented Apr 10, 2025

To do

How to test

appgurueu commented Apr 10, 2025

ProunceDev commented Apr 11, 2025

ProunceDev commented Apr 17, 2025

SmallJoker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SmallJoker Apr 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

appgurueu Apr 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grorp commented Apr 26, 2025

SmallJoker Apr 19, 2025 •

edited

Loading

appgurueu Apr 19, 2025 •

edited

Loading