-
Notifications
You must be signed in to change notification settings - Fork 30
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
Reworked expansion #86
Conversation
…ted) and time_since_join(_formatted)
import org.bukkit.event.player.PlayerJoinEvent; | ||
import org.bukkit.event.player.PlayerQuitEvent; | ||
|
||
public record PlayerListener(PlayerExpansion expansion) implements Listener { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use java 8, this applies to the rest of the files too.
compileOnly 'org.projectlombok:lombok:1.18.28' | ||
annotationProcessor 'org.projectlombok:lombok:1.18.28' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? Please remove this.
defaults = new HashMap<>() {{ | ||
put("ping",new HashMap<>() {{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is ImmutableMap#Builder from guava, please avoid using anonymous classes like this.
if (identifier.startsWith("item_in_hand")) yield requestItem(identifier,12,versionHelper.getItemInHand(p)); | ||
if (identifier.startsWith("item_in_offhand")) yield requestItem(identifier,15,p.getInventory().getItemInOffHand()); | ||
if (identifier.startsWith("armor_helmet")) yield requestItem(identifier,12,p.getInventory().getHelmet()); | ||
if (identifier.startsWith("armor_chestplate")) yield requestItem(identifier,16,p.getInventory().getChestplate()); | ||
if (identifier.startsWith("armor_leggings")) yield requestItem(identifier,14,p.getInventory().getLeggings()); | ||
if (identifier.startsWith("armor_boots")) yield requestItem(identifier,11,p.getInventory().getBoots()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoid using magic numbers (12, 15, 16 have no meaning)
|
||
public static String getLocale(final Player player) { | ||
return PLAYER_GET_LOCALE.apply(player); | ||
public static String msToSToStr(double ms) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use real names for methods
return version; | ||
public int getPing(Player player) { | ||
if (IS_1_17_OR_NEWER) return player.getPing(); | ||
try {return (int) ping.get(getHandle.invoke(player));} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
???
} | ||
@SuppressWarnings("deprecation") | ||
public int getItemDamage(ItemStack item) { | ||
return VERSION >= 1_13_0 ? item.getItemMeta() instanceof Damageable damageable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put this statement on multiple lines, it is confusing to read.
if (level <= 15) { | ||
return (level << 1) + 7; | ||
} | ||
if (level <= 30) { | ||
return (level * 5) - 38; | ||
} | ||
return (level * 9) - 158; | ||
return level <= 15 | ||
? (level << 1) + 7 | ||
: level <= 30 | ||
? (level * 5) - 38 | ||
: (level * 9) - 158; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert these changes, not everything has to be done using ternary operator.
return output == null ? null | ||
: output instanceof Boolean bool ? | ||
bool ? PlaceholderAPIPlugin.booleanTrue() | ||
: PlaceholderAPIPlugin.booleanFalse() | ||
: String.valueOf(output); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, don't use ternary operator like this.
Lots of changes 😅
Moved to Gradle
Added
%player_health_full(_rounded)%
,%player_time_since_last_played(/join)%
,%player_time_since_join%
,%player_item_in_hand_lore%
with its offhand and armor variants (closes #64) and%player_last_damage_given%
(closes #13, can't believe it took 4 years just for that)Since I'm using enhanced switch statements now (lots of them :D) I'm guessing this now requires at least Java 14, though most servers should have updated to 17 now and other expansions bumped their Java version as well so this should be fine.
Reworked
%player_colored_ping%
's config to use ranges of numbers to make it much more customizable, default config:Closes #85 #77
Included #84, #83, #65 and #37
If anyone wants to try, I tested it but just in case
Player-Expansion-2.1.0.zip