Skip to content

Initial address command implementation - #108

Open
Redstoner507 wants to merge 4 commits into
BuildTheEarth:mainfrom
Redstoner507:address-command
Open

Redstoner507 wants to merge 4 commits into
BuildTheEarth:mainfrom
Redstoner507:address-command

Conversation

@Redstoner507

Copy link
Copy Markdown
Contributor

Implemented random new address command (Issue #64)

  • Use of PhotonAPI for getting the closest address and getting an address location
    (Will be replaced by Nominatim Api )

  • '/address get' will get the player the closest address

  • '/address teleport' will theport the player to the address entered


private boolean handleGetCommand(@NonNull Player player, String @NonNull [] args) {
if (!player.hasPermission(Permissions.ADDRESS_GET)) {
player.sendMessage(ChatHelper.getErrorComponent("You don't have the required %s to %s address.", "permission",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please use Utils#sendNoPermissionMessage


private boolean handleTeleportCommand(@NonNull Player player, String @NonNull [] args) {
if (!player.hasPermission(Permissions.ADDRESS_TELEPORT)) {
player.sendMessage(ChatHelper.getErrorComponent("You don't have the required %s to %s address.", "permission",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please also use Utils#sendNoPermissionMessage

return;
}

player.sendMessage(address);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think smt like: <gray>Address: <white>{address} + make it possible to copy (with hover as explanation) would make more sense

Comment on lines +130 to +131
player.sendMessage("Address found: " + address);
player.sendMessage("Teleporting... ");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Outputting the input again doesn't really make sense.

There are two option:

  • Output the address/infos found by API
  • Combine the messages and don't output that anyway. Smt like: Address found, teleporting... - as btt info message

Comment on lines +132 to +137
Bukkit.getScheduler().runTask(
BuildTeamTools.getInstance(),
() -> player.performCommand(
"tpll " + latitude + " " + longitude
)
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Are you sure you need to schedule that on main thread?

if (sender.hasPermission(Permissions.ADDRESS_TELEPORT)) list.add("teleport");


return list.stream()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it would make sense to provide some examples for teleport


String url = BASE_URL + "reverse?lat=" + coordinates.latitude()
+ "&lon=" + coordinates.longitude()
+ "&lang=en";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not completely sure what the best way is but outputting the address in the local language also sounds quite useful

return true;
}

if (args[0].equalsIgnoreCase("get")) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

/address with no arguments crashes

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