[Bug]: Inconsistent behavior of new GameData / LoadGameConfigFile

[caxanga334]

Prerequisites

• I have checked that my issue doesn't exist yet in the issue tracker

Operating System and Version

Windows 10 22H2

Game / AppID and Version

Team Fortress 2 (440) build 9433646

SourceMod Version

1.13.0.7194

Metamod:Source Version

1.12.0-dev+1211

Version Verification

• I have updated SourceMod to the latest version and the issue persists
• I have updated SourceMod to the latest snapshot and the issue persists
• I have updated Metamod:Source to the latest snapshot and the issue persists

Updated SourceMod Version

No response

Updated Metamod:Source Version

No response

Description

The first time a plugin calls new GameData / LoadGameConfigFile it throws an error if the file doesn't exists.

If the plugin is reloaded, then it returns a valid handle (to a gamedata file that doesn't exists).

Steps to Reproduce

Plugin code:

#include <sourcemod>

public void OnPluginStart()
{
	GameData gd = null;

	gd = new GameData("this-does-not-exists");

	if (gd == null)
	{
		PrintToServer("NULL gamedata handle!");
	}
	else
	{
		PrintToServer("Got non NULL gamedata handle!");
		delete gd;
	}
}

1. Start the server with the plugin from above.
2. Wait for the server to load, the plugin should have failed to load due to a missing gamedata file.
3. Reload the plugin with sm plugins reload.
4. See if the plugin failed again.

Relevant Log Output

L 01/24/2025 - 13:54:36: [SM] Error parsing gameconfig file "D:\gameservers\tf2_ds\tf\addons\sourcemod\gamedata\this-does-not-exists.txt":
L 01/24/2025 - 13:54:36: [SM] Error 1 on line 0, col 0: Stream failed to open
L 01/24/2025 - 13:54:36: [SM] Exception reported: Unable to open this-does-not-exists: File could not be opened: The system cannot find the file specified.
L 01/24/2025 - 13:54:36: [SM] Blaming: gd-test.smx
L 01/24/2025 - 13:54:36: [SM] Call stack trace:
L 01/24/2025 - 13:54:36: [SM]   [0] GameData.GameData
L 01/24/2025 - 13:54:36: [SM]   [1] Line 16, D:\gameservers\tf2_ds\tf\addons\sourcemod\scripting\gd-test.sp::OnPluginStart
L 01/24/2025 - 13:54:37: [SM] Unable to load plugin "gd-test.smx": Error detected in plugin startup (see error logs)

sm plugins reload gd-test
Got non NULL gamedata handle!
[SM] Plugin gd-test.smx reloaded successfully.

[peace-maker]

sourcemod/core/logic/GameConfigs.cpp

Lines 1173 to 1186 in 8fc2f39

	if (m_Lookup.retrieve(file, &pConfig))
	{
	bool ret = true;
	time_t modtime = GetFileModTime(file);
	if (pConfig->m_ModTime != modtime)
	{
	pConfig->m_ModTime = modtime;
	ret = pConfig->Reparse(error, maxlength);
	}
	pConfig->AddRef();
	*_pConfig = pConfig;
	return ret;
	}

We should reparse too when pConfig->m_ModTime == 0.

[Kenzzer]

Voicing my opinion on the matter, I think we should embrace that unintended behavior. Throwing on missing gamedata file is a bad idea, and plugins out there already rely on the idea that null return == gamedata not loaded.

As per the documention
GameData A handle to the game config file or null on failure.
With no mention to the exception.

So on top of fixing the loading function, I'd recommend just removing the exception :

	if (!g_GameConfigs.LoadGameConfigFile(filename, &gc, error, sizeof(error)))
	{
-		return pCtx->ThrowNativeError("Unable to open %s: %s", filename, error);
+		return BAD_HANDLE;
	}

[psychonic]

Voicing my opinion on the matter, I think we should embrace that unintended behavior. Throwing on missing gamedata file is a bad idea, and plugins out there already rely on the idea that null return == gamedata not loaded.

As per the documention GameData A handle to the game config file or null on failure. With no mention to the exception.

So on top of fixing the loading function, I'd recommend just removing the exception :

if (!g_GameConfigs.LoadGameConfigFile(filename, &gc, error, sizeof(error)))
{

• return pCtx->ThrowNativeError("Unable to open %s: %s", filename, error);
  

• return BAD_HANDLE;
  

  }

This makes sense to me. Give them a better and consistent chance to handle the the issue, while still throwing later anyway if it's null / INVALID_HANDLE without them handling that case.