-
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
Re-save active entities more often if they move a certain distance #15605
base: master
Are you sure you want to change the base?
Re-save active entities more often if they move a certain distance #15605
Conversation
The current version is incorrect, since I accidentally compare the blocks as if they are coordinates, and we seem to could also use |
I wouldn't call this a bug per-se since there are always situations where you can kill the server and certain progress will be missing.
that's probably less than a block size (16) so not directly usable here. if we respected this value properly that'd mean we re-save the object every block (even tho the old is still active). that doesn't seem very sustainable, so might be a good idea introduce a new magic value like e.g. 6 blocks movement. |
Changed the size, so that the entity will be resaved when there is one block between the block with old (saved) position and the block with the new position (the distance between blocks is |
can we make the distance that is needed for re-save a settings option ? |
This is trading one consistency issue (player position being updated while entity position is not) for another (some entity positions get updated, others do not) plus potentially reduced performance if the chosen number turns out unsuitable for servers with plenty of decently fast-moving entities. (Unfortunately a proper consistency fix for hard crashes is hard because players and entities live in different databases.) |
As a mod creator, I agree that it'd be great to give mod developers the ability to customize this. But in this case it's value should be more meaningful, like real distance, because the current approach will, probably, be difficult to explain in the API documentation. |
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.
Looks good but I think a bit higher limit is appropriate
@@ -43,6 +43,8 @@ | |||
// A number that is much smaller than the timeout for particle spawners should/could ever be | |||
#define PARTICLE_SPAWNER_NO_EXPIRY -1024.f | |||
|
|||
#define ACTIVE_OBJECT_RESAVE_DISTANCE_SQ 2 * 2 |
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.
#define ACTIVE_OBJECT_RESAVE_DISTANCE_SQ 2 * 2 | |
static constexpr s16 ACTIVE_OBJECT_RESAVE_DISTANCE_SQ = 3 * 3; |
This patch should fix a bug when location in the database might be not updated in case of server crash, if the location in the database belongs to a block which wasn't unloaded up until the crash. The bug was fixed by adding a check of the distance between the two locations, and updating the data in case if it reached some threshold (
MAP_BLOCKSIZE
was used but it could be reconsidered.To do
This PR is Ready for Review.
How to test
paleotest
mod installed (https://content.luanti.org/packages/ElCeejo/paleotest/)SIGKILL
(pkill -9 luanti
)With this patch, after next log in the position of the mob should be near the position of the first player (as expected by users). Without this patch, the mob will be at the place where it was initially spawned, i.e. near the second player.
Remark: the latest version of the mod is buggy, and the mob might die for no reason during descend, but in other aspects it seems to work OK, and it's one of the easiest way to test the scenario I'm aware of.