fix(13-08): ship a default sidebar that renders without extra plugins

src/main/java/com/ultikits/plugins/sidebar/config/SideBarConfig.java

@@ -1,5 +1,6 @@
 package com.ultikits.plugins.sidebar.config;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -44,13 +45,13 @@ public class SideBarConfig extends AbstractConfigEntity {
         "&7欢迎, &f%player_name%",
         "",
         "&e在线人数: &f%server_online%/%server_max_players%",
-        "&e世界: &f%world_name%",
+        "&e世界: &f%player_world%",
         "",
         "&e金币: &f%vault_eco_balance_formatted%",
         "&ePing: &f%player_ping%ms",
         "",
         "&7服务器时间",
-        "&f%server_time_hh:mm:ss%",
+        "&f%server_time_HH:mm:ss%",
         "",
         "&6play.example.com"
     );
@@ -64,4 +65,54 @@ public class SideBarConfig extends AbstractConfigEntity {
     public SideBarConfig() {
         super("config/sidebar.yml");
     }
+
+    /**
+     * The pre-6.3.0 shipped default world-name line, which used the invalid PlaceholderAPI
+     * syntax {@code %world_name%} (UltiKits/UltiSideBar#13 -- the real "World" expansion
+     * placeholder, {@code %world_name_<world>%}, requires an explicit world argument).
+     * {@code AbstractConfigEntity.init()} never overwrites a key that already exists on disk,
+     * so any server that has ever started this plugin keeps this exact string in its persisted
+     * {@code sidebar.yml} forever unless it is rewritten explicitly.
+     */
+    private static final String LEGACY_WORLD_NAME_LINE = "&e世界: &f%world_name%";
+
+    /**
+     * The corrected default that replaces {@link #LEGACY_WORLD_NAME_LINE}, kept in sync by hand
+     * with the "lines" default above.
+     */
+    private static final String CURRENT_WORLD_NAME_LINE = "&e世界: &f%player_world%";
+
+    /**
+     * One-time migration for a persisted {@code sidebar.yml} whose {@code lines} list still
+     * carries the old, invalid {@link #LEGACY_WORLD_NAME_LINE} default (issue #13). Rewrites
+     * only a list entry that is byte-identical to that old default -- any operator
+     * customisation, including a line that merely mentions {@code %world_name%} alongside other
+     * text, is left untouched. Idempotent: once migrated, no entry matches
+     * {@link #LEGACY_WORLD_NAME_LINE} any more, so a second call is a no-op.
+     * <p>
+     * Must be called after {@code init(UltiToolsPlugin)} has populated {@link #lines} from
+     * disk. The caller is responsible for persisting the result with {@code save()} when this
+     * method returns {@code true} -- this method only updates the in-memory value.
+     *
+     * @return {@code true} if at least one line was rewritten, {@code false} otherwise
+     */
+    public boolean migrateLegacyWorldNameDefaultLine() {
+        if (lines == null) {
+            return false;
+        }
+        boolean changed = false;
+        List<String> migrated = new ArrayList<>(lines.size());
+        for (String line : lines) {
+            if (LEGACY_WORLD_NAME_LINE.equals(line)) {
+                migrated.add(CURRENT_WORLD_NAME_LINE);
+                changed = true;
+            } else {
+                migrated.add(line);
+            }
+        }
+        if (changed) {
+            lines = migrated;
+        }
+        return changed;
+    }
 }

src/main/java/com/ultikits/plugins/sidebar/service/SideBarService.java

@@ -15,6 +15,7 @@
 import org.bukkit.scoreboard.*;
 import org.bukkit.scheduler.BukkitTask;
 
+import java.io.IOException;
 import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
 
@@ -59,6 +60,22 @@ public void init() {
         dataOperator = plugin.getDataOperator(SideBarPreference.class);
         bukkitPlugin = Bukkit.getPluginManager().getPlugin("UltiTools");
 
+        // One-time migration (issue #13, CR-01): AbstractConfigEntity.init() -- which has
+        // already run by this point, via UltiToolsPlugin's constructor -- only fills keys that
+        // are MISSING from the persisted file and never overwrites an existing "lines" value,
+        // so a server that has ever started an older version of this plugin keeps the invalid
+        // %world_name% default forever without this explicit, exact-match rewrite. Runs again
+        // on every reload() (this method is also called from reload()), which is harmless: once
+        // migrated, the exact-match check finds nothing left to rewrite.
+        if (config.migrateLegacyWorldNameDefaultLine()) {
+            try {
+                config.save();
+            } catch (IOException e) {
+                plugin.getLogger().warn("Failed to persist the sidebar.yml %world_name% "
+                        + "placeholder migration: " + e.getMessage());
+            }
+        }
+
         // Check PlaceholderAPI
         placeholderApiAvailable = Bukkit.getPluginManager().getPlugin("PlaceholderAPI") != null;
         if (!placeholderApiAvailable) {

src/test/java/com/ultikits/plugins/sidebar/config/SideBarConfigTest.java

@@ -1,11 +1,32 @@
 package com.ultikits.plugins.sidebar.config;
 
+import com.ultikits.plugins.sidebar.UltiSideBarTestHelper;
+import com.ultikits.plugins.sidebar.service.SideBarService;
+import com.ultikits.ultitools.abstracts.UltiToolsPlugin;
+
+import me.clip.placeholderapi.PlaceholderAPI;
+import org.bukkit.configuration.file.YamlConfiguration;
+import org.bukkit.entity.Player;
 import org.junit.jupiter.api.*;
+import org.junit.jupiter.api.io.TempDir;
+import org.mockito.Answers;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
 
+import java.io.File;
+import java.lang.reflect.Method;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import static org.assertj.core.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
 
 @DisplayName("SideBarConfig Tests")
 class SideBarConfigTest {
@@ -139,6 +160,251 @@ void emptyLines() {
         }
     }
 
+    // ============================
+    // Default sidebar renderability
+    // ============================
+
+    @Nested
+    @DisplayName("Default Sidebar Renderability")
+    class DefaultSidebarRenderabilityTests {
+
+        /**
+         * Sentinel substitutions for the placeholders a real PlaceholderAPI installation (Player
+         * + Server expansions) resolves, sourced independently from PlaceholderAPI's own
+         * placeholder wiki rather than re-derived from the defaults under test in this file --
+         * so a future commit cannot introduce a broken token and "fix" this test in the same
+         * edit by adding the same name to a local allow-list.
+         * <p>
+         * {@code vault_eco_balance_formatted} is deliberately excluded (WR-03): it additionally
+         * requires Vault plus a registered economy provider, a materially larger install surface
+         * than "PlaceholderAPI is installed", so it is exempted below by name rather than
+         * silently substituted here.
+         */
+        private static final String VAULT_DEPENDENT_TOKEN = "%vault_eco_balance_formatted%";
+
+        private final Pattern serverTimeToken = Pattern.compile("%server_time_([^%]+)%");
+
+        private String stubResolve(String text) {
+            String resolved = text
+                    .replace("%player_name%", "Steve")
+                    .replace("%server_online%", "12")
+                    .replace("%server_max_players%", "100")
+                    .replace("%player_world%", "world")
+                    .replace("%player_ping%", "42");
+            // PlaceholderAPI's Server expansion accepts a SimpleDateFormat pattern as a dynamic
+            // suffix: %server_time_<SimpleDateFormat>%. A real installation only resolves it if
+            // the suffix is a legal SimpleDateFormat pattern -- an illegal pattern letter (e.g.
+            // %server_time_foo%, "f" is not a pattern letter) makes java.text.SimpleDateFormat's
+            // constructor throw IllegalArgumentException, so the real expansion cannot format it.
+            // Substituting every suffix unconditionally, as an earlier revision of this stub did,
+            // would let this test pass a shipped default that fails to render at runtime.
+            Matcher serverTimeMatcher = serverTimeToken.matcher(resolved);
+            StringBuffer buffer = new StringBuffer();
+            while (serverTimeMatcher.find()) {
+                String suffix = serverTimeMatcher.group(1);
+                String replacement;
+                try {
+                    new java.text.SimpleDateFormat(suffix).format(new java.util.Date());
+                    replacement = "12:00:00";
+                } catch (IllegalArgumentException invalidPattern) {
+                    replacement = serverTimeMatcher.group();
+                }
+                serverTimeMatcher.appendReplacement(buffer, Matcher.quoteReplacement(replacement));
+            }
+            serverTimeMatcher.appendTail(buffer);
+            return buffer.toString();
+        }
+
+        @Test
+        @DisplayName("Default lines contain no token that nothing resolves")
+        void defaultLinesContainNoTokenThatNothingResolves() throws Exception {
+            SideBarConfig config = createRealConfig();
+
+            // Route every default line through the module's own placeholder-resolution path
+            // (SideBarService.parsePlaceholders -> PlaceholderAPI.setPlaceholders) instead of
+            // checking token names against a hand-authored allow-list mirroring this same
+            // file's defaults -- that allow-list could never catch a broken token added
+            // alongside a matching allow-list entry in the same commit. The PlaceholderAPI seam
+            // is stubbed to behave the way a real installation does: a recognized placeholder is
+            // substituted, and one no registered expansion recognizes is left untouched in the
+            // output -- exactly the symptom the original issue (#13) reported.
+            SideBarService service = new SideBarService();
+            UltiSideBarTestHelper.setField(service, "placeholderApiAvailable", true);
+            Player player = Mockito.mock(Player.class);
+
+            Method parsePlaceholders = SideBarService.class
+                    .getDeclaredMethod("parsePlaceholders", Player.class, String.class);
+            parsePlaceholders.setAccessible(true);
+
+            try (MockedStatic<PlaceholderAPI> placeholderApi = Mockito.mockStatic(PlaceholderAPI.class)) {
+                placeholderApi.when(() -> PlaceholderAPI.setPlaceholders(eq(player), anyString()))
+                        .thenAnswer(invocation -> stubResolve(invocation.getArgument(1)));
+
+                List<String> unresolvedTokensRemaining = new ArrayList<>();
+                Pattern leftoverTokenPattern = Pattern.compile("%[^%]+%");
+                for (String line : config.getLines()) {
+                    String rendered = (String) parsePlaceholders.invoke(service, player, line);
+                    Matcher matcher = leftoverTokenPattern.matcher(rendered);
+                    while (matcher.find()) {
+                        String leftover = matcher.group();
+                        if (!VAULT_DEPENDENT_TOKEN.equals(leftover)) {
+                            unresolvedTokensRemaining.add(leftover);
+                        }
+                    }
+                }
+
+                assertThat(unresolvedTokensRemaining)
+                        .as("Every default line must render through the module's own placeholder "
+                                + "path with no leftover token that nothing resolves")
+                        .isEmpty();
+            }
+        }
+
+        @Test
+        @DisplayName("The stub leaves an invalid server-time pattern unresolved, matching a real installation")
+        void stubResolveRejectsAnInvalidServerTimePattern() {
+            // Regression guard for the earlier permissive regex ("%server_time_[^%]+%" ->
+            // "12:00:00" unconditionally), which would let defaultLinesContainNoTokenThatNothingResolves()
+            // pass a shipped default containing an illegal SimpleDateFormat suffix -- java.text.
+            // SimpleDateFormat throws IllegalArgumentException on an illegal pattern letter such
+            // as 'f' or 'o', so a real PlaceholderAPI Server expansion cannot format
+            // "%server_time_foo%" either. The stub must mirror that failure, not paper over it.
+            assertThat(stubResolve("%server_time_foo%")).isEqualTo("%server_time_foo%");
+            assertThat(stubResolve("&f%server_time_HH:mm:ss%")).isEqualTo("&f12:00:00");
+        }
+
+        @Test
+        @DisplayName("An operator-configured line survives init() against a persisted file that also holds the legacy default")
+        void anOperatorConfiguredLineIsUnaffected(@TempDir Path tempDir) throws Exception {
+            // Drives the real init()-mediated persisted-file-vs-default precedence (CR-01) --
+            // a bare setLines()/getLines() round-trip cannot fail for any change to
+            // SideBarConfig's default-handling behavior and proves nothing about upgrade safety.
+            File configFile = new File(tempDir.toFile(), "config/sidebar.yml");
+            Files.createDirectories(configFile.getParentFile().toPath());
+            YamlConfiguration persisted = new YamlConfiguration();
+            persisted.set("lines", Arrays.asList(
+                    "&e世界: &f%world_name%",
+                    "&aOperator's own custom line"
+            ));
+            persisted.save(configFile);
+
+            SideBarConfig config = createRealConfig();
+            config.init(mockPluginBackedBy(tempDir));
+
+            assertThat(config.getLines())
+                    .as("an operator's own persisted line must survive init() untouched")
+                    .contains("&aOperator's own custom line");
+        }
+    }
+
+    /**
+     * Builds an {@code UltiToolsPlugin} test double whose {@code getConfigFolder()}/
+     * {@code getConfigFile(String)} resolve against {@code tempDir}. Those two methods are
+     * {@code protected final} on {@code UltiToolsPlugin}, declared outside this test's package,
+     * so a normal {@code Mockito.when(mock.getConfigFolder())...} does not even compile here --
+     * this uses Mockito's {@code mock(Class, Answer)} default-answer form instead, which
+     * intercepts every method call by reflection ({@code invocation.getMethod()}) rather than by
+     * a source-level call to the (inaccessible) method.
+     */
+    private static UltiToolsPlugin mockPluginBackedBy(Path tempDir) {
+        return Mockito.mock(UltiToolsPlugin.class, invocation -> {
+            String methodName = invocation.getMethod().getName();
+            if ("getConfigFolder".equals(methodName)) {
+                return tempDir.toString();
+            }
+            if ("getConfigFile".equals(methodName)) {
+                String path = invocation.getArgument(0);
+                return new File(tempDir.toFile(), path);
+            }
+            return Answers.RETURNS_DEFAULTS.answer(invocation);
+        });
+    }
+
+    // ============================
+    // Legacy %world_name% default line migration (issue #13, CR-01)
+    // ============================
+
+    @Nested
+    @DisplayName("Legacy World-Name Default Line Migration")
+    class LegacyWorldNameLineMigration {
+
+        @TempDir
+        Path tempDir;
+
+        private UltiToolsPlugin mockPlugin;
+
+        @BeforeEach
+        void setUp() {
+            mockPlugin = mockPluginBackedBy(tempDir);
+        }
+
+        private File persistLines(List<String> lines) throws Exception {
+            File configFile = new File(tempDir.toFile(), "config/sidebar.yml");
+            Files.createDirectories(configFile.getParentFile().toPath());
+            YamlConfiguration persisted = new YamlConfiguration();
+            persisted.set("lines", lines);
+            persisted.save(configFile);
+            return configFile;
+        }
+
+        @Test
+        @DisplayName("Rewrites a persisted line byte-identical to the old %world_name% default; a custom line survives")
+        void rewritesLegacyLineButLeavesCustomLineUntouched() throws Exception {
+            File configFile = persistLines(Arrays.asList(
+                    "&7欢迎, &f%player_name%",
+                    "&e世界: &f%world_name%",
+                    "&aOperator's own custom line"
+            ));
+
+            SideBarConfig config = new SideBarConfig();
+            config.init(mockPlugin);
+
+            boolean rewritten = config.migrateLegacyWorldNameDefaultLine();
+            assertThat(rewritten).isTrue();
+            config.save();
+
+            assertThat(config.getLines())
+                    .as("the stale %world_name% line must be rewritten to the corrected default")
+                    .contains("&e世界: &f%player_world%")
+                    .doesNotContain("&e世界: &f%world_name%");
+            assertThat(config.getLines())
+                    .as("an operator's own custom line must be left untouched")
+                    .contains("&aOperator's own custom line");
+
+            YamlConfiguration onDisk = YamlConfiguration.loadConfiguration(configFile);
+            assertThat(onDisk.getStringList("lines"))
+                    .as("the migration must be persisted back to disk")
+                    .contains("&e世界: &f%player_world%", "&aOperator's own custom line")
+                    .doesNotContain("&e世界: &f%world_name%");
+        }
+
+        @Test
+        @DisplayName("Does not touch a line that merely mentions %world_name% inside other text")
+        void doesNotTouchLineThatOnlyMentionsTheLegacyToken() throws Exception {
+            persistLines(Collections.singletonList("&7Custom: &f%world_name% (renamed by admin)"));
+
+            SideBarConfig config = new SideBarConfig();
+            config.init(mockPlugin);
+
+            boolean rewritten = config.migrateLegacyWorldNameDefaultLine();
+
+            assertThat(rewritten).isFalse();
+            assertThat(config.getLines())
+                    .containsExactly("&7Custom: &f%world_name% (renamed by admin)");
+        }
+
+        @Test
+        @DisplayName("Is a no-op once the persisted line already uses the corrected placeholder")
+        void noOpWhenAlreadyMigrated() throws Exception {
+            persistLines(Collections.singletonList("&e世界: &f%player_world%"));
+
+            SideBarConfig config = new SideBarConfig();
+            config.init(mockPlugin);
+
+            assertThat(config.migrateLegacyWorldNameDefaultLine()).isFalse();
+        }
+    }
+
     /**
      * Create a real SideBarConfig instance.
      * The no-arg constructor calls super("config/sidebar.yml") which only stores the path