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

fix(extra-natives-rdr3): prevent local entities to be converted to networked objects #3227

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DaniGP17
Copy link
Contributor

Goal of this PR

Prevent the native GET_CLOSEST_OBJECT_OF_TYPE from converting a non-networked object to networked, this causes that when using this native many times it converts unwanted objects to networked and causes the synchronized object pool to fill up quickly.

How is this PR achieving the goal

Removing instructions that check if the object is already networked.

This PR applies to the following area(s)

RedM, Natives

Successfully tested on

Game builds: 1491

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 #2833

@github-actions github-actions bot added RedM Issues/PRs related to RedM invalid Requires changes before it's considered valid and can be (re)triaged labels Mar 11, 2025
@slashkeyvalue
Copy link
Contributor

Seems weird to modify this behaviour, it's the same o V. Why not use itemsets or at least implement getgamepool?

@draobrehtom
Copy link

draobrehtom commented Mar 11, 2025

@slashkeyvalue it behaves differently, more detailed explanation in issue #2833

By the way, it was mentioned by PR author, surprised how did you miss it. Especially take a look on this comment: #2833 (comment)

@slashkeyvalue
Copy link
Contributor

I see, maybe it makes more sense to override GET_CLOSEST_OBJECT_OF_TYPE and patch it to actually use the "dontNetworkObject" argument? instead of outright disabling the functionality as someone might be already be relying on that for some reason

@draobrehtom
Copy link

This parameter might make sense if it worked properly, but as mentioned in the issue, the object only becomes networked to the host and remains invisible to other players. I highly doubt anyone is relying on this 'broken' behavior. If someone wants to fix this behavior as well, you're more than welcome to do so. However, that would be more like adding a 'feature.' The more important priority here is to address the actual issue, where people use this native function without being aware of its consequences.

@spacevx
Copy link
Contributor

spacevx commented Mar 11, 2025

This parameter might make sense if it worked properly, but as mentioned in the issue, the object only becomes networked to the host and remains invisible to other players. I highly doubt anyone is relying on this 'broken' behavior. If someone wants to fix this behavior as well, you're more than welcome to do so. However, that would be more like adding a 'feature.' The more important priority here is to address the actual issue, where people use this native function without being aware of its consequences.

Well, a proper fix would be to use the parameter correctly and not just disable a feature of the native, and you don't need to be rude like this lol. If @DaniGP17 wants to do it, he will, that's all.

@DaniGP17
Copy link
Contributor Author

Well, a proper fix would be to use the parameter correctly and not just disable a feature of the native, and you don't need to be rude like this lol. If @DaniGP17 wants to do it, he will, that's all.

That was the main idea I had but I had some problems reading the parameters of this native
image

@draobrehtom
Copy link

Well, a proper fix would be to use the parameter correctly and not just disable a feature of the native

It's a highly questionable feature that seems more like a bug. If no one is available to fix it, then disabling it would be the better option - that's my point. Though I agree, the ideal solution would be to fix it.

you don't need to be rude like this lol. If @DaniGP17 wants to do it, he will, that's all.

I don't see where I was being rude - I wasn’t trying to force anyone to do something. My point was to support this PR and get it accepted as quickly as possible.

@outsider31000
Copy link
Contributor

outsider31000 commented Mar 11, 2025

No one is using this native for that specific reason entities become invisible to other clients after using this native and the network pool still increases, so no worries in breaking compat, those who use it are not aware of this issue hence why this fix is needed the way is made.

an example is here #2833 (comment)

to allow current behaviour then this issue will need to be fixed to make sense. but since you have natives to register entities as networked I dont think overriding or creating a new native is necessary, specially when fivem has the same native, which for those that do fivem and want to do redm will def confuse them.

I think current implementation is needed and is stream lined with gtav.
but this is just my opinion

@spacevx
Copy link
Contributor

spacevx commented Mar 11, 2025

Well, a proper fix would be to use the parameter correctly and not just disable a feature of the native, and you don't need to be rude like this lol. If @DaniGP17 wants to do it, he will, that's all.

That was the main idea I had but I had some problems reading the parameters of this native image

maybe @dalekenium-cfx could help u?, he did cools things on RedM

@Identity-labs
Copy link
Contributor

This native with actual behavior can broken entier net object pool, since it convert every closest object found in networked entity.
This native is currently banned from my usage since few years.

Workaround is to use ItemSet, but it's not same amount of natives called.

For me we don't need to keep current behavior, there is no use case for it.

@d22tny
Copy link
Contributor

d22tny commented Mar 11, 2025

It's easy to make a object networked.
It's not so easy to get the closest object of type.
This patch fixes getting the closest object of type.
It really seems straight-forward to me :)), i don't think anyone would have been using a broken native to make some local object networked. And also, you couldn't use this native in that regard as i've said in the issue itself, it keeps flooding with the same object / every player. 200 players -> you get to < 10 fps in 5 minutes.

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 RedM Issues/PRs related to RedM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GetClosestObjectOfType causes non-networked object to become networked
7 participants