Add Settings to in-game menu, and rewrite pause menu in Lua #14785

swagtoy · 2024-06-26T23:41:26Z

#14781 was an utter failure, so I'm following up with a new PR that:

Moves the pause menu formspec into Lua
Doesn't require any C++ rewriting of the settings menu now, as requested.

Also will add settings to the in-game menu (i haven't done it yet, but hold on a moment).

As my old PR said:

In the past, you need to quit the current game to change settings. This is tedious, especially when on servers where you need to re-enter your password. It's also frustrating as you can't fine-tune settings such as FOV.

Also, we can probably remove the "Sound" subsection. We could also move all the "Controls" configuration to this settings menu (which also lets us tweak controls on the main menu.. 2 birds dead with 1 stone!)

Not everything here is sunshines. The first case is removing client->modsLoaded from game.cpp. My change will likely force enable client modding, removing the need for a setting such as enable_client_modding. Will there be any side effects of enabling client side modding by default?

Hoping this really fixes #6722, #3165, and #6922.

There is also an opportunity to cleanup the pause menu, perhaps giving the controls text a much-needed facelift.

There was some mention on IRC of letting mods add new buttons, in a similar fashion to how sfinv works. I really like this idea and I'd be happy to implement it with no additional cost.

How to test

In minetest.conf, set enable_client_modding to true (this wont be the case later?)
Start a world
Press ESC

Also exposes Lua's io functions to the client

Probably should add a method for changing the FOV but I am tired. This also works with /set fov ###, so not sure why something like this isn't done already.

swagtoy · 2024-06-27T06:43:53Z

FOV changes are applied.

Some shader settings don't apply obviously, but the majority of things i've tested seem to work 🙂

I'd like to move my formspec events to Lua though, but I couldn't figure out how to create hooks.

grorp · 2024-06-27T12:19:54Z

Here's a version of rollerozxa's POC for the same thing, in case it's useful: grorp@a336186 and grorp@5600624

This is really hacky, but it demonstrates that everything works (kinda)

You can force reload graphics by showing the verbose debug page (press F5 a few times). It might be better to bind it to a specific keybind, but this was easier to implement and didn't require taking up another keybind.

…wireframe debug instead Still need to nuke the old FSTK settings pages and fix the main menu

sfan5 · 2024-06-28T20:44:12Z

src/script/scripting_client.cpp

@@ -85,6 +87,9 @@ void ClientScripting::InitializeModApi(lua_State *L, int top)
 	ModApiChannels::Initialize(L, top);
 	ModApiParticlesLocal::Initialize(L, top);
 	ModApiClientSound::Initialize(L, top);
+
+	ModApiMainMenu::Initialize(L, top);


the main menu api is completely unsecured, this is a no-go

Will be removed soon. I don't believe this is needed.

sfan5 · 2024-06-28T20:44:32Z

src/script/lua_api/l_client.cpp

+	NO_MAP_LOCK_REQUIRED;
+
+	std::string path = porting::path_share + DIR_DELIM + "builtin" + DIR_DELIM;
+	lua_pushstring(L, path.c_str());


leaks user paths to the secure client environment

sfan5 · 2024-06-28T20:45:09Z

src/script/lua_api/l_client.cpp

+{
+	g_gamecallback->exitToOS();
+#ifndef __ANDROID__
+	RenderingEngine::get_raw_device()->closeDevice();


pretty sure the relevant code already calls this, why is this here?

You mean the android snippet?

it wasnt my code.

I agree its weird. I can snip it out though.

sfan5 · 2024-06-28T20:45:18Z

src/script/lua_api/l_client.cpp

+{
+	g_gamecallback->unpause();
+	//lua_pushboolean(L, true);
+	return 1;


incorrect return

sfan5 · 2024-06-28T20:46:11Z

src/script/cpp_api/s_security.cpp

@@ -358,6 +367,13 @@ void ScriptApiSecurity::initializeSecurityClient()
 	copy_safe(L, os_whitelist, sizeof(os_whitelist));
 	lua_setfield(L, -3, "os");
 	lua_pop(L, 1);  // Pop old OS
+
+	// Copy safe OS functions


these functions are absolutely not safe and allow unsandboxed access to the client filesystem

Will remove this soon. Dont be fooled by this comment. It should say "Copy safe IO functions"

src/script/cpp_api/s_security.cpp

DragonWrangler1 · 2024-07-05T14:03:22Z

I'll compile and test later.

Edit: Ok. So I compiled it and tested.

First when going into settings if you click Hide: temporary settings. It crashes.
Second. I don't think that you should require client modding, since some servers don't allow that, and the menu won't open without it.

swagtoy · 2024-07-05T15:49:46Z

@sfan5 I'm going to add a new function to just read the file of the settingtypes.txt without needing to expose io. Is this okay to you? Will allow me to remove the insecure exposure.

swagtoy · 2024-07-05T15:50:39Z

First when going into settings if you click Hide: temporary settings. It crashes. Second. I don't think that you should require client modding, since some servers don't allow that, and the menu won't open without it.

There's no other way for me to rewrite this in Lua without enabling client modding, I think. The only other way would be to create an entirely new Lua context. Or to lie to the server...

First when going into settings if you click Hide: temporary settings. It crashes.

Thanks. I think there are a few ways to crash the game. I'll have to mess more with it.

swagtoy · 2024-10-06T04:57:07Z

@grorp Again...

grorp · 2024-10-06T07:43:28Z

I'll steal an old comment to explain:

WIP pull requests without progress or need for discussion are typically closed after some time. I'm just marking it so we don't forget to close it if nothing new happens.
#11470 (comment)

EDIT: Discovered that going strictly by the rules, there are still 3 months left, so removing the label again.

swagtoy · 2024-10-06T15:18:51Z

@sfan5 Hey, i discussed it on mt dev irc, but should we add a new security layer for the main menu or the "game", i suppose? maybe even separate some bits of code? This will help me stub some work for the new main menu PRs, and will prevent me from writing crappy functions and exposing them on the server side to say, get the users settings and junk.

it needs to be done at some point if we want to switch to offering game-specific customization of the main menus (discussed with greenxenith over discord unfortunately) without needing to hack over security. It would help this PR too. Was even thinking of providing an extra flag to CSM or something to make a client side mod "menu safe"

sfan5 · 2024-10-06T16:16:56Z

@sfan5 I'm going to add a new function to just read the file of the settingtypes.txt without needing to expose io. Is this okay to you?

Sure

but should we add a new security layer for the main menu or the "game", i suppose?

CSM already has a security layer and I don't see how the mainmenu is relevant here.

swagtoy · 2024-10-06T16:30:08Z

@sfan5 I'm going to add a new function to just read the file of the settingtypes.txt without needing to expose io. Is this okay to you?

Sure

but should we add a new security layer for the main menu or the "game", i suppose?

CSM already has a security layer and I don't see how the mainmenu is relevant here.

see this PR and how I've had to break security to get it to work

soon people will download main menus from games which are pretty insecure atm i think

sfan5 · 2024-10-06T16:37:38Z

and that is related how exactly?

swagtoy · 2024-10-06T16:50:50Z

Oh you replied to another thing, my bad.

I just wanted to know if we should make a new security layer that is more secure than the Client layer because we are going to let games customize the pause menu and main menu. Games you can download from the CDB

sfan5 · 2024-10-06T21:10:19Z

Perspectively we should have main and pause menu customization but it's too early to say how the implementation will look.

swagtoy · 2024-10-07T03:06:29Z

Right. Im getting ahead of myself. The only way to do this correctly without breaking security on the client side would be to just offer lua methods to return the games settings.. preferably as a table already, instead of doing it by hand like its being done. Im not a fan of how we change settings on the client game code, like fov and such, but it is what it is.

…

On Sun, Oct 6, 2024, 5:10 PM sfan5 ***@***.***> wrote: Perspectively we should have main and pause menu customization but it's too early to say how the implementation will look. — Reply to this email directly, view it on GitHub <#14785 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AY22OG4MC6MLDVXCC6YBMNLZ2GRNBAVCNFSM6AAAAABJ62LMFCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJVGU4DMNBWG4> . You are receiving this because you authored the thread.Message ID: ***@***.***>

Initial pause menu in Lua

9ebd676

swagtoy marked this pull request as draft June 26, 2024 23:41

swagtoy added 5 commits June 26, 2024 23:02

Finish pause menu

8db7485

Hackishly show settings dialog

ebeb998

Also exposes Lua's io functions to the client

Actually set settings

a13cf23

Switch between pages

96ea24f

Live FOV changes

1f52edb

Probably should add a method for changing the FOV but I am tired. This also works with /set fov ###, so not sure why something like this isn't done already.

swagtoy mentioned this pull request Jun 27, 2024

Add Settings to in-game menu, and rewrite it in C++ #14781

Closed

swagtoy added 3 commits June 27, 2024 10:58

Move pause formspec events to Lua

4b74e8d

Settings formspec demonstration

b8959a0

This is really hacky, but it demonstrates that everything works (kinda)

Reload graphics & dynamic shadows

b2792e5

You can force reload graphics by showing the verbose debug page (press F5 a few times). It might be better to bind it to a specific keybind, but this was easier to implement and didn't require taking up another keybind.

Zughy added Feature ✨ PRs that add or enhance a feature @ Builtin Concept approved Approved by a core dev: PRs welcomed! UI/UX labels Jun 28, 2024

swagtoy added 3 commits June 28, 2024 16:22

Refactor settings to not depend on FSTK, move reloadGraphics call to …

5de3ebf

…wireframe debug instead Still need to nuke the old FSTK settings pages and fix the main menu

Get rid of old formspecs

1c3afe6

Fix ugly background

7fca09f

sfan5 requested changes Jun 28, 2024

View reviewed changes

swagtoy mentioned this pull request Sep 12, 2024

Rework guiKeyChangeMenu #15152

Open

8 tasks

grorp added the Possible close label Oct 5, 2024

grorp removed the Possible close label Oct 6, 2024

swagtoy mentioned this pull request Nov 3, 2024

[no squash] Refactor & enable Lua sandbox in main menu #15380

Merged

This was referenced Jan 1, 2025

Add more settings to the pause menu. #6722

Closed

In-game settings menu using separate Lua env #15614

Merged

rubenwardy closed this Jan 2, 2025

grorp mentioned this pull request Jan 22, 2025

"Settings in the pause menu" to-do list #15701

Open

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Settings to in-game menu, and rewrite pause menu in Lua #14785

Add Settings to in-game menu, and rewrite pause menu in Lua #14785

swagtoy commented Jun 26, 2024 •

edited

Loading

swagtoy commented Jun 27, 2024

grorp commented Jun 27, 2024

sfan5 Jun 28, 2024

swagtoy Jul 5, 2024 •

edited

Loading

sfan5 Jun 28, 2024

sfan5 Jun 28, 2024

swagtoy Jul 5, 2024

sfan5 Oct 6, 2024

swagtoy Oct 7, 2024

sfan5 Jun 28, 2024

sfan5 Jun 28, 2024

swagtoy Jul 5, 2024 •

edited

Loading

DragonWrangler1 commented Jul 5, 2024 •

edited

Loading

swagtoy commented Jul 5, 2024

swagtoy commented Jul 5, 2024 •

edited

Loading

swagtoy commented Oct 6, 2024

grorp commented Oct 6, 2024 •

edited

Loading

swagtoy commented Oct 6, 2024 •

edited

Loading

sfan5 commented Oct 6, 2024

swagtoy commented Oct 6, 2024 •

edited

Loading

sfan5 commented Oct 6, 2024

swagtoy commented Oct 6, 2024

sfan5 commented Oct 6, 2024

swagtoy commented Oct 7, 2024 via email

Add Settings to in-game menu, and rewrite pause menu in Lua #14785

Add Settings to in-game menu, and rewrite pause menu in Lua #14785

Conversation

swagtoy commented Jun 26, 2024 • edited Loading

How to test

swagtoy commented Jun 27, 2024

grorp commented Jun 27, 2024

Choose a reason for hiding this comment

swagtoy Jul 5, 2024 • 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

swagtoy Jul 5, 2024 • edited Loading

Choose a reason for hiding this comment

DragonWrangler1 commented Jul 5, 2024 • edited Loading

swagtoy commented Jul 5, 2024

swagtoy commented Jul 5, 2024 • edited Loading

swagtoy commented Oct 6, 2024

grorp commented Oct 6, 2024 • edited Loading

swagtoy commented Oct 6, 2024 • edited Loading

sfan5 commented Oct 6, 2024

swagtoy commented Oct 6, 2024 • edited Loading

sfan5 commented Oct 6, 2024

swagtoy commented Oct 6, 2024

sfan5 commented Oct 6, 2024

swagtoy commented Oct 7, 2024 via email

swagtoy commented Jun 26, 2024 •

edited

Loading

swagtoy Jul 5, 2024 •

edited

Loading

swagtoy Jul 5, 2024 •

edited

Loading

DragonWrangler1 commented Jul 5, 2024 •

edited

Loading

swagtoy commented Jul 5, 2024 •

edited

Loading

grorp commented Oct 6, 2024 •

edited

Loading

swagtoy commented Oct 6, 2024 •

edited

Loading

swagtoy commented Oct 6, 2024 •

edited

Loading