Abfragen, ob sich ein Spieler nicht bewegt?

  • Hallo,

    ich wollte für ein AFK-System abfragen, ob sich ein Spieler nicht bewegt.

    Wenn ja, soll erstmal nur eine Nachricht ausgegeben werden. Die soll aber erst nach 60 Sekunden AFK-Zeit kommen.

    Das war meine Idee dazu, ich hab sie aber vermutlich mal wieder viel zu kompliziert umgesetzt:


    Jedoch kommt immer die "Test3LocUngleich"-Nachricht. Ich weiß nicht genau, wie meine Location anders sein kann, als die, wo ich vor einer Sekunde stand und mich nicht bewegt habe...

    Hat vielleicht jemand einen Fehler darin gefunden oder eine bessere, einfachere Methode?

    Vielen Dank für die Hilfe :)

  • Ich würde da nicht die "Location" speichern ist auch garnicht nötig


    Du brauchst eigentlich nur 3 Sachen eine HashMap<UUID, LocalDateTime> ein PlayerMoveEvent-Listener und ein Scheduler der denn Rest handhabt.


    (Der Code enthält Lambda also sollte deine Java Version 8+ sein! Ansonten muss der Code angepasst werden)


    Solltest du noch wissen wollen wie lange den jetzt jemand AFK war mach folgendes:

    Code
    Duration.between(localDateTime, LocalDateTime.now()).toSeconds();


    Grüße Maxi

    If you are homeless ... just buy a house, duh!

    and if you wanna have a plugin matching your conditions ... just code it yourself!

  • IDK_WHO_AM_I Ich stimme mit deiner Lösung soweit überein, bis auf zwei, drei kleinere Dinge:



    private HashMap<UUID, LocalDateTime> lastMovement;

    Eine LocalDateTime ist hier definitiv der falsche Datentyp. Du willst hier einen Moment (also einen spezifischen, unverwechselbaren Zeitpunkt) vergleichen. Es sollen ja immer 60 Sekunden vergangen sein und du willst überhaupt keine für Menschen lesbare Repräsentation dieser Zeit. Du möchtest einen Instant. Sie sind untereinander vollständig kompatibel, meinen aber wie gesagt vollständig unterschiedliche Dinge. Eine hervorragende Unterscheidung der neuen Zeit-Typen kannst du HIER erhalten.


    Code
    public void handleMovement(PlayerMoveEvent e) {                                                                                                      if (e.getFrom().getBlockX() != e.getTo().getBlockX() || e.getFrom().getBlockY() != e.getTo().getBlockY() || e.getFrom().getBlockZ() != e.getTo().getBlockZ())                                                                                                          if (e.getFrom().getPitch() != e.getTo().getPitch() || e.getFrom().getYaw() != e.getTo().getYaw())                                                                                                              lastMovement.put(e.getPlayer().getUniqueId(), LocalDateTime.now());                                                                                                  }

    Die Bedingungen würde ich mir hier schenken. Es wird ohnehin nur ein Move-Event ausgelöst, wenn sich die Position wenigstens irgendwie von der vorigen Position unterscheidet, und ob es nun innerhalb des Blocks war, oder Block-Grenzen übergreifend macht es nur ungenauer. Würde ein Nutzer so vorher an der Blockgrenze stehen, dürfte er sich weniger bewegen um trotzdem noch als AFK markiert zu werden, als wenn er in der Mitte des Blocks gestanden hätte.


    Durch deine Abfragen würden auch Teleports an die exakt selbe Position in einer anderen Welt nicht berücksichtigt. Wenn man unbedingt eine Bedingung haben möchte, kann man auch einfach die From- und To-Positionen mit Equals vergleichen, da bekäme man dann sogar noch die bessere Verarbeitung von Float und Double geschenkt. Aber es ist wie gesagt überhaupt keine Bedingung nötig, wenn ohnehin jede Art der Veränderung berücksichtigt werden soll.


    if (lastMovement.containsKey(player.getUniqueId()))
    if (lastMovement.get(player.getUniqueId()).plusMinutes(1).isAfter(LocalDateTime.now()))
    Bukkit.broadcastMessage(String.format("§e%s hasn't moved since 60 seconds"));
    //TODO do something

    Hier rückst du unnötig oft ein und verwendest keine Klammern, was insbesondere in Anbetracht der Tatsache, dass man da auch gerne mal mehrere Dinge anstoßen möchte, schnell zu Fehlern führen kann. Abgesehen davon, würde ich es aber so regeln:

    Java
            Bukkit.getScheduler().scheduleSyncRepeatingTask(pluginInstance, () -> {
                final Instant now = Instant.now;
    
                Bukkit.getOnlinePlayers().forEach(player -> {
                    Instant lastMovementMoment = lastMovement.get(player.getUniqueId());
                    if (lastMovementMoment != null && Duration.between(lastMovementMoment, now).getSeconds() > INACTIVITY_SECOND_TRESHOLD) {
                        // afk-logic here
                    }
                });
            }, 0L, 20L);

    Erstmal hole ich hier nur einmal die aktuelle Zeit. Sich den aktuellen Zeitpunkt zu beschaffen ist zwar nicht teuer, aber auch nicht gratis. Insbesondere bei der Präzision, die die neuen Time-Objekte von Java 8 anstreben, wird die präziseste, verfügbare Zeit-Ressource verwendet, um so ein Objekt zu beziehen. Das kann gerne auch mal einen Syscall beinhalten, was zwar in Ordnung ist, aber eben auch nicht so schnell/günstig wie man ggf. meinen mag. Wir brauchen den aktuellen Zeitpunkt ohnehin nur einmal, da uns die Veränderungen im Nanosekunden-Bereich während des Vergleichs egal sind.


    Eben weil uns die Präzision unter einer Sekunde nicht interessiert, habe ich den Task-Intervall übrigens auch auf 20 angehoben. Es reicht für diesen Fall sicherlich vollkommen einmal in der Sekunde nachzuprüfen, ob sich etwas neues ergeben hat.


    Dann habe ich deine beiden Bedingungen zu einer Zusammengefasst, und dabei auch das Contains durch die Nullability der Get-Methode ersetzt. Wenn das Element nicht enthalten ist, wird null zurückgegeben. Wir nutzen null niemals als echte Value, es kann also nur daher kommen. So sparen wir uns das doppelte Hashing (das aber sowieso unglaublich günstig ist) und können den Code etwas entschlacken. Der zweite Teil der Bedingung nutzt jetzt eine Konstante für den Vergleich (hier müsste also 60 zugewiesen werden) und arbeitet mit dem ermittelten Moment.


    Die Lösung ist vor Allem deshalb besser, weil sie wie gesagt nur einmal den aktuellen Vergleichs-Zeitpunkt erstellt, und weil sie generell nicht so viele Elemente erstellt. Die neue java.time-API basiert auf unveränderbaren (Immutable) Objekten. Ein Aufruf wie "plusMinutes(long)" erstellt daher extra ein neues LocalDateTime-Objekt. Wir erstellen hier jetzt zwar auch eine Duration, die wir durch den direkten Vergleich der Sekunden verhindern könnten, erhalten dafür aber eine deutlich verbesserte Lesbarkeit.


    Was aber hier noch immer nicht bedacht wurde, und was auch deine Lösung nicht abgedeckt hatte, ist der Fall, dass der Nutzer bereits als "AFK" markiert wurde. Aktuell würde also immer wieder der Teil bei "// afk-logic here" für ihn ausgeführt. Da müsste man also ggf. noch drauf achten und entsprechend filtern, um nur die neuen AFK-Nutzer zu erhalten.

  • Scrayos Was den Zeit-Typ und das Event angeht stimme ich dir zu macht um einiges mehr Sinn hab ich übersehen. Passiert ¯\_(ツ)_/¯

    Hier rückst du unnötig oft ein und verwendest keine Klammern, was insbesondere in Anbetracht der Tatsache, dass man da auch gerne mal mehrere Dinge anstoßen möchte, schnell zu Fehlern führen kann. Abgesehen davon, würde ich es aber so regeln:

    Das ist einfach nur Unsinn sowas als "Verbesserung" zu sehen den Erstens gehört es so das ein "neuer Code Block" eingerückt beginnt wer das nicht macht, "Schande über dich".

    Und warum sollte ich hier Klammern verwenden um das Code Beispiel unnötig in die Länge zu ziehen?

    Es gibt für mich hier keinen Grund einen perfekt funktionierenden Code zu verfassen, das soll schließlich keine Copy-Pasta Aktion werden sondern er soll sich mit den Klassen die ich benutzt habe vertraut machen und diese auch in Zukunft verwenden können.

    Ebenfalls gehe ich, nach seinem Code urteilend, davon aus das MadeByProxxy in der Lage ist eine einfache Klammer hinzuzufügen wenn er mehr als eine Zeile im If-Block benötigt ;D


    Was aber hier noch immer nicht bedacht wurde, und was auch deine Lösung nicht abgedeckt hatte, ist der Fall, dass der Nutzer bereits als "AFK" markiert wurde. Aktuell würde also immer wieder der Teil bei "// afk-logic here" für ihn ausgeführt. Da müsste man also ggf. noch drauf achten und entsprechend filtern, um nur die neuen AFK-Nutzer zu erhalten.

    Erneut: "Ich schreibe hier keine Fertigen Plugins", sondern gebe MadeByProxxy einen Denkanstoß in die richtige Richtung!


    Grüße Maxi

    If you are homeless ... just buy a house, duh!

    and if you wanna have a plugin matching your conditions ... just code it yourself!

  • MadeByProxxy Es freut mich, dass es nun funktioniert!


    Ich würde dennoch ungerne ein paar der von IDK_WHO_AM_I aufgeworfenen Punkte unkommentiert so stehen lassen. Dieser Post richtet sich also nicht mehr direkt an dein Problem, ist für dich und zukünftige Leser ggf. dennoch interessant und bedenkenswert. Es geht um meine allgemeine Einstellung in Bezug auf Hilfe-/Frage-Posts und um die Relevanz von Codestyle sowie verbesserter Lesbarkeit.


    IDK_WHO_AM_I Ich vermute, dass du meine Kritik falsch aufgefasst hast, bzw. ich nicht klar genug gemacht habe, was ich damit bezwecken wollte. Es ging mir überhaupt nicht darum dich auch nur im entferntesten persönlich anzugreifen oder dich darin zu kritisieren, dass du Hilfe anbietest. Im Gegenteil: Ich finde es hervorragend, dass du dir die Zeit nimmst, um jemanden mit einem Problem unentgeltlich zu helfen. Das ist definitiv nicht selbstverständlich und sollte entsprechend honoriert werden.


    Auch deinen generellen Ansatz habe ich nicht negativ kommentiert, und darin sehe ich auch keinen Grund. Der allgemeine Algorithmus und dessen Funktion stimmt ja durchaus mit dem überein, was sich der Thread-Ersteller gewünscht hat. Dennoch sind mir, wie ich oberhalb ja bereits ausgeführt habe, einige Dinge aufgefallen, die ich so für nicht gut, oder zumindest nicht uneingeschränkt optimal halte. Und insbesondere weil du dir die Zeit genommen hast, hier jemandem zu helfen und weil ich hier mit einer Antwort nicht nur dich erreichen kann, sondern auch Leute, die beispielsweise über Google auf diese Frage aufmerksam werden, habe ich mir gerne die Zeit genommen um auch darauf zu antworten und damit ggf. auch für dich einen Mehrwert zu bieten. Leider scheinst du dich davon angegriffen gefühlt zu haben, und hast dich auch innerhalb des ersten Satzes gerechtfertigt. Das wäre gar nicht nötig gewesen, es ging mir ja nicht darum dir Unvermögen oder dergleichen zu unterstellen.


    Nun aber zu meiner Meinung zu deinen Rechtfertigungen:

    Das ist einfach nur Unsinn sowas als "Verbesserung" zu sehen den Erstens gehört es so das ein "neuer Code Block" eingerückt beginnt wer das nicht macht, "Schande über dich".

    Das halte ich so für zumindest kontrovers. Codestyle und Lesbarkeit ist häufig sogar wichtiger als Mikro-Optimierungen, Feature-Überschuss oder Erweiterbarkeit. Codestyle und gute Lesbarkeit sind der Schlüssel zu guter Zusammenarbeit und Wartbarkeit. Programme werden schließlich für Menschen und nicht für den Computer geschrieben. Und auch, wenn ich deine Argumentation bezüglich des eingefügten Code-Blocks nachvollziehen kann, so stellt sich mir – insbesondere in dem konkreten Code-Beispiel – wirklich kein eigenständiger, logischer, neuer Block dar. Der innere Block besteht schließlich nur aus einer einzelnen Bedingung. Und diese Bedingung steht auch noch im direkten Zusammenhang mit der vorigen Bedingung, da du hiermit lediglich sicherstellen möchtest, dass ein fehlender Eintrag niemals größer als die 60 sein kann, die Bedingung also in diesem Fall immer falsch sein muss.


    Unnötige Einrückungen wirken sich besonders negativ auf den Lesefluss aus, wenn sie dafür sorgen, dass der darin enthaltene Code weit jenseits der (gut) lesbaren 100 Zeichen/Spalten liegt. Unter anderem deshalb gilt es, Einrückungen sinnvoll einzusetzen und eben auch zu vermeiden, wenn die Einrückung an dieser Stelle keinen Mehrwert bietet. In diesem Beispiel ist das weniger wichtig, aber grundsätzlich wirken sich separate Code-Blocks und die Aufteilung in zwei separate Bedingungen auch auf die zyklomatische Komplexität aus, was ebenfalls vermieden werden sollte.


    Was die Klammern und den Hinweis auf die AFK-Logik betrifft, so vertrete ich die Auffassung, dass die Personen die hier Hilfe anbieten so umsichtig wie möglich auf möglichst viele Leser eingehen sollten. Natürlich kann man das nicht verlangen, ist doch jeder hier freiwillig, aber wünschenswert ist es dennoch. Das Weglassen der Klammern verwirrt hier ggf. den einen oder anderen Leser und liefert ihm im Umkehrschluss keinen Vorteil. Zusätzlich können hier durch Unachtsamkeit leicht Fehler entstehen. Und auch wenn du natürlich damit recht hast, das es hier nicht um das Schreiben fertiger Plugins geht, so tut es dennoch nicht weh, hier kurz auf solche Faktoren hinzuweisen, bzw. die Klammern zu setzen. Ich glaube einfach, dass du dem Leser damit keinen gefallen tust, weil vielleicht nicht jeder so umfassend um die syntaktischen Möglichkeiten von Java informiert ist, wie du es bist.


    Dennoch möchte ich abschließend ein weiteres Mal darauf hinweisen, dass ich es grundsätzlich begrüße und loben möchte, dass du hier Leuten hilfst, und das ja auch tatsächlich nicht schlecht oder verkehrt. Aber ich hielt es für nötig mich hier zu äußern, da mir Aussagen wie "Schande über dich" trotzdem negativ aufgestoßen sind. Ich wünsche dir ungeachtet dessen einen schönen Tag und hoffe du weißt meinen Post hier richtig einzuordnen. :)

  • Hierzu wollte ich doch auch noch eine Kleinigkeit sagen, das ist etwas das Anfänger häufig vergessen: *Resourcen aufräumen*
    In diesem Fall also nicht das PlayerQuitEvent vergessen und den Eintrag aus der HashMap wegräumen.

    Alternativ wäre auch eine WeakHashMap und Player als Key möglich. Das ist aber grade in diesem Fall warscheinlich nicht die beste Lösung, da man sich darauf verlassen müsste, dass auch alle anderen Plugins (und der Server selbst) alle Referenzen zum Player aufräumen, aber ist für den einen oder anderen in einem anderen Kontext ja eventuell ein Denkanstoß ;)

    Mit freundlichen Grüßen,

    Rincewind