die null checks überall sind nicht schön, es macht vielleicht eher sinn am anfang einmal zu prüfen ob ein Eintrag vorhanden ist und wenn nicht ne leere Liste zu initialisieren..
this.replyMapping.computeIfNotPresent(() -> new ArrayList<>());
die null checks überall sind nicht schön, es macht vielleicht eher sinn am anfang einmal zu prüfen ob ein Eintrag vorhanden ist und wenn nicht ne leere Liste zu initialisieren..
`this.replyMapping.computeIfNotPresent(() -> new ArrayList<>());`
initialisierung und sofortig bedingtes überschreiben würde ich als bad practice einstufen...
Umschreiben zu
```
Component currentTargetComponent = currentTargetPlayer != null
? Main.instance().getAppliance(ChatMessages.class).getReportablePlayerName(currentTargetPlayer)
: Component.text("niemandem.");
```
das senden einzelner messages ist problematisch...
in der theorie könnten andere messages welche parallel gesendet werden so zwischen die messages rutschen. Außerdem ist jeder sendMessage aufruf ein eigenes paket, welches an den Spieler gesendet wird.
die Aufrufe sollten zusammengezogen werden zu einem einzelnen aufruf, leerzeilen können mit .appendNewLine eingefügt werden.
Hier bietet sich ggf der ComponentBuilder an, dieser kann mit Component.text() erstellt werden.
Anschließend kann darauf mit component.append(otherComponent) gearbeitet werden. Zum schluss dann sender.sendMessage(component.build())
das senden einzelner messages ist problematisch...
in der theorie könnten andere messages welche parallel gesendet werden so zwischen die messages rutschen. Außerdem ist jeder sendMessage aufruf ein eigenes paket, welches an den Spieler gesendet wird.
die Aufrufe sollten zusammengezogen werden zu einem einzelnen aufruf, leerzeilen können mit `.appendNewLine` eingefügt werden.
Hier bietet sich ggf der ComponentBuilder an, dieser kann mit `Component.text()` erstellt werden.
Anschließend kann darauf mit `component.append(otherComponent)` gearbeitet werden. Zum schluss dann `sender.sendMessage(component.build())`
lässt sich das vielleicht schlau kombinieren, dass du oldConversations schon oben erstellst und die andere prüfung auf der bestehenden Liste dann ausführst?
hier steht die gleiche logik wie in zeile 58
lässt sich das vielleicht schlau kombinieren, dass du oldConversations schon oben erstellst und die andere prüfung auf der bestehenden Liste dann ausführst?
wenn du einen weg findest finalComponent nicht überschreiben zu müssen brauchst du den array quark nicht.
ComponentBuilder<TextComponent, TextComponent.Builder> component = Component.text();
playerNames.forEach(playerName -> component.append(
Component.text("[")
.append(Component.text(playerName, NamedTextColor.GOLD))
.append(Component.text("]"))
.clickEvent(ClickEvent.runCommand("/msg " + playerName + " " + message))
.hoverEvent(HoverEvent.showText(Component.text("Klicke, um diesem Spieler zu schreiben.").color(NamedTextColor.GOLD))))
.append(Component.text(" ")));
sender.sendMessage("");
sender.sendMessage(component.build());
sender.sendMessage("");
wenn du einen weg findest finalComponent nicht überschreiben zu müssen brauchst du den array quark nicht.
```
ComponentBuilder<TextComponent, TextComponent.Builder> component = Component.text();
playerNames.forEach(playerName -> component.append(
Component.text("[")
.append(Component.text(playerName, NamedTextColor.GOLD))
.append(Component.text("]"))
.clickEvent(ClickEvent.runCommand("/msg " + playerName + " " + message))
.hoverEvent(HoverEvent.showText(Component.text("Klicke, um diesem Spieler zu schreiben.").color(NamedTextColor.GOLD))))
.append(Component.text(" ")));
sender.sendMessage("");
sender.sendMessage(component.build());
sender.sendMessage("");
```
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
/r
@@ -0,0 +25,4 @@private final Map<Player, List<Conversation>> replyMapping = new WeakHashMap<>();public void reply(Player sender, String message) {if(this.replyMapping.get(sender) != null) {hier wird ganz oft
this.replyMapping.get(sender)verwendet, rausziehen in eine eigene variable z.B.replyMap = this.replyMapping.get(sender)@@ -0,0 +24,4 @@private record Conversation(UUID target, Long lastSet) {}private final Map<Player, List<Conversation>> replyMapping = new WeakHashMap<>();public void reply(Player sender, String message) {die null checks überall sind nicht schön, es macht vielleicht eher sinn am anfang einmal zu prüfen ob ein Eintrag vorhanden ist und wenn nicht ne leere Liste zu initialisieren..
this.replyMapping.computeIfNotPresent(() -> new ArrayList<>());@@ -0,0 +44,4 @@Component currentTargetComponent = Component.text("niemandem.");Player currentTargetPlayer = Bukkit.getPlayer(youngestEntry.target());if(currentTargetPlayer != null) {currentTargetComponent = Main.instance().getAppliance(ChatMessages.class).getReportablePlayerName(currentTargetPlayer);initialisierung und sofortig bedingtes überschreiben würde ich als bad practice einstufen...
Umschreiben zu
@@ -0,0 +60,4 @@if(target == null) throw new ApplianceCommand.Error("Der Spieler " + Bukkit.getOfflinePlayer(youngestEntry.target()).getName() + " ist nicht mehr verfügbar.");this.replyMapping.get(sender).clear();sendWhisper(sender, new ResolvedPmUserArguments(target, message));this.sendWhisper
@@ -0,0 +64,4 @@return;}sender.sendMessage("");das senden einzelner messages ist problematisch...
in der theorie könnten andere messages welche parallel gesendet werden so zwischen die messages rutschen. Außerdem ist jeder sendMessage aufruf ein eigenes paket, welches an den Spieler gesendet wird.
die Aufrufe sollten zusammengezogen werden zu einem einzelnen aufruf, leerzeilen können mit
.appendNewLineeingefügt werden.Hier bietet sich ggf der ComponentBuilder an, dieser kann mit
Component.text()erstellt werden.Anschließend kann darauf mit
component.append(otherComponent)gearbeitet werden. Zum schluss dannsender.sendMessage(component.build())@@ -0,0 +70,4 @@.append(privatePrefix).append(Component.text("Das Ziel für /r hat sich bei dir in den letzten ", NamedTextColor.RED)).append(Component.text(String.valueOf(this.targetChangeTimeoutSeconds), NamedTextColor.RED)).append(Component.text(" Sekunden geändert. Wer soll deine Nachricht bekommen? ", NamedTextColor.RED))"erhalten" statt "bekommen" klingt denke ich besser
@@ -0,0 +74,4 @@);List<Conversation> oldConversations = this.replyMapping.get(sender).stream().filter(conversation -> conversation.lastSet < System.currentTimeMillis() - (targetChangeTimeoutSeconds*1000))hier steht die gleiche logik wie in zeile 58
lässt sich das vielleicht schlau kombinieren, dass du oldConversations schon oben erstellst und die andere prüfung auf der bestehenden Liste dann ausführst?
@@ -0,0 +77,4 @@.filter(conversation -> conversation.lastSet < System.currentTimeMillis() - (targetChangeTimeoutSeconds*1000)).toList();Conversation youngestOldConversation;gibt es einen Grund warum die Variable hier außerhalb allokiert wird aber nur innerhalb des if statements genutzt wird?
@@ -0,0 +91,4 @@.distinct().toList();final Component[] finalComponent = {Component.text("")};(ㆆ _ ㆆ)
@@ -0,0 +93,4 @@final Component[] finalComponent = {Component.text("")};playerNames.forEach(playerName -> finalComponent[0] = finalComponent[0].append(wenn du einen weg findest finalComponent nicht überschreiben zu müssen brauchst du den array quark nicht.
@@ -0,0 +103,4 @@sender.sendMessage("");sender.sendMessage(finalComponent[0]);sender.sendMessage("");.appendNewline()@@ -0,0 +109,4 @@public void sendWhisper(Player sender, ResolvedPmUserArguments userArguments) {if(!(this.replyMapping.get(userArguments.receiver) == null)) {klammer weg und
!=verwenden@@ -0,0 +121,4 @@System.currentTimeMillis());if(this.replyMapping.get(userArguments.receiver) == null) {hier kann durchaus ein
elseverwendet werden beim if im zusammenhang mit dem vorhergehenden if@@ -0,0 +147,4 @@);ChatMessages chatMessages = Main.instance().getAppliance(ChatMessages.class);wird in der reply methode auch aufgerufen, macht ggf sinn die chatMessages referenz als objektvariable zu halten.
@@ -0,0 +97,4 @@Component.text("[").append(Component.text(playerName, NamedTextColor.GOLD)).append(Component.text("]")).clickEvent(ClickEvent.runCommand("/msg " + playerName + " " + message))String.format
@@ -0,0 +2,4 @@import java.util.ArrayList;public class LimitedSizedList<T> extends ArrayList<T> {wenn die Klasse nicht mehr verwendet wird kann sie raus
bot
@@ -0,0 +34,4 @@.toList();this.replyMapping.get(sender).removeAll(tooOldConversations);List<Conversation> replyMap = this.replyMapping.get(sender);das kann noch über tooOldConversations und dort verwendet werden, auch bei dem removeAll