Add support for client_mods folder + allow clientside mods to edit safe tbl files #269

GooberRF · 2024-09-09T03:09:51Z

I've revamped this PR extensively since it was first put in. All the changes are detailed below, and I've also commented the code as much as possible.
Resolves #63
Resolves #62

Functionality:

Add support for a "client_mods" folder in the root RF directory. This folder is intended to be used by client_side mods, as opposed to the current/historical approach of using user_maps folders (which are supposed to be for levels) being used for this purpose.
Renamed the user_maps switch in the launcher, revised the tooltip, and made it effectively serve the role of a "legacy compatibility" switch. Most users should leave this unticked (default), but if someone wants to use the historical approach of dropping clientside mods in user_maps, they can turn it on (as they have to already).
Added a tbl file allow list which allows clientside mods to override tbl files that cannot be used for cheating.

This is a pretty extensive change. I've done a huge amount of testing of various scenarios and from everything I have seen it works absolutely perfectly as-is, but especially given how extensive it is, I would really appreciate some extra eyes on it so any potential regressions or odd behaviours are identified and mitigated before it's merged. The most extensive changes are:

Overhauled the asset override logic entirely (is_lookup_table_entry_override_allowed in vpackfile.cpp)
Rewrote load_additional_packfiles function as load_additional_packfiles_new in vpackfile.cpp

Main benefits of this change are:

Allows custom HUDs which adjust the positions of HUD elements. Currently such mods must be run as TC mods, after this change they can be run as clientside mods.
Allows translation mods (which edit strings.tbl, etc.) to be packaged as clientside mods, enhancing RF's accessibility to non-English speaking people (since they can play multiplayer with everyone else while having their translation mod loaded).
Having a separate folder for client_mods makes more logical sense, is nicer for organization, and means people can use the launcher user_maps switch to avoid unintentionally overriding core game assets without impacting their ability to use clientside mods intentionally.

tbl files that as of this PR will be able to be changed via client_side mods are:

Anything that ends with _text.tbl (HUD message config files)
strings.tbl
hud.tbl
hud_personas.tbl
personas.tbl
credits.tbl
endgame.tbl
ponr.tbl

…, populate with 7 tbl files that should be safe to mod without cheat potential

…game files" option

…d feature)

game_patch/misc/vpackfile.cpp

GooberRF · 2024-09-19T19:48:05Z

Made some significant updates to the PR, all detailed in the details in the first post above.

To note, this is a pretty extensive change. I've done a huge amount of testing of various scenarios and from everything I have seen it works absolutely perfectly as-is, but especially given how extensive it is, I would really appreciate some extra eyes on it so any potential regressions or odd behaviours are identified and mitigated before it's merged.

rafalh · 2024-12-10T21:58:22Z

game_patch/misc/vpackfile.cpp

+static bool is_tbl_file(const char* filename)
+{
+    // confirm we're working with a tbl file
+    if (stricmp(rf::file_get_ext(filename), ".tbl") == 0) {


using string_ends_with_ignore_case would probably be better

rafalh · 2024-12-10T21:58:47Z

game_patch/misc/vpackfile.cpp

+static bool is_tbl_file_a_hud_messages_file(const char* filename)
+{
+    // check if the input file ends with "_text.tbl"
+    if (strlen(filename) >= 9 && stricmp(filename + strlen(filename) - 9, "_text.tbl") == 0) {


using string_ends_with_ignore_case is really recommended here

rafalh · 2024-12-10T21:59:21Z

game_patch/misc/vpackfile.cpp

+           });
+}
+
+static bool is_tbl_file_a_hud_messages_file(const char* filename)


is_level_text_tbl_file? This feature is called level text in the code

rafalh · 2024-12-10T22:00:50Z

game_patch/misc/vpackfile.cpp

@@ -179,6 +222,7 @@ static int vpackfile_add_new(const char* filename, const char* dir)
        xlog::error("Failed to open packfile {}", full_path);
        return 0;
    }
+    xlog::info("Opened packfile {}", full_path);


this looks like something spammy

rafalh · 2024-12-10T22:02:32Z

game_patch/misc/vpackfile.cpp

-    packfile->is_user_maps = rf::vpackfile_loading_user_maps;
+    // packfile->is_user_maps = rf::vpackfile_loading_user_maps; // this is set to true for user_maps
+    // check for is_user_maps based on dir (like is_client_mods) instead of 0x01BDB21C
+    packfile->is_user_maps = (dir && (stricmp(dir, "user_maps\\multi\\") == 0 || stricmp(dir, "user_maps\\single\\") == 0));


you should prepend std functions with std::. Otherwise if you don't depend on string.h instead of cstring and not use use namespace std it is undefined behavior whether those functions will be available in global namespace

rafalh · 2024-12-10T22:05:22Z

game_patch/misc/vpackfile.cpp

    }
+    // resolve warning by having a default option, even though there should be no way to hit it


to be honest this function got very complicated. Not sure if we want to handle so many corner cases distinctly. Why do we even handle tbl mods in user_maps if we add client_mods? What's the point?

Why do we even handle tbl mods in user_maps if we add client_mods

That's a valid point. My thinking was that if we are supporting clientside mods in user_maps effectively as a compatibility switch to restore the legacy behaviour, it would be a little weird for some clientside mods to then be loaded from user_maps but not all.

That being said, loading the safe tbls as a clientside mod is new behaviour with this PR as well, so if it made sense for any clientside mods to be only usable from client_mods, it would be these. I don't know, though. I could go either way on that. Maybe it would be best to not handle safe tbl clientside mods from user_maps because it would reduce code complexity and probably no one would expect it to have worked that way anyway?

rafalh · 2024-12-10T22:07:37Z

game_patch/misc/vpackfile.cpp

@@ -529,6 +613,47 @@ static void vpackfile_cleanup_new()
    g_packfiles.clear();
 }

+static void load_vpp_files_from_directory(const char* files, const char* directory)


first argument name is not very clear. Also someone calling this function must know to use *.vpp pattern and not like *.* or it will try to load non-vpp files as vpps, despite the function claiming to load only vpps from directory. Also directory name is repeated in both args. A clearer API would be to build to build the pattern in the function using directory argument

rafalh · 2024-12-10T22:09:12Z

game_patch/misc/vpackfile.cpp

+    if (tc_mod_is_loaded()) {
+        std::string mod_name = rf::mod_param.get_arg();
+        std::string mod_dir = "mods\\" + mod_name + "\\";
+        std::string mod_file = mod_dir + "*.vpp";


more like mod_vpp_pattern

rafalh · 2024-12-10T22:09:30Z

game_patch/rf/file/file.h

@@ -98,6 +98,7 @@ namespace rf

    static auto& file_get_ext = addr_as_ref<char*(const char *path)>(0x005143F0);
    static auto& file_add_path = addr_as_ref<int(const char *path, const char *exts, bool search_on_cd)>(0x00514070);
+    //static auto& game_add_path = addr_as_ref<int(const char* path, const char* exts)>(0x004B1330);


to be deleted?

rafalh · 2024-12-10T22:09:42Z

game_patch/rf/file/packfile.h

@@ -46,6 +48,6 @@ namespace rf
    using VPackfileAddEntries_Type = uint32_t(VPackfile* packfile, const void* buf, unsigned num_files_in_block,
                                              unsigned* num_added);
    static auto& vpackfile_add_entries = addr_as_ref<VPackfileAddEntries_Type>(0x0052BD40);
-
-    static auto& vpackfile_loading_user_maps = addr_as_ref<bool>(0x01BDB21C);
+    // handled directly in vpackfile_add_new


to be deleted?

GooberRF added 6 commits September 9, 2024 00:24

Add tbl file allow list, allow loading of list members from user_maps…

c8011f5

…, populate with 7 tbl files that should be safe to mod without cheat potential

Make tbl file allow list respect player turning off "allow overwrite …

aff7c24

…game files" option

Fix potential future issue with mod_file_whitelist (currently disable…

ecead8c

…d feature)

Merge branch 'master' into usermapstbls

ff56180

Merge branch 'rafalh:master' into usermapstbls

4a677c7

Merge branch 'rafalh:master' into usermapstbls

59fe417

GooberRF marked this pull request as draft September 18, 2024 01:19

is-this-c reviewed Sep 18, 2024

View reviewed changes

game_patch/misc/vpackfile.cpp Outdated Show resolved Hide resolved

is-this-c reviewed Sep 18, 2024

View reviewed changes

game_patch/misc/vpackfile.cpp Outdated Show resolved Hide resolved

GooberRF added 2 commits September 19, 2024 16:40

support client_mods directory, revamp replacement allowance order

636fa8a

update changelog

3350c6b

GooberRF changed the title ~~Allow clientside mods to edit some safe tbl files (for translation and HUD mods)~~ Add support for client_mods folder + allow clientside mods to edit safe tbl files Sep 19, 2024

GooberRF marked this pull request as ready for review September 19, 2024 19:48

is-this-c mentioned this pull request Sep 22, 2024

Add TC mod name to main menu #277

Merged

GooberRF added 2 commits September 22, 2024 15:46

use std::aray to fix use after free

84534ef

Merge branch 'rafalh:master' into usermapstbls

5e8f9a1

GooberRF mentioned this pull request Oct 21, 2024

Expose hardcoded options to mod developers via dashoptions.tbl file #279

Closed

rafalh requested changes Dec 10, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for client_mods folder + allow clientside mods to edit safe tbl files #269

Add support for client_mods folder + allow clientside mods to edit safe tbl files #269

GooberRF commented Sep 9, 2024 •

edited

Loading

GooberRF commented Sep 19, 2024

rafalh Dec 10, 2024

rafalh Dec 10, 2024

rafalh Dec 10, 2024

rafalh Dec 10, 2024

rafalh Dec 10, 2024

rafalh Dec 10, 2024

GooberRF Dec 11, 2024

rafalh Dec 10, 2024

rafalh Dec 10, 2024

rafalh Dec 10, 2024

rafalh Dec 10, 2024

		}
		// resolve warning by having a default option, even though there should be no way to hit it

Add support for client_mods folder + allow clientside mods to edit safe tbl files #269

Are you sure you want to change the base?

Add support for client_mods folder + allow clientside mods to edit safe tbl files #269

Conversation

GooberRF commented Sep 9, 2024 • edited Loading

GooberRF commented Sep 19, 2024

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GooberRF commented Sep 9, 2024 •

edited

Loading