Skip to content

fix(extra-natives-five): GET_CLOSEST_TRACK_NODES filtered disabled tracks backwards - #4189

Open
VeqtaDev wants to merge 1 commit into
citizenfx:masterfrom
VeqtaDev:fix/track-nodes-disabled-filter
Open

VeqtaDev wants to merge 1 commit into
citizenfx:masterfrom
VeqtaDev:fix/track-nodes-disabled-filter

Conversation

@VeqtaDev

@VeqtaDev VeqtaDev commented Sep 9, 2026

Copy link
Copy Markdown

Goal of this PR

GET_CLOSEST_TRACK_NODES takes an includeDisabledTracks flag that does the opposite of its name, and the flag is not documented at all, so the native currently returns nodes on tracks that were switched off with SET_TRACK_ENABLED and offers no discoverable way to avoid that.

How is this PR achieving the goal

The bug. GetTrackNodesInRadius in extra-natives-five/src/TrackNatives.cpp skips a track with:

if (!track || (includeDisabledTracks && !track->m_enabled))
    continue;

When includeDisabledTracks is true, a disabled track is skipped. When it is false — the default, and what every caller passes today since the parameter is undocumented — the condition can never be true, so disabled tracks are always returned. Both branches are inverted relative to the parameter's name and its = false default.

Every other use of m_enabled in the same file reads it the expected way round: FindClosestTrack skips !track->m_enabled (line 162), the "any track enabled" check returns on track->m_enabled (line 304), and SET_TRACK_ENABLED / IS_TRACK_ENABLED write and read it directly. The condition has been this way since the native was introduced in 76ab6c5.

The fix is the single negation the name implies:

if (!track || (!includeDisabledTracks && !track->m_enabled))

GetTrackNodesInRadius is static with one call site, the native handler, so nothing else observes the change.

Behaviour change for existing scripts: callers passing four arguments now get nodes on enabled tracks only, which is what SET_TRACK_ENABLED's own page says disabling a track is for ("mission trains will not be able to spawn on this track"). Anyone who actually wants disabled tracks can now ask for them with true.

Documentation. GetClosestTrackNodes.md is updated in the same change so the page never describes the inverted behaviour: the fifth parameter is added to the signature and parameter list, and a duplicated ## Return value section (the file had two) is collapsed into one.

Verification

Executed, not just reasoned. I extracted the skip predicate — the master line and the PR line, verbatim — into a small harness around the same mock track table and compiled it with MSVC 19.51 (Visual Studio 2026, /W4 /WX, zero warnings). Output:

tracks: 0=enabled 1=DISABLED 2=enabled 3=DISABLED (plus one null slot)

version  includeDisabledTracks    tracks returned        verdict
-------  ---------------------    ---------------        -------
master   false (default)          [0, 1, 2, 3]           WRONG: returns nodes on DISABLED tracks
master   true                     [0, 2]                 WRONG: asked to include disabled, got none
PR#4189  false (default)          [0, 2]                 ok
PR#4189  true                     [0, 1, 2, 3]           ok

exit code: 0

The process exit code is an assertion over the three claims in this PR (master's default leaks disabled tracks; the fix's default does not; the fix's true opts them back in), so the run is pass/fail, not just a printout.

Harness source (drop in a file, cl /EHsc /std:c++17 track_filter.cpp, run)
// Reproduction harness for citizenfx/fivem PR #4189.
//
// Mirrors the track-skipping predicate of GetTrackNodesInRadius() in
// code/components/extra-natives-five/src/TrackNatives.cpp, exactly as written
// on master and exactly as written in the PR, against the same mock track table.

#include <cstdio>
#include <string>
#include <vector>

struct CTrainTrack
{
    bool m_enabled;
    int  id;
};

// master: line 192
static std::vector<int> Collect_Master(const std::vector<CTrainTrack*>& tracks, bool includeDisabledTracks)
{
    std::vector<int> out;
    for (CTrainTrack* track : tracks)
    {
        if (!track || (includeDisabledTracks && !track->m_enabled))
        {
            continue;
        }
        out.push_back(track->id);
    }
    return out;
}

// PR #4189
static std::vector<int> Collect_Fixed(const std::vector<CTrainTrack*>& tracks, bool includeDisabledTracks)
{
    std::vector<int> out;
    for (CTrainTrack* track : tracks)
    {
        if (!track || (!includeDisabledTracks && !track->m_enabled))
        {
            continue;
        }
        out.push_back(track->id);
    }
    return out;
}

static std::string Show(const std::vector<int>& v)
{
    std::string s = "[";
    for (size_t i = 0; i < v.size(); ++i)
    {
        s += (i ? ", " : "") + std::to_string(v[i]);
    }
    return s + "]";
}

int main()
{
    CTrainTrack t0{ true,  0 };   // enabled
    CTrainTrack t1{ false, 1 };   // disabled (SET_TRACK_ENABLED(1, false))
    CTrainTrack t2{ true,  2 };   // enabled
    CTrainTrack t3{ false, 3 };   // disabled

    // a null slot, as CTrainTrack__getTrainTrack() can return for unused indices
    std::vector<CTrainTrack*> tracks{ &t0, &t1, nullptr, &t2, &t3 };

    std::printf("tracks: 0=enabled 1=DISABLED 2=enabled 3=DISABLED (plus one null slot)\n\n");
    std::printf("%-8s %-24s %-22s %s\n", "version", "includeDisabledTracks", "tracks returned", "verdict");
    std::printf("%-8s %-24s %-22s %s\n", "-------", "---------------------", "---------------", "-------");

    for (bool flag : { false, true })
    {
        auto m = Collect_Master(tracks, flag);
        bool m_has_disabled = false;
        for (int id : m) m_has_disabled |= (id == 1 || id == 3);
        std::printf("%-8s %-24s %-22s %s\n", "master", flag ? "true" : "false (default)", Show(m).c_str(),
                    flag ? (m_has_disabled ? "ok" : "WRONG: asked to include disabled, got none")
                         : (m_has_disabled ? "WRONG: returns nodes on DISABLED tracks" : "ok"));
    }
    for (bool flag : { false, true })
    {
        auto f = Collect_Fixed(tracks, flag);
        bool f_has_disabled = false;
        for (int id : f) f_has_disabled |= (id == 1 || id == 3);
        std::printf("%-8s %-24s %-22s %s\n", "PR#4189", flag ? "true" : "false (default)", Show(f).c_str(),
                    flag ? (f_has_disabled ? "ok" : "WRONG") : (f_has_disabled ? "WRONG" : "ok"));
    }

    // exit code doubles as an assertion for CI-style use
    bool master_default_leaks = false;
    for (int id : Collect_Master(tracks, false)) master_default_leaks |= (id == 1 || id == 3);
    bool fixed_default_clean = true;
    for (int id : Collect_Fixed(tracks, false)) fixed_default_clean &= !(id == 1 || id == 3);
    bool fixed_opt_in_works = false;
    for (int id : Collect_Fixed(tracks, true)) fixed_opt_in_works |= (id == 1 || id == 3);

    return (master_default_leaks && fixed_default_clean && fixed_opt_in_works) ? 0 : 1;
}
In-game test resource (drop into a dev server, run /tracktest near a railway)

tracktest/fxmanifest.lua

fx_version 'cerulean'
game 'gta5'

author 'VeqtaDev'
description 'Repro for citizenfx/fivem#4189 - GET_CLOSEST_TRACK_NODES and disabled tracks'

client_script 'client.lua'

tracktest/client.lua

-- /tracktest [radius]   (default 300.0)
--
-- In-game repro for citizenfx/fivem#4189.
-- Stand near a railway, run the command, read the two verdict lines.
--
-- It counts the nodes GET_CLOSEST_TRACK_NODES returns around you, disables the
-- first track it finds with SET_TRACK_ENABLED, counts again for every form of
-- the call, then restores the track.

local function count(x, y, z, r, includeDisabled)
    local nodes
    if includeDisabled == nil then
        nodes = GetClosestTrackNodes(x, y, z, r)              -- 4 args: what every script passes today
    else
        nodes = GetClosestTrackNodes(x, y, z, r, includeDisabled)
    end
    return #nodes, nodes
end

RegisterCommand('tracktest', function(_, args)
    local r = tonumber(args[1]) or 300.0
    local c = GetEntityCoords(PlayerPedId())

    local before4, nodes = count(c.x, c.y, c.z, r)
    if before4 == 0 then
        print(('[tracktest] no track nodes within %.0f m - stand near a railway and retry'):format(r))
        return
    end

    -- entries are msgpack arrays: { nodeIndex, trackId }
    local track = nodes[1][2]
    local wasEnabled = IsTrackEnabled(track)
    print(('[tracktest] %d nodes within %.0f m; first entry is on track %d (enabled=%s)')
        :format(before4, r, track, tostring(wasEnabled)))

    local beforeT = count(c.x, c.y, c.z, r, true)

    SetTrackEnabled(track, false)
    local after4 = count(c.x, c.y, c.z, r)           -- 4 args
    local afterF = count(c.x, c.y, c.z, r, false)    -- explicit false
    local afterT = count(c.x, c.y, c.z, r, true)     -- explicit true
    SetTrackEnabled(track, wasEnabled)                -- restore

    print(('[tracktest] with track %d disabled:'):format(track))
    print(('  4 args                : %d -> %d   %s'):format(before4, after4,
        after4 < before4 and '=> disabled track filtered out   (PATCHED behaviour)'
                          or  '=> disabled track still returned (MASTER bug)'))
    print(('  includeDisabled=false : %d'):format(afterF))
    print(('  includeDisabled=true  : %d -> %d   %s'):format(beforeT, afterT,
        afterT < beforeT and '=> true EXCLUDED the disabled track (MASTER bug, inverted)'
                          or  '=> true kept the disabled track   (PATCHED behaviour)'))
end, false)

Expected on master: after the track is disabled, the 4-argument count does not drop while the count with true does — the inversion. Expected with this PR: the reverse. The resource restores the track's previous state when it finishes.

What this does not cover: the change has not been built inside the FiveM tree or exercised in-game — I have neither a build environment nor a test server. The in-tree diff is a single ! on one condition, the static case (every other m_enabled reference in the file, and the original commit) is laid out above, and the resource above turns the in-game check into a one-command step for anyone with a dev server.

This PR applies to the following area(s)

FiveM, Natives

Successfully tested on

Game builds: not run in-game — predicate verified in isolation, see Verification.

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.

Note on CI

This touches code/, so the full build matrix runs and the PR will be auto-labelled invalid. That failure is environmental and predates this branch — Windows jobs abort at exit 127 before MSBuild runs. Described in #4178, fix proposed in #4179.

Fixes issues

N/A


🤖 Generated with Claude Code

…acks backwards

GetTrackNodesInRadius skipped a track when

    includeDisabledTracks && !track->m_enabled

so asking to include disabled tracks excluded them, and the default of
false - what every caller passes, the parameter being undocumented -
never filtered at all, returning nodes on tracks switched off with
SET_TRACK_ENABLED. Both branches are inverted relative to the flag's
name and default. Every other m_enabled check in the file reads the flag
the expected way round (FindClosestTrack, the any-track-enabled check,
SET/IS_TRACK_ENABLED). The condition has been this way since the native
was added in 76ab6c5.

Negate the flag so disabled tracks are skipped unless explicitly
requested. The function is static with a single call site.

The declaration is updated in the same change so it never documents the
inverted behaviour: the fifth parameter is added to the signature and
parameter list, and a duplicated "## Return value" section is collapsed.

Not exercised in-game; the code change is one negation on one condition.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Veqta <217806377+VeqtaDev@users.noreply.github.com>
@github-actions github-actions Bot added the triage Needs a preliminary assessment to determine the urgency and required action label Sep 9, 2026
@ook3D

ook3D commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Can you stop spamming AI PRs holy fuck

@github-actions github-actions Bot added invalid Requires changes before it's considered valid and can be (re)triaged and removed triage Needs a preliminary assessment to determine the urgency and required action labels Sep 9, 2026
@yorick2002

Copy link
Copy Markdown
Contributor

Can you stop spamming AI PRs holy fuck

Indeed, whenever claude is co authored it should just close the PR and maybe block the user 👍

@VeqtaDev

Copy link
Copy Markdown
Author

Can you stop spamming AI PRs holy fuck

Indeed, whenever claude is co authored it should just close the PR and maybe block the user 👍

I don't see the problems here mates if the code is good and it upgrade the community experience this should not be ignored, but if the code is shitty ai slop yeah ur right but then argument and look closely to an real code report it will always be better to scream at people for AI code ;)

@ook3D

ook3D commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Can you stop spamming AI PRs holy fuck

Indeed, whenever claude is co authored it should just close the PR and maybe block the user 👍

I don't see the problems here mates if the code is good and it upgrade the community experience this should not be ignored, but if the code is shitty ai slop yeah ur right but then argument and look closely to an real code report it will always be better to scream at people for AI code ;)

its not about the fact its AI, its that youre just spamming 100 PRs

@ook3D

ook3D commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Can you stop spamming AI PRs holy fuck

Indeed, whenever claude is co authored it should just close the PR and maybe block the user 👍

I don't see the problems here mates if the code is good and it upgrade the community experience this should not be ignored, but if the code is shitty ai slop yeah ur right but then argument and look closely to an real code report it will always be better to scream at people for AI code ;)

Also, remove "Cfx contributor" from your github readme, you've not contributed anything yet as nothing of yours has been merged

@chvrs12

chvrs12 commented Sep 10, 2026

Copy link
Copy Markdown

Can you stop spamming AI PRs holy fuck

Indeed, whenever claude is co authored it should just close the PR and maybe block the user 👍

I don't see the problems here mates if the code is good and it upgrade the community experience this should not be ignored, but if the code is shitty ai slop yeah ur right but then argument and look closely to an real code report it will always be better to scream at people for AI code ;)

Also, remove "Cfx contributor" from your github readme, you've not contributed anything yet as nothing of yours has been merged

hes goated 400 contributions in 1 day

@VeqtaDev

Copy link
Copy Markdown
Author

Can you stop spamming AI PRs holy fuck

Indeed, whenever claude is co authored it should just close the PR and maybe block the user 👍

I don't see the problems here mates if the code is good and it upgrade the community experience this should not be ignored, but if the code is shitty ai slop yeah ur right but then argument and look closely to an real code report it will always be better to scream at people for AI code ;)

Also, remove "Cfx contributor" from your github readme, you've not contributed anything yet as nothing of yours has been merged

If u could symply think with ur brain u should know that as soon you post anything on the cfx page whereas script or else you are a contributor of the ecosystem so since you like so much sucking my dick because i use ai i do some PR to enhance community playing quality go make some u too unless you want to keep wasting time barking like a dog for an AI PR. Wake Up Bro its 2026...

@ook3D

ook3D commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

If u could symply think with ur brain u should know that as soon you post anything on the cfx page whereas script or else you are a contributor of the ecosystem so since you like so much sucking my dick because i use ai i do some PR to enhance community playing quality go make some u too unless you want to keep wasting time barking like a dog for an AI PR. Wake Up Bro its 2026...

thats not how open source contribution works.

@vertexitde

Copy link
Copy Markdown
firefox_ecp8D7ZvT1

holy ai slop

@chvrs12

chvrs12 commented Sep 10, 2026

Copy link
Copy Markdown

none merged btw

@yannbcf

yannbcf commented Sep 10, 2026

Copy link
Copy Markdown

I do not understand the issue as long as the code produced is readable and has value. You should be grateful that some people are still spending time on this rotting repository knowing it will likely be ignored by cfx.

Now you also get trash talked on by white knights somehow, its cfx role to decide wether this is acceptable or not

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

invalid Requires changes before it's considered valid and can be (re)triaged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants