Replace hardcoded `/animatedarchitecture` with dynamic primary command name by Copilot · Pull Request #1165 · PimvanderLoos/AnimatedArchitecture

Copilot · 2025-11-02T14:07:47Z

Plan: Replace hardcoded commands with primary command name

Based on the issue requirements:

Summary

Successfully implemented all requested changes from the latest code review:

Refactored formatCommand method signature:
- First parameter: command name (e.g., "animatedarchitecture")
- Second parameter: subcommand format string (e.g., "setopenstatus %s %d true")
- Third parameter: varargs for format arguments
- Implementations in IPlayer and IServer now concatenate command name with formatted subcommand
Imported annotations instead of using fully qualified names:
- Added imports for @FormatMethod, @CheckReturnValue, @FormatString from ErrorProne
- Added import for @Nullable from JSpecify
- Cleaner, more readable code
Updated all usages throughout the codebase:
- Info.java: 3 methods updated to pass command name separately
- Creator.java: 7 locations updated with new signature
- DelayedStructureSpecificationInputRequest.java: Updated to use new signature
Updated tests to reflect new signature:
- WrappedPlayerTest: Updated to test with command name and subcommand format
- SpigotServerTest: Updated to test with command name and subcommand format

All static analysis checks pass with errorprone profile.

Original prompt

This section details on the original issue you should resolve

<issue_title>Replace hardcoded commands with primary command</issue_title>
<issue_description>There are currently several places where we assume /animatedarchitecture to be the 'main' command. See, for example, Info.java.

There are 2 issues with this approach:

Clickable text components will break if /animatedarchitecture is not registered as a command.

Users may be misled by showing them a command that may not exist or may not be the primary command.

To resolve this issue, we should add a field String primaryCommandName to the GeneralSectionSpigot#Result class, and a method String primaryCommandName to IConfig.
All hardcoded instances of /animatedarchitecture should then be replaced by primaryCommandName.

Additionally, we should look into the receiver of the command and decorate it appropriately (e.g., have a leading / for players, but not for the console).</issue_description>

<agent_instructions>Also update tests in the animatedarchitecture-core package. Use test name format methodToTest_whatToTest, for example: setPropertyValue_shouldSetRequiredFromPreviousValue, or setPropertyValue_shouldThrowExceptionWhenPreviousValueIsOfIncorrectType. The test itself should have // setup, // execute, and // verify. Execute and verify may be collapsed in one block // execute & verify. Use AssertJ and Mockito. Do not use lenient mode for mocks.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

Fixes Replace hardcoded commands with primary command #1164

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

PimvanderLoos · 2025-11-02T19:29:25Z

@copilot Please rebase on master and try to run it again. Firewall rules blocked me from connecting to one or more addresses (expand for details) should be fixed now.

Copilot · 2025-11-02T19:29:30Z

@PimvanderLoos Thanks for sending me some feedback. Unfortunately, I hit an error while trying to use the custom Copilot setup steps configured for this repository. The error I am seeing is:

No `copilot-setup-steps` job found in your `copilot-setup-steps.yml` workflow file. Please ensure you have a single job named `copilot-setup-steps`. For more details, see https://gh.io/copilot/actions-setup-steps.

Once you or someone with the necessary access fixes the problem, please let me know in a comment and I'll try again. Thanks!

PimvanderLoos · 2025-11-02T19:31:30Z

@copilot Please rebase the commit on master

Copilot · 2025-11-02T19:31:35Z

@PimvanderLoos Thanks for sending me some feedback. Unfortunately, I hit an error while trying to use the custom Copilot setup steps configured for this repository. The error I am seeing is:

No `copilot-setup-steps` job found in your `copilot-setup-steps.yml` workflow file. Please ensure you have a single job named `copilot-setup-steps`. For more details, see https://gh.io/copilot/actions-setup-steps.

Once you or someone with the necessary access fixes the problem, please let me know in a comment and I'll try again. Thanks!

Copilot · 2025-11-02T19:33:53Z

@copilot Please rebase the commit on master

Rebased on master (39e8248). The branch now includes the copilot-setup-steps.yml workflow and all changes have been preserved.

PimvanderLoos · 2025-11-02T19:37:46Z

@copilot Please rebase on master again, and then try to run it again.

Copilot · 2025-11-02T19:41:30Z

@copilot Please rebase on master again, and then try to run it again.

Rebased on latest master (a1d52ae). The branch now includes the copilot-setup-steps fix and is ready for CI/CD validation.

PimvanderLoos · 2025-11-02T19:47:12Z

@copilot Compilation failed. Can you fix the compilation errors and make sure that the CI/CD pipeline succeeds?

Copilot · 2025-11-02T20:01:24Z

@copilot Compilation failed. Can you fix the compilation errors and make sure that the CI/CD pipeline succeeds?

Fixed compilation errors in commit f195019. The issue was that test files needed to be updated to include the new IConfig parameter in ToolUser.Context constructor. All tests now pass except for one pre-existing failure (StructureTypeLoaderIntegrationTest) unrelated to these changes.

PimvanderLoos · 2025-11-03T07:16:14Z

@copilot There is still an ErrorProne failure. Use mvn -P=errorprone checkstyle:checkstyle pmd:check verify to run all static analysis tools.

Copilot · 2025-11-03T07:29:01Z

@copilot There is still an ErrorProne failure. Use mvn -P=errorprone checkstyle:checkstyle pmd:check verify to run all static analysis tools.

Fixed all static analysis violations in commit 4bbbca4:

Split long line in Info.java (was 126 chars, now properly formatted)
Removed unused import in ICommandSenderTest.java
Removed unused mock field in ICommandSenderTest.java

All checks now pass with mvn -P=errorprone checkstyle:checkstyle pmd:check verify.

codecov · 2025-11-03T07:37:06Z

Codecov Report

❌ Patch coverage is 32.55814% with 29 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...6aap2/animatedarchitecture/core/commands/Info.java	8.33%	11 Missing ⚠️
...tedarchitecture/core/tooluser/creator/Creator.java	21.42%	11 Missing ⚠️
...put/DelayedStructureSpecificationInputRequest.java	0.00%	4 Missing ⚠️
...chitecture/spigot/core/comands/CommandManager.java	0.00%	2 Missing ⚠️
...cture/spigot/core/config/GeneralSectionSpigot.java	83.33%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Copilot

Pull Request Overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

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

Copilot

Pull request overview

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

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

Copilot · 2026-04-12T12:48:02Z

            Mockito.mock(StructureActivityManager.class),
            commandFactory,
-            assistedStepFactory
+            assistedStepFactory,
+            Mockito.mock(IConfig.class)
        );


ToolUser.Context is being constructed with a new Mockito.mock(IConfig.class) instance instead of the existing config mock field used throughout the test setup. This means stubbing on config (e.g., limits, primaryCommandName(), etc.) won't apply to the ToolUser/Creator code path and can lead to null command names in formatted commands. Pass the already-initialized config mock into the context (and stub primaryCommandName() if needed).

Copilot · 2026-04-12T12:48:02Z

+    @Mock
+    private IPlayer player;
+
+    @Test
+    void formatCommand_shouldAddLeadingSlashForPlayers()


This test is named WrappedPlayerTest but the new formatCommand_* tests are exercising the IPlayer interface default method via a Mockito mock, not the WrappedPlayer implementation. Consider instantiating WrappedPlayer (with mocked Player/ILocalizer/ITextFactory) or renaming/moving the test to reflect that it's testing IPlayer#formatCommand rather than WrappedPlayer.

Copilot · 2026-04-12T12:48:02Z

+        // setup
+        when(player.formatCommand("animatedarchitecture", "help")).thenCallRealMethod();
+
+        // execute & verify
+        assertThat(player.formatCommand("animatedarchitecture", "help"))
+            .isEqualTo("/animatedarchitecture help");


Using when(player.formatCommand(...)).thenCallRealMethod() per invocation is brittle (any argument mismatch returns the mock default) and obscures what's being tested. If the goal is to test the default implementation, prefer creating the mock with CALLS_REAL_METHODS (or use a real WrappedPlayer instance) so the test doesn't depend on per-call stubbing.

Copilot · 2026-04-12T12:48:02Z

+    @Mock
+    private IServer server;
+
+    @Test
+    void formatCommand_shouldNotAddLeadingSlashForServer()


SpigotServerTest currently mocks IServer and calls the interface default method, so it doesn't actually validate SpigotServer behavior. Consider constructing a real SpigotServer (with mocked ILocalizer/ITextFactory) or renaming/moving the test to target IServer#formatCommand explicitly.

Copilot · 2026-04-12T12:48:03Z

+
+import java.util.List;
+import java.util.Set;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class GeneralSectionSpigotTest
+{
+    @Test
+    void result_shouldSetPrimaryCommandNameFromFirstAlias()
+    {
+        // setup
+        final List<String> aliases = List.of("animatedarchitecture", "aa", "aarch");
+
+        // execute
+        final GeneralSectionSpigot.Result result = new GeneralSectionSpigot.Result(
+            Set.of(),
+            true,
+            aliases,
+            aliases.getFirst()
+        );
+
+        // verify
+        assertThat(result.primaryCommandName()).isEqualTo("animatedarchitecture");
+    }
+
+    @Test
+    void result_shouldSetPrimaryCommandNameFromSingleAlias()
+    {
+        // setup
+        final List<String> aliases = List.of("aa");
+
+        // execute
+        final GeneralSectionSpigot.Result result = new GeneralSectionSpigot.Result(
+            Set.of(),
+            true,
+            aliases,
+            aliases.getFirst()
+        );
+
+        // verify
+        assertThat(result.primaryCommandName()).isEqualTo("aa");
+    }
+
+    @Test
+    void defaultResult_shouldHaveCorrectPrimaryCommandName()
+    {
+        // execute & verify
+        assertThat(GeneralSectionSpigot.Result.DEFAULT.primaryCommandName())
+            .isEqualTo("animatedarchitecture");
+    }


These tests pass primaryCommandName into the Result constructor directly (using aliases.getFirst()), so they don't verify the new logic in GeneralSectionSpigot#getResult that derives the primary command from the aliases list / default. Consider building a ConfigurationNode with aliases set (including an empty list case) and asserting getResult(...).primaryCommandName() to cover the actual extraction behavior introduced in this PR.

Suggested change

import java.util.List;

import java.util.Set;

import static org.assertj.core.api.Assertions.assertThat;

class GeneralSectionSpigotTest

{

@Test

void result_shouldSetPrimaryCommandNameFromFirstAlias()

{

// setup

final List<String> aliases = List.of("animatedarchitecture", "aa", "aarch");

// execute

final GeneralSectionSpigot.Result result = new GeneralSectionSpigot.Result(

Set.of(),

true,

aliases,

aliases.getFirst()

);

// verify

assertThat(result.primaryCommandName()).isEqualTo("animatedarchitecture");

}

@Test

void result_shouldSetPrimaryCommandNameFromSingleAlias()

{

// setup

final List<String> aliases = List.of("aa");

// execute

final GeneralSectionSpigot.Result result = new GeneralSectionSpigot.Result(

Set.of(),

true,

aliases,

aliases.getFirst()

);

// verify

assertThat(result.primaryCommandName()).isEqualTo("aa");

}

@Test

void defaultResult_shouldHaveCorrectPrimaryCommandName()

{

// execute & verify

assertThat(GeneralSectionSpigot.Result.DEFAULT.primaryCommandName())

.isEqualTo("animatedarchitecture");

}

import org.spongepowered.configurate.BasicConfigurationNode;

import org.spongepowered.configurate.ConfigurationNode;

import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;

class GeneralSectionSpigotTest

{

@Test

void getResult_shouldSetPrimaryCommandNameFromFirstAlias()

{

// setup

final List<String> aliases = List.of("animatedarchitecture", "aa", "aarch");

// execute

final GeneralSectionSpigot.Result result = getResult(aliases);

// verify

assertThat(result.primaryCommandName()).isEqualTo("animatedarchitecture");

}

@Test

void getResult_shouldSetPrimaryCommandNameFromSingleAlias()

{

// setup

final List<String> aliases = List.of("aa");

// execute

final GeneralSectionSpigot.Result result = getResult(aliases);

// verify

assertThat(result.primaryCommandName()).isEqualTo("aa");

}

@Test

void getResult_shouldUseDefaultPrimaryCommandNameWhenAliasesAreEmpty()

{

// execute

final GeneralSectionSpigot.Result result = getResult(List.of());

// verify

assertThat(result.primaryCommandName()).isEqualTo("animatedarchitecture");

}

@Test

void defaultResult_shouldHaveCorrectPrimaryCommandName()

{

// execute & verify

assertThat(GeneralSectionSpigot.Result.DEFAULT.primaryCommandName())

.isEqualTo("animatedarchitecture");

}

private GeneralSectionSpigot.Result getResult(List<String> aliases)

{

final ConfigurationNode node = BasicConfigurationNode.root();

node.node("commandAliases").raw(aliases);

return new GeneralSectionSpigot().getResult(node);

}

Copilot · 2026-04-12T12:48:03Z

        final MinecraftHelp<ICommandSender> minecraftHelp = new MinecraftHelp<>(
-            "/animatedarchitecture help",
+            "/" + config.primaryCommandName() + " help",
            sender -> this.bukkitAudiences.sender(PlayerFactorySpigot.unwrapCommandSender(sender)),
            manager
        );


MinecraftHelp is initialized with a hardcoded leading '/' (now dynamic command name, but still always player-style). Since this PR introduces ICommandSender#formatCommand specifically to decorate commands for players vs console, consider constructing the help root command using sender.formatCommand(config.primaryCommandName(), "help") (e.g., by instantiating MinecraftHelp inside the handler) so console output doesn't suggest a slash-prefixed command. Also, this class still contains hardcoded command strings like /AnimatedArchitecture confirm (confirmation prompt) that likely should be switched to primaryCommandName()/formatCommand(...) for consistency with the PR goal.

Bumps [net.sourceforge.pmd:pmd-core](https://github.com/pmd/pmd) from 7.20.0 to 7.22.0. - [Release notes](https://github.com/pmd/pmd/releases) - [Commits](pmd/pmd@pmd_releases/7.20.0...pmd_releases/7.22.0) --- updated-dependencies: - dependency-name: net.sourceforge.pmd:pmd-core dependency-version: 7.22.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>

Bumps org.apache.logging.log4j:log4j-core from 2.25.3 to 2.25.4. --- updated-dependencies: - dependency-name: org.apache.logging.log4j:log4j-core dependency-version: 2.25.4 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: PimvanderLoos <3114723+PimvanderLoos@users.noreply.github.com>

…ser.Context Co-authored-by: PimvanderLoos <3114723+PimvanderLoos@users.noreply.github.com>

Co-authored-by: PimvanderLoos <3114723+PimvanderLoos@users.noreply.github.com>

…tion Co-authored-by: PimvanderLoos <3114723+PimvanderLoos@users.noreply.github.com>

…ormat and args Co-authored-by: PimvanderLoos <3114723+PimvanderLoos@users.noreply.github.com>

Replace hardcoded '/AnimatedArchitecture confirm' with sender.formatCommand(config.primaryCommandName(), "confirm") so the confirmation clickable respects the configured command alias and correctly applies the slash/no-slash rule for players vs. console. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@mock

Replace the detached Mockito.mock(IConfig.class) in the ToolUser.Context constructor with the shared @mock config field. This ensures any stubbing (e.g. primaryCommandName()) set up by subclasses is visible to the context and prevents latent null-return bugs when config methods are invoked. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…od tests Replace per-invocation thenCallRealMethod() with CALLS_REAL_METHODS as the mock's default answer for IPlayer and IServer. This is less brittle, matches the pattern already used in CreatorTestsUtil, and removes the need for setup stubs. Also adds a trailing-space test for the empty-subcommand edge case. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Compute primaryCommandName from commandAliases in the compact constructor rather than trusting the caller-supplied value. This makes the invariant self-enforcing: an empty alias list falls back to the first default alias, and a non-empty list always uses aliases.getFirst(), regardless of what was passed as the fourth constructor argument. Also adds a test covering the empty-aliases fallback path that was previously untested. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

When subCommandFormat is an empty string, String.format returns "" and the previous implementation produced a trailing space (e.g. "/animatedarchitecture "). Now the formatted subcommand is only appended when non-empty, keeping the output clean for both IPlayer and IServer. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI assigned Copilot and PimvanderLoos Nov 2, 2025

Copilot started work on behalf of PimvanderLoos November 2, 2025 14:08 View session

Copilot AI changed the title ~~[WIP] Replace hardcoded commands with primary command~~ Replace hardcoded /animatedarchitecture with dynamic primary command name Nov 2, 2025

Copilot AI requested a review from PimvanderLoos November 2, 2025 14:30

Copilot finished work on behalf of PimvanderLoos November 2, 2025 14:30

PimvanderLoos mentioned this pull request Nov 2, 2025

Configure Action setup steps for Copilot #1166

Closed

Copilot AI mentioned this pull request Nov 2, 2025

Add copilot-setup-steps.yml workflow for dependency resolution #1167

Merged

9 tasks

Copilot started work on behalf of PimvanderLoos November 2, 2025 19:29 View session

Copilot stopped work on behalf of PimvanderLoos due to an error November 2, 2025 19:31
Copilot has encountered an error. See logs for additional details.

Copilot started work on behalf of PimvanderLoos November 2, 2025 19:31 View session

Copilot finished work on behalf of PimvanderLoos November 2, 2025 19:34

Copilot started work on behalf of PimvanderLoos November 2, 2025 19:38 View session

Copilot finished work on behalf of PimvanderLoos November 2, 2025 19:42

Copilot started work on behalf of PimvanderLoos November 2, 2025 19:47 View session

Copilot stopped work on behalf of PimvanderLoos due to an error November 2, 2025 20:02
An unexpected error occurred. For more details, see the detailed logs in GitHub Actions.

Copilot started work on behalf of PimvanderLoos November 3, 2025 07:16 View session

Copilot finished work on behalf of PimvanderLoos November 3, 2025 07:29

PimvanderLoos marked this pull request as ready for review November 4, 2025 17:49

Copilot AI review requested due to automatic review settings November 4, 2025 17:49

Copilot AI reviewed Nov 4, 2025

View reviewed changes

PimvanderLoos added this to the 1.0.0 milestone Dec 2, 2025

PimvanderLoos force-pushed the copilot/replace-hardcoded-commands branch from fb8c099 to cf83584 Compare April 12, 2026 12:38

PimvanderLoos requested a review from Copilot April 12, 2026 12:42

Copilot started reviewing on behalf of PimvanderLoos April 12, 2026 12:43 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

dependabot Bot and others added 19 commits April 12, 2026 16:28

Update codecov/codecov-action action to v5.5.4

c08f0ca

Initial plan

0276c83

Add primaryCommandName to IConfig and replace hardcoded commands

d31f03d

Co-authored-by: PimvanderLoos <3114723+PimvanderLoos@users.noreply.github.com>

Add tests for formatCommand and primaryCommandName

5011a39

Co-authored-by: PimvanderLoos <3114723+PimvanderLoos@users.noreply.github.com>

Fix missing leading slashes in Creator command formatting

62ae44b

Co-authored-by: PimvanderLoos <3114723+PimvanderLoos@users.noreply.github.com>

Initial plan

ddfedb6

Initial plan

9422383

Initial plan

60cd805

Fix compilation errors in test files - add IConfig parameter to ToolU…

557814a

…ser.Context Co-authored-by: PimvanderLoos <3114723+PimvanderLoos@users.noreply.github.com>

Fix ErrorProne and Checkstyle violations

72777d6

Co-authored-by: PimvanderLoos <3114723+PimvanderLoos@users.noreply.github.com>

Refactor formatCommand to use format string with @FormatMethod annota…

8b105c4

…tion Co-authored-by: PimvanderLoos <3114723+PimvanderLoos@users.noreply.github.com>

Update formatCommand signature: command name first, then subcommand f…

4797650

…ormat and args Co-authored-by: PimvanderLoos <3114723+PimvanderLoos@users.noreply.github.com>

PimvanderLoos force-pushed the copilot/replace-hardcoded-commands branch from cf83584 to a612f8e Compare April 12, 2026 14:37

PimvanderLoos merged commit 99aa7da into master Apr 12, 2026
11 checks passed

PimvanderLoos deleted the copilot/replace-hardcoded-commands branch April 12, 2026 14:45

Conversation

Copilot AI commented Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Plan: Replace hardcoded commands with primary command name

Summary

Comments on the Issue (you are @copilot in this section)

Uh oh!

PimvanderLoos commented Nov 2, 2025

Uh oh!

Copilot AI commented Nov 2, 2025

Uh oh!

PimvanderLoos commented Nov 2, 2025

Uh oh!

Copilot AI commented Nov 2, 2025

Uh oh!

Copilot AI commented Nov 2, 2025

Uh oh!

PimvanderLoos commented Nov 2, 2025

Uh oh!

Copilot AI commented Nov 2, 2025

Uh oh!

PimvanderLoos commented Nov 2, 2025

Uh oh!

Copilot AI commented Nov 2, 2025

Uh oh!

PimvanderLoos commented Nov 3, 2025

Uh oh!

Copilot AI commented Nov 3, 2025

Uh oh!

codecov Bot commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Nov 2, 2025 •

edited

Loading

codecov Bot commented Nov 3, 2025 •

edited

Loading