Skip to content

✨ ActionBar & Personal chargeuptime#1

Open
WinteruOfficiel wants to merge 1 commit intoShatteredSoftware:mainfrom
WinteruOfficiel:main
Open

✨ ActionBar & Personal chargeuptime#1
WinteruOfficiel wants to merge 1 commit intoShatteredSoftware:mainfrom
WinteruOfficiel:main

Conversation

@WinteruOfficiel
Copy link

  • Added Message "x seconds until launch"
  • chargeuptime is now personal, they're in the player pref yaml file
  • Added one sound

+ Added Message "x seconds until launch"
+ chargeuptime is now personal, they're in the player pref yaml file
+ Added one sound
@hhenrichsen
Copy link
Collaborator

Hey, sorry I've neglected this repo; it's been in my old account that I don't get notifications anymore. I'll review this and look at getting it merged and released if you'd still like.

Copy link
Collaborator

@hhenrichsen hhenrichsen left a comment

Choose a reason for hiding this comment

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

I'm happy to make these changes myself to the main branch if you'd prefer not to go through a literal year-old code review. Again apologies for missing this, I've moved it to my plugins org where I actually get notifications on both accounts.

Comment on lines +152 to +153
// this.plugin.getMessenger().sendMessage(commandSender, "personalchargeup set
// to " + prefs.personalChargeupTicks, true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be deleted.

@Override
public boolean onCommand(final CommandSender commandSender, final Command command, final String s,
final String[] args) {
final String[] args) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like a lot of auto-formatter changes in this PR that don't match what mine is doing. Can you revert these so the changes are the smallest set possible to do your changes?

0.1F);
if (time < sePlayer.getPrefs().personalChargeupTicks) {
final int seconds = (sePlayer.getPrefs().personalChargeupTicks - time) / 20;
player.spigot().sendMessage(ChatMessageType.ACTION_BAR,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you make this message configurable?

this.plugin.getMessenger().sendMessage(commandSender, "firework-disabled", true);
}
}
if (args[0].equalsIgnoreCase("personalchargeup")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This probably should have a permission check as well, and should be added to the plugin.yml as a false-by-default permission. In order for people to be able to update to this, they should be able to decide if their players should have access to this command.

if (event.isSneaking()) {
playerManager.getPlayer(player).setChargeUpTicks(0);
} else {
if (playerManager.getPlayer(player).getChargeUpTicks() >= playerManager.getPlayer(player)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again, this should probably be the main change to this file. I'm seeing a lot of other format-related changes.

@WinteruOfficiel
Copy link
Author

WinteruOfficiel commented May 20, 2024

I'll look into it !
Also, sorry for the auto format, I didn't pay attention.

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

Comments