Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ dependencies {
implementation(libs.alpslib.utils) {
exclude(group = "com.github.cryptomorin", module = "XSeries")
}
implementation(libs.alpslib.geo)
implementation(libs.com.alpsbte.canvas)
implementation(libs.com.github.cryptomorin.xseries)
implementation(libs.net.wesjd.anvilgui)
Expand Down
2 changes: 2 additions & 0 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ fawe-bom = "1.56" # Ref: https://github.com/IntellectualSites/bom
bstats = "3.+" # https://central.sonatype.com/artifact/org.bstats/bstats-bukkit
bluemap-api = "2.+" # Ref: https://github.com/BlueMap-Minecraft/BlueMapAPI
net-buildtheearth-projection = "1.+" # Ref: https://github.com/BuildTheEarth/projection
alpslib-geo = "1.0.0" # https://mvn.alps-bte.com/service/rest/repository/browse/alps-bte/com/alpsbte/alpslib/alpslib-geo/

[libraries]
com-alpsbte-alpslib-alpslib-libpsterra = { module = "com.alpsbte.alpslib:alpslib-libpsterra", version.ref = "com-alpsbte-alpslib-alpslib-libpsterra" }
Expand All @@ -41,6 +42,7 @@ fawe-bom = { module = "com.intellectualsites.bom:bom-newest", version.ref = "faw
bstats-bukkit = { module = "org.bstats:bstats-bukkit", version.ref = "bstats" }
bluemap-api = { module = "de.bluecolored:bluemap-api", version.ref = "bluemap-api" }
net-buildtheearth-projection = { module = "net.buildtheearth:projection", version.ref = "net-buildtheearth-projection"}
alpslib-geo = { module = "com.alpsbte.alpslib:alpslib-geo", version.ref = "alpslib-geo" }

[plugins]
lombok = { id = "io.freefair.lombok", version.ref = "io-freefair-lombok" }
Expand Down
2 changes: 1 addition & 1 deletion settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ dependencyResolutionManagement {
repositoriesMode.set(RepositoriesMode.FAIL_ON_PROJECT_REPOS)
repositories {
mavenCentral()
//mavenLocal() // NEVER use in Production/Commits!
// mavenLocal() // NEVER use in Production/Commits!
maven {
url = uri("https://repo.papermc.io/repository/maven-public/")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.util.Objects;

@UtilityClass
public class NavUtils {
public static void sendPlayerToConnectedServer(Player player, String server) {
Expand Down Expand Up @@ -173,4 +175,18 @@ public static Location getLocationFromCoordinatesYawPitch(GeographicalCoordinate
public static Location getLocationFromCoordinates(GeographicalCoordinate coordinate) {
return getLocationFromCoordinatesYawPitch(coordinate, 0, 0);
}

/**
* Returns the CCA2 code of the country of the given country name.
*/
public static String getCCA2FromCountryName(String countryName, Player clickPlayer) {
var region = Objects.requireNonNull(NetworkModule.getInstance().getBuildTeam()).getRegions().stream().filter(regionF -> regionF.getName().equals(countryName)).findFirst();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Try to avoid long method chains like this. You can use a helper or facade method for this instead.

if (region.isPresent()) {
return region.get().getCountryCodeCca2();
} else {
clickPlayer.sendMessage(ChatHelper.getErrorString("Could not find the country of the location! Please report that"));
return "";
}

}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package net.buildtheearth.buildteamtools.modules.navigation;

import com.alpsbte.alpslib.geo.rgc.RgcHandler;
import com.alpsbte.alpslib.utils.ChatHelper;
import lombok.Getter;
import net.buildtheearth.buildteamtools.BuildTeamTools;
import net.buildtheearth.buildteamtools.modules.Module;
Expand All @@ -21,6 +23,15 @@
import net.buildtheearth.buildteamtools.utils.io.ConfigPaths;
import net.buildtheearth.buildteamtools.utils.io.ConfigUtil;
import org.bukkit.Bukkit;
import org.jetbrains.annotations.Nullable;

import java.io.File;
import java.io.FileOutputStream;
import java.net.URI;
import java.net.URL;
import java.nio.channels.Channels;
import java.nio.channels.FileChannel;
import java.nio.channels.ReadableByteChannel;

/**
* Manages all things related to universal tpll
Expand All @@ -36,7 +47,9 @@ public class NavigationModule extends Module {
private TpllComponent tpllComponent;
@Getter
private BluemapComponent bluemapComponent;

@Getter
@Nullable
private RgcHandler rgcHandler = null;
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Quite sure that will not cause issues because we always read from main thread. also Rgc handler itself is not thread save


private static NavigationModule instance = null;

Expand All @@ -60,9 +73,41 @@ public void enable() {
navigatorComponent = new NavigatorComponent();
tpllComponent = new TpllComponent();

var navConfig = BuildTeamTools.getInstance().getConfig(ConfigUtil.NAVIGATION);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is deeply nested code plus a lot of if/elses, which makes it hard to read and expand in the future. It would be better to use a switch statement here and/or more seperate functions.


if (navConfig.getBoolean(ConfigPaths.Navigation.RGC_LOCAL_DB_ENABLED, false)) {
File rgcFile = BuildTeamTools.getInstance().getDataPath().resolve("modules/navigation").resolve(navConfig.getString(ConfigPaths.Navigation.RGC_LOCAL_DB_PATH, "bs.file")).toFile();
ChatHelper.logDebug("Reverse Geocode local database support is enabled. Checking for local database file at: %s", rgcFile.getAbsolutePath());
if (rgcFile.exists()) {
rgcHandler = new RgcHandler(rgcFile, BuildTeamTools.getInstance().getSLF4JLogger(), false);
} else {
BuildTeamTools.getInstance().getComponentLogger().info("Reverse Geocode local database is enabled but the file does not exist at the specified path, installing it from the configured url.");
Bukkit.getScheduler().runTaskAsynchronously(BuildTeamTools.getInstance(), () -> {
try {
if (!rgcFile.getParentFile().mkdirs()) {
BuildTeamTools.getInstance().getComponentLogger().warn("Failed to create parent directories for Reverse Geocode local database file. Make sure the plugin has the necessary permissions to create directories and files in the plugin data folder.");
}
URL url = URI.create(navConfig.getString(ConfigPaths.Navigation.RGC_LOCAL_DB_UPDATE_URL, "")).toURL();
try (ReadableByteChannel readableByteChannel = Channels.newChannel(url.openStream())) {
try (FileOutputStream fileOutputStream = new FileOutputStream(rgcFile)) {
FileChannel fileChannel = fileOutputStream.getChannel();
fileChannel.transferFrom(readableByteChannel, 0, Long.MAX_VALUE);
}
}
Bukkit.getScheduler().runTask(BuildTeamTools.getInstance(), () -> {
rgcHandler = new RgcHandler(rgcFile, BuildTeamTools.getInstance().getSLF4JLogger(), false);
BuildTeamTools.getInstance().getComponentLogger().info("Successfully downloaded Reverse Geocode local database and enabled local database support for Reverse Geocoding.");
});
} catch (Exception e) {
BuildTeamTools.getInstance().getComponentLogger().error("Failed to download Reverse Geocode local database from the configured url, disabling local database support for Reverse Geocoding.", e);
navConfig.set(ConfigPaths.Navigation.RGC_LOCAL_DB_ENABLED, false);
}
});
}
}

// Check if BlueMap plugin is enabled and config allows BlueMap integration
boolean bluemapConfigEnabled = BuildTeamTools.getInstance().getConfig(ConfigUtil.NAVIGATION)
.getBoolean(ConfigPaths.Navigation.BLUEMAP_ENABLED, true);
boolean bluemapConfigEnabled = navConfig.getBoolean(ConfigPaths.Navigation.BLUEMAP_ENABLED, true);

if (Bukkit.getPluginManager().isPluginEnabled("BlueMap") && bluemapConfigEnabled) {
bluemapComponent = new BluemapComponent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ public static void createWarp(@NonNull Player creator, WarpGroup group) {
String regionName = result[0];
String countryCodeCCA2 = result[1].toUpperCase();

if (countryCodeCCA2.isEmpty()) countryCodeCCA2 = NavUtils.getCCA2FromCountryName(regionName, creator);

//Check if the team owns this region/country
boolean ownsRegion = NetworkModule.getInstance().ownsRegion(regionName, countryCodeCCA2);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import net.buildtheearth.OutOfProjectionBoundsException;
import net.buildtheearth.Projection;
import net.buildtheearth.buildteamtools.BuildTeamTools;
import net.buildtheearth.buildteamtools.modules.navigation.NavUtils;
import net.buildtheearth.buildteamtools.modules.navigation.components.warps.model.Warp;
import net.buildtheearth.buildteamtools.modules.network.NetworkModule;
import net.buildtheearth.buildteamtools.modules.network.api.OpenStreetMapAPI;
Expand Down Expand Up @@ -144,6 +145,9 @@ protected void setItemClickEventsAsync() {
String regionName = result[0];
String countryCodeCCA2 = result[1].toUpperCase();

if (countryCodeCCA2.isEmpty())
countryCodeCCA2 = NavUtils.getCCA2FromCountryName(regionName, clickPlayer);

//Check if the team owns this region/country
boolean ownsRegion = NetworkModule.getInstance().ownsRegion(regionName, countryCodeCCA2);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
package net.buildtheearth.buildteamtools.modules.network.api;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You are using deeply nested code and a chain of if/elses here again. Use switch statements and/or seperate functions for this.


import com.alpsbte.alpslib.geo.AdminLevel;
import com.alpsbte.alpslib.utils.ChatHelper;
import net.buildtheearth.buildteamtools.BuildTeamTools;
import net.buildtheearth.buildteamtools.modules.navigation.NavigationModule;
import org.bukkit.Bukkit;
import org.jetbrains.annotations.NotNull;
import org.json.simple.JSONArray;
import org.json.simple.JSONObject;

import java.io.IOException;
import java.util.Objects;
import java.util.concurrent.CompletableFuture;

public class OpenStreetMapAPI extends API {
Expand All @@ -16,6 +21,37 @@
*/
public static @NotNull CompletableFuture<String[]> getCountryFromLocationAsync(double @NotNull [] coordinates) {
CompletableFuture<String[]> future = new CompletableFuture<>();

if (NavigationModule.getInstance().isEnabled() && NavigationModule.getInstance().getRgcHandler() != null) {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Quite sure that will not cause issues

ChatHelper.logDebug("Using custom file API to get country from location: %s, %s", coordinates[0], coordinates[1]);

// If we are not on main thread, schedule lookup on main thread and complete the outer future there
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Try to avoid comments (especially in the way you wrote them) like this as you should already be able to read from function names what it is supposed to do. You can of course still add a comment if something is cases where it is really necessary to do so to understand something, but here that is not the case (same for line 44).

if (!Bukkit.isPrimaryThread()) {
ChatHelper.logDebug("Not on main thread: scheduling RGC lookup on main thread...");
Bukkit.getScheduler().runTask(BuildTeamTools.getInstance(), () -> {
try {
var rgcGeoLocation = NavigationModule.getInstance().getRgcHandler()
.locationFromCoordinates((float) coordinates[0], (float) coordinates[1]);
ChatHelper.logDebug("RGC lookup successful: %s", rgcGeoLocation);

Check notice on line 35 in src/main/java/net/buildtheearth/buildteamtools/modules/network/api/OpenStreetMapAPI.java

View workflow job for this annotation

GitHub Actions / Qodana for JVM

Constant values

Value `rgcGeoLocation` is always 'null'

Check warning on line 35 in src/main/java/net/buildtheearth/buildteamtools/modules/network/api/OpenStreetMapAPI.java

View workflow job for this annotation

GitHub Actions / Qodana for JVM

Confusing argument to varargs method

Confusing argument `rgcGeoLocation`, unclear if a varargs or non-varargs call is desired
future.complete(new String[]{rgcGeoLocation.get(AdminLevel.COUNTRY), ""});
} catch (Exception ex) {
future.completeExceptionally(ex);
}
});
return future;
}

// We're on the main thread already — run synchronously
try {
var location = Objects.requireNonNull(NavigationModule.getInstance().getRgcHandler()).locationFromCoordinates((float) coordinates[0], (float) coordinates[1]);
ChatHelper.logDebug("RGC lookup successful: %s", location);

Check notice on line 47 in src/main/java/net/buildtheearth/buildteamtools/modules/network/api/OpenStreetMapAPI.java

View workflow job for this annotation

GitHub Actions / Qodana for JVM

Constant values

Value `location` is always 'null'

Check warning on line 47 in src/main/java/net/buildtheearth/buildteamtools/modules/network/api/OpenStreetMapAPI.java

View workflow job for this annotation

GitHub Actions / Qodana for JVM

Confusing argument to varargs method

Confusing argument `location`, unclear if a varargs or non-varargs call is desired
future.complete(new String[]{location.get(AdminLevel.COUNTRY), ""});
} catch (Exception ex) {
future.completeExceptionally(ex);
}
return future;
}

String url = "https://photon.komoot.io/reverse?lat=" + coordinates[0] + "&lon=" + coordinates[1] + "&lang=en";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here it says "&lang=en". Did you check whether it always returns English names? Also it may be a good idea to expand the code like so that we can add more languages in the future, because of course not everyone in BTE speaks English. You do not have to implement this already, but there should at least be a class (or enum?) where you can define this easily.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What about disputed areas on Earth? What names does it give back for that? This may not seem an issue, but people across the world - and thus, also in BTE - have different beliefs on what regions on Earth have what names and it may cause unnecessary discussions in-game or on our socials. I do not know the solution to this myself right now. Maybe give back multiple names for disputed areas? We could discuss this internally.


ChatHelper.logDebug("Requesting country from location: %s", url);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ public static class Navigation {
// BlueMap Integration
private static final String BLUEMAP = "bluemap.";
public static final String BLUEMAP_ENABLED = BLUEMAP + "enabled";

// Reverse Geocode
private static final String RGC_LOCAL_DB = "reverse-geocode.local-database.";
public static final String RGC_LOCAL_DB_ENABLED = RGC_LOCAL_DB + "enabled";
public static final String RGC_LOCAL_DB_UPDATE_URL = RGC_LOCAL_DB + "url";
public static final String RGC_LOCAL_DB_PATH = RGC_LOCAL_DB + "path";
}

public static class PlotSystem {
Expand Down
13 changes: 12 additions & 1 deletion src/main/resources/modules/navigation/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,16 @@ bluemap:
# Enables or disables the BlueMap integration for displaying warps [true|false]
enabled: true

reverse-geocode:
local-database:
# Whether to use the local database for reverse geocoding or not. If false, the online Photon/Komoot API will be used.
# Currently, we use only https://photon.komoot.io/ for online reverse geocoding.
enabled: true
# The URL to the local database. More infos: https://github.com/kno10/reversegeocode/blob/master/data/README.md
# The file is automatically downloaded once if it does not exist.
url: "https://data.ub.uni-muenchen.de/61/8/osm-20151130-0.001-2.bin"
# The relative path to the local database.
path: "reversegeocode/osm-20151130-0.001-2.bin"

# NOTE: Do not change
config-version: 1.6
config-version: 1.7
Loading