Surgery: switch penalty, old RPCharacters warning and physician failure report

src/main/java/net/tfminecraft/surgery/SurgeryPlugin.java

@@ -43,9 +43,21 @@ public void onEnable() {
         getCommand("surgery").setExecutor(surgeryCommand);
         getCommand("surgery").setTabCompleter(surgeryCommand);
 
+        // The first tick runs once every plugin has enabled.
+        getServer().getScheduler().runTask(this, this::warnIfNoCharacterProvider);
+
         getLogger().info("surgery has been enabled!");
     }
 
+    /** RPCharacters before 2.16.0 registers no TLibs character provider, so every offer would fail. */
+    void warnIfNoCharacterProvider() {
+        if (getServer().getPluginManager().isPluginEnabled("RPCharacters")
+                && !net.tfminecraft.tlibs.character.Characters.available()) {
+            getLogger().warning("RPCharacters provides no character identity, so every surgery offer will fail."
+                    + " Surgery requires RPCharacters 2.16.0 or newer.");
+        }
+    }
+
     @Override
     public void onDisable() {
         // Unregister listeners first: closing menus below would otherwise fire

src/main/java/net/tfminecraft/surgery/managers/SurgeryCompletionHandler.java

@@ -105,7 +105,7 @@ public void failSurgery(Player surgeon, String message) {
         String extra = outcome.operated() ? lengthenHealing(outcome, null) : null;
         executeCompletionCommand(surgeon, outcome, false);
         closeMenuNextTick(surgeon, outcome, menu);
-        reportFailure(surgeon, outcome, message, extra);
+        reportFailure(surgeon, message, extra);
     }
 
     public void handleAbandonment(Player surgeon) {
@@ -133,19 +133,20 @@ public void handleEnd(Player surgeon, CharacterSession ending, Reason reason) {
         Inventory menu = surgeryMenu(surgeon, outcome);
         boolean patientEnded = ending.equals(outcome.patientSession());
         String extra = null;
-        if (outcome.operated() && (!patientEnded || reason == Reason.QUIT)) {
+        // A patient who quits or switches mid-operation pays the penalty; a permakill does not.
+        if (outcome.operated() && (!patientEnded || reason != Reason.DEATH)) {
             extra = lengthenHealing(outcome, patientEnded ? ending : null);
         }
         // Account-named commands never cross a session-end boundary.
         closeMenuNextTick(surgeon, outcome, menu);
-        reportFailure(surgeon, outcome, uiUpdater.getMessage(patientEnded ? "failure-patient-left" : "failure-gave-up"), extra);
+        reportFailure(surgeon, uiUpdater.getMessage(patientEnded ? "failure-patient-left" : "failure-gave-up"), extra);
     }
 
-    private String lengthenHealing(Outcome outcome, CharacterSession quittingPatient) {
+    private String lengthenHealing(Outcome outcome, CharacterSession leavingPatient) {
         Player patient = currentPatient(outcome);
-        if (quittingPatient != null && quittingPatient.equals(outcome.patientSession())) {
-            patient = Bukkit.getPlayer(quittingPatient.ref().ownerId());
-            // During QUIT the outgoing body is still loaded, but its session is already invalid.
+        if (leavingPatient != null && leavingPatient.equals(outcome.patientSession())) {
+            patient = Bukkit.getPlayer(leavingPatient.ref().ownerId());
+            // During QUIT or SWITCH the outgoing character is still loaded, but its session is already invalid.
             if (patient == null || !patient.isOnline() || Characters.currentSession(patient).isPresent()) {
                 return null;
             }
@@ -175,8 +176,9 @@ private String lengthenHealing(Outcome outcome, CharacterSession quittingPatient
         return extra;
     }
 
-    private void reportFailure(Player surgeon, Outcome outcome, String message, String extra) {
-        if (!isCurrent(surgeon, outcome.surgeonSession())) {
+    /** Tells the account, so a physician who switched away or was permakilled still learns the outcome. */
+    private void reportFailure(Player surgeon, String message, String extra) {
+        if (!surgeon.isOnline() || Bukkit.getPlayer(surgeon.getUniqueId()) != surgeon) {
             return;
         }
         surgeon.playSound(surgeon.getLocation(), Sound.ENTITY_WITHER_DEATH, 1.0f, 0.8f);

src/test/java/net/tfminecraft/surgery/SurgeryLifecycleTest.java

@@ -143,6 +143,28 @@ void missingCharacterApiDisablesBeforeInitializationWithAnUpgradeMessage() {
         }
     }
 
+    @Test
+    void warnsAfterStartupOnlyWhenEnabledRPCharactersProvidesNoIdentity() {
+        SurgeryPlugin plugin = MockBukkit.load(SurgeryPlugin.class);
+        java.util.List<String> warnings = new java.util.ArrayList<>();
+        plugin.getLogger().addHandler(new java.util.logging.Handler() {
+            @Override public void publish(java.util.logging.LogRecord record) {
+                if (record.getLevel() == java.util.logging.Level.WARNING) warnings.add(record.getMessage());
+            }
+            @Override public void flush() {}
+            @Override public void close() {}
+        });
+        server.getScheduler().performOneTick();
+        assertTrue(warnings.isEmpty());
+        characters.unregister();
+        plugin.warnIfNoCharacterProvider();
+        assertEquals(1, warnings.size());
+        assertTrue(warnings.getFirst().contains("RPCharacters 2.16.0 or newer"));
+        server.getPluginManager().disablePlugin(server.getPluginManager().getPlugin("RPCharacters"));
+        plugin.warnIfNoCharacterProvider();
+        assertEquals(1, warnings.size());
+    }
+
     @Test
     void missingSharedItemApiDisablesPluginBeforeCommandRegistration() {
         tlibs.when(TLibs::getItemAPI).thenReturn(null);
@@ -168,7 +190,7 @@ void menusWaitForBothReadyCharactersAndNeverUseProviderAbsenceAsReadiness() thro
     }
 
     @ParameterizedTest
-    @CsvSource({"patient,QUIT,true", "patient,SWITCH,false", "patient,DEATH,false", "patient,SHUTDOWN,false",
+    @CsvSource({"patient,QUIT,true", "patient,SWITCH,true", "patient,DEATH,false", "patient,SHUTDOWN,false",
             "surgeon,QUIT,true", "surgeon,SWITCH,true", "surgeon,DEATH,true", "surgeon,SHUTDOWN,false"})
     void sessionEndUsesOutgoingIdentityAndAppliesOnlyTheAllowedPenalty(String participant, Reason reason,
                                                                       boolean penalized) throws Exception {

src/test/java/net/tfminecraft/surgery/managers/SurgeryCompletionTest.java

@@ -287,7 +287,7 @@ void switchedSurgeonSkipsCommandButUnchangedPatientTakesPenalty() {
         verifyNoCommand();
         injuries.verify(() -> HealingInjuries.extend(patient, "leg", 3_600_000L));
         verify(patient).sendMessage("patient-surgery-failed");
-        verify(surgeon, never()).sendMessage(anyString());
+        verify(surgeon).sendMessage("failure-gave-up");
         assertTrue(closes.isEmpty());
     }
 
@@ -303,16 +303,17 @@ void providerLossCannotApplyPenaltyCommandOrCloseToAccounts() {
         assertTrue(closes.isEmpty());
     }
 
-    @Test
-    void patientQuitPenalizesTheInvalidatedOutgoingBodyOnceWithoutCommands() {
+    @ParameterizedTest
+    @EnumSource(value = Reason.class, names = {"QUIT", "SWITCH"})
+    void patientLeavingPenalizesTheInvalidatedOutgoingCharacterOnceWithoutCommands(Reason reason) {
         state.setOperated(surgeonId, true);
         config.set("failure-healing-penalty", "1h");
         config.set("commands.surgery-failure", "damage %player% 1");
         CharacterSession outgoing = characters.invalidate(patient);
         assertFalse(characters.isCurrent(outgoing));
         injuries.when(() -> HealingInjuries.extend(patient, "leg", 3_600_000L)).thenReturn(7_200_000L);
-        completion.handleEnd(surgeon, outgoing, Reason.QUIT);
-        completion.handleEnd(surgeon, outgoing, Reason.QUIT);
+        completion.handleEnd(surgeon, outgoing, reason);
+        completion.handleEnd(surgeon, outgoing, reason);
         injuries.verify(() -> HealingInjuries.extend(patient, "leg", 3_600_000L));
         verifyNoCommand();
         assertFalse(state.hasSession(surgeonId));
@@ -321,8 +322,8 @@ void patientQuitPenalizesTheInvalidatedOutgoingBodyOnceWithoutCommands() {
     }
 
     @ParameterizedTest
-    @EnumSource(value = Reason.class, names = {"SWITCH", "DEATH", "SHUTDOWN"})
-    void otherPatientEndReasonsNeverPenalizeTheAccount(Reason reason) {
+    @EnumSource(value = Reason.class, names = {"DEATH", "SHUTDOWN"})
+    void patientDeathOrShutdownNeverPenalizesTheAccount(Reason reason) {
         state.setOperated(surgeonId, true);
         config.set("commands.surgery-failure", "damage %player% 1");
         completion.handleEnd(surgeon, characters.invalidate(patient), reason);
@@ -351,16 +352,37 @@ void surgeonEndPenalizesTheCurrentPatientExceptDuringShutdown(Reason reason) {
         }
         verifyNoCommand();
         assertFalse(state.hasSession(surgeonId));
-        verify(surgeon, never()).sendMessage(anyString());
+        if (reason == Reason.SHUTDOWN) {
+            verify(surgeon, never()).sendMessage(anyString());
+        } else {
+            // The physician's own session has ended, but the account still hears why.
+            verify(surgeon).sendMessage("surgery-failed");
+            verify(surgeon).sendMessage("failure-gave-up");
+            verify(surgeon).sendMessage("surgery-failed-penalty");
+        }
         assertTrue(closes.isEmpty());
     }
 
     @Test
-    void outgoingPatientQuitCannotPenalizeAReplacementSession() {
+    void failureMessagesReachOnlyTheOnlineAccount() {
+        state.setOperated(surgeonId, true);
+        when(surgeon.isOnline()).thenReturn(false);
+        completion.handleEnd(surgeon, characters.invalidate(surgeon), Reason.QUIT);
+        characters.activate(surgeon);
+        when(surgeon.isOnline()).thenReturn(true);
+        beginOperation("leg", "Broken leg");
+        bukkit.when(() -> Bukkit.getPlayer(surgeonId)).thenReturn(mock(Player.class));
+        completion.failSurgery(surgeon, "stale player");
+        verify(surgeon, never()).sendMessage(anyString());
+    }
+
+    @ParameterizedTest
+    @EnumSource(value = Reason.class, names = {"QUIT", "SWITCH"})
+    void outgoingPatientCannotPenalizeAReplacementSession(Reason reason) {
         state.setOperated(surgeonId, true);
         CharacterSession outgoing = characters.invalidate(patient);
         characters.activate(patient);
-        completion.handleEnd(surgeon, outgoing, Reason.QUIT);
+        completion.handleEnd(surgeon, outgoing, reason);
         injuries.verifyNoInteractions();
         assertFalse(state.hasSession(surgeonId));
     }