feat(redm/script): add patch for overriding train tracks and trolley cables from resource

[Sage-of-Mirrors]

Goal of this PR

Allow a resource developer to replace the default traintracks.xml and trolleyCableTracks.xml files, thus allowing for RedM servers to have proper custom train tracks. And trolley cables, if so desired.

How is this PR achieving the goal

The fxmanifest metadata statements replace_traintrack_file <path> and replace_trolley_cable_file <path> have been added. When present, they store their paths to global variables in a patch, located at gta-core-rdr3/src/PatchTrainTrackFileOverride.cpp. The patch intercepts any calls to LoadTrackXML(), and if either of the override paths are valid, it will pass them into the track loading native rather than the default filepaths that were passed in.

This PR applies to the following area(s)

RedM, Server, ScRT: Lua

Successfully tested on

Game builds: b1311, b1491

Platforms: Windows

Checklist

• Code compiles and has been tested successfully.
• Code explains itself well and/or is documented.
• My commit message explains what the changes do and what they are for.
• No extra compilation warnings are added by these changes.

Fixes issues

resolves #2424

[Sage-of-Mirrors]

This resource can be used to verify that the patch is working correctly: tracktest.zip. However, additional resources allowing the player to spawn trains and manipulate junctions are required to see anything beyond the trace() messages.

[outsider31000]

I would suggest you test this also in a newer build. b1491 is the latest, and what everyone should and are using

[Sage-of-Mirrors]

I would suggest you test this also in a newer build. b1491 is the latest, and what everyone should and are using

Yeah I'm not sure why my copy is so out of date. Tonight, I'll make sure that Steam updates it to latest.

[Sage-of-Mirrors]

Tested on build 1491.

[prikolium-cfx]

Hey @Sage-of-Mirrors,

Thanks a lot for your hard work; we really value what you've brought to the table. Just a heads up, though, your implementation doesn't quite line up with our in-house rules for how mods and metadata should be handled. Specifically, we need to make sure that any modded files are unloaded whenever someone disconnects from the server.

[Sage-of-Mirrors]

I see, thank you for the feedback. Question, though - when a user disconnects from a server, does the level get destroyed? If so, the custom track data will automatically be destroyed as it was loaded into the same global as the base-game data.

Otherwise, can you point me to examples of custom content being unloaded at disconnect? I can think through how to bring my code in line with CFX's standards.

Thanks!

[prikolium-cfx]

I see, thank you for the feedback. Question, though - when a user disconnects from a server, does the level get destroyed? If so, the custom track data will automatically be destroyed as it was loaded into the same global as the base-game data.

Otherwise, can you point me to examples of custom content being unloaded at disconnect? I can think through how to bring my code in line with CFX's standards.

Thanks!

As I can see it will be enough to reset g_overrideTrolleyCableFilePath and g_overrideTrainTrackFilePath to "" on disconnect, otherwise it will be loaded again on the next connection to server.

[Sage-of-Mirrors]

As I can see it will be enough to reset g_overrideTrolleyCableFilePath and g_overrideTrainTrackFilePath to "" on disconnect, otherwise it will be loaded again on the next connection to server.

Ahhh, that completely slipped my mind. I will fix that later today.

[Sage-of-Mirrors]

Would it be acceptable to "consume" the override path, i.e. clear it after passing it to the native parsing function, like overriding the level name does?

e.g.:

if (!g_overrideTrainTrackFilePath.empty() && strcmp(origXmlFileName, DEFAULT_TRACK_FILE) == 0)
{
    trace("Replacing default traintracks.xml with %s\n", g_overrideTrainTrackFilePath.data());
    g_origLoadTrackXML(g_overrideTrainTrackFilePath.data(), dstPool);

    g_overrideTrainTrackFilePath.clear();
}
else if (!g_overrideTrolleyCableFilePath.empty() && strcmp(origXmlFileName, DEFAULT_CABLES_FILE) == 0)
{
    trace("Replacing default trolleyCableTracks.xml with %s\n", g_overrideTrolleyCableFilePath.data());
    g_origLoadTrackXML(g_overrideTrolleyCableFilePath.data(), dstPool);

    g_overrideTrolleyCableFilePath.clear();
}

Otherwise I believe doing it on disconnect would be:

static InitFunction initFunction([]()
{
    OnKillNetwork.Connect([=]()
    {
	g_overrideTrainTrackFilePath.clear();
        g_overrideTrolleyCableFilePath.clear();
    });
});

[Sage-of-Mirrors]

Added a subscription to OnKillNetworkDone to clear the XML override paths when the user disconnects from a server.

[Disquse]

As was discussed in DMs, the way this PR achieves the desired behavior isn't suitable for us. Creating fxmanifest entries for every single thing that must be loaded as a part of level meta isn't something we would like to do. As an alternative, I suggested creating runtime natives to manipulate train tracks (similar to what was done for water-related stuff in FiveM). There's no need to further improve this specific PR (unless it will be reused for the implementation of the runtime natives idea). I'm sorry for not leaving a public comment about it.

[outsider31000]

Instead of closing this thread, why not provide some guidance to @Sage-of-Mirrors so it's suitable ?
Make it a work in progress perhaps?

@Sage-of-Mirrors prooved to be up to the task and put hard work on it and been asking how to implement it etc.

closing this thread doesnt make anyone want to make pull requests if they just get closed.
Giving guidance should be the best thing to do, due to How behind RedM state is.

[Sage-of-Mirrors]

I am confused then - did @prikolium-cfx approve of this approach? Why did they seem amenable to it as long as I fixed the cleanup bug?

[Sage-of-Mirrors]

While Disquse and I have discussed how this could be done in a different way in DMs, I would like to argue that my addition of REPLACE_TRAINTRACK_FILE (and the associated TROLLEYCABLE version) is necessary and would not open the door to other REPLACE_<datafile>_FILE-type statements.

data_file 'TRAINTRACK_FILE' <xml path> does not work for this case. It simply invokes CTrainConfigFileLoader::LoadTrackXML(), which only adds to the tracks array - it does not replace what has already been loaded. This is a critical blocker for custom tracks for RP, as both tracks on either side of a junction must reference each other in order for a train to properly cross from one to another. As far as I am aware, this is a unique case amongst all of the data_files; as such, very little if not none of them would have need of a specific manifest statement for overriding them. The only other file I can think of that would benefit from a specific override function would be TRAINCONFIG_FILE, as not all servers make use of the base game's train config data.

REPLACE_LEVEL_META currently does not work for RDR3, either. Disquse has informed me that while he is/was working on it, there are bugs which cause it not to work correctly; unless those bugs can be sourced and fixed, this is not an option for train tracks. Additionally, FiveM has deprecated manifest statements before - BEFORE_LEVEL_META and AFTER_LEVEL_META. If and when REPLACE_LEVEL_META is fully implemented, REPLACE_TRAINTRACK_FILE could be deprecated in favor of REPLACE_LEVEL_META. I argue that the utility of custom track data is great enough to RP-based servers to warrant a possibly temporary solution for the junction issue.

Thank you, and I appreciate your time.

[Disquse]

Instead of closing this thread, why not provide some guidance to @Sage-of-Mirrors so it's suitable ?

I already did that in DMs, quoting: "The only viable option I see is to make runtime natives like we have for water stuff in FiveM"

closing this thread doesnt make anyone want to make pull requests if they just get closed.

As I mentioned, @Sage-of-Mirrors can re-open it if it will be decided to reuse this PR.

@prikolium-cfx decided to review this while not being aware of the context, sorry for misunderstanding.

The huge blocker for this functionality is the lack of level replace meta support. We shouldn't stick to ad-hoc solutions for specific functionality when there's an obvious area of responsibility and the code that is supposed to fit this area. Why should we implement anything when we clearly understand that it will become deprecated as soon as some parts of our functionality are ported? Deprecations aren't something we can afford due to our policy of avoiding breaking backward compatibility as much as possible. The "deprecated code" will stay in our codebase for years, if not indefinitely. People will continue to use the deprecated features regardless of any warnings or better solutions we may offer.

[Sage-of-Mirrors]

Ok, sorry