-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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(net/five): correct train track node on ownership change #2911
base: master
Are you sure you want to change the base?
Conversation
Excellent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! It looks like a really valuable change. Left some minor comments.
But also, do you have a good way to reproduce/test it? I tried to play with it locally:
- Put one player in a carriage of a train. Make sure that this player is the owner.
- Get second player on the train as well.
- See
netObjectMgrBase__ChangeOwner
being called, but the code never reaches theCTrain__SetTrainCoord(train, -1, -1);
part. Seems like(CTrain__IsCarriageEngine(train) && *(int*)((char*)train + TrainTrackNodeIndexOffset) == 0)
check is always false in my case. - Disconnect the first player.
- After some time train disappears even tho second player in in a carriage.
Do I misunderstand the use case or this? Am I doing something wrong?
// Make sure that this is a train and that we are now the new owner of it | ||
if (object->objectType == (uint16_t)NetObjEntityType::Train && targetPlayer->physicalPlayerIndex() == rage::GetLocalPlayer()->physicalPlayerIndex()) | ||
{ | ||
// Ensure that the vehicle isn't a nullptr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: This comment is redundant. It's clear whats going on without it.
This issue isn't related to ownership changing between players as in most (if not all cases) the client becoming owner should already have knowledge of the track node the train is located on. However, in cases where the server was the previous owner (in cases where no player is in the trains scope to have ownership over the train) the client becoming owner will have no knowledge of the trains current track node. Causing the train to reset to node 0 regardless of where it actually is.
I've also attached the repro resource i used to test this. (find a train track, do /train on the client to create a train. Leave the scope and return) |
ecc189a
to
15365dd
Compare
Goal of this PR
Fixes scenarios where a train becomes owned by the server (orphan natives) and upon a player re-entering the trains scope and becoming owner of the train, The train resets to the node 0 of the track it is currently on
How is this PR achieving the goal
By forcing the train to find its current track node based on the trains current position, only upon ownership change to a player from a server and if the new client has no knowledge of the trains current track node.
This PR applies to the following area(s)
FiveM
Successfully tested on
Game builds: 1604, 2372, 2545, 2802, 3258
Platforms: Windows
Checklist
Fixes issues