Fix double-dialing race and cross-server landing race
Double-dial: isOpen() only becomes true once the chevron animation finishes, so a second click during that animation (another player, or the same player double-clicking) passed the existing guard and started an overlapping dial. RuntimeGate now has a separate isDialing() flag set the instant dial() starts and cleared only when closeGate() runs, so a gate stays blocked for its entire dialing-through-open lifecycle, not just while fully open. GateManager.dial() also defensively re-checks this itself and returns false if either gate is busy, in case two calls race in before the caller's own guard catches it. Cross-server landing: the proxy only sends its "deliver" message after confirming the server switch succeeded, so there's no guaranteed ordering between that message arriving and this backend finishing its own join processing for the player. A single Bukkit.getPlayer() null-check at message-receipt time could lose that race and silently drop the landing teleport, leaving the player at the server's normal spawn instead of the gate - which is what was being reported. CrossServerBridge now stashes the pending delivery and retries on PlayerJoinEvent as well, so it lands regardless of which order the message and the join actually resolve in.
This commit is contained in:
@@ -68,7 +68,10 @@ material, unsealed interior, too far from the sign, etc.
|
|||||||
water) and travel opens up. It auto-closes after `dialing.open-seconds`. That water
|
water) and travel opens up. It auto-closes after `dialing.open-seconds`. That water
|
||||||
is contained to exactly the iris blocks - the plugin actively cancels any attempt
|
is contained to exactly the iris blocks - the plugin actively cancels any attempt
|
||||||
by Minecraft's own fluid physics to spread it further, so it can't leak out through
|
by Minecraft's own fluid physics to spread it further, so it can't leak out through
|
||||||
a gap or an odd ring shape and flood the ground around the gate.
|
a gap or an odd ring shape and flood the ground around the gate. A gate that's
|
||||||
|
already dialing or open won't dial again - the animation itself, not just the fully
|
||||||
|
open state, blocks re-entry - so two people clicking the same sign can't start
|
||||||
|
overlapping dials or connect it to two destinations at once.
|
||||||
- **Shift + left-click** the sign: destroys the gate on the spot (same permission
|
- **Shift + left-click** the sign: destroys the gate on the spot (same permission
|
||||||
check as `/sg destroy` - owner or `stargate.admin`), breaking the sign as if
|
check as `/sg destroy` - owner or `stargate.admin`), breaking the sign as if
|
||||||
you'd mined it. Quicker than looking at the sign and typing the command.
|
you'd mined it. Quicker than looking at the sign and typing the command.
|
||||||
|
|||||||
+14
-6
@@ -323,10 +323,16 @@ public class GateManager {
|
|||||||
|
|
||||||
// ---- Dialing ----
|
// ---- Dialing ----
|
||||||
|
|
||||||
/** Starts the chevron-lighting animation, then opens the gate and connects it to the destination. */
|
/**
|
||||||
public void dial(RuntimeGate from, RuntimeGate to, Player initiator) {
|
* Starts the chevron-lighting animation, then opens the gate and connects it to the
|
||||||
if (from.isOpen()) closeGate(from);
|
* destination. Returns false (does nothing) if either gate is already dialing/open - the
|
||||||
if (to.isOpen()) closeGate(to);
|
* caller's own isDialing()/isOpen() check should normally catch this first, but this is a
|
||||||
|
* defensive second guard against two calls racing in before either sets the flag.
|
||||||
|
*/
|
||||||
|
public boolean dial(RuntimeGate from, RuntimeGate to, Player initiator) {
|
||||||
|
if (from.isDialing() || from.isOpen() || to.isDialing() || to.isOpen()) return false;
|
||||||
|
from.setDialing(true);
|
||||||
|
to.setDialing(true);
|
||||||
|
|
||||||
List<GateStructure.ChevronSlot> chevrons = from.getStructure() != null ? from.getStructure().getChevrons() : List.of();
|
List<GateStructure.ChevronSlot> chevrons = from.getStructure() != null ? from.getStructure().getChevrons() : List.of();
|
||||||
int delay = Math.max(1, config.chevronTickDelay);
|
int delay = Math.max(1, config.chevronTickDelay);
|
||||||
@@ -340,7 +346,7 @@ public class GateManager {
|
|||||||
|
|
||||||
if (chevrons.isEmpty()) {
|
if (chevrons.isEmpty()) {
|
||||||
finish.run();
|
finish.run();
|
||||||
return;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
final int[] i = {0};
|
final int[] i = {0};
|
||||||
@@ -359,6 +365,7 @@ public class GateManager {
|
|||||||
i[0]++;
|
i[0]++;
|
||||||
}, 0L, delay);
|
}, 0L, delay);
|
||||||
from.setDialTask(taskHolder[0]);
|
from.setDialTask(taskHolder[0]);
|
||||||
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
public void openGate(RuntimeGate gate, RuntimeGate connectedTo) {
|
public void openGate(RuntimeGate gate, RuntimeGate connectedTo) {
|
||||||
@@ -386,6 +393,7 @@ public class GateManager {
|
|||||||
if (gate.getDialTask() != null) { gate.getDialTask().cancel(); gate.setDialTask(null); }
|
if (gate.getDialTask() != null) { gate.getDialTask().cancel(); gate.setDialTask(null); }
|
||||||
if (gate.getCloseTask() != null) { gate.getCloseTask().cancel(); gate.setCloseTask(null); }
|
if (gate.getCloseTask() != null) { gate.getCloseTask().cancel(); gate.setCloseTask(null); }
|
||||||
gate.setOpen(false);
|
gate.setOpen(false);
|
||||||
|
gate.setDialing(false);
|
||||||
gate.setOutgoing(false);
|
gate.setOutgoing(false);
|
||||||
RuntimeGate other = gate.getConnectedTo();
|
RuntimeGate other = gate.getConnectedTo();
|
||||||
gate.setConnectedTo(null);
|
gate.setConnectedTo(null);
|
||||||
@@ -395,7 +403,7 @@ public class GateManager {
|
|||||||
c.getBlock().setType(c.getRestingMaterial());
|
c.getBlock().setType(c.getRestingMaterial());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (other != null && other.isOpen()) {
|
if (other != null && (other.isOpen() || other.isDialing())) {
|
||||||
closeGate(other);
|
closeGate(other);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -9,6 +9,10 @@ public class RuntimeGate {
|
|||||||
private final Gate gate;
|
private final Gate gate;
|
||||||
private GateStructure structure; // null if structure could not be (re)scanned
|
private GateStructure structure; // null if structure could not be (re)scanned
|
||||||
private boolean open = false;
|
private boolean open = false;
|
||||||
|
// True from the moment dialing starts until the gate closes - NOT just while it's open.
|
||||||
|
// isOpen() alone doesn't become true until the chevron animation finishes, so guarding
|
||||||
|
// only on isOpen() lets a second click during that animation start an overlapping dial.
|
||||||
|
private boolean dialing = false;
|
||||||
private boolean outgoing = false; // true only for the gate that was dialed - travel is one-way
|
private boolean outgoing = false; // true only for the gate that was dialed - travel is one-way
|
||||||
private RuntimeGate connectedTo = null;
|
private RuntimeGate connectedTo = null;
|
||||||
private BukkitTask closeTask;
|
private BukkitTask closeTask;
|
||||||
@@ -25,6 +29,8 @@ public class RuntimeGate {
|
|||||||
public void setStructure(GateStructure structure) { this.structure = structure; }
|
public void setStructure(GateStructure structure) { this.structure = structure; }
|
||||||
public boolean isOpen() { return open; }
|
public boolean isOpen() { return open; }
|
||||||
public void setOpen(boolean open) { this.open = open; }
|
public void setOpen(boolean open) { this.open = open; }
|
||||||
|
public boolean isDialing() { return dialing; }
|
||||||
|
public void setDialing(boolean dialing) { this.dialing = dialing; }
|
||||||
public boolean isOutgoing() { return outgoing; }
|
public boolean isOutgoing() { return outgoing; }
|
||||||
public void setOutgoing(boolean outgoing) { this.outgoing = outgoing; }
|
public void setOutgoing(boolean outgoing) { this.outgoing = outgoing; }
|
||||||
public RuntimeGate getConnectedTo() { return connectedTo; }
|
public RuntimeGate getConnectedTo() { return connectedTo; }
|
||||||
|
|||||||
+6
-3
@@ -76,7 +76,7 @@ public class SignInteractListener implements Listener {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private void dial(RuntimeGate from, org.bukkit.entity.Player player) {
|
private void dial(RuntimeGate from, org.bukkit.entity.Player player) {
|
||||||
if (from.isOpen()) {
|
if (from.isOpen() || from.isDialing()) {
|
||||||
player.sendMessage(Component.text("This gate is already active.", NamedTextColor.RED));
|
player.sendMessage(Component.text("This gate is already active.", NamedTextColor.RED));
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -98,7 +98,7 @@ public class SignInteractListener implements Listener {
|
|||||||
player.sendMessage(Component.text("Destination gate not found.", NamedTextColor.RED));
|
player.sendMessage(Component.text("Destination gate not found.", NamedTextColor.RED));
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (to.isOpen()) {
|
if (to.isOpen() || to.isDialing()) {
|
||||||
player.sendMessage(Component.text("Destination gate is busy.", NamedTextColor.RED));
|
player.sendMessage(Component.text("Destination gate is busy.", NamedTextColor.RED));
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -109,8 +109,11 @@ public class SignInteractListener implements Listener {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!gateManager.dial(from, to, player)) {
|
||||||
|
player.sendMessage(Component.text("This gate is busy.", NamedTextColor.RED));
|
||||||
|
return;
|
||||||
|
}
|
||||||
player.sendMessage(Component.text("Dialing " + to.getGate().getName() + "...", NamedTextColor.AQUA));
|
player.sendMessage(Component.text("Dialing " + to.getGate().getName() + "...", NamedTextColor.AQUA));
|
||||||
gateManager.dial(from, to, player);
|
|
||||||
SignRenderer.render(from, gateManager);
|
SignRenderer.render(from, gateManager);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+58
-22
@@ -7,14 +7,21 @@ import dev.skywalker3200.stargate.common.network.StargateChannel;
|
|||||||
import dev.skywalker3200.stargate.paper.StargatePlugin;
|
import dev.skywalker3200.stargate.paper.StargatePlugin;
|
||||||
import dev.skywalker3200.stargate.paper.gate.GateManager;
|
import dev.skywalker3200.stargate.paper.gate.GateManager;
|
||||||
import dev.skywalker3200.stargate.paper.gate.RuntimeGate;
|
import dev.skywalker3200.stargate.paper.gate.RuntimeGate;
|
||||||
|
import net.kyori.adventure.text.Component;
|
||||||
|
import net.kyori.adventure.text.format.NamedTextColor;
|
||||||
import org.bukkit.Bukkit;
|
import org.bukkit.Bukkit;
|
||||||
import org.bukkit.Location;
|
import org.bukkit.Location;
|
||||||
import org.bukkit.entity.Player;
|
import org.bukkit.entity.Player;
|
||||||
|
import org.bukkit.event.EventHandler;
|
||||||
|
import org.bukkit.event.Listener;
|
||||||
|
import org.bukkit.event.player.PlayerJoinEvent;
|
||||||
import org.bukkit.plugin.messaging.PluginMessageListener;
|
import org.bukkit.plugin.messaging.PluginMessageListener;
|
||||||
import org.bukkit.util.Vector;
|
import org.bukkit.util.Vector;
|
||||||
|
|
||||||
import java.io.ByteArrayInputStream;
|
import java.io.ByteArrayInputStream;
|
||||||
import java.io.DataInputStream;
|
import java.io.DataInputStream;
|
||||||
|
import java.util.HashMap;
|
||||||
|
import java.util.Map;
|
||||||
import java.util.UUID;
|
import java.util.UUID;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -22,12 +29,23 @@ import java.util.UUID;
|
|||||||
* player dialing a gate hosted on another backend server actually gets moved there and then
|
* player dialing a gate hosted on another backend server actually gets moved there and then
|
||||||
* warped to the right spot once they land, facing the same direction they entered with.
|
* warped to the right spot once they land, facing the same direction they entered with.
|
||||||
*/
|
*/
|
||||||
public class CrossServerBridge implements PluginMessageListener {
|
public class CrossServerBridge implements PluginMessageListener, Listener {
|
||||||
|
|
||||||
private final StargatePlugin plugin;
|
private final StargatePlugin plugin;
|
||||||
private final GateManager gateManager;
|
private final GateManager gateManager;
|
||||||
private boolean enabled = false;
|
private boolean enabled = false;
|
||||||
|
|
||||||
|
// The proxy sends its "deliver" message only after confirming the server switch, which
|
||||||
|
// means this backend's own join processing for that player may already be running - or
|
||||||
|
// may not have started yet. There's no guaranteed ordering between "plugin message
|
||||||
|
// arrives" and "Bukkit finishes registering the player as online", so a single
|
||||||
|
// Bukkit.getPlayer() check at message-receipt time can race and silently drop the landing,
|
||||||
|
// leaving the player at whatever this server's normal spawn is. Stashing the pending
|
||||||
|
// delivery and also re-checking it on PlayerJoinEvent covers both orderings.
|
||||||
|
private final Map<UUID, PendingDelivery> pending = new HashMap<>();
|
||||||
|
|
||||||
|
private record PendingDelivery(UUID gateId, float yaw, float pitch) {}
|
||||||
|
|
||||||
public CrossServerBridge(StargatePlugin plugin, GateManager gateManager) {
|
public CrossServerBridge(StargatePlugin plugin, GateManager gateManager) {
|
||||||
this.plugin = plugin;
|
this.plugin = plugin;
|
||||||
this.gateManager = gateManager;
|
this.gateManager = gateManager;
|
||||||
@@ -37,6 +55,7 @@ public class CrossServerBridge implements PluginMessageListener {
|
|||||||
this.enabled = true;
|
this.enabled = true;
|
||||||
Bukkit.getMessenger().registerOutgoingPluginChannel(plugin, StargateChannel.CHANNEL);
|
Bukkit.getMessenger().registerOutgoingPluginChannel(plugin, StargateChannel.CHANNEL);
|
||||||
Bukkit.getMessenger().registerIncomingPluginChannel(plugin, StargateChannel.CHANNEL, this);
|
Bukkit.getMessenger().registerIncomingPluginChannel(plugin, StargateChannel.CHANNEL, this);
|
||||||
|
Bukkit.getPluginManager().registerEvents(this, plugin);
|
||||||
plugin.getLogger().info("[Stargate] Cross-server bridge registered on channel " + StargateChannel.CHANNEL);
|
plugin.getLogger().info("[Stargate] Cross-server bridge registered on channel " + StargateChannel.CHANNEL);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -70,31 +89,48 @@ public class CrossServerBridge implements PluginMessageListener {
|
|||||||
float yaw = in.readFloat();
|
float yaw = in.readFloat();
|
||||||
float pitch = in.readFloat();
|
float pitch = in.readFloat();
|
||||||
|
|
||||||
RuntimeGate rg = gateManager.getById(gateId);
|
|
||||||
if (rg == null) {
|
|
||||||
plugin.getLogger().warning("[Stargate] Received teleport-deliver for unknown gate " + gateId);
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
Bukkit.getScheduler().runTask(plugin, () -> {
|
Bukkit.getScheduler().runTask(plugin, () -> {
|
||||||
Player p = Bukkit.getPlayer(playerId);
|
pending.put(playerId, new PendingDelivery(gateId, yaw, pitch));
|
||||||
if (p == null) return;
|
tryDeliver(playerId);
|
||||||
if (rg.getGate().isIrisClosed()) {
|
// in case the player never actually finishes joining (disconnected mid-transfer),
|
||||||
// the shield closed between the request and arrival - can't send them back
|
// don't hang onto this forever
|
||||||
// through the proxy, so just leave them at this server's default spawn
|
Bukkit.getScheduler().runTaskLater(plugin, () -> pending.remove(playerId), 20L * 30);
|
||||||
p.sendMessage(net.kyori.adventure.text.Component.text(
|
|
||||||
"The iris on the other end closed before you arrived.",
|
|
||||||
net.kyori.adventure.text.format.NamedTextColor.RED));
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
double yawRad = Math.toRadians(yaw);
|
|
||||||
Vector direction = new Vector(-Math.sin(yawRad), 0, Math.cos(yawRad));
|
|
||||||
Location landing = gateManager.computeSafeLanding(rg, direction, yaw, pitch);
|
|
||||||
if (landing == null) return;
|
|
||||||
p.teleport(landing);
|
|
||||||
});
|
});
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
plugin.getLogger().warning("[Stargate] Failed to handle cross-server teleport message: " + e.getMessage());
|
plugin.getLogger().warning("[Stargate] Failed to handle cross-server teleport message: " + e.getMessage());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@EventHandler
|
||||||
|
public void onJoin(PlayerJoinEvent event) {
|
||||||
|
if (pending.containsKey(event.getPlayer().getUniqueId())) {
|
||||||
|
tryDeliver(event.getPlayer().getUniqueId());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Must run on the main thread. No-op if the player isn't online yet or there's nothing pending for them. */
|
||||||
|
private void tryDeliver(UUID playerId) {
|
||||||
|
PendingDelivery delivery = pending.get(playerId);
|
||||||
|
if (delivery == null) return;
|
||||||
|
Player p = Bukkit.getPlayer(playerId);
|
||||||
|
if (p == null) return;
|
||||||
|
pending.remove(playerId);
|
||||||
|
|
||||||
|
RuntimeGate rg = gateManager.getById(delivery.gateId());
|
||||||
|
if (rg == null) {
|
||||||
|
plugin.getLogger().warning("[Stargate] Received teleport-deliver for unknown gate " + delivery.gateId());
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if (rg.getGate().isIrisClosed()) {
|
||||||
|
// the shield closed between the request and arrival - can't send them back
|
||||||
|
// through the proxy, so just leave them at this server's default spawn
|
||||||
|
p.sendMessage(Component.text("The iris on the other end closed before you arrived.", NamedTextColor.RED));
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
double yawRad = Math.toRadians(delivery.yaw());
|
||||||
|
Vector direction = new Vector(-Math.sin(yawRad), 0, Math.cos(yawRad));
|
||||||
|
Location landing = gateManager.computeSafeLanding(rg, direction, delivery.yaw(), delivery.pitch());
|
||||||
|
if (landing == null) return;
|
||||||
|
p.teleport(landing);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user