oss/luanti
oss/luanti
1
0
Fork 0
mirror of https://github.com/luanti-org/luanti.git synced 2025-09-30 19:22:14 +00:00
Code Wiki Activity

Refactor server-side node placement or dig prediction handling

Browse source 
The code for handling `TOSERVER_INTERACT` packets has some style-related
problems:
* There are five places with mapblock-sending code to undo predictions in
  the large `handleCommand_Interact` method in `serverpackethandler.cpp`,
  so these can be abstracted away into a function to shorten the code.
* In `networkprotocol.h`, below `TOSERVER_INTERACT`,
  there is an outdated copy of the `InteractAction` enum
  documentation and the player position information is not mentioned.
* In `serverpackethandler.cpp`, there is a (newer) copy of the
  `TOSERVER_INTERACT` documentation from `networkprotocol.h`.

Furthermore, the way the server undoes or confirms client-side node placement
or dig predictions looks inconsistent, thus erroneous:
* If a player tries to place a node in a distance too far away or if he/she
  is dead,
  only the mapblock containing `pointed.node_undersurface` is triggered to be
  sent although the player may have placed the node at
  `pointed.node_abovesurface` and this position may be in a different mapblock.
* Similarly, if a player without interact privilege tries to place a node,
  only the mapblock containing `pointed.node_abovesurface` is triggered to be
  sent although the player may have placed the node at
  `pointed.node_undersurface` and this position may be in a different mapblock.

This change addresses the problems listed above:
* Fix the documentation comments related to the TOSERVER_INTERACT package
  content
* Move mapblock sending code related to predictions to ClientInterface
* If the player tries to place but has no interact privilege, is dead,
  or exceeds the permitted distance,
  send both the mapblock corresponding to `pointed.node_undersurface` and
  `pointed.node_abovesurface` if these mapblocks differ
This commit is contained in:
HybridDog 2024-01-24 22:03:11 +01:00
parent dc7a7a0ed9
commit 6327101607
4 changed files with 64 additions and 88 deletions
Download patch file Download diff file Expand all files Collapse all files

25
src/network/networkprotocol.h
View file

@ -1006,17 +1006,20 @@ enum ToServerCommand : u16
TOSERVER_INTERACT = 0x39,
/*
[0] u16 command
[2] u8 action
[3] u16 item
[5] u32 length of the next item
[9] serialized PointedThing
actions:
0: start digging (from undersurface) or use
1: stop digging (all parameters ignored)
2: digging completed
3: place block or item (to abovesurface)
4: use item
u16 command
u8 action (InteractAction)
u16 item
u32 length of the next item
u8[length] serialized PointedThing
v3s32 player position multiplied by 100 and converted to integers
v3s32 player speed multiplied by 100 and converted to integers
s32 player pitch multiplied by 100 and converted to an integer
s32 player yaw multiplied by 100 and converted to an integer
u32 keyPressed
u8 fov
u8 wanted drawing range
optional:
u8 flags; it is 0x01 if the camera is inverted
*/
TOSERVER_REMOVED_SOUNDS = 0x3a,

78
src/network/serverpackethandler.cpp
View file

@ -921,15 +921,6 @@ static inline void getWieldedItem(const PlayerSAO *playersao, std::optional<Item
void Server::handleCommand_Interact(NetworkPacket *pkt)
{
/*
[0] u16 command
[2] u8 action
[3] u16 item
[5] u32 length of the next item (plen)
[9] serialized PointedThing
[9 + plen] player position information
*/
InteractAction action;
u16 item_i;
@ -966,13 +957,7 @@ void Server::handleCommand_Interact(NetworkPacket *pkt)
if (playersao->isDead()) {
actionstream << "Server: " << player->getName()
<< " tried to interact while dead; ignoring." << std::endl;
if (pointed.type == POINTEDTHING_NODE) {
// Re-send block to revert change on client-side
RemoteClient *client = getClient(peer_id);
v3s16 blockpos = getNodeBlockPos(pointed.node_undersurface);
client->SetBlockNotSent(blockpos);
}
// Call callbacks
getClient(peer_id)->respondToInteraction(action, pointed, false);
m_script->on_cheat(playersao, "interacted_while_dead");
return;
}
@ -1012,22 +997,7 @@ void Server::handleCommand_Interact(NetworkPacket *pkt)
if (!checkPriv(player->getName(), "interact")) {
actionstream << player->getName() << " attempted to interact with " <<
pointed.dump() << " without 'interact' privilege" << std::endl;
if (pointed.type != POINTEDTHING_NODE)
return;
// Re-send block to revert change on client-side
RemoteClient *client = getClient(peer_id);
// Digging completed -> under
if (action == INTERACT_DIGGING_COMPLETED) {
v3s16 blockpos = getNodeBlockPos(pointed.node_undersurface);
client->SetBlockNotSent(blockpos);
}
// Placement -> above
else if (action == INTERACT_PLACE) {
v3s16 blockpos = getNodeBlockPos(pointed.node_abovesurface);
client->SetBlockNotSent(blockpos);
}
getClient(peer_id)->respondToInteraction(action, pointed, false);
return;
}
@ -1055,12 +1025,7 @@ void Server::handleCommand_Interact(NetworkPacket *pkt)
float d = playersao->getEyePosition().getDistanceFrom(target_pos);
if (!checkInteractDistance(player, d, pointed.dump())) {
if (pointed.type == POINTEDTHING_NODE) {
// Re-send block to revert change on client-side
RemoteClient *client = getClient(peer_id);
v3s16 blockpos = getNodeBlockPos(pointed.node_undersurface);
client->SetBlockNotSent(blockpos);
}
getClient(peer_id)->respondToInteraction(action, pointed, false);
return;
}
}
@ -1213,14 +1178,12 @@ void Server::handleCommand_Interact(NetworkPacket *pkt)
if (is_valid_dig && n.getContent() != CONTENT_IGNORE)
m_script->node_on_dig(p_under, n, playersao);
v3s16 blockpos = getNodeBlockPos(p_under);
RemoteClient *client = getClient(peer_id);
// Send unusual result (that is, node not being removed)
if (m_env->getMap().getNode(p_under).getContent() != CONTENT_AIR)
// Re-send block to revert change on client-side
client->SetBlockNotSent(blockpos);
else
client->ResendBlockIfOnWire(blockpos);
// For whatever reason we assume that the client always predicts that a
// dug node is air irrespective of the node's node_dig_prediction
bool prediction_success =
m_env->getMap().getNode(p_under).getContent() == CONTENT_AIR;
getClient(peer_id)->respondToInteraction(action, pointed,
prediction_success);
return;
} // action == INTERACT_DIGGING_COMPLETED
@ -1264,24 +1227,11 @@ void Server::handleCommand_Interact(NetworkPacket *pkt)
SendInventory(player, true);
}
if (pointed.type != POINTEDTHING_NODE)
return;
// If item has node placement prediction, always send the
// blocks to make sure the client knows what exactly happened
RemoteClient *client = getClient(peer_id);
v3s16 blockpos = getNodeBlockPos(pointed.node_abovesurface);
v3s16 blockpos2 = getNodeBlockPos(pointed.node_undersurface);
if (had_prediction) {
client->SetBlockNotSent(blockpos);
if (blockpos2 != blockpos)
client->SetBlockNotSent(blockpos2);
} else {
client->ResendBlockIfOnWire(blockpos);
if (blockpos2 != blockpos)
client->ResendBlockIfOnWire(blockpos2);
}
// Since we do not known if the client has predicted the node at
// pointed.above or pointed.under,
// we assume that a prediction is always wrong.
getClient(peer_id)->respondToInteraction(action, pointed,
!had_prediction);
return;
} // action == INTERACT_PLACE

33
src/server/clientiface.cpp
View file

@ -31,6 +31,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
#include "server/luaentity_sao.h"
#include "server/player_sao.h"
#include "log.h"
#include "util/pointedthing.h"
#include "util/srp.h"
#include "util/string.h"
#include "face_position_cache.h"
@ -77,14 +78,6 @@ RemoteClient::RemoteClient() :
{
}
void RemoteClient::ResendBlockIfOnWire(v3s16 p)
{
// if this block is on wire, mark it for sending again as soon as possible
if (m_blocks_sending.find(p) != m_blocks_sending.end()) {
SetBlockNotSent(p);
}
}
static LuaEntitySAO *getAttachedObject(PlayerSAO *sao, ServerEnvironment *env)
{
ServerActiveObject *ao = sao;
@ -469,6 +462,30 @@ void RemoteClient::SetBlocksNotSent(const std::vector<v3s16> &blocks)
}
}
void RemoteClient::respondToInteraction(InteractAction action,
const PointedThing &pointed, bool prediction_success)
{
if ((action != INTERACT_PLACE && action != INTERACT_DIGGING_COMPLETED)
|| pointed.type != POINTEDTHING_NODE)
// The client has not predicted not node changes
return;
// The client may have an outdated mapblock if the placement or dig
// prediction was wrong or if an old mapblock is still being sent to it.
v3s16 blockpos = getNodeBlockPos(pointed.node_undersurface);
if (!prediction_success
|| m_blocks_sending.find(blockpos) != m_blocks_sending.end()) {
SetBlockNotSent(blockpos);
}
if (action != INTERACT_PLACE)
return;
v3s16 blockpos2 = getNodeBlockPos(pointed.node_abovesurface);
if (blockpos2 != blockpos && (!prediction_success
|| m_blocks_sending.find(blockpos2) != m_blocks_sending.end())) {
SetBlockNotSent(blockpos2);
}
}
void RemoteClient::notifyEvent(ClientStateEvent event)
{
std::ostringstream myerror;

16
src/server/clientiface.h
View file

@ -38,6 +38,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
#include <mutex>
class MapBlock;
class PointedThing;
class ServerEnvironment;
class EmergeManager;
@ -270,12 +271,17 @@ public:
void SetBlocksNotSent(const std::vector<v3s16> &blocks);
/**
* tell client about this block being modified right now.
* this information is required to requeue the block in case it's "on wire"
* while modification is processed by server
* @param p position of modified block
* Trigger block sending to undo client-side predicted node changes
*
* \param action The interact action to determine which predictions the
* client could have made
* \param pointed The positions where the client has interacted
* \param prediction_success If true, assume that the client has made a
* correct prediction and send the mapblock only if an outdated mapblock
* is currently "on wire", which can erroneously override the prediction
*/
void ResendBlockIfOnWire(v3s16 p);
void respondToInteraction(InteractAction action,
const PointedThing &pointed, bool prediction_success);
u32 getSendingCount() const { return m_blocks_sending.size(); }