ServerEnv: Clean up object lifecycle handling (#6414)

* ServerEnv: Clean up object lifecycle handling
This commit is contained in:
sfan5 2017-09-15 12:19:01 +02:00 committed by Loïc Blot
parent edbc533414
commit 04839f233f
8 changed files with 175 additions and 213 deletions

@ -59,7 +59,7 @@ public:
m_age += dtime;
if(m_age > 10)
{
m_removed = true;
m_pending_removal = true;
return;
}
@ -397,12 +397,11 @@ void LuaEntitySAO::step(float dtime, bool send_recommended)
{
ServerMap *map = dynamic_cast<ServerMap *>(&m_env->getMap());
assert(map);
if (!m_pending_deactivation &&
if (!m_pending_removal &&
map->saoPositionOverLimit(m_base_position)) {
infostream << "Remove SAO " << m_id << "(" << m_init_name
infostream << "Removing SAO " << m_id << "(" << m_init_name
<< "), outside of limits" << std::endl;
m_pending_deactivation = true;
m_removed = true;
m_pending_removal = true;
return;
}
}
@ -550,9 +549,9 @@ int LuaEntitySAO::punch(v3f dir,
ServerActiveObject *puncher,
float time_from_last_punch)
{
if (!m_registered){
if (!m_registered) {
// Delete unknown LuaEntities when punched
m_removed = true;
m_pending_removal = true;
return 0;
}
@ -596,12 +595,10 @@ int LuaEntitySAO::punch(v3f dir,
}
if (getHP() == 0) {
m_removed = true;
m_pending_removal = true;
m_env->getScriptIface()->luaentity_on_death(m_id, puncher);
}
return result.wear;
}
@ -1353,11 +1350,10 @@ void PlayerSAO::setWieldIndex(int i)
}
}
// Erase the peer id and make the object for removal
void PlayerSAO::disconnected()
{
m_peer_id = 0;
m_removed = true;
m_pending_removal = true;
}
void PlayerSAO::unlinkPlayerSessionAndSave()

@ -1124,8 +1124,8 @@ void Server::handleCommand_Interact(NetworkPacket* pkt)
playersao->noCheatDigStart(p_under);
}
else if (pointed.type == POINTEDTHING_OBJECT) {
// Skip if object has been removed
if (pointed_object->m_removed)
// Skip if object can't be interacted with anymore
if (pointed_object->isGone())
return;
actionstream<<player->getName()<<" punches object "
@ -1283,8 +1283,8 @@ void Server::handleCommand_Interact(NetworkPacket* pkt)
if (pointed.type == POINTEDTHING_OBJECT) {
// Right click object
// Skip if object has been removed
if (pointed_object->m_removed)
// Skip if object can't be interacted with anymore
if (pointed_object->isGone())
return;
actionstream << player->getName() << " right-clicks object "

@ -343,6 +343,10 @@ void ScriptApiBase::objectrefGetOrCreate(lua_State *L,
ObjectRef::create(L, cobj);
} else {
push_objectRef(L, cobj->getId());
if (cobj->isGone())
warningstream << "ScriptApiBase::objectrefGetOrCreate(): "
<< "Pushing ObjectRef to removed/deactivated object"
<< ", this is probably a bug." << std::endl;
}
}

@ -642,7 +642,7 @@ int ModApiEnvMod::l_get_objects_inside_radius(lua_State *L)
std::vector<u16>::const_iterator iter = ids.begin();
for(u32 i = 0; iter != ids.end(); ++iter) {
ServerActiveObject *obj = env->getActiveObject(*iter);
if (!obj->m_removed) {
if (!obj->isGone()) {
// Insert object reference into table
script->objectrefGetOrCreate(L, obj);
lua_rawseti(L, -2, ++i);

@ -140,15 +140,14 @@ int ObjectRef::l_remove(lua_State *L)
return 0;
const std::unordered_set<int> &child_ids = co->getAttachmentChildIds();
std::unordered_set<int>::const_iterator it;
for (it = child_ids.begin(); it != child_ids.end(); ++it) {
for (int child_id : child_ids) {
// Child can be NULL if it was deleted earlier
if (ServerActiveObject *child = env->getActiveObject(*it))
if (ServerActiveObject *child = env->getActiveObject(child_id))
child->setAttachment(0, "", v3f(0, 0, 0), v3f(0, 0, 0));
}
verbosestream<<"ObjectRef::l_remove(): id="<<co->getId()<<std::endl;
co->m_removed = true;
verbosestream << "ObjectRef::l_remove(): id=" << co->getId() << std::endl;
co->m_pending_removal = true;
return 0;
}

@ -971,24 +971,17 @@ void ServerEnvironment::clearObjects(ClearObjectsMode mode)
<< "Removing all active objects" << std::endl;
std::vector<u16> objects_to_remove;
for (auto &it : m_active_objects) {
u16 id = it.first;
ServerActiveObject* obj = it.second;
if (obj->getType() == ACTIVEOBJECT_TYPE_PLAYER)
continue;
u16 id = it.first;
// Delete static object if block is loaded
if (obj->m_static_exists) {
MapBlock *block = m_map->getBlockNoCreateNoEx(obj->m_static_block);
if (block) {
block->m_static_objects.remove(id);
block->raiseModified(MOD_STATE_WRITE_NEEDED,
MOD_REASON_CLEAR_ALL_OBJECTS);
obj->m_static_exists = false;
}
}
deleteStaticFromBlock(obj, id, MOD_REASON_CLEAR_ALL_OBJECTS, true);
// If known by some client, don't delete immediately
if (obj->m_known_by_count > 0) {
obj->m_pending_deactivation = true;
obj->m_removed = true;
obj->m_pending_removal = true;
continue;
}
@ -1302,16 +1295,14 @@ void ServerEnvironment::step(float dtime)
for (auto &ao_it : m_active_objects) {
ServerActiveObject* obj = ao_it.second;
// Don't step if is to be removed or stored statically
if(obj->m_removed || obj->m_pending_deactivation)
if (obj->isGone())
continue;
// Step object
obj->step(dtime, send_recommended);
// Read messages from object
while(!obj->m_messages_out.empty())
{
m_active_object_messages.push(
obj->m_messages_out.front());
while (!obj->m_messages_out.empty()) {
m_active_object_messages.push(obj->m_messages_out.front());
obj->m_messages_out.pop();
}
}
@ -1322,9 +1313,6 @@ void ServerEnvironment::step(float dtime)
*/
if (m_object_management_interval.step(dtime, 0.5)) {
ScopeProfiler sp(g_profiler, "SEnv: remove removed objs avg /.5s", SPT_AVG);
/*
Remove objects that satisfy (m_removed && m_known_by_count==0)
*/
removeRemovedObjects();
}
@ -1444,7 +1432,7 @@ void ServerEnvironment::getAddedActiveObjects(PlayerSAO *playersao, s16 radius,
player_radius_f = 0;
/*
Go through the object list,
- discard m_removed objects,
- discard removed/deactivated objects,
- discard objects that are too far away,
- discard objects that are found in current_objects.
- add remaining objects to added_objects
@ -1457,8 +1445,7 @@ void ServerEnvironment::getAddedActiveObjects(PlayerSAO *playersao, s16 radius,
if (object == NULL)
continue;
// Discard if removed or deactivating
if(object->m_removed || object->m_pending_deactivation)
if (object->isGone())
continue;
f32 distance_f = object->getBasePosition().
@ -1497,9 +1484,9 @@ void ServerEnvironment::getRemovedActiveObjects(PlayerSAO *playersao, s16 radius
/*
Go through current_objects; object is removed if:
- object is not found in m_active_objects (this is actually an
error condition; objects should be set m_removed=true and removed
only after all clients have been informed about removal), or
- object has m_removed=true, or
error condition; objects should be removed only after all clients
have been informed about removal), or
- object is to be removed or deactivated, or
- object is too far away
*/
for (u16 id : current_objects) {
@ -1512,7 +1499,7 @@ void ServerEnvironment::getRemovedActiveObjects(PlayerSAO *playersao, s16 radius
continue;
}
if (object->m_removed || object->m_pending_deactivation) {
if (object->isGone()) {
removed_objects.push(id);
continue;
}
@ -1684,7 +1671,7 @@ u16 ServerEnvironment::addActiveObjectRaw(ServerActiveObject *object,
}
/*
Remove objects that satisfy (m_removed && m_known_by_count==0)
Remove objects that satisfy (isGone() && m_known_by_count==0)
*/
void ServerEnvironment::removeRemovedObjects()
{
@ -1692,62 +1679,54 @@ void ServerEnvironment::removeRemovedObjects()
for (auto &ao_it : m_active_objects) {
u16 id = ao_it.first;
ServerActiveObject* obj = ao_it.second;
// This shouldn't happen but check it
if(obj == NULL)
{
infostream<<"NULL object found in ServerEnvironment"
<<" while finding removed objects. id="<<id<<std::endl;
// Id to be removed from m_active_objects
if (!obj) {
errorstream << "ServerEnvironment::removeRemovedObjects(): "
<< "NULL object found. id=" << id << std::endl;
objects_to_remove.push_back(id);
continue;
}
/*
We will delete objects that are marked as removed or thatare
waiting for deletion after deactivation
We will handle objects marked for removal or deactivation
*/
if (!obj->m_removed && !obj->m_pending_deactivation)
if (!obj->isGone())
continue;
/*
Delete static data from block if is marked as removed
Delete static data from block if removed
*/
if(obj->m_static_exists && obj->m_removed)
{
MapBlock *block = m_map->emergeBlock(obj->m_static_block, false);
if (block) {
block->m_static_objects.remove(id);
block->raiseModified(MOD_STATE_WRITE_NEEDED,
MOD_REASON_REMOVE_OBJECTS_REMOVE);
obj->m_static_exists = false;
} else {
infostream<<"Failed to emerge block from which an object to "
<<"be removed was loaded from. id="<<id<<std::endl;
}
}
if (obj->m_pending_removal)
deleteStaticFromBlock(obj, id, MOD_REASON_REMOVE_OBJECTS_REMOVE, false);
// If m_known_by_count > 0, don't actually remove. On some future
// If still known by clients, don't actually remove. On some future
// invocation this will be 0, which is when removal will continue.
if(obj->m_known_by_count > 0)
continue;
/*
Move static data from active to stored if not marked as removed
Move static data from active to stored if deactivated
*/
if(obj->m_static_exists && !obj->m_removed){
if (!obj->m_pending_removal && obj->m_static_exists) {
MapBlock *block = m_map->emergeBlock(obj->m_static_block, false);
if (block) {
std::map<u16, StaticObject>::iterator i =
block->m_static_objects.m_active.find(id);
if(i != block->m_static_objects.m_active.end()){
if (i != block->m_static_objects.m_active.end()) {
block->m_static_objects.m_stored.push_back(i->second);
block->m_static_objects.m_active.erase(id);
block->raiseModified(MOD_STATE_WRITE_NEEDED,
MOD_REASON_REMOVE_OBJECTS_DEACTIVATE);
} else {
warningstream << "ServerEnvironment::removeRemovedObjects(): "
<< "id=" << id << " m_static_exists=true but "
<< "static data doesn't actually exist in "
<< PP(obj->m_static_block) << std::endl;
}
} else {
infostream<<"Failed to emerge block from which an object to "
<<"be deactivated was loaded from. id="<<id<<std::endl;
infostream << "Failed to emerge block from which an object to "
<< "be deactivated was loaded from. id=" << id << std::endl;
}
}
@ -1760,7 +1739,6 @@ void ServerEnvironment::removeRemovedObjects()
if(obj->environmentDeletes())
delete obj;
// Id to be removed from m_active_objects
objects_to_remove.push_back(id);
}
// Remove references from m_active_objects
@ -1855,6 +1833,7 @@ void ServerEnvironment::activateObjects(MapBlock *block, u32 dtime_s)
// This will also add the object to the active static list
addActiveObjectRaw(obj, false, dtime_s);
}
// Clear stored list
block->m_static_objects.m_stored.clear();
// Add leftover failed stuff to stored list
@ -1862,15 +1841,6 @@ void ServerEnvironment::activateObjects(MapBlock *block, u32 dtime_s)
block->m_static_objects.m_stored.push_back(s_obj);
}
// Turn the active counterparts of activated objects not pending for
// deactivation
for (auto &i : block->m_static_objects.m_active) {
u16 id = i.first;
ServerActiveObject *object = getActiveObject(id);
assert(object);
object->m_pending_deactivation = false;
}
/*
Note: Block hasn't really been modified here.
The objects have just been activated and moved from the stored
@ -1906,8 +1876,8 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
if(!force_delete && !obj->isStaticAllowed())
continue;
// If pending deactivation, let removeRemovedObjects() do it
if(!force_delete && obj->m_pending_deactivation)
// removeRemovedObjects() is responsible for these
if(!force_delete && obj->isGone())
continue;
u16 id = ao_it.first;
@ -1924,47 +1894,25 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
!m_active_blocks.contains(obj->m_static_block) &&
m_active_blocks.contains(blockpos_o))
{
v3s16 old_static_block = obj->m_static_block;
// Delete from block where object was located
deleteStaticFromBlock(obj, id, MOD_REASON_STATIC_DATA_REMOVED, false);
// Save to block where object is located
MapBlock *block = m_map->emergeBlock(blockpos_o, false);
if(!block){
errorstream<<"ServerEnvironment::deactivateFarObjects(): "
<<"Could not save object id="<<id
<<" to it's current block "<<PP(blockpos_o)
<<std::endl;
continue;
}
std::string staticdata_new;
obj->getStaticData(&staticdata_new);
StaticObject s_obj(obj->getType(), objectpos, staticdata_new);
block->m_static_objects.insert(id, s_obj);
obj->m_static_block = blockpos_o;
block->raiseModified(MOD_STATE_WRITE_NEEDED,
MOD_REASON_STATIC_DATA_ADDED);
// Save to block where object is located
saveStaticToBlock(blockpos_o, id, obj, s_obj, MOD_REASON_STATIC_DATA_ADDED);
// Delete from block where object was located
block = m_map->emergeBlock(old_static_block, false);
if(!block){
errorstream<<"ServerEnvironment::deactivateFarObjects(): "
<<"Could not delete object id="<<id
<<" from it's previous block "<<PP(old_static_block)
<<std::endl;
continue;
}
block->m_static_objects.remove(id);
block->raiseModified(MOD_STATE_WRITE_NEEDED,
MOD_REASON_STATIC_DATA_REMOVED);
continue;
}
// If block is active, don't remove
// If block is still active, don't remove
if(!force_delete && m_active_blocks.contains(blockpos_o))
continue;
verbosestream<<"ServerEnvironment::deactivateFarObjects(): "
<<"deactivating object id="<<id<<" on inactive block "
<<PP(blockpos_o)<<std::endl;
verbosestream << "ServerEnvironment::deactivateFarObjects(): "
<< "deactivating object id=" << id << " on inactive block "
<< PP(blockpos_o) << std::endl;
// If known by some client, don't immediately delete.
bool pending_delete = (obj->m_known_by_count > 0 && !force_delete);
@ -1972,7 +1920,6 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
/*
Update the static data
*/
if(obj->isStaticAllowed())
{
// Create new static object
@ -1983,6 +1930,7 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
bool stays_in_same_block = false;
bool data_changed = true;
// Check if static data has changed considerably
if (obj->m_static_exists) {
if (obj->m_static_block == blockpos_o)
stays_in_same_block = true;
@ -2001,108 +1949,47 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
(static_old.pos - objectpos).getLength() < save_movem)
data_changed = false;
} else {
errorstream<<"ServerEnvironment::deactivateFarObjects(): "
<<"id="<<id<<" m_static_exists=true but "
<<"static data doesn't actually exist in "
<<PP(obj->m_static_block)<<std::endl;
warningstream << "ServerEnvironment::deactivateFarObjects(): "
<< "id=" << id << " m_static_exists=true but "
<< "static data doesn't actually exist in "
<< PP(obj->m_static_block) << std::endl;
}
}
}
/*
While changes are always saved, blocks are only marked as modified
if the object has moved or different staticdata. (see above)
*/
bool shall_be_written = (!stays_in_same_block || data_changed);
u32 reason = shall_be_written ? MOD_REASON_STATIC_DATA_CHANGED : MOD_REASON_UNKNOWN;
// Delete old static object
if(obj->m_static_exists)
{
MapBlock *block = m_map->emergeBlock(obj->m_static_block, false);
if(block)
{
block->m_static_objects.remove(id);
obj->m_static_exists = false;
// Only mark block as modified if data changed considerably
if(shall_be_written)
block->raiseModified(MOD_STATE_WRITE_NEEDED,
MOD_REASON_STATIC_DATA_CHANGED);
}
}
deleteStaticFromBlock(obj, id, reason, false);
// Add to the block where the object is located in
v3s16 blockpos = getNodeBlockPos(floatToInt(objectpos, BS));
// Get or generate the block
MapBlock *block = NULL;
try{
block = m_map->emergeBlock(blockpos);
} catch(InvalidPositionException &e){
// Handled via NULL pointer
// NOTE: emergeBlock's failure is usually determined by it
// actually returning NULL
}
if(block)
{
if (block->m_static_objects.m_stored.size() >= g_settings->getU16("max_objects_per_block")) {
warningstream << "ServerEnv: Trying to store id = " << obj->getId()
<< " statically but block " << PP(blockpos)
<< " already contains "
<< block->m_static_objects.m_stored.size()
<< " objects."
<< " Forcing delete." << std::endl;
force_delete = true;
} else {
// If static counterpart already exists in target block,
// remove it first.
// This shouldn't happen because the object is removed from
// the previous block before this according to
// obj->m_static_block, but happens rarely for some unknown
// reason. Unsuccessful attempts have been made to find
// said reason.
if(id && block->m_static_objects.m_active.find(id) != block->m_static_objects.m_active.end()){
warningstream<<"ServerEnv: Performing hack #83274"
<<std::endl;
block->m_static_objects.remove(id);
}
// Store static data
u16 store_id = pending_delete ? id : 0;
block->m_static_objects.insert(store_id, s_obj);
// Only mark block as modified if data changed considerably
if(shall_be_written)
block->raiseModified(MOD_STATE_WRITE_NEEDED,
MOD_REASON_STATIC_DATA_CHANGED);
obj->m_static_exists = true;
obj->m_static_block = block->getPos();
}
}
else{
if(!force_delete){
v3s16 p = floatToInt(objectpos, BS);
errorstream<<"ServerEnv: Could not find or generate "
<<"a block for storing id="<<obj->getId()
<<" statically (pos="<<PP(p)<<")"<<std::endl;
continue;
}
}
u16 store_id = pending_delete ? id : 0;
if (!saveStaticToBlock(blockpos, store_id, obj, s_obj, reason))
force_delete = true;
}
/*
If known by some client, set pending deactivation.
Otherwise delete it immediately.
*/
if(pending_delete && !force_delete)
{
verbosestream<<"ServerEnvironment::deactivateFarObjects(): "
<<"object id="<<id<<" is known by clients"
<<"; not deleting yet"<<std::endl;
verbosestream << "ServerEnvironment::deactivateFarObjects(): "
<< "object id=" << id << " is known by clients"
<< "; not deleting yet" << std::endl;
obj->m_pending_deactivation = true;
continue;
}
verbosestream<<"ServerEnvironment::deactivateFarObjects(): "
<<"object id="<<id<<" is not known by clients"
<<"; deleting"<<std::endl;
verbosestream << "ServerEnvironment::deactivateFarObjects(): "
<< "object id=" << id << " is not known by clients"
<< "; deleting" << std::endl;
// Tell the object about removal
obj->removingFromEnvironment();
@ -2122,6 +2009,69 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
}
}
void ServerEnvironment::deleteStaticFromBlock(
ServerActiveObject *obj, u16 id, u32 mod_reason, bool no_emerge)
{
if (!obj->m_static_exists)
return;
MapBlock *block;
if (no_emerge)
block = m_map->getBlockNoCreateNoEx(obj->m_static_block);
else
block = m_map->emergeBlock(obj->m_static_block, false);
if (!block) {
if (!no_emerge)
errorstream << "ServerEnv: Failed to emerge block " << PP(obj->m_static_block)
<< " when deleting static data of object from it. id=" << id << std::endl;
return;
}
block->m_static_objects.remove(id);
if (mod_reason != MOD_REASON_UNKNOWN) // Do not mark as modified if requested
block->raiseModified(MOD_STATE_WRITE_NEEDED, mod_reason);
obj->m_static_exists = false;
}
bool ServerEnvironment::saveStaticToBlock(
v3s16 blockpos, u16 store_id,
ServerActiveObject *obj, const StaticObject &s_obj,
u32 mod_reason)
{
MapBlock *block = nullptr;
try {
block = m_map->emergeBlock(blockpos);
} catch (InvalidPositionException &e) {
// Handled via NULL pointer
// NOTE: emergeBlock's failure is usually determined by it
// actually returning NULL
}
if (!block) {
errorstream << "ServerEnv: Failed to emerge block " << PP(obj->m_static_block)
<< " when saving static data of object to it. id=" << store_id << std::endl;
return false;
}
if (block->m_static_objects.m_stored.size() >= g_settings->getU16("max_objects_per_block")) {
warningstream << "ServerEnv: Trying to store id = " << store_id
<< " statically but block " << PP(blockpos)
<< " already contains "
<< block->m_static_objects.m_stored.size()
<< " objects." << std::endl;
return false;
}
block->m_static_objects.insert(store_id, s_obj);
if (mod_reason != MOD_REASON_UNKNOWN) // Do not mark as modified if requested
block->raiseModified(MOD_STATE_WRITE_NEEDED, mod_reason);
obj->m_static_exists = true;
obj->m_static_block = blockpos;
return true;
}
PlayerDatabase *ServerEnvironment::openPlayerDatabase(const std::string &name,
const std::string &savedir, const Settings &conf)
{

@ -35,6 +35,7 @@ class PlayerDatabase;
class PlayerSAO;
class ServerEnvironment;
class ActiveBlockModifier;
struct StaticObject;
class ServerActiveObject;
class Server;
class ServerScripting;
@ -368,7 +369,7 @@ private:
u16 addActiveObjectRaw(ServerActiveObject *object, bool set_changed, u32 dtime_s);
/*
Remove all objects that satisfy (m_removed && m_known_by_count==0)
Remove all objects that satisfy (isGone() && m_known_by_count==0)
*/
void removeRemovedObjects();
@ -388,6 +389,14 @@ private:
*/
void deactivateFarObjects(bool force_delete);
/*
A few helpers used by the three above methods
*/
void deleteStaticFromBlock(
ServerActiveObject *obj, u16 id, u32 mod_reason, bool no_emerge);
bool saveStaticToBlock(v3s16 blockpos, u16 store_id,
ServerActiveObject *obj, const StaticObject &s_obj, u32 mod_reason);
/*
Member variables
*/

@ -212,23 +212,27 @@ public:
it anymore.
- Removal is delayed to preserve the id for the time during which
it could be confused to some other object by some client.
- This is set to true by the step() method when the object wants
to be deleted.
- This can be set to true by anything else too.
- This is usually set to true by the step() method when the object wants
to be deleted but can be set by anything else too.
*/
bool m_removed = false;
bool m_pending_removal = false;
/*
This is set to true when an object should be removed from the active
object list but couldn't be removed because the id has to be
reserved for some client.
Same purpose as m_pending_removal but for deactivation.
deactvation = save static data in block, remove active object
The environment checks this periodically. If this is true and also
m_known_by_count is true, object is deleted from the active object
list.
If this is set alongside with m_pending_removal, removal takes
priority.
*/
bool m_pending_deactivation = false;
/*
A getter that unifies the above to answer the question:
"Can the environment still interact with this object?"
*/
inline bool isGone() const
{ return m_pending_removal || m_pending_deactivation; }
/*
Whether the object's static data has been stored to a block
*/