CraftAttack/spawn
5
0
Fork 0
Code Issues 3 Pull Requests Packages Projects Releases Wiki Activity

develop-chatReply #5

Merged
Pupsi merged 11 commits from develop-chatReply into master 2024-10-06 13:52:59 +00:00
Conversation 21 Commits 11 Files Changed 9 +13 -14
1 changed files with 13 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 8462e43e89 - Show all commits

27
src/main/java/eu/mhsl/craftattack/spawn/appliances/privateMessage/PrivateMessage.java
View File

@ -29,20 +29,20 @@ public class PrivateMessage extends Appliance {
public void reply(Player sender, String message) {
this.replyMapping.computeIfAbsent(sender, player -> new ArrayList<>());
List<Conversation> tooOldConversations = this.replyMapping.get(sender).stream()
List<Conversation> replyList = this.replyMapping.get(sender);
List<Conversation> tooOldConversations = replyList.stream()
.filter(conversation -> conversation.lastSet < System.currentTimeMillis() - (conversationTimeoutMinutes*60*1000))
.toList();
this.replyMapping.get(sender).removeAll(tooOldConversations);
replyList.removeAll(tooOldConversations);
MineTec marked this conversation as resolved Outdated
MineTec commented 2024-10-06 13:25:52 +00:00
Outdated
Review
Copy Link

das kann noch über tooOldConversations und dort verwendet werden, auch bei dem removeAll

das kann noch über tooOldConversations und dort verwendet werden, auch bei dem removeAll
List<Conversation> replyMap = this.replyMapping.get(sender);
if(replyMap.isEmpty()) throw new ApplianceCommand.Error("Du führst aktuell keine Konversation.");
if(replyList.isEmpty()) throw new ApplianceCommand.Error("Du führst aktuell keine Konversation.");
Component privatePrefix = Component.text("[Privat] ", NamedTextColor.LIGHT_PURPLE);
Conversation youngestEntry = replyMap.stream()
Conversation youngestEntry = replyList.stream()
.max(Comparator.comparingLong(o -> o.lastSet))
.orElse(replyMap.getLast());
.orElse(replyList.getLast());
if(message.isBlank()) {
Pupsi marked this conversation as resolved
MineTec commented 2024-10-06 11:21:22 +00:00
Review
Copy Link

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.");
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."); ```
Player currentTargetPlayer = Bukkit.getPlayer(youngestEntry.target());
@ -59,15 +59,15 @@ public class PrivateMessage extends Appliance {
return;
}
List<Conversation> oldConversations = replyMap.stream()
List<Conversation> oldConversations = replyList.stream()
.filter(conversation -> conversation.lastSet < System.currentTimeMillis() - (targetChangeTimeoutSeconds*1000))
Pupsi marked this conversation as resolved Outdated
MineTec commented 2024-10-06 11:24:05 +00:00
Outdated
Review
Copy Link

this.sendWhisper

this.sendWhisper
.toList();
if(oldConversations.contains(youngestEntry) || replyMap.size() == 1) {
if(oldConversations.contains(youngestEntry) || replyList.size() == 1) {
Player target = Bukkit.getPlayer(youngestEntry.target());
Pupsi marked this conversation as resolved Outdated
MineTec commented 2024-10-06 11:25:50 +00:00
Outdated
Review
Copy Link

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())`
if(target == null) throw new ApplianceCommand.Error("Der Spieler " + Bukkit.getOfflinePlayer(youngestEntry.target()).getName() + " ist nicht mehr verfügbar.");
this.replyMapping.get(sender).clear();
replyList.clear();
this.sendWhisper(sender, new ResolvedPmUserArguments(target, message));
return;
}
Pupsi marked this conversation as resolved Outdated
MineTec commented 2024-10-06 11:29:16 +00:00
Outdated
Review
Copy Link

"erhalten" statt "bekommen" klingt denke ich besser

"erhalten" statt "bekommen" klingt denke ich besser
@ -88,12 +88,11 @@ public class PrivateMessage extends Appliance {
Conversation youngestOldConversation = oldConversations.stream()
.max(Comparator.comparingLong(o -> o.lastSet))
.orElse(oldConversations.getLast());
this.replyMapping.get(sender).removeAll(oldConversations);
this.replyMapping.get(sender).add(youngestOldConversation);
replyMap = this.replyMapping.get(sender);
replyList.removeAll(oldConversations);
replyList.add(youngestOldConversation);
}
Pupsi marked this conversation as resolved Outdated
MineTec commented 2024-10-06 11:35:41 +00:00
Outdated
Review
Copy Link

(ㆆ _ ㆆ)

(ㆆ _ ㆆ)
List<String> playerNames = replyMap.stream()
List<String> playerNames = replyList.stream()
.map(conversation -> Bukkit.getOfflinePlayer(conversation.target()).getName())
Pupsi marked this conversation as resolved Outdated
MineTec commented 2024-10-06 11:40:32 +00:00
Outdated
Review
Copy Link

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(""); ```
.distinct()
.toList();