fix: harden reward payout and store integrity

src/main/java/tfmc/justin/activity/commands/ActivityCommand.java

@@ -7,6 +7,7 @@
 import org.bukkit.command.CommandSender;
 import org.bukkit.command.TabCompleter;
 import org.bukkit.entity.Player;
+import tfmc.justin.activity.config.ActivityConfiguration;
 import tfmc.justin.activity.config.Messages;
 import tfmc.justin.activity.gui.ActivityGui;
 import tfmc.justin.activity.managers.ActivityManager;
@@ -99,7 +100,8 @@ private void handleReset(CommandSender sender, String[] args) {
         }
 
         PlayerData data = manager.getStore().get(target.getUniqueId());
-        data.reset(manager.getConfiguration().currentWeekKey(), manager.getConfiguration().currentDayKey());
+        ActivityConfiguration.Keys keys = manager.getConfiguration().currentKeys();
+        data.reset(keys.week(), keys.day());
         manager.getStore().markDirty();
 
         // The resolved name, not what was typed - casing and the cache decide

src/main/java/tfmc/justin/activity/config/ActivityConfiguration.java

@@ -9,7 +9,6 @@
 import tfmc.justin.activity.utils.Weeks;
 
 import java.time.DayOfWeek;
-import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -77,6 +76,10 @@ public void load() {
         loadActivities(config.getConfigurationSection("activities"));
 
         rewardCommands = config.getStringList("rewards.commands");
+        if (rewardCommands.isEmpty()) {
+            plugin.getLogger().warning("rewards.commands is empty - a full bar can be reached but nothing"
+                + " can ever be claimed.");
+        }
         rewardDisplay = config.getStringList("rewards.display");
 
         guiTitle = config.getString("gui.title", "&8Weekly Activity");
@@ -112,15 +115,6 @@ private void loadActivities(ConfigurationSection section) {
                 continue;
             }
 
-            // The old key: its meaning moved to 'every' with 'points' per award,
-            // so an unconverted entry would hand out its old 50-point award per
-            // single action. Loud and skipped rather than silently wrong.
-            if (entry.contains("daily-goal")) {
-                plugin.getLogger().warning("Activity '" + id + "' still uses 'daily-goal' - skipping it."
-                    + " Convert to every/points/daily-cap (see the default config.yml).");
-                continue;
-            }
-
             int points = wholeNumber(entry, id, "points", 0);
             if (points <= 0) {
                 plugin.getLogger().warning("Activity '" + id + "' is worth " + points
@@ -231,16 +225,16 @@ public ActivityDef activity(String id) {
     }
 
     // ====================================
-    // The keys "right now" falls in. Every caller used to spell out the same
-    // LocalDateTime.now() plus two Weeks calls; a reset boundary belongs to
-    // the config that defines it.
+    // Both keys off one clock reading. Asking for them separately can straddle
+    // a midnight tick and produce a day key from the new day with a week key
+    // from the old one, which reads as a rollover that never happened.
     // ====================================
-    public String currentWeekKey() {
-        return Weeks.weekKey(LocalDateTime.now(), resetDay, resetHour);
+    public Keys currentKeys() {
+        LocalDateTime now = LocalDateTime.now();
+        return new Keys(Weeks.weekKey(now, resetDay, resetHour), Weeks.dayKey(now.toLocalDate()));
     }
 
-    public String currentDayKey() {
-        return Weeks.dayKey(LocalDate.now());
+    public record Keys(String week, String day) {
     }
 
     public DayOfWeek resetDay() {

src/main/java/tfmc/justin/activity/gui/ActivityGui.java

@@ -115,7 +115,7 @@ private ItemStack rewardItem(ActivityConfiguration config, Messages messages, Pl
         List<String> lore = new ArrayList<>();
         lore.add(messages.get("gui.reward-lore-header", "%every%", config.rewardEvery()));
         lore.addAll(Messages.colorize(config.rewardDisplay()));
-        lore.add(messages.get("gui.reward-lore-claimed", "%claimed%", data.claimed(),
+        lore.add(messages.get("gui.reward-lore-claimed", "%claimed%", data.claimedPoints() / config.rewardEvery(),
             "%total%", config.barMax() / config.rewardEvery()));
         lore.add(due > 0
             ? messages.get("gui.reward-click", "%count%", due)
@@ -152,9 +152,40 @@ public void onClick(InventoryClickEvent event) {
             return;
         }
 
+        // The permission is checked on /activity, but an inventory can outlive
+        // the permission that opened it - a revoked player must not still be
+        // able to click a reward out of a window they left open
+        if (!player.hasPermission("activity.use")) {
+            return;
+        }
+
         ActivityConfiguration config = manager.getConfiguration();
         if (manager.claim(player) == 0) {
-            player.sendMessage(config.messages().get("reward-nothing"));
+            // ====================================
+            // A claim of 0 is either "nothing was due" or a refusal, and every
+            // refusal has already told the player why - saying "nothing to
+            // claim yet" on top of that contradicts it. So the only two lines
+            // sent here are the ones claim() cannot send itself: the missing
+            // reward commands, which it leaves silent on purpose, and the
+            // genuinely empty claim, which still owes the player an answer.
+            // ====================================
+            // A store that never loaded refuses every claim and has already
+            // said so - what is or is not configured is beside the point
+            if (!manager.getStore().isLoaded()) {
+                return;
+            }
+
+            if (config.rewardCommands().isEmpty()) {
+                player.sendMessage(config.messages().get("reward-unconfigured"));
+                return;
+            }
+
+            // peek, not get: deciding which line to send must not create an
+            // entry or dirty the store
+            PlayerData data = manager.getStore().peek(player.getUniqueId());
+            if (data == null || data.claimable(config.rewardEvery()) == 0) {
+                player.sendMessage(config.messages().get("reward-nothing"));
+            }
             return;
         }
         event.getView().getTopInventory().setItem(REWARD_SLOT,

src/main/java/tfmc/justin/activity/hooks/PlaceholderHook.java

@@ -61,7 +61,10 @@ public String onRequest(OfflinePlayer player, String params) {
         ActivityConfiguration config = manager.getConfiguration();
         PlayerData data = manager.getStore().peek(player.getUniqueId());
 
-        boolean currentWeek = data != null && data.weekKey().equals(config.currentWeekKey());
+        // One clock reading for both keys: asked separately they can straddle
+        // a midnight tick and disagree about which day this week it is
+        ActivityConfiguration.Keys keys = config.currentKeys();
+        boolean currentWeek = data != null && data.weekKey().equals(keys.week());
         // Clamped here too: peek() never runs the clamp get() does, so a lowered
         // bar.max would otherwise read over 100% until the next action
         int points = currentWeek ? Math.min(data.points(), config.barMax()) : 0;
@@ -99,7 +102,7 @@ public String onRequest(OfflinePlayer player, String params) {
             }
             // "Done" is the daily cap reached, or - for an uncapped activity -
             // at least one award earned today
-            int today = currentWeek && data.dayKey().equals(config.currentDayKey())
+            int today = currentWeek && data.dayKey().equals(keys.day())
                 ? def.worth(data.count(def.id())) : 0;
             boolean done = def.dailyCap() > 0 ? today >= def.dailyCap() : today > 0;
             return Utils.colorize(config.messages().raw(done ? "placeholder.done" : "placeholder.not-done"));

src/main/java/tfmc/justin/activity/listeners/VoteListener.java

@@ -8,8 +8,10 @@
 import org.bukkit.event.EventHandler;
 import org.bukkit.event.EventPriority;
 import org.bukkit.event.Listener;
+import org.bukkit.plugin.IllegalPluginAccessException;
 import org.bukkit.plugin.java.JavaPlugin;
 import tfmc.justin.activity.managers.ActivityManager;
+import tfmc.justin.activity.utils.Utils;
 
 import java.util.UUID;
 
@@ -46,35 +48,45 @@ public void onVote(PlayerVoteEvent event) {
             return;
         }
 
-        UUID uuid = resolve(event);
-        if (uuid == null) {
-            return;
-        }
+        // Only plain data is read off the vote thread; every lookup that
+        // touches server state happens on the tick below
+        VotingPluginUser user = event.getVotingPluginUser();
+        UUID javaUuid = user == null ? null : user.getJavaUUID();
+        String name = event.getPlayer();
 
         // The scheduler throws for a disabled plugin, and a vote arriving
         // during shutdown has nowhere to be scored anyway
-        if (manager == null || !plugin.isEnabled()) {
+        if (!plugin.isEnabled()) {
             return;
         }
 
-        Bukkit.getScheduler().runTask(plugin, () -> manager.recordAction(uuid, "vote", 1));
+        // isEnabled() was true a line ago, but a disable landing in between
+        // would throw on VotingPlugin's thread rather than ours
+        try {
+            Bukkit.getScheduler().runTask(plugin, () -> {
+                UUID uuid = resolve(javaUuid, name);
+                if (uuid != null) {
+                    manager.recordAction(uuid, "vote", 1);
+                }
+            });
+        } catch (IllegalPluginAccessException | IllegalStateException e) {
+            // Plugin is being disabled, vote dropped
+        }
     }
 
     // ====================================
-    // Every fallback here reads a local cache only, which is what makes this
-    // safe on the async vote thread. Bukkit.getOfflinePlayer(String) is not
-    // used at all: it blocks on a Mojang lookup and invents a UUID for names
-    // that have never played, which would quietly fill players.yml with junk.
+    // Main thread only: Bukkit.getPlayerExact and the offline-player cache are
+    // not safe off a tick. Bukkit.getOfflinePlayer(String) is not used at all:
+    // it blocks on a Mojang lookup and invents a UUID for names that have
+    // never played, which would quietly fill players.yml with junk.
     // ====================================
-    private UUID resolve(PlayerVoteEvent event) {
-        VotingPluginUser user = event.getVotingPluginUser();
-        if (user != null && user.getJavaUUID() != null) {
-            return user.getJavaUUID();
+    private UUID resolve(UUID javaUuid, String name) {
+        if (javaUuid != null) {
+            return javaUuid;
         }
 
         // Proxy-forwarded votes can arrive with no user attached; the name is
         // always set
-        String name = event.getPlayer();
         if (name == null || name.isBlank()) {
             plugin.getLogger().warning("Ignoring a vote with neither a user nor a player name.");
             return null;
@@ -90,7 +102,8 @@ private UUID resolve(PlayerVoteEvent event) {
             return cached.getUniqueId();
         }
 
-        plugin.getLogger().info("Dropping a vote from '" + name + "' - no cached player by that name.");
+        plugin.getLogger().info("Dropping a vote from '" + Utils.safeForLog(name)
+            + "' - no cached player by that name.");
         return null;
     }
 }