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

Decouple island location from island center. #1659

Merged
merged 15 commits into from
Feb 13, 2021
Merged

Conversation

tastybento
Copy link
Member

This PR enables the protection area to move anywhere within the island boundaries. This is required to enable a new game mode I'm working on where the island protection zone starts small and increases based on achievements/challenges. I need a way to move the protection zone anywhere within the island boundaries and not be locked to the center.

I added an admin command to enable manual setting of the island location. I also expanded the info command to show the island location.

In summary, the island location is the center of the protection area. It is usually the center of the island but can be moved. The protection area will not go outside the island range. However, as it is possible for the island location to be right on the edge of the island range, the protection range value can now be up to the diameter of the island, i.e., 2 * island range.

@tastybento tastybento added the Type: Enhancement Improvement or modification which is usually a new feature. label Jan 23, 2021
@BONNe
Copy link
Member

BONNe commented Jan 23, 2021

:D Nice...

// Range cannot be greater than the island distance
range = Math.min(range, plugin.getIWM().getIslandDistance(island.getWorld()));
// Range cannot be greater than the island distance * 2
range = Math.min(range, 2 * plugin.getIWM().getIslandDistance(island.getWorld()));
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if this is a correct way how to do it.

I would prefer to calculate by offset from the actual island center.

Copy link
Member Author

Choose a reason for hiding this comment

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

The protection range is now relative to the protection center location, so if the player is to be given the ability to use all of their island area, then their protection range must be able to go up to 2x island distance away from that point.

For example:

Island range = 400
Island center = 0,0
Island location = 399, 0
Island protection range = 800

This last value will enable them to operate anywhere with all of the island space.

@tastybento
Copy link
Member Author

I still need to fix the tests, but I don't expect any issue there.

@Poslovitch
Copy link
Member

However, as it is possible for the island location to be right on the edge of the island range, the protection range value can now be up to the diameter of the island, i.e., 2 * island range.

I didn't review the PR yet, but want to discuss what this implies.

Will the island protection zone "bleed" into the "inter-island space" if the island location is located right at the edge of the island range ?

Could you please elaborate what the use case would be for this ?

How could we make sure this does not get confusing ? I'm already a tad confused by the island center / island location / protection range / island range stuff. Who's who ? 🙂

@Poslovitch
Copy link
Member

@tastybento Could you please check #1659 (comment) ?

@tastybento
Copy link
Member Author

Will the island protection zone "bleed" into the "inter-island space" if the island location is located right at the edge of the island range ?

No. The protection zone is all in the Island class if you want to check it.

Could you please elaborate what the use case would be for this ?

In the new game mode you start with a tiny island, ie., a tiny protection area. There is a game mechanic where you can throw an enderpearl to move your island to another location to obtain better resources. A player can move the island anywhere within the island distance multiple times. The protection zone can grow over time so it would be possible for a player to move the location to a spot near the edge of their island maximum.

All that is really happening is that we are now able to move the protection area around inside the island space instead of having it locked to the center point.

How could we make sure this does not get confusing ? I'm already a tad confused by the island center / island location / protection range / island range stuff. Who's who ? 🙂

This is primarily for programmers, but it is logical:

  • Island center - the center of the island space. Never moves. It's the dot on the grid.
  • Island range - the distance between islands.
  • Island location - the spot inside the island range where the island actually is. This may be the same as the island center but does not have to be.
  • Protection zone - the area around the island's location that is protected. Defined by the protection range. The protection zone never extends further than the island range.

A diagram would make this easier to understand so that could be added to the docs.

@tastybento
Copy link
Member Author

One change to make it clearer could be to use protectionCenter so it would be getProtectionCenter() instead of getLocation().

@tastybento
Copy link
Member Author

@Poslovitch Any thoughts on the above? ^^^

The speed option was doing nothing except causing repeated actions on
the same chunks.
Conflicts:
	src/main/java/world/bentobox/bentobox/util/DeleteIslandChunks.java
@Poslovitch
Copy link
Member

One change to make it clearer could be to use protectionCenter so it would be getProtectionCenter() instead of getLocation().

I agree to that.

Please also resolve the conflict 😉

@Poslovitch
Copy link
Member

And thanks for your explanation. They indeed make the PR easier to understand. It might be worth creating a page on the Wiki for this. We have tons of things to explain about that already, and this new "layer" is surely going to confuse admins.

@Poslovitch Poslovitch added the Status: Need review Waiting for a review to be made on this Pull Request label Feb 7, 2021
@Poslovitch Poslovitch added this to the 1.16.0 - Home sweet home... milestone Feb 7, 2021
Copy link
Member

@Poslovitch Poslovitch left a comment

Choose a reason for hiding this comment

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

Everything looks good. Do not forget to rename the placeholders and the commands appropriately, to keep up with the suggested change.

Thanks for your work, and sorry for the time it took me to finally leave a review.

@Poslovitch Poslovitch removed the Status: Need review Waiting for a review to be made on this Pull Request label Feb 7, 2021
@Poslovitch Poslovitch added the Status: Need revision Waiting for a revision to be made on this Pull Request label Feb 7, 2021
Added logic to expand the deletion area based on moving the island
protection location and range instead of just deleting everything. This
will keep deletion time to a minimum for BSkyBlock, etc.
Conflicts resolved:
	src/test/java/world/bentobox/bentobox/listeners/PortalTeleportationListenerTest.java
@tastybento tastybento removed the Status: Need revision Waiting for a revision to be made on this Pull Request label Feb 13, 2021
@tastybento tastybento merged commit 1d9ce72 into develop Feb 13, 2021
@tastybento tastybento deleted the island_location branch February 13, 2021 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Improvement or modification which is usually a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants