Prompt to confirm (or -f to force) moving a player already in a faction via addmember by Copilot · Pull Request #1919 · Dans-Plugins/Medieval-Factions

Copilot · 2025-12-13T05:26:54Z

Summary

Changes /f addmember behavior when the target player is already in another faction. Previously the command simply refused. Now:

Default: the admin is shown a clickable confirmation prompt; confirming removes the player from their current faction and adds them to the target.
-f flag (requires mf.force.addmember / mf.force.join): skips the prompt and moves the player immediately. Both old and new factions are notified.

Lang strings for the prompt/notifications added across all five locales.

⚠️ Reviewer note — behavior & data change

This introduces a new faction-membership mutation: addmember can now remove a player from their existing faction (on confirm or -f), where before it only refused. Please confirm: (1) removing the member via currentFaction.copy(members = …filter…) cleanly handles the player's role/standing in the old faction, and (2) the confirmation-prompt command path is safe (it re-runs addmember … -f from a clickable component). It mutates membership only — no claims/power/faction records are deleted.

Dev-loop grooming

Rebased on main (was 226 behind) — resolved 3 additive lang conflicts (de/fr/pt), no key loss.
Fixed the build: the original failure was a lintKotlinMain error (unused dropFirst import after the command switched to stdlib args.drop(1)), not a compile error. Removed it; ./gradlew clean build is now green (container + CI build/docker-build on a9467a18).
Documented the -f flag / confirmation behavior in COMMANDS.md.
mf.force.addmember / mf.force.join are already registered in plugin.yml.

Known gap

No tests for the new confirmation/force/move logic. Sibling command tests (e.g. MfFactionDenyAppCommandTest) exist to mirror; recommend adding coverage (guard branches, -f removes-then-adds, confirmation path) before merge.

Data-safety statement

No schema migrations; no DPC contract change; no config.yml default change. Lang changes additive. Behavior change to membership handling flagged above for review.

Groomed by Claude on behalf of Daniel Stephenson.

…in faction Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Copilot

Pull request overview

This PR adds confirmation prompts when using the addmember command to add a player who is already in another faction. Previously, the command would reject such attempts. Now it prompts the admin for confirmation and provides a force flag (-f) to complete the operation.

Changes:

Added force flag (-f) handling to allow bypassing the confirmation prompt and forcefully moving players between factions
Implemented confirmAddMember() method that displays interactive buttons for confirming or canceling the operation
Added 8 new localization keys for confirmation messages, though translations are only complete for English variants

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/main/kotlin/com/dansplugins/factionsystem/command/faction/addmember/MfFactionAddMemberCommand.kt	Added force flag handling, confirmation flow, and logic to remove players from their current faction before adding to target faction
src/main/resources/lang/lang_en_US.properties	Added 8 English (US) localization keys for the confirmation feature
src/main/resources/lang/lang_en_GB.properties	Added 8 English (GB) localization keys for the confirmation feature
src/main/resources/lang/lang_fr_FR.properties	Added 8 localization key entries, but content is in English instead of French
src/main/resources/lang/lang_pt_BR.properties	Added 8 localization key entries, but content is in English instead of Portuguese
src/main/resources/lang/lang_de_DE.properties	Added 8 localization key entries, but content is in English instead of German

Comments suppressed due to low confidence (1)

src/main/kotlin/com/dansplugins/factionsystem/command/faction/addmember/MfFactionAddMemberCommand.kt:110

The target faction member limit check occurs after attempting to remove the player from their current faction. This creates a scenario where a player could be removed from their current faction, but then the add operation fails due to the target faction being full, leaving the player in no faction at all. The member limit check should be performed before removing the player from their current faction to ensure the operation can complete successfully. Move lines 106-110 to before line 86.

        val maxMembers = plugin.config.getInt("factions.maxMembers")
        if (maxMembers > 0 && targetFaction.members.size >= maxMembers) {
            sender.sendMessage("${ChatColor.RED}${plugin.language["CommandFactionAddMemberTargetFactionFull"]}")
            return true
        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-01T12:17:34Z

+CommandFactionAddMemberConfirmRemoval={0} is currently in {1}. Do you want to remove them and add them to {2}?
+CommandFactionAddMemberConfirmButton=[Confirm]
+CommandFactionAddMemberConfirmButtonHover=Click to confirm and remove the player from their current faction
+CommandFactionAddMemberCancelButton=[Cancel]
+CommandFactionAddMemberCancelButtonHover=Click to cancel
+CommandFactionAddMemberFailedToRemoveFromCurrentFaction=Failed to remove player from their current faction.
+CommandFactionAddMemberRemovedFromFactionTitle={0} was removed from the faction
+CommandFactionAddMemberRemovedFromFactionBody={0} was forcefully removed from the faction.


The new language keys added for confirmation messages are in English only, while the other language files (French, Portuguese BR, German) should have these translated. Currently, lines 1064-1071 in lang_fr_FR.properties, lang_pt_BR.properties, and lang_de_DE.properties contain English text instead of French, Portuguese, and German translations respectively.

Suggested change

CommandFactionAddMemberConfirmRemoval={0} is currently in {1}. Do you want to remove them and add them to {2}?

CommandFactionAddMemberConfirmButton=[Confirm]

CommandFactionAddMemberConfirmButtonHover=Click to confirm and remove the player from their current faction

CommandFactionAddMemberCancelButton=[Cancel]

CommandFactionAddMemberCancelButtonHover=Click to cancel

CommandFactionAddMemberFailedToRemoveFromCurrentFaction=Failed to remove player from their current faction.

CommandFactionAddMemberRemovedFromFactionTitle={0} was removed from the faction

CommandFactionAddMemberRemovedFromFactionBody={0} was forcefully removed from the faction.

CommandFactionAddMemberConfirmRemoval={0} est actuellement dans {1}. Voulez-vous le retirer de {1} et l'ajouter � {2} ?

CommandFactionAddMemberConfirmButton=[Confirmer]

CommandFactionAddMemberConfirmButtonHover=Cliquez pour confirmer et retirer le joueur de sa faction actuelle

CommandFactionAddMemberCancelButton=[Annuler]

CommandFactionAddMemberCancelButtonHover=Cliquez pour annuler

CommandFactionAddMemberFailedToRemoveFromCurrentFaction=�chec de la suppression du joueur de sa faction actuelle.

CommandFactionAddMemberRemovedFromFactionTitle={0} a �t� retir� de la faction

CommandFactionAddMemberRemovedFromFactionBody={0} a �t� retir� de force de la faction.

French translations added in commit b32a3b0.

Copilot · 2026-02-01T12:17:35Z

+CommandFactionAddMemberConfirmRemoval={0} is currently in {1}. Do you want to remove them and add them to {2}?
+CommandFactionAddMemberConfirmButton=[Confirm]
+CommandFactionAddMemberConfirmButtonHover=Click to confirm and remove the player from their current faction
+CommandFactionAddMemberCancelButton=[Cancel]
+CommandFactionAddMemberCancelButtonHover=Click to cancel
+CommandFactionAddMemberFailedToRemoveFromCurrentFaction=Failed to remove player from their current faction.
+CommandFactionAddMemberRemovedFromFactionTitle={0} was removed from the faction
+CommandFactionAddMemberRemovedFromFactionBody={0} was forcefully removed from the faction.


The new language keys added for confirmation messages are in English only and should be translated to Portuguese (BR). Currently, lines 1064-1071 contain English text instead of Portuguese translations.

Suggested change

CommandFactionAddMemberConfirmRemoval={0} is currently in {1}. Do you want to remove them and add them to {2}?

CommandFactionAddMemberConfirmButton=[Confirm]

CommandFactionAddMemberConfirmButtonHover=Click to confirm and remove the player from their current faction

CommandFactionAddMemberCancelButton=[Cancel]

CommandFactionAddMemberCancelButtonHover=Click to cancel

CommandFactionAddMemberFailedToRemoveFromCurrentFaction=Failed to remove player from their current faction.

CommandFactionAddMemberRemovedFromFactionTitle={0} was removed from the faction

CommandFactionAddMemberRemovedFromFactionBody={0} was forcefully removed from the faction.

CommandFactionAddMemberConfirmRemoval={0} atualmente está em {1}. Deseja removê-lo(a) e adicioná-lo(a) a {2}?

CommandFactionAddMemberConfirmButton=[Confirmar]

CommandFactionAddMemberConfirmButtonHover=Clique para confirmar e remover o jogador da facção atual

CommandFactionAddMemberCancelButton=[Cancelar]

CommandFactionAddMemberCancelButtonHover=Clique para cancelar

CommandFactionAddMemberFailedToRemoveFromCurrentFaction=Falha ao remover o jogador da facção atual.

CommandFactionAddMemberRemovedFromFactionTitle={0} foi removido da facção

CommandFactionAddMemberRemovedFromFactionBody={0} foi removido(a) à força da facção.

Portuguese (BR) translations added in commit b32a3b0.

Copilot · 2026-02-01T12:17:35Z

+CommandFactionAddMemberConfirmRemoval={0} is currently in {1}. Do you want to remove them and add them to {2}?
+CommandFactionAddMemberConfirmButton=[Confirm]
+CommandFactionAddMemberConfirmButtonHover=Click to confirm and remove the player from their current faction
+CommandFactionAddMemberCancelButton=[Cancel]
+CommandFactionAddMemberCancelButtonHover=Click to cancel
+CommandFactionAddMemberFailedToRemoveFromCurrentFaction=Failed to remove player from their current faction.
+CommandFactionAddMemberRemovedFromFactionTitle={0} was removed from the faction
+CommandFactionAddMemberRemovedFromFactionBody={0} was forcefully removed from the faction.


The new language keys added for confirmation messages are in English only and should be translated to German. Currently, lines 1064-1071 contain English text instead of German translations.

Suggested change

CommandFactionAddMemberConfirmRemoval={0} is currently in {1}. Do you want to remove them and add them to {2}?

CommandFactionAddMemberConfirmButton=[Confirm]

CommandFactionAddMemberConfirmButtonHover=Click to confirm and remove the player from their current faction

CommandFactionAddMemberCancelButton=[Cancel]

CommandFactionAddMemberCancelButtonHover=Click to cancel

CommandFactionAddMemberFailedToRemoveFromCurrentFaction=Failed to remove player from their current faction.

CommandFactionAddMemberRemovedFromFactionTitle={0} was removed from the faction

CommandFactionAddMemberRemovedFromFactionBody={0} was forcefully removed from the faction.

CommandFactionAddMemberConfirmRemoval={0} ist derzeit Mitglied von {1}. M�chtest du ihn aus dieser Fraktion entfernen und zu {2} hinzuf�gen?

CommandFactionAddMemberConfirmButton=[Best�tigen]

CommandFactionAddMemberConfirmButtonHover=Klicken, um zu best�tigen und den Spieler aus seiner aktuellen Fraktion zu entfernen

CommandFactionAddMemberCancelButton=[Abbrechen]

CommandFactionAddMemberCancelButtonHover=Klicken, um abzubrechen

CommandFactionAddMemberFailedToRemoveFromCurrentFaction=Spieler konnte nicht aus seiner aktuellen Fraktion entfernt werden.

CommandFactionAddMemberRemovedFromFactionTitle={0} wurde aus der Fraktion entfernt

CommandFactionAddMemberRemovedFromFactionBody={0} wurde zwangsweise aus der Fraktion entfernt.

German translations added in commit b32a3b0.

Copilot · 2026-02-01T12:17:35Z

+        val force = if (args.lastOrNull() == "-f") {
+            lastArgOffset = 1
+            true
+        } else {
+            false
+        }


The force flag is accepted without validating that the sender has the appropriate permission. In MfFactionJoinCommand.kt (lines 31-38), the pattern is to check hasUninvitedJoinPermission before accepting the force flag. Similarly, here the force flag should only be accepted if the sender has the required permission, otherwise users could attempt to use the flag inappropriately. The force flag check should be: val force = if (sender.hasPermission("mf.force.addmember") || sender.hasPermission("mf.force.join")) && args.lastOrNull() == "-f")

Permission check added for force flag following the MfFactionJoinCommand pattern in commit b32a3b0. Now only users with mf.force.addmember or mf.force.join permission can use the -f flag.

Copilot · 2026-02-01T12:17:36Z

+                updatedCurrentFaction.sendMessage(
+                    plugin.language["CommandFactionAddMemberRemovedFromFactionTitle", targetName],
+                    plugin.language["CommandFactionAddMemberRemovedFromFactionBody", targetName]
+                )


When a player is forcibly removed from their current faction before being added to a new faction, the target player should be notified about this removal. The current implementation only notifies the old faction members but doesn't send a message to the target player themselves. Consider sending a direct message to the target player (if online) informing them that they have been moved from one faction to another.

Suggested change

)

)

// Notify target player (if online) about being moved from one faction to another

val targetPlayer = plugin.server.getPlayer(targetMfPlayer.id)

if (targetPlayer != null) {

targetPlayer.sendMessage(

"${ChatColor.YELLOW}" +

"You have been moved from faction ${currentFaction.name} to faction ${targetFaction.name}."

)

}

Target player notification added in commit b32a3b0. If the target player is online, they now receive a message: "You have been moved from faction {old} to faction {new}." with localized versions in all supported languages.

Copilot · 2026-02-01T12:17:36Z

+                    return true
+                }
+                // Notify old faction
+                var targetName = targetMfPlayer.name ?: plugin.language["CommandFactionAddMemberUnknownNewPlayerFaction"]


The variable targetName is declared as var but should be declared as val since it's never reassigned. On line 98, you retrieve the name or set a default, but you don't reassign it later. Using val instead of var is more idiomatic in Kotlin for immutable references.

Suggested change

var targetName = targetMfPlayer.name ?: plugin.language["CommandFactionAddMemberUnknownNewPlayerFaction"]

val targetName = targetMfPlayer.name ?: plugin.language["CommandFactionAddMemberUnknownNewPlayerFaction"]

Changed to val in commit b32a3b0.

dmccoystephenson · 2026-02-01T12:23:17Z

@copilot address comments

…ication, and code improvements Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

…emove-faction # Conflicts: # src/main/resources/lang/lang_de_DE.properties # src/main/resources/lang/lang_fr_FR.properties # src/main/resources/lang/lang_pt_BR.properties

The command now uses stdlib args.drop(1); the dropFirst extension import is unused and failed lintKotlinMain. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

dmccoystephenson

Self-review rubric (grooming pass — adversarial):

Scope: PASS — MfFactionAddMemberCommand + 5 lang files + the grooming COMMANDS.md doc. No unrelated churn.
Tests-new: FAIL (flagged) — no test covers the new confirmation/-f/move logic. Not auto-fixed: this is a human-gated behavior change (membership mutation), so coverage should be added once the design below is confirmed.
Sibling structure: PASS — command follows existing addmember/command conventions.
Lang-externalized: PASS — 9 new keys present in all 5 locales (rebase kept main's keys, Latin-1 preserved, no loss).
Docs: PASS (fixed in groom) — COMMANDS.md now documents the -f flag and confirmation behavior.
Plugin.yml: PASS — mf.force.addmember / mf.force.join already registered.
Migrations-additive / DPC-contract / Config-defaults: PASS (vacuous) — none touched.
Main-thread-safety: PASS (no signal) — follows the existing async-save pattern in the command.
CI: PASS — build + docker-build green on a9467a18; build-verified in a clean container (the original red was a ktlint unused-import, now removed).

Lead finding (behavior/data — reviewer decision): addmember now performs a membership mutation it never did before — removing the target from their current faction (on confirm or -f). Confirm the old-faction cleanup (role/standing) is correct and that the clickable-confirmation re-invocation path is safe. No records are deleted; membership only.

Rebased, build fixed, documented, marked ready. Left un-merged pending the behavior review + test coverage.

Self-review by Claude on behalf of Daniel Stephenson.

Initial plan

b8c541d

Copilot AI assigned Copilot and dmccoystephenson Dec 13, 2025

Copilot started work on behalf of dmccoystephenson December 13, 2025 05:27 View session

Copilot AI and others added 2 commits December 13, 2025 05:34

Add confirmation prompt for addmember command when player is already …

b0e1e83

…in faction Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Improve parameter naming for clarity in confirmAddMember method

fde889b

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Prompt user about faction removal on addmember command~~ Prompt for confirmation when addmember targets player already in faction Dec 13, 2025

Copilot AI requested a review from dmccoystephenson December 13, 2025 05:38

Copilot finished work on behalf of dmccoystephenson December 13, 2025 05:38

dmccoystephenson requested a review from Copilot February 1, 2026 12:11

Copilot started reviewing on behalf of dmccoystephenson February 1, 2026 12:11 View session

Copilot AI reviewed Feb 1, 2026

View reviewed changes

Copilot started work on behalf of dmccoystephenson February 1, 2026 12:23 View session

Address PR feedback: add translations, permission check, player notif…

b32a3b0

…ication, and code improvements Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Copilot finished work on behalf of dmccoystephenson February 1, 2026 12:26

dmccoystephenson changed the base branch from develop to main April 25, 2026 21:50

BuildTools and others added 3 commits June 6, 2026 18:15

Merge remote-tracking branch 'origin/main' into copilot/prompt-user-r…

5b3f2f7

…emove-faction # Conflicts: # src/main/resources/lang/lang_de_DE.properties # src/main/resources/lang/lang_fr_FR.properties # src/main/resources/lang/lang_pt_BR.properties

Groom #1919: remove unused dropFirst import (ktlint)

2a2e635

The command now uses stdlib args.drop(1); the dropFirst extension import is unused and failed lintKotlinMain. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Groom #1919: document addmember -f flag and confirmation in COMMANDS.md

a9467a1

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

dmccoystephenson marked this pull request as ready for review June 7, 2026 00:34

dmccoystephenson changed the title ~~Prompt for confirmation when addmember targets player already in faction~~ Prompt to confirm (or -f to force) moving a player already in a faction via addmember Jun 7, 2026

dmccoystephenson reviewed Jun 7, 2026

View reviewed changes

dmccoystephenson mentioned this pull request Jun 8, 2026

Add configurable moderator approval for war/ally/vassalize declarations #1954

Open

4 tasks

-                )
+                )
+                // Notify target player (if online) about being moved from one faction to another
+                val targetPlayer = plugin.server.getPlayer(targetMfPlayer.id)
+                if (targetPlayer != null) {
+                    targetPlayer.sendMessage(
+                        "${ChatColor.YELLOW}" +
+                            "You have been moved from faction ${currentFaction.name} to faction ${targetFaction.name}."
+                    )
+                }

	var targetName = targetMfPlayer.name ?: plugin.language["CommandFactionAddMemberUnknownNewPlayerFaction"]
	val targetName = targetMfPlayer.name ?: plugin.language["CommandFactionAddMemberUnknownNewPlayerFaction"]

Conversation

Copilot AI commented Dec 13, 2025 • edited by dmccoystephenson Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

⚠️ Reviewer note — behavior & data change

Dev-loop grooming

Known gap

Data-safety statement

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

dmccoystephenson commented Feb 1, 2026

Uh oh!

dmccoystephenson left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Dec 13, 2025 •

edited by dmccoystephenson

Loading