Skip to content

feat: Add filter registration option for random teleport locations #1292

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.

Sign up for GitHub

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

Open
vLuckyyy wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: Add filter registration option for random teleport locations #1292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
47 changes: 47 additions & 0 deletions ...c/main/java/com/eternalcode/core/feature/randomteleport/RandomTeleportLocationFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package com.eternalcode.core.feature.randomteleport;

import org.bukkit.Location;

/**
* Filter for validating random teleport locations.
* <p>
* Allows external plugins to add custom validation logic for random teleport locations.
* For example, a plot plugin could implement this to prevent teleporting into claimed plots.
* <p>
* Example usage:
* <pre>{@code
* public class PlotFilter implements RandomTeleportLocationFilter {
* @Override
* public boolean isValid(Location location) {
* // Check if location is inside a claimed plot
* return !plotAPI.isPlotClaimed(location);
* }
*
* @Override
* public String getFilterName() {
* return "PlotFilter";
* }
* }
*
* // Register the filter
* EternalCoreApi api = EternalCoreApiProvider.provide();
* api.getRandomTeleportService().registerFilter(new PlotFilter());
* }</pre>
*/
public interface RandomTeleportLocationFilter {

/**
* Checks if the given location is valid for random teleportation.
*
* @param location The location to validate
* @return true if the location is valid, false otherwise
*/
boolean isValid(Location location);

/**
* Gets the name of this filter for debugging and logging purposes.
*
* @return The filter name
*/
String getFilterName();
}
26 changes: 26 additions & 0 deletions ...-api/src/main/java/com/eternalcode/core/feature/randomteleport/RandomTeleportService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,30 @@ public interface RandomTeleportService {
*/
CompletableFuture<Location> getSafeRandomLocationInWorldBorder(World world, int attemptCount);

/**
* Registers a custom location filter that will be used to validate random teleport locations.
* <p>
* Filters are checked after the built-in safety checks (unsafe blocks, world border, etc.).
* All registered filters must return true for a location to be considered valid.
*
* @param filter The filter to register
* @throws IllegalArgumentException if a filter with the same name is already registered
*/
void registerFilter(RandomTeleportLocationFilter filter);

/**
* Unregisters a previously registered location filter.
*
* @param filterName The name of the filter to unregister
* @return true if the filter was found and removed, false otherwise
*/
boolean unregisterFilter(String filterName);

/**
* Gets all currently registered location filters.
*
* @return An unmodifiable collection of registered filters
*/
Collection<RandomTeleportLocationFilter> getRegisteredFilters();

}
19 changes: 17 additions & 2 deletions ...n/java/com/eternalcode/core/feature/randomteleport/RandomTeleportSafeLocationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,18 @@ class RandomTeleportSafeLocationService {

private final RandomTeleportSettings randomTeleportSettings;
private final LocationsConfiguration locationsConfiguration;
private final RandomTeleportServiceImpl randomTeleportService;
private final Random random = new Random();

@Inject
RandomTeleportSafeLocationService(
RandomTeleportSettings randomTeleportSettings,
LocationsConfiguration locationsConfiguration
LocationsConfiguration locationsConfiguration,
RandomTeleportServiceImpl randomTeleportService
) {
Comment on lines 32 to 36
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Feb 5, 2026

Choose a reason for hiding this comment

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

high

Injecting RandomTeleportServiceImpl here introduces a circular dependency, as RandomTeleportServiceImpl already depends on RandomTeleportSafeLocationService. This is a significant design flaw that can lead to issues with object instantiation and makes the code harder to understand, maintain, and test.

To resolve this, you should break the cycle. A good approach is to pass the required data (the collection of filters) from RandomTeleportServiceImpl to the methods of RandomTeleportSafeLocationService that need it, instead of injecting the entire service.

For example, you could modify getSafeRandomLocation:

// In RandomTeleportSafeLocationService
public CompletableFuture<Location> getSafeRandomLocation(World world, RandomTeleportRadius radius, int attemptCount, Collection<RandomTeleportLocationFilter> filters) {
    // ...
    if (this.isSafeLocation(chunk, generatedLocation, filters)) {
        // ...
    }
    // ...
}

private boolean isSafeLocation(Chunk chunk, Location location, Collection<RandomTeleportLocationFilter> filters) {
    // ...
    for (RandomTeleportLocationFilter filter : filters) {
        if (!filter.isValid(location)) {
            return false;
        }
    }
    return true;
}

Then, RandomTeleportServiceImpl would pass the filters when calling the method:

// In RandomTeleportServiceImpl
@Override
public CompletableFuture<Location> getSafeRandomLocation(World world, int attemptCount) {
    // ...
    return this.safeLocationService.getSafeRandomLocation(world, radius, attemptCount, this.registeredFilters.values());
}

This refactoring would eliminate the circular dependency and also allow you to remove the package-private getRegisteredFiltersInternal() method.

this.randomTeleportSettings = randomTeleportSettings;
this.locationsConfiguration = locationsConfiguration;
this.randomTeleportService = randomTeleportService;
}

public CompletableFuture<Location> getSafeRandomLocation(World world, RandomTeleportRadius radius, int attemptCount) {
Expand Down Expand Up @@ -103,9 +106,21 @@ private boolean isSafeLocation(Chunk chunk, Location location) {
return false;
}

return switch (world.getEnvironment()) {
boolean environmentValid = switch (world.getEnvironment()) {
case NORMAL, THE_END, CUSTOM -> true;
case NETHER -> location.getY() <= NETHER_MAX_HEIGHT;
};

if (!environmentValid) {
return false;
}

for (RandomTeleportLocationFilter filter : this.randomTeleportService.getRegisteredFiltersInternal()) {
if (!filter.isValid(location)) {
return false;
}
}

return true;
}
}
40 changes: 40 additions & 0 deletions .../src/main/java/com/eternalcode/core/feature/randomteleport/RandomTeleportServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
import com.eternalcode.core.injector.annotations.component.Service;
import io.papermc.lib.PaperLib;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;
import org.bukkit.Location;
import org.bukkit.World;
Expand All @@ -25,6 +27,7 @@ class RandomTeleportServiceImpl implements RandomTeleportService {
private final RandomTeleportSettings randomTeleportSettings;
private final RandomTeleportSafeLocationService safeLocationService;
private final EventCaller eventCaller;
private final Map<String, RandomTeleportLocationFilter> registeredFilters;

@Inject
RandomTeleportServiceImpl(
Expand All @@ -34,6 +37,7 @@ class RandomTeleportServiceImpl implements RandomTeleportService {
this.randomTeleportSettings = randomTeleportSettings;
this.safeLocationService = safeLocationService;
this.eventCaller = eventCaller;
this.registeredFilters = new ConcurrentHashMap<>();
}

@Override
Expand Down Expand Up @@ -150,4 +154,40 @@ private RandomTeleportRadius getWorldBorderRadius(World world) {
int borderRadius = (int) (worldBorder.getSize() / 2);
return RandomTeleportRadius.of(-borderRadius, borderRadius, -borderRadius, borderRadius);
}

@Override
public void registerFilter(RandomTeleportLocationFilter filter) {
if (filter == null) {
throw new IllegalArgumentException("Filter cannot be null");
}

String filterName = filter.getFilterName();
if (filterName == null || filterName.isEmpty()) {
throw new IllegalArgumentException("Filter name cannot be null or empty");
}

if (this.registeredFilters.containsKey(filterName)) {
throw new IllegalArgumentException("Filter with name '" + filterName + "' is already registered");
}

this.registeredFilters.put(filterName, filter);
Comment on lines +169 to +173
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Feb 5, 2026

Choose a reason for hiding this comment

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

medium

The current implementation for checking if a filter is already registered and then adding it is not an atomic operation. While ConcurrentHashMap is thread-safe, using putIfAbsent is a more idiomatic and robust way to perform this check-then-act operation atomically. This makes the code's intent clearer and prevents potential race conditions if multiple threads try to register a filter with the same name concurrently.

Suggested change
if (this.registeredFilters.containsKey(filterName)) {
throw new IllegalArgumentException("Filter with name '" + filterName + "' is already registered");
}
this.registeredFilters.put(filterName, filter);
if (this.registeredFilters.putIfAbsent(filterName, filter) != null) {
throw new IllegalArgumentException("Filter with name '" + filterName + "' is already registered");
}

}

@Override
public boolean unregisterFilter(String filterName) {
if (filterName == null || filterName.isEmpty()) {
return false;
}

return this.registeredFilters.remove(filterName) != null;
}
Comment on lines +177 to +183
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Feb 5, 2026

Choose a reason for hiding this comment

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

security-medium medium

The unregisterFilter method allows any caller with access to the RandomTeleportService to remove any registered filter by its name. Since filter names are discoverable via getRegisteredFilters(), a malicious or poorly-behaved plugin can unregister filters registered by other plugins. This can lead to a bypass of critical security checks, such as those preventing teleportation into protected regions or claimed plots. It is recommended to modify the API to require the RandomTeleportLocationFilter instance itself for unregistration, ensuring that only a caller with a reference to the filter object can remove it.


@Override
public Collection<RandomTeleportLocationFilter> getRegisteredFilters() {
return Collections.unmodifiableCollection(this.registeredFilters.values());
}

Collection<RandomTeleportLocationFilter> getRegisteredFiltersInternal() {
return this.registeredFilters.values();
}
}