no async world access #496

Merged
YoyoNow merged 7 commits from Hut512/SteamWar:fix/thread-safe-world-access into main 2026-11-06 09:02:52 +01:00
Contributor

Race Conditions behoben in:

  • SmartPlaceListener
  • NoClipCommand
  • AntiNocom
  • CursorListener
  • SteamEditWand
  • TechHiderWrapper
  • (legacy TechHider); Besser in Zukunft zu TechHider wechseln

Zusammenfassung

Dieser PR ersetzt Netty-Paketfilter wo es geht durch Event-Handler. Überflüssiger Code wurde entfernt und dabei viele Bugs behoben:

  • SmartPlaceListener platziert Block bei (0,0,0)
  • Fehler in Slot-Index-Berechnung in NoClipCommand
  • Blöcke in NoClip Platzieren aktiviert für 1 Tick die Physik -> Man bleibt an Blöcken hängen
  • Floodfill vei SteamEditWand hat nicht richtig funktioniert
  • SteamEditWand funktioniert nur, wenn der Spieler bereits eine FAWE-Auswahl hat
  • SteamEditWand crasht bei FAWE-Kreisauswahl
  • Falsche Entity ID im TechHider
  • ...
Race Conditions behoben in: - [x] SmartPlaceListener - [x] NoClipCommand - [x] AntiNocom - [x] CursorListener - [x] SteamEditWand - [x] TechHiderWrapper - [ ] (legacy TechHider); Besser in Zukunft zu TechHider wechseln # Zusammenfassung Dieser PR ersetzt Netty-Paketfilter wo es geht durch Event-Handler. Überflüssiger Code wurde entfernt und dabei viele Bugs behoben: - SmartPlaceListener platziert Block bei (0,0,0) - Fehler in Slot-Index-Berechnung in NoClipCommand - Blöcke in NoClip Platzieren aktiviert für 1 Tick die Physik -> Man bleibt an Blöcken hängen - Floodfill vei SteamEditWand hat nicht richtig funktioniert - SteamEditWand funktioniert nur, wenn der Spieler bereits eine FAWE-Auswahl hat - SteamEditWand crasht bei FAWE-Kreisauswahl - Falsche Entity ID im TechHider - ...
Hut512 force-pushed fix/thread-safe-world-access from e1a75402ac to f92ff2177b 2026-09-22 01:00:33 +02:00 Compare
Hut512 force-pushed fix/thread-safe-world-access from 9b4340895a to df52ed2f37 2026-10-01 20:00:30 +02:00 Compare
Hut512 marked the pull request as ready for review 2026-10-03 18:15:59 +02:00
Owner

Wie sieht es bei diesem PR aus? Der build failed noch in der pipeline?

Wie sieht es bei diesem PR aus? Der build failed noch in der pipeline?
Owner

Könntest du zudem auf main Rebasen, sodass hier die neuen Änderungen aus main drin sind/angepasst ist darauf.

Könntest du zudem auf main Rebasen, sodass hier die neuen Änderungen aus main drin sind/angepasst ist darauf.
YoyoNow requested changes 2026-10-28 16:01:43 +01:00
Dismissed
YoyoNow left a comment
Owner

Kleiner Tipp fürs nächste mal, mach aus solchen Refactors einzelne PRs, welche dadurch viel kleiner und einfacher zu durchblicken sind.

Kleiner Tipp fürs nächste mal, mach aus solchen Refactors einzelne PRs, welche dadurch viel kleiner und einfacher zu durchblicken sind.
@@ -47,4 +47,4 @@
import java.util.stream.Collectors;
@Linked
public class SmartPlaceListener implements Listener {
Owner

Den Diff der Klasse durchblicke ich aktuell noch nicht. Das muss ich mir glaube mal genauer anschauen.

Den Diff der Klasse durchblicke ich aktuell noch nicht. Das muss ich mir glaube mal genauer anschauen.
Author
Contributor

Habe aus dem PacketFilter einen EventHandler gemacht und die Lookups (CONTAINERS + IGNORED) vereinfacht.

Habe aus dem PacketFilter einen EventHandler gemacht und die Lookups (CONTAINERS + IGNORED) vereinfacht.
Owner

Alles klar ok.

Alles klar ok.
YoyoNow marked this conversation as resolved
@@ -98,9 +99,13 @@ public class Hull {
return players.contains(player) && region.inRegion(location) && !visibility.get(new IntVector(location).toId(region));
}
public boolean isEntityHidden(Player player, int entityId) {
Owner

Hier musst du dir mal den HullHider PR anschauen

Hier musst du dir mal den HullHider PR anschauen
Author
Contributor

Diese race condition wird nicht in #517 behoben. Siehe todo:
// TODO will require entity tracking on the netty thread to prevent future race conditions

Diese race condition wird nicht in #517 behoben. Siehe todo: `// TODO will require entity tracking on the netty thread to prevent future race conditions`
@@ -37,4 +0,0 @@
import java.util.concurrent.ConcurrentHashMap;
@Linked
public class AntiNocom implements Listener {
Owner

Warum entfernst du diese Klasse?

Warum entfernst du diese Klasse?
Author
Contributor
PaperMC hat bereits einen robusteren Fix: https://github.com/PaperMC/Paper/blob/4d854e66b80dad9898d71a0f0b1f83cd39b97119/paper-server/patches/sources/net/minecraft/server/network/ServerGamePacketListenerImpl.java.patch#L1155-L1160
Owner

Gefühlt würde ich das nochmal drin lassen, weil der Path in paper ist zwar von vor 8 Jahren aber trotzdem ist dieser Bug aufgekommen und war exploitable.

Gefühlt würde ich das nochmal drin lassen, weil der Path in paper ist zwar von vor 8 Jahren aber trotzdem ist dieser Bug aufgekommen und war exploitable.
@@ -75,3 +73,2 @@
private boolean noWand(Player player) {
return player.getInventory().getItemInMainHand().getType() != SteamEditWand.MATERIAL;
private static final int MAX_CONNECTED_DEPTH = 256;
Owner

Is that enough to select a whole (M)WG / WS / AS?

Is that enough to select a whole (M)WG / WS / AS?
Author
Contributor

MAX_CONNECTED_DEPTH ist der maximale Abstand in Blöcken vom Startpunkt. 265 sollte für die meisten Sachen reichen. Das begrenzt den Lag auf gefühlt ~500ms. Für viel größere Bereiche müsste man die Arbeit auf mehrere Ticks aufteilen.

MAX_CONNECTED_DEPTH ist der maximale Abstand in Blöcken vom Startpunkt. 265 sollte für die meisten Sachen reichen. Das begrenzt den Lag auf gefühlt ~500ms. Für viel größere Bereiche müsste man die Arbeit auf mehrere Ticks aufteilen.
Owner

500ms an Lag ist eher unangenehm und sollte vermieden werden. Generell versuche ich eigentlich das der Tick nicht länger als 1000/20 dauert also 50ms wobei ich meistens solchen Tool calls maximal 30-40ms pro Tick brauchen dürfen und es sonst auf mehrere Ticks aufgeteilt wird.

500ms an Lag ist eher unangenehm und sollte vermieden werden. Generell versuche ich eigentlich das der Tick nicht länger als 1000/20 dauert also 50ms wobei ich meistens solchen Tool calls maximal 30-40ms pro Tick brauchen dürfen und es sonst auf mehrere Ticks aufgeteilt wird.
@@ -26,10 +20,6 @@ public class CursorListener implements Listener {
instance = this;
}
TinyProtocol.instance.addFilter(ServerboundMovePlayerPacket.Pos.class, this::updateCursorFromPacket);
Owner

Das hier war schon absichtlich so gebaut, sodass der Cursor snappier sich anfühlt. Paper Events sind nicht immer die schnellsten insbesondere beim Movement Packet

Das hier war schon absichtlich so gebaut, sodass der Cursor snappier sich anfühlt. Paper Events sind nicht immer die schnellsten insbesondere beim Movement Packet
Author
Contributor

Der Cursor wird doch bereits 20 Mal pro Sekunde aktualisiert. Ich habe beide Varianten (mit und ohne PacketFilter) direkt nacheinander getestet und konnte keinen Unterschied feststellen. Außerdem sollte man Cursor::render nicht von Netty aufrufen, weil es sehr rechenintensiv ist und auf die Welt zugreift (z.B. Raytracing).

Der Cursor wird doch bereits 20 Mal pro Sekunde aktualisiert. Ich habe beide Varianten (mit und ohne PacketFilter) direkt nacheinander getestet und konnte keinen Unterschied feststellen. Außerdem sollte man Cursor::render nicht von Netty aufrufen, weil es sehr rechenintensiv ist und auf die Welt zugreift (z.B. Raytracing).
YoyoNow marked this conversation as resolved
@@ -203,3 +203,3 @@
processors.put(ClientboundAnimatePacket.class, this.buildEntityPacketProcessor(ClientboundAnimatePacket::getId));
// 7.1.35 Entity Event: entity id based signal.
processors.put(ClientboundEntityEventPacket.class, this.buildEntityPacketProcessor(ClientboundEntityEventPacket::getEventId));
processors.put(ClientboundEntityEventPacket.class, this.buildEntityPacketProcessor((ClientboundEntityEventPacket p) -> p.entityId));
Owner

Den fix gibt es bald schon in main. Siehe HullHider PR

Den fix gibt es bald schon in main. Siehe HullHider PR
Author
Contributor

Diese PR ist trotzdem relevant.

Diese PR ist trotzdem relevant.
YoyoNow marked this conversation as resolved
Hut512 force-pushed fix/thread-safe-world-access from 1d88d3a8e0 to 17d56cd0b2 2026-10-29 10:49:10 +01:00 Compare
Owner

Kannst du den PR bitte nochmal Rebasen auf den aktuellen main?

Kannst du den PR bitte nochmal Rebasen auf den aktuellen main?
Author
Contributor

ConnectedSearch.java ist von Claude und ungefähr 20x schneller als der alte Algorithmus: ~10ms für ein Warship. Würde auch ohne funktionieren aber dann ist die Nutzererfahrung deutlich schlechter.

ConnectedSearch.java ist von Claude und ungefähr 20x schneller als der alte Algorithmus: ~10ms für ein Warship. Würde auch ohne funktionieren aber dann ist die Nutzererfahrung deutlich schlechter.
Hut512 added 7 commits 2026-11-04 21:15:00 +01:00
Hut512 force-pushed fix/thread-safe-world-access from 2dedff6442 to 63b9f4886f 2026-11-04 21:15:00 +01:00 Compare
YoyoNow approved these changes 2026-11-06 09:02:41 +01:00
YoyoNow merged commit 78ebef7d1b into main 2026-11-06 09:02:52 +01:00
YoyoNow deleted branch fix/thread-safe-world-access 2026-11-06 09:02:52 +01:00
Sign in to join this conversation.