Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
2 changes: 2 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ Version 1.9.0 (not released yet)
- Add Kill Reward settings for dedicated servers
- Do not load unnecessary VPPs in dedicated server mode
- Add level filename to "Level Initializing" console message
- Allow clientside mods to edit table files that can't be used to cheat (strings, hud, hud_personas, personas, credits, endgame, ponr)
- Add support for `client_mods` folder for loading clientside mods and made launcher switch restore legacy behavior
- Properly handle WM_PAINT in dedicated server, may improve performance (DF bug)

Version 1.8.0 (released 2022-09-17)
Expand Down
7 changes: 6 additions & 1 deletion game_patch/misc/misc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ bool g_in_mp_game = false;
bool g_jump_to_multi_server_list = false;
std::optional<JoinMpGameData> g_join_mp_game_seq_data;

bool tc_mod_is_loaded()
{
return rf::mod_param.found();
}

CodeInjection critical_error_hide_main_wnd_patch{
0x0050BA90,
[]() {
Expand Down Expand Up @@ -381,7 +386,7 @@ CodeInjection vfile_read_stack_corruption_fix{
CodeInjection game_set_file_paths_injection{
0x004B1810,
[]() {
if (rf::mod_param.found()) {
if (tc_mod_is_loaded()) {
std::string mod_dir = "mods\\";
mod_dir += rf::mod_param.get_arg();
rf::file_add_path(mod_dir.c_str(), ".bik", false);
Expand Down
1 change: 1 addition & 0 deletions game_patch/misc/misc.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ void start_join_multi_game_sequence(const rf::NetAddr& addr, const std::string&
bool multi_join_game(const rf::NetAddr& addr, const std::string& password);
void ui_get_string_size(int* w, int* h, const char* s, int s_len, int font_num);
void g_solid_render_ui();
bool tc_mod_is_loaded();
148 changes: 137 additions & 11 deletions game_patch/misc/vpackfile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
#include <shlwapi.h>
#include "vpackfile.h"
#include "../main/main.h"
#include "../misc/misc.h"
#include "../rf/file/file.h"
#include "../rf/file/packfile.h"
#include "../rf/crt.h"
#include "../rf/multi.h"
#include "../rf/os/os.h"
#include "../os/console.h"

#define CHECK_PACKFILE_CHECKSUM 0 // slow (1 second on SSD on first load after boot)
Expand Down Expand Up @@ -47,8 +49,11 @@ static bool g_is_overriding_disabled = false;

const char* mod_file_allow_list[] = {
"reticle_0.tga",
"reticle_1.tga",
"scope_ret_0.tga",
"scope_ret_1.tga",
"reticle_rocket_0.tga",
"reticle_rocket_1.tga",
};

static bool is_mod_file_in_whitelist(const char* Filename)
Expand All @@ -61,6 +66,44 @@ static bool is_mod_file_in_whitelist(const char* Filename)

#endif // MOD_FILE_WHITELIST

// allow list of table files that can't be used for cheating, but for which modding in clientside mods has utility
// Examples include translation packs (translated strings, endgame, etc.) and custom HUD mods that change coords of HUD elements
constexpr std::array<std::string_view, 7> tbl_mod_allow_list = {
"strings.tbl",
"hud.tbl",
"hud_personas.tbl",
"personas.tbl",
"credits.tbl",
"endgame.tbl",
"ponr.tbl"
};

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) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using string_ends_with_ignore_case would probably be better

return true;
}
return false;
}

static bool is_tbl_file_in_allowlist(const char* filename)
{
// compare the input file against the tbl file allowlist
return is_tbl_file(filename) && std::ranges::any_of(tbl_mod_allow_list, [filename](std::string_view allowed_tbl) {
return stricmp(allowed_tbl.data(), filename) == 0;
});
}

static bool is_tbl_file_a_hud_messages_file(const char* filename)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_level_text_tbl_file? This feature is called level text in the code

{
// check if the input file ends with "_text.tbl"
if (strlen(filename) >= 9 && stricmp(filename + strlen(filename) - 9, "_text.tbl") == 0) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using string_ends_with_ignore_case is really recommended here

return true;
}
return false;
}

#if CHECK_PACKFILE_CHECKSUM

static unsigned hash_file(const char* Filename)
Expand Down Expand Up @@ -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);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like something spammy


auto packfile = std::make_unique<rf::VPackfile>();
std::strncpy(packfile->filename, filename, sizeof(packfile->filename) - 1);
Expand All @@ -187,8 +231,13 @@ static int vpackfile_add_new(const char* filename, const char* dir)
packfile->path[sizeof(packfile->path) - 1] = '\0';
packfile->field_a0 = 0;
packfile->num_files = 0;
// this is set to true for user_maps
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));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

packfile->is_client_mods = (dir && stricmp(dir, "client_mods\\") == 0);

xlog::debug("Packfile {} is from {}user_maps, {}client_mods", filename, packfile->is_user_maps ? "" : "NOT ",
packfile->is_client_mods ? "" : "NOT ");

// Process file header
char buf[0x800];
Expand Down Expand Up @@ -291,27 +340,62 @@ static int vpackfile_build_file_list_new(const char* ext_filter, char*& filename

static bool is_lookup_table_entry_override_allowed(rf::VPackfileEntry* old_entry, rf::VPackfileEntry* new_entry)
{
// dirs are loaded in this order: base game, user_maps, client_mods, mods
if (g_is_overriding_disabled) {
// Don't allow overriding files after game is initialized because it can lead to crashes
return false;
}
if (!new_entry->parent->is_user_maps) {
// Allow overriding by packfiles from game root and from mods
// Allow overriding by packfiles from game root and from mods (not user_maps or client_mods)
else if (!new_entry->parent->is_user_maps && !new_entry->parent->is_client_mods) {
return true;
}
if (!old_entry->parent->is_user_maps && !stricmp(rf::file_get_ext(new_entry->name), ".tbl")) {
// Always skip overriding tbl files from game by user_maps
return false;
}
#ifdef MOD_FILE_WHITELIST
if (is_mod_file_in_whitelist(new_entry->file_name)) {
else if (is_mod_file_in_whitelist(new_entry->name)) {
// Always allow overriding for specific files
return true;
}
#endif
if (!g_game_config.allow_overwrite_game_files) {
return false;
// Process if in client_mods
else if (new_entry->parent->is_client_mods) {
if (is_tbl_file_in_allowlist(new_entry->name) || is_tbl_file_a_hud_messages_file(new_entry->name)) {
// allow overriding of tbl files from client_mods if they are on allow list or are _text.tbl
g_is_modded_game = true; // goober todo: confirm what this is used for. If anticheat, can remove from here
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is used anywhere currently. But yes, it was supposed to be something related to anticheat until I realized half of the playerbase has mods in user_maps

return true;
}
else if (is_tbl_file(new_entry->name)) {
// deny all other tbl overrides
return false;
}
g_is_modded_game = true;
return true;
}
// Process if in user_maps
else if (new_entry->parent->is_user_maps) {
if (old_entry->parent->is_user_maps) {
// allow files from user_maps to override other files in user_maps, even if the switch is off
// files from user_maps can't override files from client_mods because client_mods is parsed later
return true;
}
else if (!g_game_config.allow_overwrite_game_files)
{
// if the switch is off, no overwriting with files from user_maps, except for other files in user_maps
return false;
}
else if (is_tbl_file_in_allowlist(new_entry->name) || is_tbl_file_a_hud_messages_file(new_entry->name))
{
// allow overriding of tbl files from user_maps if they are on allow list or are _text.tbl
g_is_modded_game = true; // goober todo: confirm what this is used for. If anticheat, can remove from here
return true;
}
else if (is_tbl_file(new_entry->name))
{
// deny all other tbl overrides
return false;
}
g_is_modded_game = true;
return true;
}
// resolve warning by having a default option, even though there should be no way to hit it
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

g_is_modded_game = true;
return true;
}
Expand Down Expand Up @@ -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)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

{
WIN32_FIND_DATA find_file_data;
HANDLE hFind = FindFirstFile(files, &find_file_data);

if (hFind != INVALID_HANDLE_VALUE) {
do {
if (!(find_file_data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) {
// Call vpackfile_add_new function directly
if (vpackfile_add_new(find_file_data.cFileName, directory) == 0) {
xlog::warn("Failed to load additional VPP file: {}", find_file_data.cFileName);
}
}
} while (FindNextFile(hFind, &find_file_data) != 0);
FindClose(hFind);
}
else {
xlog::info("No VPP files found in {}.", directory);
}
}

static void load_additional_packfiles_new()
{
// load VPP files from user_maps\single and user_maps\multi first
load_vpp_files_from_directory("user_maps\\single\\*.vpp", "user_maps\\single\\");
load_vpp_files_from_directory("user_maps\\multi\\*.vpp", "user_maps\\multi\\");

// then load VPP files from client_mods
load_vpp_files_from_directory("client_mods\\*.vpp", "client_mods\\");

// lastly load VPP files from the loaded TC mod if applicable
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";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more like mod_vpp_pattern

xlog::info("Loading packfiles from mod: {}.", mod_name);
//rf::game_add_path(mod_dir.c_str(), ".vpp");
load_vpp_files_from_directory(mod_file.c_str(), mod_dir.c_str());
}
}

void vpackfile_apply_patches()
{
// VPackfile handling implemetation getting rid of all limits
Expand All @@ -539,6 +664,7 @@ void vpackfile_apply_patches()
AsmWriter(0x0052C220).jmp(vpackfile_find_new);
AsmWriter(0x0052BB60).jmp(vpackfile_init_new);
AsmWriter(0x0052BC80).jmp(vpackfile_cleanup_new);
AsmWriter(0x004B15E0).jmp(load_additional_packfiles_new);

// Don't return success from vpackfile_open if offset points out of file contents
vpackfile_open_check_seek_result_injection.install();
Expand Down
1 change: 1 addition & 0 deletions game_patch/rf/file/file.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be deleted?


static auto& root_path = addr_as_ref<char[max_path_len]>(0x018060E8);
}
6 changes: 4 additions & 2 deletions game_patch/rf/file/packfile.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ namespace rf
#endif
uint32_t file_size;
#ifdef DASH_FACTION
// track whether the packfile was from user_maps or client_mods
bool is_user_maps;
bool is_client_mods;
#endif
};
#ifndef DASH_FACTION
Expand All @@ -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
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be deleted?

//static auto& vpackfile_loading_user_maps = addr_as_ref<bool>(0x01BDB21C);
}
2 changes: 1 addition & 1 deletion launcher/DashFactionLauncher.rc
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ STYLE DS_SHELLFONT | WS_CHILD | WS_VISIBLE | DS_CONTROL
FONT 8, "MS Shell Dlg"
BEGIN
AUTOCHECKBOX "Fast Start",IDC_FAST_START_CHECK,0,0,66,10
AUTOCHECKBOX "Allow overriding game files by packages in user_maps",IDC_ALLOW_OVERWRITE_GAME_CHECK,0,12,260,10
AUTOCHECKBOX "Allow clientside mods to be loaded from user_maps",IDC_ALLOW_OVERWRITE_GAME_CHECK,0,12,260,10
AUTOCHECKBOX "Run game at reduced speed when window doesn't have focus",IDC_REDUCED_SPEED_IN_BG_CHECK,0,24,260,10
AUTOCHECKBOX "Beep when another player joins multiplayer game and window doesn't have focus",
IDC_PLAYER_JOIN_BEEP_CHECK,0,36,260,18,BS_AUTOCHECKBOX | BS_MULTILINE | WS_TABSTOP | BS_TOP
Expand Down
2 changes: 1 addition & 1 deletion launcher/OptionsMiscDlg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ void OptionsMiscDlg::InitToolTip()
{
m_tool_tip.Create(*this);
m_tool_tip.AddTool(GetDlgItem(IDC_FAST_START_CHECK), "Skip game intro videos and go straight to Main Menu");
m_tool_tip.AddTool(GetDlgItem(IDC_ALLOW_OVERWRITE_GAME_CHECK), "Enable this if you want to modify game content by putting mods into user_maps folder. Can have side effect of level packfiles modyfing common textures/sounds.");
m_tool_tip.AddTool(GetDlgItem(IDC_ALLOW_OVERWRITE_GAME_CHECK), "Allows files in user_maps folders to override core game files. When left disabled, only files in client_mods can do this. Enabling this can unintentionally mean stock game assets get replaced by installed levels.");
m_tool_tip.AddTool(GetDlgItem(IDC_AUTOSAVE_CHECK), "Automatically save the game after a level transition");
}

Expand Down