Add removeElementData client side#4745
Conversation
removeElementData can now be called on the client which actually deletes (rather than just sets to nil or false) an element data. It also sends a new packet type to the server and syncs it with clients.
|
Tested everything with 2 PCs, fixed that client variable wasn't being sent to onElementDataChange when a client used removeElementData. |
|
I've made the requested changes and re-tested it. |
|
Requested changes have been added and re-tested. |
| enum eServerRPCFunctions | ||
| { | ||
| PLAYER_INGAME_NOTICE, | ||
| INITIAL_DATA_STREAM, | ||
| PLAYER_TARGET, | ||
| PLAYER_WEAPON, | ||
| KEY_BIND, | ||
| CURSOR_EVENT, | ||
| REQUEST_STEALTH_KILL, | ||
| REMOVE_ELEMENT_DATA_RPC, | ||
| }; |
There was a problem hiding this comment.
I wonder why there are so few RPCs here. I feel like there are other ways of defining RPCs that perhaps we're missing?
Like surely there should be a regular setElementData RPC here?
There was a problem hiding this comment.
Okay, an LLM figured it out. It uses PACKET_ID_CUSTOM_DATA, which is simultaneously used to broadcast to all clients.
I think we should consider reusing that packet, if possible.
There was a problem hiding this comment.
For example, here:
mtasa-blue/Server/mods/deathmatch/logic/packets/CCustomDataPacket.cpp
Lines 36 to 39 in 7d8fd38
We could do something like this:
if (m_Value.ReadFromBitStream(BitStream)) {
return true;
} else {
m_bDelete = true;
return true;
}| bool isSynced; | ||
| CLuaArgument* currentVariable = Entity.GetCustomData(name, false, &isSynced); | ||
| if (!currentVariable) | ||
| return false; |
There was a problem hiding this comment.
We should be able to skip this check if the entity is a local entity, as DeleteCustomData checks if the field already exists: https://sourcegraph.com/r/github.com/multitheftauto/mtasa-blue@7d8fd38976d6e9fc1edf767697cde2e05b937631/-/blob/Client/mods/deathmatch/logic/CClientEntity.cpp?L501-506
qaisjp
left a comment
There was a problem hiding this comment.
Left some feedback about how we can preserve ordering + reuse the same packet, instead of using a new packet.
|
Sorry! I know it can be frustrating to go through multiple rounds of review :( |
|
Made it so the new RPC isn't needed, checked for compliance with coding guidelines, and re-tested. |
Summary
Fixes #594
removeElementData can now be called on the client which actually deletes (rather than just sets to nil or false) an element data. It also sends a new packet type to the server and syncs it with clients.
Motivation
To actually delete element data so that nil or false isn't being unnecessarily synced to future clients.
Test plan
Although I didn't test with 2 PCs I used it on an object then reconnected to verify that new clients are no longer being synced a removed element data.
Edit: tested with 2 PCs and the remote players have the element data removed correctly.