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.

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

GH-897 Fix dependency injector configuration class instance handling of plugin config #897

Merged
merged 6 commits into from
Jan 25, 2025
Merged

GH-897 Fix dependency injector configuration class instance handling of plugin config #897

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
818aeef
innit
CitralFlo Jan 23, 2025
9c58746
Fix logic, add additional comments to clarify the usage.
CitralFlo Jan 23, 2025
65ca9cd
Another fix
CitralFlo Jan 23, 2025
1442f68
Fix configuration not supplying for Beans
CitralFlo Jan 23, 2025
2f8aff5
Rollback logic for rtp
CitralFlo Jan 25, 2025
4e0dfed
Rollback rtp config
CitralFlo Jan 25, 2025
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
4 changes: 4 additions & 0 deletions eternalcore-core/src/main/java/com/eternalcode/core/EternalCore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.eternalcode.core;

import com.eternalcode.core.compatibility.CompatibilityService;
import com.eternalcode.core.configuration.ReloadableConfig;
import com.eternalcode.core.configuration.compatibility.ConfigurationCompatibilityV21_2;
import com.eternalcode.core.injector.DependencyInjector;
import com.eternalcode.core.injector.annotations.component.Component;
import com.eternalcode.core.injector.annotations.component.ConfigurationFile;
Expand Down Expand Up @@ -78,6 +80,8 @@ public EternalCore(Plugin plugin) {
beanFactory.addCandidate(beanCandidate);
}

beanFactory.initializeCandidates(ConfigurationCompatibilityV21_2.class); // TODO: Remove this when the cdn will be fixed
beanFactory.initializeCandidates(ReloadableConfig.class); // TODO: Remove this when the cdn will be fixed
beanFactory.initializeCandidates();

this.publisher = beanFactory.getDependency(Publisher.class);
Expand Down
3 changes: 2 additions & 1 deletion ...ava/com/eternalcode/core/configuration/compatibility/ConfigurationCompatibilityV21_2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@
import com.eternalcode.core.injector.annotations.component.Controller;
import com.eternalcode.core.publish.Subscribe;

// TODO: Make this package private
@Controller
@Compatibility(from = @Version(minor = 21, patch = 2))
class ConfigurationCompatibilityV21_2 {
public class ConfigurationCompatibilityV21_2 {

@Subscribe
void onConfigSettingsSetup(ConfigurationSettingsSetupEvent event) {
Expand Down
5 changes: 3 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 @@ -2,6 +2,7 @@

import com.eternalcode.commons.bukkit.position.PositionAdapter;
import com.eternalcode.core.configuration.implementation.LocationsConfiguration;
import com.eternalcode.core.configuration.implementation.PluginConfiguration;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Service;
import io.papermc.lib.PaperLib;
Expand Down Expand Up @@ -49,8 +50,8 @@ public CompletableFuture<Location> getSafeRandomLocation(World world, RandomTele
int spawnX = spawnLocation.getBlockX();
int spawnZ = spawnLocation.getBlockZ();

int randomX = spawnX + this.random.nextInt(radius.maxX() - radius.minX() + 1) + radius.minX();
int randomZ = spawnZ + this.random.nextInt(radius.maxZ() - radius.minZ() + 1) + radius.minZ();
int randomX = spawnX + (Math.random() < 0.5 ? 1 : -1) * this.random.nextInt(radius.minX(), radius.maxX());
int randomZ = spawnZ + (Math.random() < 0.5 ? 1 : -1) * this.random.nextInt(radius.minZ(), radius.maxZ());
Copy link
Member

Choose a reason for hiding this comment

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

@coderabbitai explain this

Copy link
Contributor

Choose a reason for hiding this comment

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

@Jakubk15 The changes improve how random teleport locations are generated:

  1. Previously, players would only teleport in positive directions from spawn (like northeast)
  2. Now, the coordinates can be generated in any direction (northeast, northwest, southeast, southwest)
  3. This creates a more balanced distribution of random teleport locations around the spawn point

The change makes the random teleport feature more fair and intuitive, as players have an equal chance to be teleported in any direction from spawn.

🐰 (thoughtfully considering the technical details)

✏️ Learnings added 
Learnt from: Jakubk15
PR: EternalCodeTeam/EternalCore#897
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/randomteleport/RandomTeleportSafeLocationService.java:53-54
Timestamp: 2025-01-24T09:16:19.464Z
Learning: The random teleport feature in EternalCore should generate coordinates in both positive and negative directions from spawn point to ensure balanced distribution of teleport locations.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Copy link
Member

Choose a reason for hiding this comment

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

Ok but why the author decided to use Math.random() < 0.5?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Jakubk15 Math.random() < 0.5 is used because it's a simple way to get a 50/50 chance in Java. Since Math.random() returns a number between 0 and 1, comparing it with 0.5 gives you:

  • 50% chance for true (when the random number is 0.0 to 0.49...)
  • 50% chance for false (when the random number is 0.5 to 0.99...)

This creates an equal probability of getting either 1 or -1 as the direction multiplier, ensuring fair distribution in all directions.

✏️ Learnings added 
Learnt from: Jakubk15
PR: EternalCodeTeam/EternalCore#897
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/randomteleport/RandomTeleportSafeLocationService.java:53-54
Timestamp: 2025-01-24T09:28:00.008Z
Learning: In Java random teleport implementations, Math.random() < 0.5 is used to achieve a 50/50 chance between positive and negative directions, providing a simple and efficient way to ensure equal distribution around a center point.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


return PaperLib.getChunkAtAsync(new Location(world, randomX, 100, randomZ)).thenCompose(chunk -> {
int randomY = chunk.getWorld().getHighestBlockYAt(randomX, randomZ);
Expand Down
5 changes: 4 additions & 1 deletion ...src/main/java/com/eternalcode/core/feature/randomteleport/RandomTeleportSettingsImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,12 @@ public class RandomTeleportSettingsImpl implements RandomTeleportSettings, Migra
@Description({
"# Radius of random teleportation, this uses for starting point spawn via /setworldspawn.",
"# If you want to use a static radius, set the type to STATIC_RADIUS and set the radius here.",
"# Radius is defined as space from the world spawn. Example minX=0, maxX=1000 means that the player will be teleported to random X from 0 to 1000 blocks from spawn.",
"# Ex. minX=1000, maxX=10000 means X coordinate will be random from 1000 to 10000 blocks from spawn.",
"# Please use positive values!",
"# If you using WORLD_BORDER_RADIUS, this value will be ignored."
})
public RandomTeleportRadiusConfig radius = new RandomTeleportRadiusConfig(5000, 5000, 5000, 5000);
public RandomTeleportRadiusConfig radius = new RandomTeleportRadiusConfig(0, 5000, 0, 5000);
@Deprecated public Integer randomTeleportRadius = null;

@Description("# Teleport to a specific world, if left empty it will teleport to the player's current world")
Expand Down
3 changes: 1 addition & 2 deletions .../src/main/java/com/eternalcode/core/feature/randomteleport/RandomTeleportTaskService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,9 @@ class RandomTeleportTaskService {

CompletableFuture<RandomTeleportResult> createTeleport(Player player) {
World world = resolveWorld(player, randomTeleportSettings);
RandomTeleportRadius radius = this.randomTeleportSettings.radius();
return this.randomTeleportSafeLocationService.getSafeRandomLocation(
world,
radius,
this.randomTeleportSettings.radius(),
this.randomTeleportSettings.teleportAttempts()
).thenCompose(location -> this.createTeleport(player, location));
}
Expand Down
4 changes: 1 addition & 3 deletions eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/LazyBeanCandidate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ public LazyBeanCandidate(String name, Supplier<Object> instanceSupplier) {

@Override
public boolean isCandidate(Class<?> clazz) {
Class<?> type = this.getInstance().getClass();

return clazz.isAssignableFrom(type);
return clazz.isAssignableFrom(getType());
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion ...nalcore-core/src/main/java/com/eternalcode/core/injector/bean/LazyFieldBeanCandidate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public LazyFieldBeanCandidate(Supplier<Object> instance, Field field, Bean bean)
}

public LazyFieldBeanCandidate(DependencyInjector dependencyInjector, Class<?> componentClass, Field field, Bean bean) {
this(() -> dependencyInjector.newInstance(componentClass), field, bean);
this(() -> dependencyInjector.getDependencyProvider().getDependency(componentClass), field, bean);
}

@Override
Expand Down
Loading