SteamWar/SpigotCore
Archiviert
13
0

Packet System + Bungee GUI #59

Manuell gemergt
YoyoNow hat 10 Commits von packet-system nach master 2020-09-26 09:20:52 +02:00 zusammengeführt
Besitzer

TODO: Add License

TODO: Add License
Lixfel hat 2020-09-20 09:05:58 +02:00 Änderungen angefragt
@ -0,0 +1,10 @@
package de.steamwar.coms.receiver;
Besitzer

Üblicherweise kürzt man comms mit doppel-m ab

Üblicherweise kürzt man comms mit doppel-m ab
@ -0,0 +2,4 @@
import com.google.common.io.ByteArrayDataInput;
public abstract class Handler {
Besitzer

Handler ist doch etwas arg generisch. Vllt. den Verwendungszweck des Handlers beschreiben.

Handler ist doch etwas arg generisch. Vllt. den Verwendungszweck des Handlers beschreiben.
@ -0,0 +1,33 @@
package de.steamwar.coms.receiver;
Besitzer

Ist das System so groß, dass es auch noch Untermodule benötigt?

Ist das System so groß, dass es auch noch Untermodule benötigt?
@ -0,0 +11,4 @@
import java.util.HashMap;
import java.util.Map;
public class PacketHandler implements PluginMessageListener {
Besitzer

Auch PacketHandler zu generisch.

Auch PacketHandler zu generisch.
@ -0,0 +16,4 @@
private static Map<String, Handler> handlerMap = new HashMap<>();
public static void registerHandler(Handler handler) {
handlerMap.put(handler.getName(), handler);
Besitzer

So wie ich das sehe, ist das der einzige Fall, wo .getName() benötigt wird. Evtl. stattdessen eine Zahl als Identifier und Runnable hernehmen, dann müsste man nur eine Funktion übergeben und nicht (unbedingt) für jede Funktionalität eine neue Klasse aufmachen.

So wie ich das sehe, ist das der einzige Fall, wo .getName() benötigt wird. Evtl. stattdessen eine Zahl als Identifier und Runnable hernehmen, dann müsste man nur eine Funktion übergeben und nicht (unbedingt) für jede Funktionalität eine neue Klasse aufmachen.
@ -0,0 +15,4 @@
@Override
public void handle(ByteArrayDataInput byteArrayDataInput) {
Player player = Bukkit.getPlayer(UUID.fromString(byteArrayDataInput.readUTF()));
Besitzer

Überlegung: Statt String für den Spieler die SteamWarUser-id?

Überlegung: Statt String für den Spieler die SteamWarUser-id?
@ -0,0 +28,4 @@
int length = byteArrayDataInput.readInt();
Map<Integer, SWItem> items = new HashMap<>();
for (int i = 0; i < length; i++) {
JsonObject object = new JsonParser().parse(byteArrayDataInput.readUTF()).getAsJsonObject();
Besitzer

Wenn wir hier schon JSON verwenden, könnte man nicht stattdessen das gesamte Paket als JSON übertragen? (Oder das ganze Protokoll als JSON aufbauen?). Dann sparst du dir length, weil das einfach eine JSON-Liste ist.

Wenn wir hier schon JSON verwenden, könnte man nicht stattdessen das gesamte Paket als JSON übertragen? (Oder das ganze Protokoll als JSON aufbauen?). Dann sparst du dir length, weil das einfach eine JSON-Liste ist.
@ -0,0 +36,4 @@
}
if(object.has("enchanted"))
item.setEnchanted(true);
if(object.has("hideAttributes"))
Besitzer

Ich glaube, die Verzauberungen verstecken wir immer.

Ich glaube, die Verzauberungen verstecken wir immer.
@ -0,0 +9,4 @@
import java.util.UUID;
public class PingHandler extends Handler {
Besitzer

Wozu ist das nötig?

Wozu ist das nötig?
Autor
Besitzer

Dass bei @Player ein kleiner Ton kommt.

Dass bei @Player ein kleiner Ton kommt.
@ -0,0 +5,4 @@
import java.io.Serializable;
public abstract class Packet implements Serializable {
Besitzer

Packet. Auch wieder sehr generisch.

Packet. Auch wieder sehr generisch.
@ -0,0 +7,4 @@
public class PacketSender {
public static void sendPacket(Packet packet, Player player) {
Besitzer

Die Funktion würde ich eher in Packet integrieren, dafür braucht es nicht eine extra Klasse.

Die Funktion würde ich eher in Packet integrieren, dafür braucht es nicht eine extra Klasse.
@ -62,2 +63,4 @@
if(version >= 12)
ErrorLogger.init();
getServer().getMessenger().registerIncomingPluginChannel(this, "sw:bridge", new PacketHandler());
getServer().getMessenger().registerOutgoingPluginChannel(this, "sw:return");
Besitzer

Wozu braucht es den Channel?

Wozu braucht es den Channel?
@ -138,3 +140,3 @@
}
private void hideAttributes() {
public void hideAttributes() {
Besitzer

Üblicherweise hiden wir immer, daher kann das glaube ich private bleiben

Üblicherweise hiden wir immer, daher kann das glaube ich private bleiben
@ -194,2 +196,4 @@
itemStack.setItemMeta(itemMeta);
}
public String parseToJson(int position) {
Besitzer

Ich glaube, wir müssen den Bungee nicht das ganze Objekt zurückschicken, er wird ja auch angeordnet haben, was da angezeigt werden soll.

Ich glaube, wir müssen den Bungee nicht das ganze Objekt zurückschicken, er wird ja auch angeordnet haben, was da angezeigt werden soll.
Chaoscaot hat den Titel von WIP: Packet System + Bungee GUI zu Packet System + Bungee GUI 2020-09-22 18:31:09 +02:00 geändert
Lixfel hat 2020-09-24 11:00:07 +02:00 Änderungen angefragt
Lixfel hat einen Kommentar hinterlassen
Besitzer

Auch hier sehe ich (wie auch beim BungeeCore), dass die Paket-Objekte vom Typ "Fire-And-Forget" sind. Auch da wäre die Umsetzung als static-Methoden Garbage-Collector freundlicher (ist aber auch so vollkommen in Ordnung und Objektorientiert sauber, musst du entscheiden, ob du dass so lassen willst oder umarbeiten willst.

Auch hier sehe ich (wie auch beim BungeeCore), dass die Paket-Objekte vom Typ "Fire-And-Forget" sind. Auch da wäre die Umsetzung als static-Methoden Garbage-Collector freundlicher (ist aber auch so vollkommen in Ordnung und Objektorientiert sauber, musst du entscheiden, ob du dass so lassen willst oder umarbeiten willst.
@ -0,0 +1,11 @@
package de.steamwar.comms;
Besitzer

Lizenzheader fehlt.

Lizenzheader fehlt.
@ -0,0 +47,4 @@
JsonObject itemJson = array.get(i).getAsJsonObject();
SWItem item = null;
try {
item = new SWItem(Material.valueOf(itemJson.get("material").getAsString()), itemJson.get("title").getAsString());
Besitzer

Evtl. einfach einen neuen SWItem-Konstruktor erstellen, der als einziges Argument ein JsonObject nimmt und dann sich die Parameter aus dem JsonObject entnimmt? Das wäre meiner Meinung nach eine elegantere Lösung.

Evtl. einfach einen neuen SWItem-Konstruktor erstellen, der als einziges Argument ein JsonObject nimmt und dann sich die Parameter aus dem JsonObject entnimmt? Das wäre meiner Meinung nach eine elegantere Lösung.
@ -0,0 +69,4 @@
item.setCallback(click -> {
new InventoryCallbackPacket().setPosition(itemJson.get("position").getAsInt()).setClick(click)
.setCallback(InventoryCallbackPacket.CallbackType.CLICK)
.setOwner(SteamwarUser.get(player).getId()).send(Bukkit.getPlayer(player));
Besitzer

Der Aufruf ist doch etwas aufwändig. Evtl. wäre es sinnvoller, auch da als Parameter einfach das itemJson im Konstruktor zu übergeben, oder das SWItem selbst (in Verbindung mit dem vorhergehenden Vorschlag)

Der Aufruf ist doch etwas aufwändig. Evtl. wäre es sinnvoller, auch da als Parameter einfach das itemJson im Konstruktor zu übergeben, oder das SWItem selbst (in Verbindung mit dem vorhergehenden Vorschlag)
@ -26,3 +25,4 @@
import java.sql.SQLException;
import java.time.Instant;
import javax.xml.bind.DatatypeConverter;
Besitzer

Nenene, keinen DatatypeConverter bitte mehr! (einfach mal bitte den neuen master reinmergen)

Nenene, keinen DatatypeConverter bitte mehr! (einfach mal bitte den neuen master reinmergen)
Besitzer

I changed my Mind: Lass die Packets bitte als Objekte, denn dann haben wir da eine einfachere Erweiterungsmöglichkeit in anderen Systemen.

I changed my Mind: Lass die Packets bitte als Objekte, denn dann haben wir da eine einfachere Erweiterungsmöglichkeit in anderen Systemen.
Lixfel hat die Änderungen 2020-09-25 09:33:37 +02:00 genehmigt
YoyoNow hat diesen Pull-Request 2020-09-26 09:20:52 +02:00 geschlossen
Lixfel löschte die Branch packet-system 2020-09-26 11:44:55 +02:00
Dieses Repo ist archiviert. Du kannst Pull-Requests nicht kommentieren.
Keine Beschreibung angegeben.