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

Island delete #466

Merged
merged 9 commits into from
Jan 13, 2019
Merged

Island delete #466

merged 9 commits into from
Jan 13, 2019

Conversation

tastybento
Copy link
Member

See commit message.

Addresses #457. Should help with #447.

Island deletion is done a few chunks at a time per tick. Current speed
is 5 chunks per tick per world (e.g., 15 chunks per tick if nether and
end islands are used).

Chunks are deleted based on the all-time maximum protection range of the
island. This is because the protection range can grow bigger or smaller
over the island's lifetime. To ensure all possible chunks are erased,
the largest every protection range is remembered and used.

Very large protection ranges will take a long time to fully delete.

Info on islands that are being deleted is stored in the database. If the
server shuts down mid-deletion, deletion will restart when the server
restarts.

While an island is being deleted, new islands cannot occupy that spot
and the spot cannot be reserved by the admin.

In addition, async approaches to island saving and player saving were
removed. Async will be implemented another way.

Now, instead of saving the full island or player database, individual
database entries are saved instead to be more efficient.
@tastybento tastybento requested a review from Poslovitch January 13, 2019 01:14
Both seem to work fine, but probably need more real-world testing.
@Poslovitch Poslovitch added this to the 1.1 milestone Jan 13, 2019
}
Vector location = center.toVector();
user.sendMessage("commands.admin.info.island-location", "[xyz]", Util.xyz(location));
Vector from = center.toVector().subtract(new Vector(range, 0, range)).setY(0);
Vector to = center.toVector().add(new Vector(range-1, 0, range-1)).setY(center.getWorld().getMaxHeight());
user.sendMessage("commands.admin.info.island-coords", "[xz1]", Util.xyz(from), "[xz2]", Util.xyz(to));
user.sendMessage("commands.admin.info.protection-range", "[range]", String.valueOf(protectionRange));
user.sendMessage("commands.admin.info.max-protection-range", "[range]", String.valueOf(maxEverProtectionRange));
Copy link
Member

Choose a reason for hiding this comment

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

This might only lead to admins being confused. Why aren't you using the range (island distance) instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Quick answer - using island distance takes too long and is not necessary because only the protection range chunks should need to be deleted.

To avoid admin confusion, I could delete that line 701 and just keep it an internal number.

By the way, I did some experiments with deleting large island ranges running in the background they can take a very long time to delete (minutes), so that's why I wanted to avoid having to regenerate all those chunks. Once 4 or 5 islands were in deletion, the server became very sluggish. So, it's a tradeoff.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can make this small change on the main develop branch if required. Let me know what you think.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, I understand why you didn't use range then. That makes sense.

Then, I'd like to remove line 701. Just to avoid any kind of "support request" that could occur because of that.

@Poslovitch
Copy link
Member

Overall, the code's good.

@tastybento tastybento merged commit 6557d2f into develop Jan 13, 2019
@Poslovitch
Copy link
Member

Checks failed because I wasn't using the updated pom. Now that it's been merged in develop, that'll be fine.

@Poslovitch Poslovitch deleted the IslandDelete branch January 13, 2019 16:10
@tastybento
Copy link
Member Author

I'm also adding the MariaDB deletedID method now. Commit in a moment.

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

Successfully merging this pull request may close these issues.

2 participants