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(gamestate/server): Apply routing bucket changes to child attachments #2847

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

Conversation

tens0rfl0w
Copy link
Contributor

Goal of this PR

Changing the routing bucket of an entity does not update the routing buckets of possible attached child entities. This causes various problems, one of which is broken collision data for entities that get re-attached to other entities with the same handle.

How is this PR achieving the goal

Sync data is only parent-focused, so we have no info about possible child attachments. This makes it necessary to iterate through the server's entity list and traverse possible chained attachments to make sure we catch every possible attachment path to the root entity.

I tried to keep recursive calls as low as possible with tracking already traversed paths, which should also prevent cyclic attachments from causing infinite recursions.

wow.mp4

This PR applies to the following area(s)

FiveM, Server

Successfully tested on

Game builds: 3258

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

/

@github-actions github-actions bot added the invalid Requires changes before it's considered valid and can be (re)triaged label Oct 7, 2024
@CeebDev
Copy link

CeebDev commented Oct 7, 2024

Could be great to also change routing buckets of passagers when we change the routing bucket of a vehicle instead of iterate on them QoL

@Identity-labs
Copy link
Contributor

Great news :)
Any solution for non attached entities like pet ? or maybe vehicle or horse ?
to be switched automatically as well ?

@tens0rfl0w
Copy link
Contributor Author

Could be great to also change routing buckets of passagers when we change the routing bucket of a vehicle instead of iterate on them QoL

Yeah, this definitely could be added, though I'm not sure if this is a generally desired behavior. Would be great to get some additional feedback on this.

Great news :) Any solution for non attached entities like pet ? or maybe vehicle or horse ? to be switched automatically as well ?

Could you elaborate on what you mean with 'entities like pet'? Not sure If I got you right here, are you also referring to automatically applying bucket changes to vehicle passengers?

@JnKTechstuff
Copy link

JnKTechstuff commented Oct 8, 2024

Could be great to also change routing buckets of passagers when we change the routing bucket of a vehicle instead of iterate on them QoL

Yeah, this definitely could be added, though I'm not sure if this is a generally desired behavior. Would be great to get some additional feedback on this.

Well would it be desired behavior to set a vehicle into a bucket and all passengers fall out and the vehicle disappear? Logically thinking about it, setting a vehicle's bucket should set all players and entities inside said vehicle into the same bucket. If people wish to remove people from a vehicle plenty of methods exist that don't involve removing it from that player's existence.

I think its safe to do the same with vehicle and pax if not already done.

@Identity-labs
Copy link
Contributor

Could you elaborate on what you mean with 'entities like pet'? Not sure If I got you right here, are you also referring to automatically applying bucket changes to vehicle passengers?

Today if you have peds following you, with relationship/range & co you have to remove the ped before changing bucket.
Because ped will no longer have owner in original bucket and stay alone in this bucket.

May be add a native to tag entities to be linked to root entity, like that they are migrated in bucket when root entity is changing bucket same way as attached entity ?

@tens0rfl0w tens0rfl0w force-pushed the tweak/routingbucket-child-attachments branch from d327b58 to 43590a2 Compare October 8, 2024 20:42
@tens0rfl0w
Copy link
Contributor Author

Thank you for the feedback!

This now also factors in vehicle occupants and applies bucket changes to those as well.

For anyone reviewing this: A possible short-path for checking for occupants would be to use the data in CVehicleGameStateNodeData, but this wouldn't factor in possible chained attachments of the occupant entity.

Today if you have peds following you, with relationship/range & co you have to remove the ped before changing bucket. Because ped will no longer have owner in original bucket and stay alone in this bucket.

This would be out-of-scope of this PR, as this only focuses on (chained) attachments/connections to the root entity. Checking for possible ped relations would make this not deterministic and therefore not possible to check this way.

…tities

This applies routing bucket changes to entities that are connected to the entity that got its bucket changed.
@tens0rfl0w tens0rfl0w force-pushed the tweak/routingbucket-child-attachments branch from 43590a2 to a792e04 Compare October 8, 2024 21:02
@github-actions github-actions bot added triage Needs a preliminary assessment to determine the urgency and required action invalid Requires changes before it's considered valid and can be (re)triaged and removed invalid Requires changes before it's considered valid and can be (re)triaged triage Needs a preliminary assessment to determine the urgency and required action labels Oct 8, 2024
@Shinzuh
Copy link

Shinzuh commented Oct 8, 2024

Could be great to also change routing buckets of passagers when we change the routing bucket of a vehicle instead of iterate on them QoL

There is scenarios where we want to change the routing bucket of the driver and not the passengers.
The best option would be to let a way to chose.

@CeebDev
Copy link

CeebDev commented Oct 9, 2024

There is scenarios where we want to change the routing bucket of the driver and not the passengers. The best option would be to let a way to chose.

Give example of case where you only need to change bucket of vehicle but not passengers

@Shinzuh
Copy link

Shinzuh commented Oct 9, 2024

Thinking of it I don't have any special exemples for my use case but the backward compatibilty scare me a bit.
Scripts weren't expecting that a change of a bucket for a player could change it for other players too.
A bool to activate this new comportment seems to be the strict necessary

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.

5 participants