citizenfx / fivem

The source code for the Cfx.re modification frameworks, such as FiveM, RedM and LibertyM, as well as FXServer.
https://cfx.re/
3.52k stars 2.07k forks source link

tweak(natives/rpc): Use client-native names and apply aliases #2550

Closed tens0rfl0w closed 4 months ago

tens0rfl0w commented 4 months ago

Goal of this PR

Commit https://github.com/citizenfx/natives/commit/e6f473573f6eb7e2921b2ef48668117cbbe84bbb changed the native name from '_SET_PED_HAIR_COLOR' to 'SET_PED_HAIR_TINT' which makes the RPC native definition invalid.

How is this PR achieving the goal

We now also check for alias matches when generating RPC decl. in codegen and apply client-native aliases to RPC natives. This allows us to keep RPC native naming in sync with client-native changes while maintaining backward compatibility with old native naming (like on the client side).

This PR applies to the following area(s)

FiveM, Natives (RPC)

Successfully tested on

Game builds: 2699

Platforms: Windows

Checklist

Fixes issues

fixes #2549 fixes #2447 fixes https://forum.cfx.re/t/5226975

DaniGP17 commented 4 months ago

Why some functions names start with _?

AvarianKnight commented 4 months ago

Names that start with underscore mean that the native name isn't known.

freedy69 commented 4 months ago

maybe CALL_MINIMAP_SCALEFORM_FUNCTION can also be renamed to BEGIN_SCALEFORM_MOVIE_METHOD_ON_MINIMAP?

tens0rfl0w commented 4 months ago

maybe CALL_MINIMAP_SCALEFORM_FUNCTION can also be renamed to BEGIN_SCALEFORM_MOVIE_METHOD_ON_MINIMAP?

Why should this native be renamed? This is a custom native, not a base-game native.

Also, PRs are generally not the best place to suggest other stuff as this PR is about RPC natives.

freedy69 commented 4 months ago

maybe CALL_MINIMAP_SCALEFORM_FUNCTION can also be renamed to BEGIN_SCALEFORM_MOVIE_METHOD_ON_MINIMAP?

Why should this native be renamed? This is a custom native, not a base-game native.

Also, PRs are generally not the best place to suggest other stuff as this PR is about RPC natives.

Im just saying because it would fit naming formatting of other scaleform natives. Would make more sense, im not really suggesting, more so pointing out since this is about renaming a native

tens0rfl0w commented 4 months ago

This PR is not about renaming a native. The natives name was already changed months ago. This just aims to fix the RPC native naming, so it can be invoked from the server-side again.

gottfriedleibniz commented 4 months ago

To avoid breaking compatibility here, it should be safe (i.e., requires testing) to update getNative to also search through aliases, e.g.,

local function getNative(nativeName)
    -- get the native
    for k, v in ipairs(natives) do
        if #v.apiset == 0 or v.apiset[1] == 'client' then
            if v.name == nativeName then
                return v
            end

            for _,aliasName in ipairs(v.aliases) do
                if nativeName == aliasName then
                    return v
                end
            end
        end
    end
    return nil
end

Updating this script to use a reverse name lookup would probably be better. That can be done later though.

tens0rfl0w commented 4 months ago

@gottfriedleibniz Tested your provided snippet. Works to keep backwards compatibility for old native naming. I also applied client aliases to RPC natives and used the 'real' client-native naming to keep RPC natives in sync while keeping backwards compatibility.

Couldn't find any regressions when testing this change, but not sure.

(Those code styling fixes can be removed if unwanted)