From 01c6f91e1809101c830b80109d02d5e41e23f52a Mon Sep 17 00:00:00 2001 From: caucow Date: Fri, 8 May 2020 05:54:57 -0500 Subject: [PATCH 1/4] Make DB access async, add nanotime to debug, move mark to NameRecord. --- .../minecraft/wordbank/NameRecord.java | 65 +++++++++++++++ .../minecraft/wordbank/WordBank.java | 62 ++++++++++++++ .../minecraft/wordbank/WordBankConfig.java | 23 ++++++ .../wordbank/actions/ActionListener.java | 82 ++++++++++++------- .../minecraft/wordbank/data/WordBankData.java | 3 + .../wordbank/util/NameConstructor.java | 11 ++- 6 files changed, 209 insertions(+), 37 deletions(-) create mode 100644 src/main/java/com/programmerdan/minecraft/wordbank/NameRecord.java diff --git a/src/main/java/com/programmerdan/minecraft/wordbank/NameRecord.java b/src/main/java/com/programmerdan/minecraft/wordbank/NameRecord.java new file mode 100644 index 0000000..0e44aca --- /dev/null +++ b/src/main/java/com/programmerdan/minecraft/wordbank/NameRecord.java @@ -0,0 +1,65 @@ +package com.programmerdan.minecraft.wordbank; + +import com.programmerdan.minecraft.wordbank.data.WordBankData; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.SQLException; +import java.util.Objects; +import java.util.logging.Level; + +/** + * + * @author caucow + */ +public class NameRecord { + public final String key; + public final String value; + private boolean marked; + + public NameRecord(String key, String value, boolean marked) { + this.key = Objects.requireNonNull(key); + this.value = Objects.requireNonNull(value); + this.marked = marked; + } + + /** + * Marks the wordbank key/value pair as used in the database, along with + * player UUID and the item type it was applied to. + * @param wbplugin WordBank plugin (for database access) + * @param playerId player UUID + * @param itemType item type name was applied to + * @param force force a database update with the current pID and itemType + * regardless of whether this name is already marked + */ + public void mark(WordBank wbplugin, String playerId, String itemType, boolean force) { + if (marked && !force) { + return; + } + marked = true; + if (wbplugin.config().hasDB()) { + try { + if (wbplugin.config().isDebug()) wbplugin.logger().info(" - Inserting item record"); + long startTime = 0, endTime = 0; + startTime = System.nanoTime(); + try ( + Connection connection = wbplugin.data().getConnection(); + PreparedStatement insert = connection.prepareStatement(WordBankData.insert)) { + insert.setString(1, key); + insert.setString(2, playerId); + insert.setString(3, itemType); + insert.setString(4, value); + insert.executeUpdate(); + insert.close(); + endTime = System.nanoTime(); + if (wbplugin.config().isDebug()) wbplugin.logger().log(Level.INFO, "Wrote key/value {0}={1} to database in {2}ms", new Object[] { key, value, (endTime - startTime) / 1_000_000.0F }); + } + } catch (SQLException se) { + wbplugin.logger().log(Level.WARNING, "Failed to insert key utilization", se); + } + } + } + + public boolean isMarked() { + return marked; + } +} diff --git a/src/main/java/com/programmerdan/minecraft/wordbank/WordBank.java b/src/main/java/com/programmerdan/minecraft/wordbank/WordBank.java index 40f55e2..716d5dd 100644 --- a/src/main/java/com/programmerdan/minecraft/wordbank/WordBank.java +++ b/src/main/java/com/programmerdan/minecraft/wordbank/WordBank.java @@ -1,5 +1,8 @@ package com.programmerdan.minecraft.wordbank; +import com.google.common.cache.LoadingCache; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; import java.sql.SQLException; import java.util.logging.Level; import java.util.logging.Logger; @@ -12,6 +15,12 @@ import com.programmerdan.minecraft.wordbank.actions.ActionListener; import com.programmerdan.minecraft.wordbank.actions.CommandListener; import com.programmerdan.minecraft.wordbank.data.WordBankData; +import com.programmerdan.minecraft.wordbank.util.NameConstructor; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.util.concurrent.TimeUnit; +import org.bukkit.ChatColor; /** * See README.md for details. Simple Bukkit plugin using some Cool Stuff under the hood. @@ -25,6 +34,7 @@ public class WordBank extends JavaPlugin { private static WordBank plugin; private WordBankConfig config; private WordBankData data; + private LoadingCache nameCache; // laggy database hits? that's not very cache money tbh public void onEnable() { WordBank.plugin = this; @@ -40,6 +50,54 @@ public void onEnable() { // Do DB provision data = new WordBankData(); + nameCache = CacheBuilder.newBuilder() + .expireAfterAccess(config.getNamecacheInvalidateMinutes(), TimeUnit.MINUTES) + .expireAfterWrite(config.getNamecacheInvalidateMinutes(), TimeUnit.MINUTES) + .maximumSize(config.getNamecacheMaxSize()) + .build(new CacheLoader() { + @Override + public NameRecord load(String key) throws Exception { + String value = null; + long startTime = 0, endTime = 0; + if (config().hasDB()) { + try { + startTime = System.nanoTime(); + try (Connection connection = data().getConnection(); + PreparedStatement statement = connection.prepareStatement(WordBankData.getvalue)) { + statement.setString(1, key); + try (ResultSet rs = statement.executeQuery()) { + if (rs.next()) { + value = rs.getString("val"); + } + } + statement.close(); + } + endTime = System.nanoTime(); + } catch (SQLException se) { + logger().log(Level.WARNING, "Failed to retrieve key/value data!", se); + if (config().isFailRenameOnDbError()) { + throw se; + } + } + if (value != null) { + // value exists in database, return record with marked=TRUE + NameRecord record = new NameRecord(key, value, true); + if (config().isDebug()) logger().log(Level.INFO, "Retrieved value {0}={1} from database in {2}ms", new Object[] { record.key, record.value, (endTime - startTime) / 1_000_000.0F }); + return record; + } else { + // value did not exist in database. debug time and carry on + if (config().isDebug()) logger().log(Level.INFO, "Database entry not found for {0} in {1}ms", new Object[] { key, (endTime - startTime) / 1_000_000.0F }); + } + } + // no value in database, return record with marked=FALSE + startTime = System.nanoTime(); + NameRecord record = NameConstructor.buildName(key); + endTime = System.nanoTime(); + if (config().isDebug()) logger().log(Level.INFO, "Generated name for value {0}={1} in {2}ms", new Object[] { record.key, record.value, (endTime - startTime) / 1_000_000.0F }); + return record; + } + } + ); Bukkit.getPluginManager().registerEvents(new ActionListener(), this); plugin.getCommand("wordbank").setExecutor(new CommandListener()); } @@ -70,4 +128,8 @@ public WordBankConfig config() { public WordBankData data() { return data; } + + public LoadingCache nameCache() { + return nameCache; + } } diff --git a/src/main/java/com/programmerdan/minecraft/wordbank/WordBankConfig.java b/src/main/java/com/programmerdan/minecraft/wordbank/WordBankConfig.java index e727f99..24772cc 100644 --- a/src/main/java/com/programmerdan/minecraft/wordbank/WordBankConfig.java +++ b/src/main/java/com/programmerdan/minecraft/wordbank/WordBankConfig.java @@ -30,6 +30,9 @@ public class WordBankConfig { private boolean debug; private WordBank plugin; private long confirm_delay; + private boolean fail_rename_on_db_error; + private int namecache_invalidate_minutes; + private int namecache_max_size; public WordBankConfig(ConfigurationSection config) throws InvalidPluginException { this(config, null); @@ -68,6 +71,14 @@ public WordBankConfig(ConfigurationSection config, WordBank plugin) throws Inval this.confirm_delay = config.getLong("confirm_delay", 10000l); + // true if, should the database be enabled AND throw an exception when + // getting a value for a key name, wordbank should skip trying to generate + // a new value for that key (prevents possible ambiguity in names if + // the configs change and the che cache can't access the database) + this.fail_rename_on_db_error = config.getBoolean("fail_rename_on_db_error", true); + this.namecache_invalidate_minutes = config.getInt("namecache_invalidate_minutes", 5); + this.namecache_max_size = config.getInt("namecache_max_size", 500); + // dbconfig this.db_config = null; ConfigurationSection db = config.getConfigurationSection("db"); @@ -150,4 +161,16 @@ public boolean hasDB() { public HikariConfig database() { return this.db_config; } + + public boolean isFailRenameOnDbError() { + return fail_rename_on_db_error; + } + + public int getNamecacheInvalidateMinutes() { + return namecache_invalidate_minutes; + } + + public int getNamecacheMaxSize() { + return namecache_max_size; + } } diff --git a/src/main/java/com/programmerdan/minecraft/wordbank/actions/ActionListener.java b/src/main/java/com/programmerdan/minecraft/wordbank/actions/ActionListener.java index 1c1a178..d6afed8 100644 --- a/src/main/java/com/programmerdan/minecraft/wordbank/actions/ActionListener.java +++ b/src/main/java/com/programmerdan/minecraft/wordbank/actions/ActionListener.java @@ -1,5 +1,6 @@ package com.programmerdan.minecraft.wordbank.actions; +import com.programmerdan.minecraft.wordbank.NameRecord; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.SQLException; @@ -26,8 +27,8 @@ import org.bukkit.scheduler.BukkitTask; import com.programmerdan.minecraft.wordbank.WordBank; -import com.programmerdan.minecraft.wordbank.data.WordBankData; -import com.programmerdan.minecraft.wordbank.util.NameConstructor; +import java.util.concurrent.ExecutionException; +import org.bukkit.Bukkit; /** * Manages the detection and application of WordBank keys. @@ -46,7 +47,7 @@ public ActionListener() { } public ActionListener(WordBank plugin) { this.plugin = plugin; - pendingMarks = new HashMap(); + pendingMarks = new HashMap<>(); } protected WordBank plugin() { @@ -80,6 +81,7 @@ public void TableTouch(PlayerInteractEvent event) { if (plugin().config().isDebug()) plugin().logger().info(" - has no Makers Mark lore"); String curName = meta.getDisplayName(); + String unmodifiedName = curName; if (plugin().config().isActivateAnyLength() || curName.length() == plugin().config().getActivationLength()) { if (plugin().config().isDebug()) plugin().logger().info(" - is eligible"); @@ -89,12 +91,23 @@ public void TableTouch(PlayerInteractEvent event) { if (pInv.containsAtLeast(plugin().config().getCost(), plugin().config().getCost().getAmount())) { final UUID puid = event.getPlayer().getUniqueId(); BukkitTask firstHit = pendingMarks.get(puid); + + // moved this out of if/else so the proper length name is used in + // both code blocks rather than just one (required by cache) + if (curName.length() > plugin().config().getActivationLength()) { + curName = curName.substring(0, plugin().config().getActivationLength()); + } else if (curName.length() < plugin().config().getActivationLength()) { + int diff = plugin().config().getActivationLength() - curName.length(); + curName = curName.concat( new String(new char[diff]) + .replaceAll("\0", plugin().config().getPadding())); + } + if (firstHit == null) { long confirmDelay = plugin().config().getConfirmDelay(); plugin().logger().log(Level.INFO, "Pending a mark for player {0}", puid); event.getPlayer().sendMessage(String.format("Hit the table a second time in the next %d seconds to confirm renaming using %s.", - confirmDelay / 1000l, curName)); + confirmDelay / 1000l, unmodifiedName)); pendingMarks.put(puid, new BukkitRunnable() { @@ -104,6 +117,20 @@ public void run() { } }.runTaskLater(plugin(), confirmDelay / 50l) // convert to ticks ); + + // Schedule ASYNC task to load and cache the mapped name + // Do it now so it'll be ready when the player clicks again + { + final String finalCurName = curName; + Bukkit.getScheduler().runTaskAsynchronously(plugin(), () -> { + try { + System.out.println(plugin().nameCache().get(finalCurName).value); + } catch (ExecutionException e) { + + } + }); + } + event.setCancelled(true); return; } else { @@ -116,26 +143,29 @@ public void run() { // ignore overflow? } } else { - firstHit.cancel(); - pendingMarks.remove(puid); try { if (plugin().config().isDebug()) plugin().logger().info(" - Paid and updating item"); - if (curName.length() > plugin().config().getActivationLength()) { - curName = curName.substring(0, plugin().config().getActivationLength()); - } else if (curName.length() < plugin().config().getActivationLength()) { - int diff = plugin().config().getActivationLength() - curName.length(); - curName = curName.concat( new String(new char[diff]) - .replaceAll("\0", plugin().config().getPadding())); + NameRecord newNameRecord = plugin().nameCache().getIfPresent(curName); + + if (newNameRecord == null) { + event.getPlayer().sendMessage(String.format("%sThat name is still generating (or there was an error), wait a few seconds.", ChatColor.WHITE)); + event.setCancelled(true); + return; + } else { + // Move the task cancel here so spamming wordbank + // doesn't spawn crap tons of threads all blocking + // while waiting for the first cache load + firstHit.cancel(); + pendingMarks.remove(puid); } - String newName = NameConstructor.buildName(curName, true); if (plugin().config().isDebug()) plugin().logger().log( Level.INFO, " - Using key {0} to generate {1}", - new Object[]{curName, newName}); + new Object[]{curName, newNameRecord.value}); - meta.setDisplayName(newName); + meta.setDisplayName(newNameRecord.value); ArrayList lore = new ArrayList(); lore.add(plugin().config().getMakersMark()); meta.setLore(lore); @@ -146,22 +176,12 @@ ChatColor.WHITE, plugin().config().getMakersMark(), meta.getDisplayName(), ChatColor.WHITE, item.getType().toString())); - if (plugin().config().hasDB()) { - try { - if (plugin().config().isDebug()) plugin().logger().info(" - Inserting item record"); - Connection connection = plugin().data().getConnection(); - PreparedStatement insert = connection.prepareStatement(WordBankData.insert); - insert.setString(1, curName); - insert.setString(2, event.getPlayer().getUniqueId().toString()); - insert.setString(3, item.getType().toString()); - insert.setString(4, newName); - insert.executeUpdate(); - insert.close(); - connection.close(); - } catch (SQLException se) { - plugin().logger().log(Level.WARNING, "Failed to insert key utilization", se); - } - } + // Schedule ASYNC task to add an entry to the database + // force=true for... logging purposes? to the database for some reason? + // why does it need player UUID and item type just to keep a unique key/value? + Bukkit.getScheduler().runTaskAsynchronously(plugin(), () -> { + newNameRecord.mark(plugin(), event.getPlayer().getUniqueId().toString(), item.getType().toString(), true); + }); } catch (Exception e) { plugin().logger().log(Level.WARNING, "Something went very wrong while renaming", e); event.getPlayer().sendMessage(String.format("Mystic renaming of %s has %sfailed%s. %sPlease report via /helpop.", diff --git a/src/main/java/com/programmerdan/minecraft/wordbank/data/WordBankData.java b/src/main/java/com/programmerdan/minecraft/wordbank/data/WordBankData.java index 1742816..5ba9c05 100644 --- a/src/main/java/com/programmerdan/minecraft/wordbank/data/WordBankData.java +++ b/src/main/java/com/programmerdan/minecraft/wordbank/data/WordBankData.java @@ -36,6 +36,9 @@ public class WordBankData { "SELECT uuid, count(*) AS cnt, count(DISTINCT target) AS targets " + " FROM wordbank_utilization WHERE wbkey = ? GROUP BY uuid LIMIT ? OFFSET ?;"; + public static final String getvalue = + "SELECT wbkey, wbname AS val FROM wordbank_utilization WHERE wbkey = ? LIMIT 1;"; + private HikariDataSource datasource; private WordBank plugin; diff --git a/src/main/java/com/programmerdan/minecraft/wordbank/util/NameConstructor.java b/src/main/java/com/programmerdan/minecraft/wordbank/util/NameConstructor.java index 157fe85..fc02302 100644 --- a/src/main/java/com/programmerdan/minecraft/wordbank/util/NameConstructor.java +++ b/src/main/java/com/programmerdan/minecraft/wordbank/util/NameConstructor.java @@ -1,6 +1,7 @@ package com.programmerdan.minecraft.wordbank.util; import com.programmerdan.minecraft.wordbank.CharConfig; +import com.programmerdan.minecraft.wordbank.NameRecord; import com.programmerdan.minecraft.wordbank.WordBank; /** @@ -18,11 +19,10 @@ public class NameConstructor { * All these parts are joined and returned. * * @param key The character sequence used to construct a WordBank name. - * @param mark If true, save key as used; otherwise, do not. * @return The converted key. */ - public static String buildName(String key, boolean mark) { - return buildName(key, mark, WordBank.instance()); + public static NameRecord buildName(String key) { + return buildName(key, WordBank.instance()); } /** @@ -30,8 +30,7 @@ public static String buildName(String key, boolean mark) { * * @param plugin the WordBank instance to use. Good for unit testing. */ - public static String buildName(String key, boolean mark, WordBank plugin) { - // TODO: add mark storage; mark is ignored for now, tbd. + public static NameRecord buildName(String key, WordBank plugin) { // First, compute color. float whichColor = executeConfig(plugin.config().getColor(), key); @@ -51,7 +50,7 @@ public static String buildName(String key, boolean mark, WordBank plugin) { )); } - return name.toString(); + return new NameRecord(key, name.toString(), false); } public static float executeConfig(CharConfig conf, String key) { From 0ee4dfdcec2a3dd0e3313ed7857064bd8e79a223 Mon Sep 17 00:00:00 2001 From: caucow Date: Fri, 8 May 2020 14:22:20 -0500 Subject: [PATCH 2/4] Add force-mark as a config option "force_mark_all_renames" (def: false) --- .../programmerdan/minecraft/wordbank/WordBankConfig.java | 6 ++++++ .../minecraft/wordbank/actions/ActionListener.java | 5 +---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/programmerdan/minecraft/wordbank/WordBankConfig.java b/src/main/java/com/programmerdan/minecraft/wordbank/WordBankConfig.java index 24772cc..a67deb6 100644 --- a/src/main/java/com/programmerdan/minecraft/wordbank/WordBankConfig.java +++ b/src/main/java/com/programmerdan/minecraft/wordbank/WordBankConfig.java @@ -33,6 +33,7 @@ public class WordBankConfig { private boolean fail_rename_on_db_error; private int namecache_invalidate_minutes; private int namecache_max_size; + private boolean force_mark_all_renames; public WordBankConfig(ConfigurationSection config) throws InvalidPluginException { this(config, null); @@ -78,6 +79,7 @@ public WordBankConfig(ConfigurationSection config, WordBank plugin) throws Inval this.fail_rename_on_db_error = config.getBoolean("fail_rename_on_db_error", true); this.namecache_invalidate_minutes = config.getInt("namecache_invalidate_minutes", 5); this.namecache_max_size = config.getInt("namecache_max_size", 500); + this.force_mark_all_renames = config.getBoolean("force_mark_all_renames", false); // dbconfig this.db_config = null; @@ -173,4 +175,8 @@ public int getNamecacheInvalidateMinutes() { public int getNamecacheMaxSize() { return namecache_max_size; } + + public boolean getForceMarkAllRenames() { + return force_mark_all_renames; + } } diff --git a/src/main/java/com/programmerdan/minecraft/wordbank/actions/ActionListener.java b/src/main/java/com/programmerdan/minecraft/wordbank/actions/ActionListener.java index d6afed8..a49df77 100644 --- a/src/main/java/com/programmerdan/minecraft/wordbank/actions/ActionListener.java +++ b/src/main/java/com/programmerdan/minecraft/wordbank/actions/ActionListener.java @@ -1,9 +1,6 @@ package com.programmerdan.minecraft.wordbank.actions; import com.programmerdan.minecraft.wordbank.NameRecord; -import java.sql.Connection; -import java.sql.PreparedStatement; -import java.sql.SQLException; import java.util.ArrayList; import java.util.HashMap; import java.util.Map; @@ -180,7 +177,7 @@ ChatColor.WHITE, plugin().config().getMakersMark(), // force=true for... logging purposes? to the database for some reason? // why does it need player UUID and item type just to keep a unique key/value? Bukkit.getScheduler().runTaskAsynchronously(plugin(), () -> { - newNameRecord.mark(plugin(), event.getPlayer().getUniqueId().toString(), item.getType().toString(), true); + newNameRecord.mark(plugin(), event.getPlayer().getUniqueId().toString(), item.getType().toString(), plugin().config().getForceMarkAllRenames()); }); } catch (Exception e) { plugin().logger().log(Level.WARNING, "Something went very wrong while renaming", e); From fd0ca8a3e7613f98d8b3a0a1c66434d21a257fab Mon Sep 17 00:00:00 2001 From: caucow Date: Sat, 9 May 2020 02:39:27 -0500 Subject: [PATCH 3/4] Bugfix + predictable switch-item behavior + rename recent config option With the async code put in place of the old non-async code, the player would be charged the cost of a wordbank item even if the new name hadn't finished loading yet. That's been fixed. The unintentional cooldown on wordbank usage while a player waits for a name to load, except sometimes not if player switches items, that's been made more predictable now: if a player queues a name then tries to queue another name without applying the first name, it'll warn them first and either prevent them from loading the new name before the last name loads, or allow them to queue the next name load regardless of whether the last finished loading or not. Configurable with "prevent_dblookup_spam", defaults to true, preventing players from queueing second+ name lookups until the first has finished or the hit-to-confirm timer ends. The config option added in the last commit "force_mark_all_renames" has been renamed to "dblog_all_item_marks" following crimeo's (very) valid criticism of how awful and confusing the old name was. The in-game error messages were also changed to red to be more noticeable. --- .../minecraft/wordbank/WordBankConfig.java | 22 ++++- .../wordbank/actions/ActionListener.java | 81 ++++++++++++------- 2 files changed, 72 insertions(+), 31 deletions(-) diff --git a/src/main/java/com/programmerdan/minecraft/wordbank/WordBankConfig.java b/src/main/java/com/programmerdan/minecraft/wordbank/WordBankConfig.java index a67deb6..6b88ae8 100644 --- a/src/main/java/com/programmerdan/minecraft/wordbank/WordBankConfig.java +++ b/src/main/java/com/programmerdan/minecraft/wordbank/WordBankConfig.java @@ -33,7 +33,8 @@ public class WordBankConfig { private boolean fail_rename_on_db_error; private int namecache_invalidate_minutes; private int namecache_max_size; - private boolean force_mark_all_renames; + private boolean dblog_all_item_marks; + private boolean prevent_dblookup_spam; public WordBankConfig(ConfigurationSection config) throws InvalidPluginException { this(config, null); @@ -79,7 +80,16 @@ public WordBankConfig(ConfigurationSection config, WordBank plugin) throws Inval this.fail_rename_on_db_error = config.getBoolean("fail_rename_on_db_error", true); this.namecache_invalidate_minutes = config.getInt("namecache_invalidate_minutes", 5); this.namecache_max_size = config.getInt("namecache_max_size", 500); - this.force_mark_all_renames = config.getBoolean("force_mark_all_renames", false); + // Before the change to use async load/generate, every time a player + // used wordbank to generate a name for an item, it added a new entry to + // the database regardless of whether an entry already existed with that + // wbkey. Setting this to true will keep that old behavior (for... + // counting number of times a name is used or something? idk) + this.dblog_all_item_marks = config.getBoolean("dblog_all_item_marks", false); + // Set to true if players are spamming wordbank too fast and slowing the + // database. Defaulting this to false for now since it *shouldn't* really + // be a big problem, but leaving the option there. + this.prevent_dblookup_spam = config.getBoolean("prevent_dblookup_spam", true); // dbconfig this.db_config = null; @@ -176,7 +186,11 @@ public int getNamecacheMaxSize() { return namecache_max_size; } - public boolean getForceMarkAllRenames() { - return force_mark_all_renames; + public boolean isDBLogAllItemMarks() { + return dblog_all_item_marks; + } + + public boolean isPreventDBLookupSpam() { + return prevent_dblookup_spam; } } diff --git a/src/main/java/com/programmerdan/minecraft/wordbank/actions/ActionListener.java b/src/main/java/com/programmerdan/minecraft/wordbank/actions/ActionListener.java index a49df77..adee797 100644 --- a/src/main/java/com/programmerdan/minecraft/wordbank/actions/ActionListener.java +++ b/src/main/java/com/programmerdan/minecraft/wordbank/actions/ActionListener.java @@ -37,7 +37,7 @@ public class ActionListener implements Listener { private WordBank plugin; - private HashMap pendingMarks; + private HashMap pendingMarks; public ActionListener() { this(null); @@ -87,7 +87,7 @@ public void TableTouch(PlayerInteractEvent event) { Inventory pInv = event.getPlayer().getInventory(); if (pInv.containsAtLeast(plugin().config().getCost(), plugin().config().getCost().getAmount())) { final UUID puid = event.getPlayer().getUniqueId(); - BukkitTask firstHit = pendingMarks.get(puid); + PendingMarkTask firstHit = pendingMarks.get(puid); // moved this out of if/else so the proper length name is used in // both code blocks rather than just one (required by cache) @@ -106,14 +106,9 @@ public void TableTouch(PlayerInteractEvent event) { event.getPlayer().sendMessage(String.format("Hit the table a second time in the next %d seconds to confirm renaming using %s.", confirmDelay / 1000l, unmodifiedName)); - pendingMarks.put(puid, - new BukkitRunnable() { - @Override - public void run() { - pendingMarks.remove(puid); - } - }.runTaskLater(plugin(), confirmDelay / 50l) // convert to ticks - ); + PendingMarkTask task = new PendingMarkTask(puid, curName); + task.runTaskLater(plugin(), confirmDelay / 50l); // convert to ticks + pendingMarks.put(puid, task); // Schedule ASYNC task to load and cache the mapped name // Do it now so it'll be ready when the player clicks again @@ -121,7 +116,10 @@ public void run() { final String finalCurName = curName; Bukkit.getScheduler().runTaskAsynchronously(plugin(), () -> { try { - System.out.println(plugin().nameCache().get(finalCurName).value); + NameRecord record = plugin().nameCache().get(finalCurName); + if (plugin().config().isDebug()) plugin().logger().log( + Level.INFO, " - Used key {0} to load/generate {1}", + new Object[]{record.key, record.value}); } catch (ExecutionException e) { } @@ -131,6 +129,31 @@ public void run() { event.setCancelled(true); return; } else { + NameRecord newNameRecord = plugin().nameCache().getIfPresent(firstHit.key); + // Original queued name does NOT match current item's name + if (!curName.equals(firstHit.key)) { + if (newNameRecord == null && plugin().config().isPreventDBLookupSpam()) { + event.getPlayer().sendMessage(String.format("%sThat doesn't match the name you queued. %sWait for the queued name %sto finish loading before trying another name.", ChatColor.RED, ChatColor.GRAY, ChatColor.RED)); + } else { + event.getPlayer().sendMessage(String.format("%sThat doesn't match the name you queued. %sClick again %sto queue the new name.", ChatColor.RED, ChatColor.GRAY, ChatColor.RED)); + firstHit.cancel(); + pendingMarks.remove(firstHit.puid); + } + event.setCancelled(true); + return; + } + if (newNameRecord == null) { + event.getPlayer().sendMessage(String.format("%sThat name is still loading (or there was an error), wait a few seconds.", ChatColor.RED)); + event.setCancelled(true); + return; + } else { + // Move the task cancel here so spamming wordbank + // doesn't spawn crap tons of threads all blocking + // while waiting for the first cache load + firstHit.cancel(); + pendingMarks.remove(puid); + } + HashMap incomplete = pInv.removeItem(plugin().config().getCost()); if (incomplete != null && !incomplete.isEmpty()) { if (plugin().config().isDebug()) plugin().logger().info(" - lacks enough to pay for it"); @@ -144,22 +167,8 @@ public void run() { try { if (plugin().config().isDebug()) plugin().logger().info(" - Paid and updating item"); - NameRecord newNameRecord = plugin().nameCache().getIfPresent(curName); - - if (newNameRecord == null) { - event.getPlayer().sendMessage(String.format("%sThat name is still generating (or there was an error), wait a few seconds.", ChatColor.WHITE)); - event.setCancelled(true); - return; - } else { - // Move the task cancel here so spamming wordbank - // doesn't spawn crap tons of threads all blocking - // while waiting for the first cache load - firstHit.cancel(); - pendingMarks.remove(puid); - } - if (plugin().config().isDebug()) plugin().logger().log( - Level.INFO, " - Using key {0} to generate {1}", + Level.INFO, " - Used key {0} to generate {1}", new Object[]{curName, newNameRecord.value}); meta.setDisplayName(newNameRecord.value); @@ -177,7 +186,7 @@ ChatColor.WHITE, plugin().config().getMakersMark(), // force=true for... logging purposes? to the database for some reason? // why does it need player UUID and item type just to keep a unique key/value? Bukkit.getScheduler().runTaskAsynchronously(plugin(), () -> { - newNameRecord.mark(plugin(), event.getPlayer().getUniqueId().toString(), item.getType().toString(), plugin().config().getForceMarkAllRenames()); + newNameRecord.mark(plugin(), event.getPlayer().getUniqueId().toString(), item.getType().toString(), plugin().config().isDBLogAllItemMarks()); }); } catch (Exception e) { plugin().logger().log(Level.WARNING, "Something went very wrong while renaming", e); @@ -239,4 +248,22 @@ public void ItemPrevention(PrepareAnvilEvent event) { result.setItemMeta(resultMeta); } } + + private class PendingMarkTask extends BukkitRunnable { + public final UUID puid; + public final String key; + + public PendingMarkTask(UUID puid, String key) { + this.puid = puid; + this.key = key; + } + + @Override + public void run() { + PendingMarkTask task = pendingMarks.get(puid); + if (this == task) { + pendingMarks.remove(puid); + } + } + } } From 0fea161bd12466ba6cd33470a59512376665de72 Mon Sep 17 00:00:00 2001 From: caucow Date: Tue, 12 May 2020 05:03:07 -0500 Subject: [PATCH 4/4] Remove config comment + add new options to config.yml. --- .../programmerdan/minecraft/wordbank/WordBankConfig.java | 3 +-- src/main/resources/config.yml | 7 +++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/programmerdan/minecraft/wordbank/WordBankConfig.java b/src/main/java/com/programmerdan/minecraft/wordbank/WordBankConfig.java index 6b88ae8..2bdfbdd 100644 --- a/src/main/java/com/programmerdan/minecraft/wordbank/WordBankConfig.java +++ b/src/main/java/com/programmerdan/minecraft/wordbank/WordBankConfig.java @@ -87,8 +87,7 @@ public WordBankConfig(ConfigurationSection config, WordBank plugin) throws Inval // counting number of times a name is used or something? idk) this.dblog_all_item_marks = config.getBoolean("dblog_all_item_marks", false); // Set to true if players are spamming wordbank too fast and slowing the - // database. Defaulting this to false for now since it *shouldn't* really - // be a big problem, but leaving the option there. + // database. this.prevent_dblookup_spam = config.getBoolean("prevent_dblookup_spam", true); // dbconfig diff --git a/src/main/resources/config.yml b/src/main/resources/config.yml index 175fde3..f55a0d8 100644 --- a/src/main/resources/config.yml +++ b/src/main/resources/config.yml @@ -9,6 +9,13 @@ padding: ' add_serial: false makers_mark: "Marked Item" debug: false +namecache_invalidate_minutes: 5 +namecache_max_size: 500 +prevent_dblookup_spam: true +# Generally leave this false to keep the database clean +dblog_all_item_marks: false +# Leave this true to prevent wordlist changes from breaking old renames +fail_rename_on_db_error: true db: driver: mysql host: localhost