Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

tens0rfl0w
Copy link
Contributor

@tens0rfl0w tens0rfl0w commented May 18, 2024

Goal of this PR

Commit citizenfx/natives@e6f4735 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

  • 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

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

@github-actions github-actions bot added the triage Needs a preliminary assessment to determine the urgency and required action label May 18, 2024
@DaniGP17
Copy link

Why some functions names start with _?

@AvarianKnight
Copy link
Contributor

AvarianKnight commented May 18, 2024

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

@freedy69
Copy link
Contributor

maybe CALL_MINIMAP_SCALEFORM_FUNCTION can also be renamed to BEGIN_SCALEFORM_MOVIE_METHOD_ON_MINIMAP?

@tens0rfl0w
Copy link
Contributor Author

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
Copy link
Contributor

freedy69 commented May 19, 2024

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
Copy link
Contributor Author

tens0rfl0w commented May 19, 2024

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
Copy link
Contributor

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 tens0rfl0w force-pushed the fix/SET_PED_HAIR_TINT-rpc-native branch from 64b7d36 to dcacaa5 Compare May 19, 2024 13:30
* RPC natives now use the natives name from the client decl.
* Aliases from the client decl. are now applied to RPC natives
* This allows naming changes of client natives without changing the RPC decl. and breaking backwards compatibility on the server-side

* Code styling fixes
@tens0rfl0w tens0rfl0w force-pushed the fix/SET_PED_HAIR_TINT-rpc-native branch from dcacaa5 to 327f8d0 Compare May 19, 2024 13:33
@tens0rfl0w
Copy link
Contributor Author

tens0rfl0w commented May 19, 2024

@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)

@tens0rfl0w tens0rfl0w changed the title fix(natives/rpc): Update native name for SET_PED_HAIR_TINT tweak(natives/rpc): Use client-native names and apply aliases May 19, 2024
@gottfriedleibniz gottfriedleibniz added ready-to-merge This PR is enqueued for merging and removed triage Needs a preliminary assessment to determine the urgency and required action labels May 20, 2024
@prikolium-cfx prikolium-cfx merged commit bad411a into citizenfx:master May 28, 2024
9 checks passed
@tens0rfl0w tens0rfl0w deleted the fix/SET_PED_HAIR_TINT-rpc-native branch June 11, 2024 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR is enqueued for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Server side RPC native SetPedHairColor doesn't exist Error on SetPedHairColor - 0xBB43F090
6 participants