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

Object sync - fixes and improvements #3405

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

FileEX
Copy link
Contributor

@FileEX FileEX commented May 26, 2024

I undertook the repair and improvement of object synchronization, which has been disabled since 2011.

I was unable to reproduce the bugs described in #460

I realize that this is quite a big PR, so I will be very grateful for any kind of feedback and reliable tests to eliminate any bugs and to safely merge this PR. I tested everything myself on the local server and I don't have the opportunity to test it with other players, so I'm not sure if all the problems have been eliminated.

Changes

Fixes

  • Code refactoring
  • Small changes

New events

  • Added onObjectDamage event based on synchronization
  • Added onObjectBreak event based on synchronization
  • Added onObjectMoveStart event
  • Added onObjectMoveStop event based on synchronization

Changes

  • Added almost all objects functions to server-side: setObjectProperty, getObjectProperty
  • Added getElementVelocity & getElementAngularVeloity functions for objects on the server-side
  • Added isElementInWater function for objects (shared)
  • Move eObjectProperty enum to the shared

Resolved issues: #460, #2752, #386 (probably)

Parts of this PR:

Comment on lines 267 to 279
// Sync properties
if (pData->ucFlags & 0x40)
pObject->SetMass(pData->fMass);
if (pData->ucFlags & 0x80)
pObject->SetTurnMass(pData->fTurnMass);
if (pData->ucFlags & 0x100)
pObject->SetAirResistance(pData->fAirResistance);
if (pData->ucFlags & 0x200)
pObject->SetElasticity(pData->fElasticity);
if (pData->ucFlags & 0x400)
pObject->SetBuoyancyConstant(pData->fBuoyancyConstant);
if (pData->ucFlags & 0x800)
pObject->SetCenterOfMass(pData->vecCenterOfMass);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need to sync static data

Copy link
Contributor Author

@FileEX FileEX May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Data is only sent if it has changed. If the object is created on the server side, you must first get its properties, which are available on the client side, otherwise the properties on the server and client side would be different

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But why do we need that?
If you create object on server, you should change properties on server.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is illogical. When creating an object on the server side, its properties will be default (-1, zero vector), and on the client side the properties will have some values.

When we create a vehicle on the server side, it also has handling, plate and other data just like on the client side. It's as if upgrades were only on the client side by default, and there were no upgrades on the server side because they had to be set on the server side as well.

This is strange behavior imo

Copy link
Member

@TheNormalnij TheNormalnij May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When creating an object on the server side, its properties will be default (-1, zero vector)

So server should have a config. All object properties should be available for a new object immediately.

When we create a vehicle on the server side, it also has handling, plate and other data just like on the client side.

Server has configs for that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, your solution adds uninitialized state for server objects. So it's much complex, that the solution with serverside config

Copy link
Contributor Author

@FileEX FileEX May 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is normal behavior. The object has its properties set at the time of synchronization and this is normal, because on the client side if the object is not streamed, its properties are also default (-1, zero vector). The properties have the appropriate values ​​when the object is streamed, and if the object is streamed, there is a good chance that it is also synchronized. I don't understand why you should complicate such a trivial thing as object properties. If this is really such a bad way, then I will remove access to the server-side properties, because implementing the same in a more difficult way requires investigation and a lot of work for the same effect

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not normal and unnecessary complex

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not normal and unnecessary complex

Adding feedback label due to this, cc @FileEX to please provide counterarguments or chat with eachother in dev discord about your POV's. Only after that comes to a conclusion, this can be marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will work on it. Now I'm breaking the entire PR into several smaller ones to make it easier to review. One of them is already ready and waiting to be merged #3450

Server/mods/deathmatch/logic/CObjectSync.cpp Outdated Show resolved Hide resolved
@FileEX
Copy link
Contributor Author

FileEX commented Jun 9, 2024

Ok, this is quite a large PR, so I decided to split it into several smaller PRs to make the review easier.

@FileEX FileEX marked this pull request as draft June 9, 2024 00:21
@FileEX FileEX mentioned this pull request Jun 9, 2024
@Dutchman101 Dutchman101 added the feedback Further information is requested label Jun 30, 2024
@lopezloo
Copy link
Member

Maybe split this work into:

  1. Health and broken state of object sync.
  2. Position, velocity sync.
  3. Server controlled object properties.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feedback Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants