Automate Gamedata Reparsing

[Scags]

Allows runtime re-parsing of cached gamedata files. Uses the same mannerisms as sm_reload_databases.

[ramonberrutti]

The reload doesn't guarantee the reuse of the game data.
Most of the extensions initialize at first use.

Can you give a example when can be useful?

[Scags]

This is mainly geared towards plugin-creators that are actively working on something, especially those who are working with some serious memory hacks.

Let's say I'm working on a plugin that does some offset manipulation off of a memory address and my gamedata Offset is 4 bytes short. The only way I can update that Key in the file is to restart the server. Reloading the plugin will not update any new writes to the gamedata file. Another scenario, one I've run into multiple times, is adding a new gamedata-requiring feature to a plugin that already uses a file. Once again, I would have to restart the server to implement it fully.

It's also not common knowledge that gamedata is cached after you load it for the first time. Not to mention a command really beats the hell out of restarting your server.

[peace-maker]

This is cool. A message telling the user, that this doesn't update old users of the gamedata automatically could help avoid confusion in the future.

[Wend4r]

This is strange, I have immediately opens the file and reads the keys without cache uses new GameData() or LoadGameConfigFile(). Checked on Windows

 SourceMod Version Information:
    SourceMod Version: 1.11.0.6623
    SourcePawn Engine: 1.11.0.6623, jit-x86 (build 1.11.0.6623)
    SourcePawn API: v1 = 5, v2 = 12
    Compiled on: Aug 26 2020 11:56:10
    Built from: https://github.com/alliedmodders/sourcemod/commit/ea3f55f0
    Build ID: 6623:ea3f55f0
    http://www.sourcemod.net/

[Scags]

@peace-maker Where should I place that? Should I append it to the description part in DefineCommand?

[peace-maker]

@Scags Yes, and additionally as a message to the console in the command handler using rootmenu->ConsolePrint.

[asherkin]

This is neat, but seems even more of a crutch than the other sm_reload_ commands - how hard would it be to just have it be loaded from disk automatically as required?

[Scags]

Are you talking about some sort of file-watcher? That seems a bit overkill don't you think?

[asherkin]

No, just when loading is requested. Either by always re-parsing from the master file, or storing mtimes for each leaf.

Everything is a lot faster than when these cache-heavy systems were designed, and gamedata doesn't see a lot of inter-plugin sharing (for very good reason) and core's files are verboten.

[Scags]

Right. I was able to get an automated system, though I still think that the command is beneficial. Plugins could be caching their gamedata and retrieving values at runtime; static Handles, etc.

Caching also seems to have a mind of its own (see https://discordapp.com/channels/335290997317697536/335290997317697536/684566494717542442 and https://discordapp.com/channels/335290997317697536/335290997317697536/720173946938589204). So the command could be treated as a last resort or as an alternative depending on plugin complexity.

I also want to correct what I said earlier. The caching process, as I've learned, properly reparses with each new GameData Handle. Though if another plugin tries to reference a gamedata file that is in use, it will not be reparsed and refreshed. In most cases, a plugin restart will refresh the needed data but it's not an uncommon problem to run into instances where currently held data does not reflect what is written into the file.

[Scags]

Nevermind, scratch most everything in my last message. I think I've figured out why gamedata sometimes refuses to recache. When LoadGameConfigFile() or the GameData constructor fails to initialize, it permanently prevents the file from being reparsed. That gamedata file apparently isn't Release()d properly, and is perpetually kept in m_Lookup.

The method I've used fixes this problem, though I don't think it's a proper solution; more of a band-aid. I'll push it in a second.

Should I pursue this further and get a more solid fix of this memory leak? It looks like this just doesn't release itself if Reparse() fails. https://github.com/alliedmodders/sourcemod/blob/master/core/logic/GameConfigs.cpp#L1140-L1154

[KyleSanderson]

This is a really complicated one to solve. If you're not up for the (extended) journey on this one I fully understand (and still thank you very much for trying to make this better) as there's a lot of technical debt you're going to be working on.

Plugins take gamedata once at startup (wasn't part of the API, but just a common pattern) and hold those values as tried and true until the bitter end. At the very least if these files change, we need to perform a damage graph and reload the dependents on the gamedata file (in reality it would be offset/sig, but that's really invasive and this isn't heart surgery - so a simple reload should suffice).

I looked at this one with Paul in 2012~ and the above is what stopped us. However as it's now 8 years later, plugin_settings.cfg is gone (where you could say: reload all of my plugins every mapchange!) and I'm more than happy to work with you on this one as there was no cheap win at the time. Additionally, a lot has changed 😄.

Hopefully that helps explain the nuances that are present with the current cset, and how we can make this better.

EDIT:
PS this was the last time I really worked on this stuff: https://bugs.alliedmods.net/show_bug.cgi?id=5875

[asherkin]

Plugins take gamedata once at startup (wasn't part of the API, but just a common pattern) and hold those values as tried and true until the bitter end. At the very least if these files change, we need to perform a damage graph and reload the dependents on the gamedata file (in reality it would be offset/sig, but that's really invasive and this isn't heart surgery - so a simple reload should suffice).

Gamedata is owned by the plugin - what you're talking about is interaction with the auto-updater which doesn't apply to 99.9% of thirdparty plugins - this diff makes it so when a plugin is reloaded all it's gamedata is re-parsed (i.e. what people need during development and currently causes a lot of pain for users), magically reloading plugins whenever gamedata files change on disk feels extremely magic, brittle, and unnecessary.

IMO, the correct solution for the problem you're talking about (the gamedata updater updates things automatically and things don't know) is to have it flush the cache and a global forward that plugins can opt-in to, but that's very unrelated to this.

[Scags]

So, what Kyle mentioned doesn't exactly have to do with what this PR changes. Am I reading that correctly?

So should I pursue anything further or will this suffice as-is?

[KyleSanderson]

Asher was right - I was trying to boil the ocean instead of trying to catch the kettle of fish. While this isn't as complete as I would like, it is a UX improvement.