From 26def21881822486374937ceac3c07737286c11c Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 17 Oct 2026 17:59:24 +0200 Subject: [PATCH 1/3] feat: answer vehicle faction, guild, ally and vassal access Registers VehicleFramework 2.15's VehicleAccessGroups service. Pool and installation vehicles always admit their paying faction, and their leader may switch allies and vassals on. Guild access counts player guilds only. Drops the claim bridge for VehicleFramework releases before 2.13, whose event no longer exists. Co-Authored-By: Claude Opus 5.5 --- .../simplefactions/SimpleFactions.java | 2 + .../owners/LegacyVehicleClaimsOff.java | 15 -- .../vehicles/owners/VehicleClaimGuards.java | 8 +- .../vehicles/owners/VehicleGroupAccess.java | 102 ++++++++++ .../vehicles/owners/VehicleOwnershipGate.java | 7 +- .../owners/VehicleOwnershipServices.java | 28 ++- .../PluginLifecycleCoverageTest.java | 3 + .../vehicles/owners/VehicleClaimsOffTest.java | 27 +-- .../owners/VehicleGroupAccessTest.java | 182 ++++++++++++++++++ .../owners/VehicleOwnershipGateTest.java | 26 ++- .../owners/VehicleOwnershipServicesTest.java | 25 +++ 11 files changed, 376 insertions(+), 49 deletions(-) delete mode 100644 src/main/java/net/tfminecraft/simplefactions/vehicles/owners/LegacyVehicleClaimsOff.java create mode 100644 src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleGroupAccess.java create mode 100644 src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleGroupAccessTest.java diff --git a/src/main/java/net/tfminecraft/simplefactions/SimpleFactions.java b/src/main/java/net/tfminecraft/simplefactions/SimpleFactions.java index 41615c9f..64dbdb5b 100644 --- a/src/main/java/net/tfminecraft/simplefactions/SimpleFactions.java +++ b/src/main/java/net/tfminecraft/simplefactions/SimpleFactions.java @@ -1008,6 +1008,8 @@ void startVehicles() { if (full) { VehicleOwnershipServices.registerAuthority(getServer().getServicesManager(), this, () -> started && LeaderStamps.enabled()); + VehicleOwnershipServices.registerGroupAccess(getServer().getServicesManager(), this, vehicleRegistry, + () -> started && LeaderStamps.enabled()); LeaderStamps.enable(true); getServer().getScheduler().runTaskTimer(this, this::reconcileVehicleUpkeep, VEHICLE_RECONCILE_TICKS, VEHICLE_RECONCILE_TICKS); diff --git a/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/LegacyVehicleClaimsOff.java b/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/LegacyVehicleClaimsOff.java deleted file mode 100644 index 1888258d..00000000 --- a/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/LegacyVehicleClaimsOff.java +++ /dev/null @@ -1,15 +0,0 @@ -package net.tfminecraft.simplefactions.vehicles.owners; - -import org.bukkit.event.EventHandler; -import org.bukkit.event.EventPriority; -import org.bukkit.event.Listener; - -import net.tfminecraft.vehicleframework.events.VehicleOwnerClaimedEvent; - -/** Compatibility bridge, loaded only when the provider lacks the modern event. */ -public final class LegacyVehicleClaimsOff implements Listener { - @EventHandler(priority = EventPriority.LOWEST, ignoreCancelled = true) - public void onClaim(VehicleOwnerClaimedEvent event) { - VehicleClaimGuards.refuse(event.getPlayer(), event); - } -} diff --git a/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimGuards.java b/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimGuards.java index 5e4dfde2..bcb73e5b 100644 --- a/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimGuards.java +++ b/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimGuards.java @@ -13,13 +13,11 @@ public final class VehicleClaimGuards { private VehicleClaimGuards() {} public static Listener create() { - return create("net.tfminecraft.vehicleframework.events.VehicleOwnershipChangeEvent", - "net.tfminecraft.vehicleframework.events.VehicleOwnerClaimedEvent"); + return create("net.tfminecraft.vehicleframework.events.VehicleOwnershipChangeEvent"); } - static Listener create(String modernEvent, String legacyEvent) { - if (present(modernEvent)) return new VehicleClaimsOff(); - if (present(legacyEvent)) return new LegacyVehicleClaimsOff(); + static Listener create(String claimEvent) { + if (present(claimEvent)) return new VehicleClaimsOff(); throw new IllegalStateException("VehicleFramework has no supported claim event"); } diff --git a/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleGroupAccess.java b/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleGroupAccess.java new file mode 100644 index 00000000..519e581a --- /dev/null +++ b/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleGroupAccess.java @@ -0,0 +1,102 @@ +package net.tfminecraft.simplefactions.vehicles.owners; + +import java.util.Optional; +import java.util.Set; +import java.util.UUID; +import java.util.function.BooleanSupplier; + +import net.tfminecraft.simplefactions.guild.Guild; +import net.tfminecraft.simplefactions.managers.FactionManager; +import net.tfminecraft.simplefactions.managers.RelationManager; +import net.tfminecraft.simplefactions.objects.Faction; +import net.tfminecraft.simplefactions.vehicles.pool.FactionVehiclePoolService; +import net.tfminecraft.simplefactions.vehicles.registry.PlayerVehicleRecord; +import net.tfminecraft.simplefactions.vehicles.registry.PlayerVehicleRegistry; +import net.tfminecraft.vehicleframework.api.AccessGroup; +import net.tfminecraft.vehicleframework.api.VehicleAccessGroups; + +/** + * VehicleFramework's group switches, answered from current membership. A pool or installation + * vehicle counts from its paying faction and always admits it; any other vehicle counts from its + * owner's character. Guilds mean player guilds, never a faction's main guild, and faction + * vehicles have none. Vassals are the direct vassals only. + */ +public final class VehicleGroupAccess implements VehicleAccessGroups { + private final PlayerVehicleRegistry registry; + private final BooleanSupplier ready; + + public VehicleGroupAccess(PlayerVehicleRegistry registry, BooleanSupplier ready) { + this.registry = registry; + this.ready = ready; + } + + @Override + public boolean ready() { + return ready.getAsBoolean(); + } + + @Override + public boolean admits(AccessGroup group, UUID vehicleId, String owner, UUID rider) { + if (group == null || rider == null) return false; + return switch (group) { + case FACTION -> same(faction(vehicleId, owner), FactionManager.getByMember(rider)); + case GUILD -> { + Guild guild = guild(vehicleId, owner); + yield guild != null && guild == playerGuild(rider); + } + case ALLY -> { + Faction faction = faction(vehicleId, owner); + Faction other = FactionManager.getByMember(rider); + yield faction != null && other != null && !same(faction, other) && RelationManager.isAlly(faction, other); + } + case VASSAL -> { + Faction faction = faction(vehicleId, owner); + Faction other = FactionManager.getByMember(rider); + yield faction != null && other != null && RelationManager.isOverlord(other, faction); + } + }; + } + + @Override + public Set alwaysOpen(UUID vehicleId, String owner) { + return factionRow(vehicleId) != null ? Set.of(AccessGroup.FACTION) : Set.of(); + } + + @Override + public Optional describe(AccessGroup group, UUID vehicleId, String owner) { + if (group == AccessGroup.GUILD) { + return Optional.ofNullable(guild(vehicleId, owner)).map(Guild::getName); + } + return Optional.ofNullable(faction(vehicleId, owner)).map(Faction::getName); + } + + /** The faction a vehicle's groups count from: its paying faction, else its owner's faction. */ + private Faction faction(UUID vehicleId, String owner) { + PlayerVehicleRecord row = factionRow(vehicleId); + if (row != null) return FactionVehiclePoolService.payingFaction(row); + UUID character = OwnerTags.characterId(owner).orElse(null); + return character == null ? null : FactionManager.getByMember(character); + } + + private Guild guild(UUID vehicleId, String owner) { + if (factionRow(vehicleId) != null) return null; + return OwnerTags.characterId(owner).map(VehicleGroupAccess::playerGuild).orElse(null); + } + + private PlayerVehicleRecord factionRow(UUID vehicleId) { + if (vehicleId == null) return null; + PlayerVehicleRecord row = registry.getByVehicleUuid(vehicleId.toString()).orElse(null); + return LeaderStamps.isFactionRow(row) ? row : null; + } + + /** The character's player guild, as member or leader; a faction's main guild does not count. */ + static Guild playerGuild(UUID character) { + Guild guild = FactionManager.getGuildByMember(character); + if (guild == null || guild.isBase()) guild = FactionManager.getGuildByLeader(character); + return guild == null || guild.isBase() ? null : guild; + } + + private static boolean same(Faction a, Faction b) { + return a != null && b != null && a.getId().equalsIgnoreCase(b.getId()); + } +} diff --git a/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipGate.java b/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipGate.java index cf547e99..cdc031f3 100644 --- a/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipGate.java +++ b/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipGate.java @@ -50,6 +50,9 @@ public final class VehicleOwnershipGate implements Listener { public static final String FAILED = "§cVehicle ownership could not be checked, so nothing changed."; public static final String HELD = "§cThis vehicle is held for staff; ask them to assign its owner."; static final int REMEMBERED_OPERATIONS = 1024; + /** The only controls a faction leader may use on a pool or installation vehicle. */ + static final Set LEADER_SWITCHES = + Set.of(OwnershipReason.ALLY_ACCESS_TOGGLE, OwnershipReason.VASSAL_ACCESS_TOGGLE); private final PlayerVehicleRegistry registry; private final VehicleRegistryClaimService claims; @@ -81,7 +84,7 @@ public void onChange(VehicleOwnershipChangeEvent event) { refuse(event, event.getPlayer(), HELD); return; } - if (LeaderStamps.isFactionRow(row(vehicle))) { + if (LeaderStamps.isFactionRow(row(vehicle)) && !LEADER_SWITCHES.contains(event.getReason())) { refuse(event, event.getPlayer(), VehicleRegistryClaimListener.FACTION_VEHICLE); return; } @@ -230,7 +233,7 @@ public void onEditCheck(VehicleOwnershipEditCheckEvent event) { if (LeaderStamps.isStale(record, event.getVehicle().getOwnerData().getOwner())) { event.deny(); LeaderStamps.scheduleRestamp(record); - } else if (!event.isMenu()) { + } else if (!event.isMenu() && !LEADER_SWITCHES.contains(event.getReason().orElseThrow())) { event.deny(); } } catch (RuntimeException | LinkageError error) { diff --git a/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipServices.java b/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipServices.java index 520a0b6f..b78027a8 100644 --- a/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipServices.java +++ b/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipServices.java @@ -9,12 +9,16 @@ import org.bukkit.plugin.ServicePriority; import org.bukkit.plugin.ServicesManager; +import net.tfminecraft.simplefactions.vehicles.registry.PlayerVehicleRegistry; import net.tfminecraft.vehicleframework.api.QuarantinedVehicle; +import net.tfminecraft.vehicleframework.api.VehicleAccessGroups; import net.tfminecraft.vehicleframework.api.VehicleOwnershipAuthority; import net.tfminecraft.vehicleframework.api.VehicleQuarantineSource; -/** The two VehicleFramework services SimpleFactions provides; only loaded once the API probe passed. */ +/** The VehicleFramework services SimpleFactions provides; only loaded once the API probe passed. */ public final class VehicleOwnershipServices { + static final String GROUP_ACCESS_API = "net.tfminecraft.vehicleframework.api.VehicleAccessGroups"; + private VehicleOwnershipServices() {} /** Lists held-back vehicles for {@code /vf admin quarantine}, even when the vehicle stage stopped early. */ @@ -32,6 +36,28 @@ public boolean ready() { }, plugin, ServicePriority.Normal); } + /** + * Answers the vehicle group switches. VehicleFramework before 2.15 has no such service, so + * nothing is registered and vehicles keep their whitelists only. + */ + public static boolean registerGroupAccess(ServicesManager services, Plugin plugin, PlayerVehicleRegistry registry, + BooleanSupplier ready) { + return registerGroupAccess(services, plugin, registry, ready, GROUP_ACCESS_API); + } + + static boolean registerGroupAccess(ServicesManager services, Plugin plugin, PlayerVehicleRegistry registry, + BooleanSupplier ready, String apiClass) { + try { + Class.forName(apiClass); + } catch (ClassNotFoundException | LinkageError missing) { + plugin.getLogger().warning("VehicleFramework 2.15 or newer is needed for faction, guild, ally and vassal " + + "vehicle access; vehicles keep their whitelists only"); + return false; + } + services.register(VehicleAccessGroups.class, new VehicleGroupAccess(registry, ready), plugin, ServicePriority.Normal); + return true; + } + static List quarantined(VehicleQuarantine quarantine) { List out = new ArrayList<>(); for (VehicleQuarantine.Entry entry : quarantine.list()) { diff --git a/src/test/java/net/tfminecraft/simplefactions/PluginLifecycleCoverageTest.java b/src/test/java/net/tfminecraft/simplefactions/PluginLifecycleCoverageTest.java index 2db5e977..5e2f3052 100644 --- a/src/test/java/net/tfminecraft/simplefactions/PluginLifecycleCoverageTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/PluginLifecycleCoverageTest.java @@ -1605,6 +1605,9 @@ void vehiclesStartRightAfterFactionsWhenVehicleFrameworkIsAlreadyEnabled() throw var authority = services().load(net.tfminecraft.vehicleframework.api.VehicleOwnershipAuthority.class); assertTrue(authority.ready()); + var groups = services().load(net.tfminecraft.vehicleframework.api.VehicleAccessGroups.class); + assertTrue(groups.ready()); + assertTrue(groups.alwaysOpen(UUID.randomUUID(), "char_" + leader).isEmpty()); assertEquals( 1, services() diff --git a/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimsOffTest.java b/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimsOffTest.java index cf7e7008..f32dc7c0 100644 --- a/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimsOffTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimsOffTest.java @@ -7,7 +7,6 @@ import org.bukkit.entity.Player; import org.junit.jupiter.api.Test; import net.tfminecraft.vehicleframework.api.OwnershipReason; -import net.tfminecraft.vehicleframework.events.VehicleOwnerClaimedEvent; import net.tfminecraft.vehicleframework.events.VehicleOwnershipChangeEvent; import net.tfminecraft.vehicleframework.vehicles.ActiveVehicle; @@ -31,30 +30,10 @@ void withoutCharacterOwnershipOnlyStaffMayClaim() { } @Test - void legacyBridgeKeepsTheOldStaffAndMissingPlayerPolicy() { - LegacyVehicleClaimsOff guard = new LegacyVehicleClaimsOff(); - Player player = mock(Player.class); - Player staff = mock(Player.class); - when(staff.hasPermission("simplefactions.admin")).thenReturn(true); - VehicleOwnerClaimedEvent claim = new VehicleOwnerClaimedEvent(player, null, "none", "player_A"); - VehicleOwnerClaimedEvent allowed = new VehicleOwnerClaimedEvent(staff, null, "none", "player_Staff"); - VehicleOwnerClaimedEvent absent = new VehicleOwnerClaimedEvent(null, null, "none", "player_X"); - guard.onClaim(claim); - guard.onClaim(allowed); - guard.onClaim(absent); - assertTrue(claim.isCancelled()); - verify(player).sendMessage(VehicleClaimsOff.MESSAGE); - assertFalse(allowed.isCancelled()); - assertFalse(absent.isCancelled()); - } - - @Test - void selectsModernBeforeLegacyAndRefusesAnUnknownProvider() { - String legacy = VehicleOwnerClaimedEvent.class.getName(); + void selectsTheOwnershipEventGuardAndRefusesAnUnknownProvider() { assertInstanceOf(VehicleClaimsOff.class, VehicleClaimGuards.create()); - assertInstanceOf(LegacyVehicleClaimsOff.class, VehicleClaimGuards.create("missing.Modern", legacy)); - assertInstanceOf(LegacyVehicleClaimsOff.class, VehicleClaimGuards.create(BrokenEvent.class.getName(), legacy)); - assertThrows(IllegalStateException.class, () -> VehicleClaimGuards.create("missing.Modern", "missing.Legacy")); + assertThrows(IllegalStateException.class, () -> VehicleClaimGuards.create("missing.Modern")); + assertThrows(IllegalStateException.class, () -> VehicleClaimGuards.create(BrokenEvent.class.getName())); } static final class BrokenEvent { diff --git a/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleGroupAccessTest.java b/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleGroupAccessTest.java new file mode 100644 index 00000000..81012e6f --- /dev/null +++ b/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleGroupAccessTest.java @@ -0,0 +1,182 @@ +package net.tfminecraft.simplefactions.vehicles.owners; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; + +import java.util.Optional; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +import net.tfminecraft.simplefactions.guild.Guild; +import net.tfminecraft.simplefactions.managers.FactionManager; +import net.tfminecraft.simplefactions.managers.RelationManager; +import net.tfminecraft.simplefactions.objects.Faction; +import net.tfminecraft.simplefactions.vehicles.pool.FactionVehiclePoolService; +import net.tfminecraft.simplefactions.vehicles.registry.OwnershipMode; +import net.tfminecraft.simplefactions.vehicles.registry.PlayerVehicleRecord; +import net.tfminecraft.simplefactions.vehicles.registry.PlayerVehicleRegistry; +import net.tfminecraft.vehicleframework.api.AccessGroup; + +class VehicleGroupAccessTest { + private final PlayerVehicleRegistry registry = new PlayerVehicleRegistry(); + private final AtomicBoolean ready = new AtomicBoolean(true); + private final VehicleGroupAccess access = new VehicleGroupAccess(registry, ready::get); + private final UUID vehicle = UUID.randomUUID(); + private final UUID owner = UUID.randomUUID(); + private final UUID rider = UUID.randomUUID(); + private final String stamp = OwnerTags.character(owner); + private final Faction bog = faction("The_Bog", "The Bog"); + private final Faction marsh = faction("Marsh", "Marsh"); + private MockedStatic factions; + private MockedStatic relations; + private MockedStatic pool; + + @BeforeEach + void setUp() { + factions = mockStatic(FactionManager.class); + relations = mockStatic(RelationManager.class); + pool = mockStatic(FactionVehiclePoolService.class); + factions.when(() -> FactionManager.getByMember(owner)).thenReturn(bog); + } + + @AfterEach + void tearDown() { + pool.close(); + relations.close(); + factions.close(); + } + + private static Faction faction(String id, String name) { + Faction faction = mock(Faction.class); + when(faction.getId()).thenReturn(id); + when(faction.getName()).thenReturn(name); + return faction; + } + + private static Guild guild(String name, boolean base) { + Guild guild = mock(Guild.class); + when(guild.getName()).thenReturn(name); + when(guild.isBase()).thenReturn(base); + return guild; + } + + private void factionVehicle(Faction payer) { + PlayerVehicleRecord row = new PlayerVehicleRecord(owner, vehicle.toString(), "gunboat", OwnershipMode.POOL, null, + "The_Bog"); + registry.register(row); + pool.when(() -> FactionVehiclePoolService.payingFaction(row)).thenReturn(payer); + } + + @Test + void readinessFollowsSimpleFactionsOnEveryCall() { + assertTrue(access.ready()); + ready.set(false); + assertFalse(access.ready()); + } + + @Test + void factionAccessCountsFromTheOwnersCurrentFaction() { + assertFalse(access.admits(AccessGroup.FACTION, vehicle, stamp, rider), "a rider without a faction"); + factions.when(() -> FactionManager.getByMember(rider)).thenReturn(marsh); + assertFalse(access.admits(AccessGroup.FACTION, vehicle, stamp, rider)); + Faction lowerCase = faction("the_bog", "The Bog"); + factions.when(() -> FactionManager.getByMember(rider)).thenReturn(lowerCase); + assertTrue(access.admits(AccessGroup.FACTION, vehicle, stamp, rider), "faction ids match without case"); + assertFalse(access.admits(AccessGroup.FACTION, vehicle, "char_none", rider), "no owner character, no faction"); + assertFalse(access.admits(AccessGroup.FACTION, vehicle, stamp, null)); + assertFalse(access.admits(null, vehicle, stamp, rider)); + assertEquals(Optional.of("The Bog"), access.describe(AccessGroup.FACTION, vehicle, stamp)); + assertEquals(Set.of(), access.alwaysOpen(vehicle, stamp)); + assertEquals(Set.of(), access.alwaysOpen(null, stamp)); + } + + @Test + void aFactionVehicleAlwaysAdmitsItsPayingFactionWhateverTheStamp() { + factionVehicle(bog); + factions.when(() -> FactionManager.getByMember(rider)).thenReturn(bog); + assertEquals(Set.of(AccessGroup.FACTION), access.alwaysOpen(vehicle, "char_none")); + assertTrue(access.admits(AccessGroup.FACTION, vehicle, "char_none", rider)); + assertEquals(Optional.of("The Bog"), access.describe(AccessGroup.ALLY, vehicle, "char_none")); + factionVehicle(null); + assertFalse(access.admits(AccessGroup.FACTION, vehicle, stamp, rider), "a row whose faction is gone admits nobody"); + } + + @Test + void personalRowsCountFromTheOwner() { + registry.register(new PlayerVehicleRecord(owner, vehicle.toString(), "cart", OwnershipMode.PERSONAL, null)); + factions.when(() -> FactionManager.getByMember(rider)).thenReturn(bog); + assertEquals(Set.of(), access.alwaysOpen(vehicle, stamp)); + assertTrue(access.admits(AccessGroup.FACTION, vehicle, stamp, rider)); + } + + @Test + void guildAccessMeansTheSamePlayerGuildNeverTheMainGuild() { + Guild smiths = guild("Smiths", false); + factions.when(() -> FactionManager.getGuildByMember(owner)).thenReturn(smiths); + assertFalse(access.admits(AccessGroup.GUILD, vehicle, stamp, rider)); + Guild masons = guild("Masons", false); + factions.when(() -> FactionManager.getGuildByMember(rider)).thenReturn(masons); + assertFalse(access.admits(AccessGroup.GUILD, vehicle, stamp, rider)); + factions.when(() -> FactionManager.getGuildByMember(rider)).thenReturn(null); + factions.when(() -> FactionManager.getGuildByLeader(rider)).thenReturn(smiths); + assertTrue(access.admits(AccessGroup.GUILD, vehicle, stamp, rider), "a guild's leader counts as in it"); + assertEquals(Optional.of("Smiths"), access.describe(AccessGroup.GUILD, vehicle, stamp)); + Guild main = guild("The Bog", true); + factions.when(() -> FactionManager.getGuildByMember(owner)).thenReturn(main); + factions.when(() -> FactionManager.getGuildByLeader(rider)).thenReturn(main); + assertFalse(access.admits(AccessGroup.GUILD, vehicle, stamp, rider), "the main guild is the whole faction"); + assertEquals(Optional.empty(), access.describe(AccessGroup.GUILD, vehicle, stamp)); + assertFalse(access.admits(AccessGroup.GUILD, vehicle, "none", rider)); + factions.when(() -> FactionManager.getGuildByMember(rider)).thenReturn(main); + factions.when(() -> FactionManager.getGuildByLeader(rider)).thenReturn(smiths); + factions.when(() -> FactionManager.getGuildByMember(owner)).thenReturn(smiths); + assertTrue(access.admits(AccessGroup.GUILD, vehicle, stamp, rider), "a leader still listed in the main guild"); + } + + @Test + void factionVehiclesHaveNoGuild() { + Guild smiths = guild("Smiths", false); + factions.when(() -> FactionManager.getGuildByMember(any(UUID.class))).thenReturn(smiths); + factionVehicle(bog); + assertFalse(access.admits(AccessGroup.GUILD, vehicle, stamp, rider)); + assertEquals(Optional.empty(), access.describe(AccessGroup.GUILD, vehicle, stamp)); + } + + @Test + void allyAccessNeedsTheOwnersFactionToCallTheRidersFactionAnAlly() { + assertFalse(access.admits(AccessGroup.ALLY, vehicle, stamp, rider), "a rider without a faction"); + factions.when(() -> FactionManager.getByMember(rider)).thenReturn(marsh); + assertFalse(access.admits(AccessGroup.ALLY, vehicle, stamp, rider)); + relations.when(() -> RelationManager.isAlly(bog, marsh)).thenReturn(true); + assertTrue(access.admits(AccessGroup.ALLY, vehicle, stamp, rider)); + relations.when(() -> RelationManager.isAlly(bog, bog)).thenReturn(true); + factions.when(() -> FactionManager.getByMember(rider)).thenReturn(bog); + assertFalse(access.admits(AccessGroup.ALLY, vehicle, stamp, rider), "the owner's own faction is not an ally"); + assertFalse(access.admits(AccessGroup.ALLY, vehicle, "none", rider)); + } + + @Test + void vassalAccessIsForDirectVassalsOfTheOwnersFaction() { + assertFalse(access.admits(AccessGroup.VASSAL, vehicle, stamp, rider), "a rider without a faction"); + factions.when(() -> FactionManager.getByMember(rider)).thenReturn(marsh); + assertFalse(access.admits(AccessGroup.VASSAL, vehicle, stamp, rider)); + relations.when(() -> RelationManager.isOverlord(marsh, bog)).thenReturn(true); + assertTrue(access.admits(AccessGroup.VASSAL, vehicle, stamp, rider)); + relations.when(() -> RelationManager.isOverlord(bog, marsh)).thenReturn(true); + relations.when(() -> RelationManager.isOverlord(marsh, bog)).thenReturn(false); + assertFalse(access.admits(AccessGroup.VASSAL, vehicle, stamp, rider), "the overlord is not the vassal"); + assertFalse(access.admits(AccessGroup.VASSAL, vehicle, "none", rider)); + assertEquals(Optional.empty(), access.describe(AccessGroup.VASSAL, vehicle, "none")); + } +} diff --git a/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipGateTest.java b/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipGateTest.java index 71920ab9..4b1b55ad 100644 --- a/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipGateTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipGateTest.java @@ -137,7 +137,7 @@ private void factionRow() { @ParameterizedTest @EnumSource(value = OwnershipReason.class, names = {"CLAIM", "CLEAR", "WHITELIST_ADD", "WHITELIST_REMOVE", - "WHITELIST_TOGGLE", "TICKETS_TOGGLE"}) + "WHITELIST_TOGGLE", "TICKETS_TOGGLE", "FACTION_ACCESS_TOGGLE", "GUILD_ACCESS_TOGGLE"}) void everyNonStaffChangeToAFactionVehicleIsRefused(OwnershipReason reason) { factionRow(); VehicleOwnershipChangeEvent event = change(reason, false); @@ -148,6 +148,19 @@ void everyNonStaffChangeToAFactionVehicleIsRefused(OwnershipReason reason) { verify(player).sendMessage(VehicleRegistryClaimListener.FACTION_VEHICLE); } + @ParameterizedTest + @EnumSource(value = OwnershipReason.class, names = {"ALLY_ACCESS_TOGGLE", "VASSAL_ACCESS_TOGGLE"}) + void theLeaderMayOpenAFactionVehicleToAlliesAndVassals(OwnershipReason reason) { + factionRow(); + VehicleOwnershipChangeEvent event = change(reason, false); + + gate.onChange(event); + gate.onClaimFee(event); + + assertFalse(event.isCancelled()); + verifyNoInteractions(player); + } + @Test void staffChangesPassEvenOnFactionVehiclesAndWithoutReadyPayments() { factionRow(); @@ -454,7 +467,7 @@ private VehicleOwnershipEditCheckEvent check(OwnershipReason reason, boolean sta } @Test - void factionLeadersMayOpenTheMenuButUseNoControlAndStaleStampsAreHiddenAndFixed() { + void factionLeadersMayOpenTheMenuAndOnlyTheAllyAndVassalSwitchesAndStaleStampsAreHiddenAndFixed() { UUID leader = UUID.randomUUID(); Faction red = mock(Faction.class); when(red.getId()).thenReturn("red"); @@ -470,6 +483,15 @@ void factionLeadersMayOpenTheMenuButUseNoControlAndStaleStampsAreHiddenAndFixed( VehicleOwnershipEditCheckEvent toggle = check(OwnershipReason.WHITELIST_TOGGLE, false); gate.onEditCheck(toggle); assertFalse(toggle.isAllowed()); + VehicleOwnershipEditCheckEvent guild = check(OwnershipReason.GUILD_ACCESS_TOGGLE, false); + gate.onEditCheck(guild); + assertFalse(guild.isAllowed(), "a faction vehicle has no guild"); + VehicleOwnershipEditCheckEvent allies = check(OwnershipReason.ALLY_ACCESS_TOGGLE, false); + gate.onEditCheck(allies); + assertTrue(allies.isAllowed()); + VehicleOwnershipEditCheckEvent vassals = check(OwnershipReason.VASSAL_ACCESS_TOGGLE, false); + gate.onEditCheck(vassals); + assertTrue(vassals.isAllowed()); VehicleOwnershipEditCheckEvent staff = check(OwnershipReason.CLEAR, true); gate.onEditCheck(staff); assertTrue(staff.isAllowed()); diff --git a/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipServicesTest.java b/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipServicesTest.java index 58e8cc62..dac66b2a 100644 --- a/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipServicesTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipServicesTest.java @@ -20,7 +20,9 @@ import org.junit.jupiter.api.io.TempDir; import org.mockito.ArgumentCaptor; +import net.tfminecraft.simplefactions.vehicles.registry.PlayerVehicleRegistry; import net.tfminecraft.vehicleframework.api.QuarantinedVehicle; +import net.tfminecraft.vehicleframework.api.VehicleAccessGroups; import net.tfminecraft.vehicleframework.api.VehicleOwnershipAuthority; import net.tfminecraft.vehicleframework.api.VehicleQuarantineSource; @@ -46,6 +48,29 @@ void theQuarantineSourceListsHeldBackVehiclesByIdForStaff() throws Exception { source.getValue().quarantinedVehicles()); } + @Test + void groupAccessIsRegisteredOnlyWhenVehicleFrameworkHasTheService() { + ServicesManager services = mock(ServicesManager.class); + Plugin plugin = mock(Plugin.class); + java.util.logging.Logger logger = mock(java.util.logging.Logger.class); + org.mockito.Mockito.when(plugin.getLogger()).thenReturn(logger); + AtomicBoolean ready = new AtomicBoolean(true); + + assertTrue(VehicleOwnershipServices.registerGroupAccess(services, plugin, new PlayerVehicleRegistry(), ready::get)); + + ArgumentCaptor groups = ArgumentCaptor.forClass(VehicleAccessGroups.class); + verify(services).register(eq(VehicleAccessGroups.class), groups.capture(), eq(plugin), eq(ServicePriority.Normal)); + assertTrue(groups.getValue() instanceof VehicleGroupAccess); + ready.set(false); + assertFalse(groups.getValue().ready()); + + ServicesManager old = mock(ServicesManager.class); + assertFalse(VehicleOwnershipServices.registerGroupAccess(old, plugin, new PlayerVehicleRegistry(), ready::get, + "net.tfminecraft.vehicleframework.api.Missing")); + org.mockito.Mockito.verifyNoInteractions(old); + verify(logger).warning(org.mockito.ArgumentMatchers.contains("VehicleFramework 2.15 or newer")); + } + @Test void theAuthorityAnswersReadyFromSimpleFactionsOnEveryCall() { ServicesManager services = mock(ServicesManager.class); From a2dd98f86579518f7c8c9164fe88b6ecf070d6c7 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 17 Oct 2026 18:26:55 +0200 Subject: [PATCH 2/3] build: build against VehicleFramework 2.15.0 Co-Authored-By: Claude Opus 5.5 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index d9641926..ebe82b3f 100644 --- a/pom.xml +++ b/pom.xml @@ -16,7 +16,7 @@ 0.7.0 2.16.0 2.6.0 - 2.13.0 + 2.15.0 2.2.0 From 376734c2981898ae827798ed58846f0375a179d5 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 17 Oct 2026 19:56:22 +0200 Subject: [PATCH 3/3] fix: keep vehicle integration loading on older VehicleFramework Leader switches are matched by reason name, so the gate still links on VehicleFramework 2.13 and 2.14. Before 2.13 the legacy claim event is guarded again, registered by name so nothing links to the removed class. Co-Authored-By: Claude Opus 5.5 --- .../simplefactions/SimpleFactions.java | 3 +- .../vehicles/owners/VehicleClaimGuards.java | 45 +++++++--- .../vehicles/owners/VehicleOwnershipGate.java | 12 +-- .../vehicles/owners/VehicleClaimsOffTest.java | 85 ++++++++++++++++++- 4 files changed, 123 insertions(+), 22 deletions(-) diff --git a/src/main/java/net/tfminecraft/simplefactions/SimpleFactions.java b/src/main/java/net/tfminecraft/simplefactions/SimpleFactions.java index 64dbdb5b..91f520a7 100644 --- a/src/main/java/net/tfminecraft/simplefactions/SimpleFactions.java +++ b/src/main/java/net/tfminecraft/simplefactions/SimpleFactions.java @@ -1102,8 +1102,7 @@ private void registerVehicleIntegration(boolean full) { vehicleRegistryClaimService, vehicleFeeStore, vehicleFeeConfirmations, this::saveVehicleFees, getLogger()), this); } else { - getServer().getPluginManager().registerEvents( - net.tfminecraft.simplefactions.vehicles.owners.VehicleClaimGuards.create(), this); + net.tfminecraft.simplefactions.vehicles.owners.VehicleClaimGuards.register(getServer().getPluginManager(), this); } // VehicleFramework loads its vehicles as it enables, so wait before reading owners. getServer().getScheduler().runTaskLater(this, this::recordVehicleOwners, 200L); diff --git a/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimGuards.java b/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimGuards.java index bcb73e5b..4f4db471 100644 --- a/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimGuards.java +++ b/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimGuards.java @@ -2,31 +2,54 @@ import org.bukkit.entity.Player; import org.bukkit.event.Cancellable; +import org.bukkit.event.Event; +import org.bukkit.event.EventPriority; import org.bukkit.event.Listener; +import org.bukkit.plugin.Plugin; +import org.bukkit.plugin.PluginManager; import net.tfminecraft.simplefactions.utils.Permissions; -/** Selects a claim guard without loading listener types absent from the installed provider. */ +/** Registers a claim guard without loading listener types absent from the installed provider. */ public final class VehicleClaimGuards { public static final String MESSAGE = "§cClaiming vehicles is off until VehicleFramework is updated."; + static final String CLAIM_EVENT = "net.tfminecraft.vehicleframework.events.VehicleOwnershipChangeEvent"; + /** The claim event of VehicleFramework before 2.13; 2.14 removed it, so it is only named, never linked. */ + static final String LEGACY_CLAIM_EVENT = "net.tfminecraft.vehicleframework.events.VehicleOwnerClaimedEvent"; private VehicleClaimGuards() {} - public static Listener create() { - return create("net.tfminecraft.vehicleframework.events.VehicleOwnershipChangeEvent"); + public static void register(PluginManager plugins, Plugin plugin) { + register(plugins, plugin, CLAIM_EVENT, LEGACY_CLAIM_EVENT); } - static Listener create(String claimEvent) { - if (present(claimEvent)) return new VehicleClaimsOff(); - throw new IllegalStateException("VehicleFramework has no supported claim event"); + static void register(PluginManager plugins, Plugin plugin, String claimEvent, String legacyEvent) { + if (eventClass(claimEvent) != null) { + plugins.registerEvents(new VehicleClaimsOff(), plugin); + return; + } + Class legacy = eventClass(legacyEvent); + if (legacy == null) throw new IllegalStateException("VehicleFramework has no supported claim event"); + plugins.registerEvent(legacy, new Listener() {}, EventPriority.LOWEST, + (listener, event) -> refuseLegacy(event), plugin, true); + } + + private static Class eventClass(String name) { + try { + return Class.forName(name).asSubclass(Event.class); + } catch (ClassNotFoundException | ClassCastException | LinkageError missing) { + return null; + } } - private static boolean present(String eventClass) { + /** The legacy event is only a claim; its player is read by name. */ + static void refuseLegacy(Event event) { + if (!(event instanceof Cancellable cancellable)) return; try { - Class.forName(eventClass); - return true; - } catch (ClassNotFoundException | LinkageError missing) { - return false; + Object player = event.getClass().getMethod("getPlayer").invoke(event); + refuse(player instanceof Player p ? p : null, cancellable); + } catch (ReflectiveOperationException | LinkageError unreadable) { + // Without a readable player the claim cannot be judged; VehicleFramework keeps its own rules. } } diff --git a/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipGate.java b/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipGate.java index cdc031f3..c8f0a328 100644 --- a/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipGate.java +++ b/src/main/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleOwnershipGate.java @@ -50,9 +50,11 @@ public final class VehicleOwnershipGate implements Listener { public static final String FAILED = "§cVehicle ownership could not be checked, so nothing changed."; public static final String HELD = "§cThis vehicle is held for staff; ask them to assign its owner."; static final int REMEMBERED_OPERATIONS = 1024; - /** The only controls a faction leader may use on a pool or installation vehicle. */ - static final Set LEADER_SWITCHES = - Set.of(OwnershipReason.ALLY_ACCESS_TOGGLE, OwnershipReason.VASSAL_ACCESS_TOGGLE); + /** + * The only controls a faction leader may use on a pool or installation vehicle. Matched by + * name: VehicleFramework before 2.15 has no such reasons, and a constant would not link there. + */ + static final Set LEADER_SWITCHES = Set.of("ALLY_ACCESS_TOGGLE", "VASSAL_ACCESS_TOGGLE"); private final PlayerVehicleRegistry registry; private final VehicleRegistryClaimService claims; @@ -84,7 +86,7 @@ public void onChange(VehicleOwnershipChangeEvent event) { refuse(event, event.getPlayer(), HELD); return; } - if (LeaderStamps.isFactionRow(row(vehicle)) && !LEADER_SWITCHES.contains(event.getReason())) { + if (LeaderStamps.isFactionRow(row(vehicle)) && !LEADER_SWITCHES.contains(event.getReason().name())) { refuse(event, event.getPlayer(), VehicleRegistryClaimListener.FACTION_VEHICLE); return; } @@ -233,7 +235,7 @@ public void onEditCheck(VehicleOwnershipEditCheckEvent event) { if (LeaderStamps.isStale(record, event.getVehicle().getOwnerData().getOwner())) { event.deny(); LeaderStamps.scheduleRestamp(record); - } else if (!event.isMenu() && !LEADER_SWITCHES.contains(event.getReason().orElseThrow())) { + } else if (!event.isMenu() && !LEADER_SWITCHES.contains(event.getReason().orElseThrow().name())) { event.deny(); } } catch (RuntimeException | LinkageError error) { diff --git a/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimsOffTest.java b/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimsOffTest.java index f32dc7c0..5b0d882a 100644 --- a/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimsOffTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/vehicles/owners/VehicleClaimsOffTest.java @@ -3,9 +3,21 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; + import java.util.UUID; import org.bukkit.entity.Player; +import org.bukkit.event.Cancellable; +import org.bukkit.event.Event; +import org.bukkit.event.EventPriority; +import org.bukkit.event.HandlerList; +import org.bukkit.event.Listener; +import org.bukkit.plugin.EventExecutor; +import org.bukkit.plugin.Plugin; +import org.bukkit.plugin.PluginManager; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; import net.tfminecraft.vehicleframework.api.OwnershipReason; import net.tfminecraft.vehicleframework.events.VehicleOwnershipChangeEvent; import net.tfminecraft.vehicleframework.vehicles.ActiveVehicle; @@ -30,16 +42,81 @@ void withoutCharacterOwnershipOnlyStaffMayClaim() { } @Test - void selectsTheOwnershipEventGuardAndRefusesAnUnknownProvider() { - assertInstanceOf(VehicleClaimsOff.class, VehicleClaimGuards.create()); - assertThrows(IllegalStateException.class, () -> VehicleClaimGuards.create("missing.Modern")); - assertThrows(IllegalStateException.class, () -> VehicleClaimGuards.create(BrokenEvent.class.getName())); + void registersTheOwnershipEventGuardFirstAndRefusesAnUnknownProvider() { + PluginManager plugins = mock(PluginManager.class); + Plugin plugin = mock(Plugin.class); + VehicleClaimGuards.register(plugins, plugin); + verify(plugins).registerEvents(any(VehicleClaimsOff.class), eq(plugin)); + assertThrows(IllegalStateException.class, + () -> VehicleClaimGuards.register(plugins, plugin, "missing.Modern", "missing.Legacy")); + assertThrows(IllegalStateException.class, () -> VehicleClaimGuards.register(plugins, plugin, + BrokenEvent.class.getName(), NotAnEvent.class.getName())); + } + + @Test + void beforeVehicleFramework213TheLegacyClaimEventIsGuardedByName() throws Exception { + PluginManager plugins = mock(PluginManager.class); + Plugin plugin = mock(Plugin.class); + VehicleClaimGuards.register(plugins, plugin, "missing.Modern", LegacyClaim.class.getName()); + ArgumentCaptor executor = ArgumentCaptor.forClass(EventExecutor.class); + verify(plugins).registerEvent(eq(LegacyClaim.class), any(Listener.class), eq(EventPriority.LOWEST), + executor.capture(), eq(plugin), eq(true)); + Player player = mock(Player.class); + Player staff = mock(Player.class); + when(staff.hasPermission("simplefactions.admin")).thenReturn(true); + LegacyClaim claim = new LegacyClaim(player); + LegacyClaim staffClaim = new LegacyClaim(staff); + LegacyClaim absent = new LegacyClaim(null); + executor.getValue().execute(null, claim); + executor.getValue().execute(null, staffClaim); + executor.getValue().execute(null, absent); + assertTrue(claim.isCancelled()); + verify(player).sendMessage(VehicleClaimGuards.MESSAGE); + assertFalse(staffClaim.isCancelled()); + assertFalse(absent.isCancelled()); + VehicleClaimGuards.refuseLegacy(new PlainEvent()); + NoPlayerClaim unreadable = new NoPlayerClaim(); + VehicleClaimGuards.refuseLegacy(unreadable); + assertFalse(unreadable.isCancelled()); } static final class BrokenEvent { static { if (true) throw new NoClassDefFoundError("missing dependency"); } } + static final class NotAnEvent { + } + + public static class LegacyClaim extends Event implements Cancellable { + private static final HandlerList HANDLERS = new HandlerList(); + private final Player player; + private boolean cancelled; + + LegacyClaim(Player player) { + this.player = player; + } + + public Player getPlayer() { + return player; + } + + @Override public boolean isCancelled() { return cancelled; } + @Override public void setCancelled(boolean cancel) { cancelled = cancel; } + @Override public HandlerList getHandlers() { return HANDLERS; } + public static HandlerList getHandlerList() { return HANDLERS; } + } + + static final class NoPlayerClaim extends Event implements Cancellable { + private boolean cancelled; + @Override public boolean isCancelled() { return cancelled; } + @Override public void setCancelled(boolean cancel) { cancelled = cancel; } + @Override public HandlerList getHandlers() { return new HandlerList(); } + } + + static final class PlainEvent extends Event { + @Override public HandlerList getHandlers() { return new HandlerList(); } + } + private static VehicleOwnershipChangeEvent claim(Player player, OwnershipReason reason) { return new VehicleOwnershipChangeEvent(player, mock(ActiveVehicle.class), reason, UUID.randomUUID(), null, false, null, null, "none", "player_A");