Shopkeepers/TODO.txt

101 lines
8.5 KiB
Plaintext
Raw Normal View History

High priority:
*
Mid priority:
* For any commands allowing for a targeted shopkeeper: Print 'Ambiguous shopkeeper. Specify the shopkeeper explicitly instead.' error in case more than one shopkeeper is targeted?
* Instead of either only using the first shopkeeper, or applying the operation to all of them
Low priority:
* Make all config keys consistent: Avoid spaces (eg. 'owner uuid') and use hyphens rather than camelcase?
FallbackArgument delegates to another argument for its fallback implementation now. Moved some of the logic for determining which exception to use when handling fallbacks into Command and FallbackArgument itself. For the most part the behavior there is as before, but if a fallback failes due to missing an argument or requiring a player, the parsing exception of the original (root) argument is preferred. If the parsing context has changed (ag. if some of the following command arguments were able to parse something), the original argument may get reevaluated to determine the new error. Other command related changes: * Clarified CommandArgument#isOptional javadoc. * Added some notes and TODO entries on remaining issues with ambiguities. * Renamed FallbackArgumentException#getArgument to #getFallbackArgument. * Added FallbackArgument#getOriginalArgument. * Added MissingArgumentException and InvalidArgumentException (which ArgumentRejectedException extends) which allows specifically handling those types of exceptions. * Renamed CommandArgument#missingArgument and #invalidArgument to #missingArgumentError and #invalidArgumentError. * Added CommandArgument#requiresPlayerError, RequiresPlayerArgumentException and a corresponding default message (msg-command-argument-requires-player) for arguments that require a player as executor. * PostponedArgumentParseException wraps another exception now which gets thrown by the command in place of the postponed exception itself. This allows providing context about the type of the postponed exception. * Added PostponedArgumentParseException#getRootException. * Using an Optional to indicate whether an argument's parent has already been set vs whether it has no parent. * Making sure that the command argument parent isn't set to the argument itself. * Fixed parent validation in Command#addArgument. * Added: Validating that no arguments with the same name are added to the same command. Other changes: * Added Utils#concat() for arrays.
2019-10-17 06:28:53 -07:00
* German translation:
* Rename all occurrences of 'Shop/Shops' with 'Laden/Läden/Händler'?
* Command library revamp:
2019-11-18 12:04:24 -08:00
* Reverse the setup of arguments in parsing chains:
* Currently the parent argument delegates to child arguments, making the setup unnatural since fallbacks need to be specified first
* Instead, allow specifying the chain of arguments more naturally: argument.fallback(fallback).fallback(fallback) etc.
* Support advanced text features (eg. in helps page)
* Allow arguments to change the following arguments, similar to how subcommands have different chains of arguments
* Eg. /shopkeeper info ['own'|'admin'|<player>] <shopkeeper>: Different suggestions/allowed arguments for the second argument depending on the first argument
* But: Avoid duplicate definition of common argument chains, if different chains are not required/used
* Eg. by defining an argument chain once and then reusing it for different sub-commands/arguments
* Maybe even dynamically, by allowing arguments that get parameterized by the arguments parsed before
* Eg. /region <region> removeMember <member>: Only suggest members that are part of the region
* Bind executing code directly to an argument chain, to avoid having to manually check the CommandContext for which arguments got actually parsed
* Allow for easier debugging of command execution and argument parsing (without introducing plugin-specific logging dependencies into the library classes).
FallbackArgument delegates to another argument for its fallback implementation now. Moved some of the logic for determining which exception to use when handling fallbacks into Command and FallbackArgument itself. For the most part the behavior there is as before, but if a fallback failes due to missing an argument or requiring a player, the parsing exception of the original (root) argument is preferred. If the parsing context has changed (ag. if some of the following command arguments were able to parse something), the original argument may get reevaluated to determine the new error. Other command related changes: * Clarified CommandArgument#isOptional javadoc. * Added some notes and TODO entries on remaining issues with ambiguities. * Renamed FallbackArgumentException#getArgument to #getFallbackArgument. * Added FallbackArgument#getOriginalArgument. * Added MissingArgumentException and InvalidArgumentException (which ArgumentRejectedException extends) which allows specifically handling those types of exceptions. * Renamed CommandArgument#missingArgument and #invalidArgument to #missingArgumentError and #invalidArgumentError. * Added CommandArgument#requiresPlayerError, RequiresPlayerArgumentException and a corresponding default message (msg-command-argument-requires-player) for arguments that require a player as executor. * PostponedArgumentParseException wraps another exception now which gets thrown by the command in place of the postponed exception itself. This allows providing context about the type of the postponed exception. * Added PostponedArgumentParseException#getRootException. * Using an Optional to indicate whether an argument's parent has already been set vs whether it has no parent. * Making sure that the command argument parent isn't set to the argument itself. * Fixed parent validation in Command#addArgument. * Added: Validating that no arguments with the same name are added to the same command. Other changes: * Added Utils#concat() for arrays.
2019-10-17 06:28:53 -07:00
* Maybe define separate logging interface for the command lib, current the Log class is used directly
* The same applies for any other utilities and plugin-specific stuff
* Dealing with ambiguities:
* Example: /list [player (default:sender)] [page (default:1)])
* "/list 123" ("123" is a valid player name!)
* Also: "/list 123 2" would currently try to parse "123" as page number and then print an error due to the unexpected "2" due to the fallback mechanic used for the player argument
* Warning if command/arguments allow for ambiguity? Eg. by letting arguments provide a list of examples and evaluating all possible combinations
* Parse all possible argument assignments and print a warning if the input is ambigous?
* Allow resolving ambiguities by explicitly binding arguments, eg. "/list player=blablubbabc page=3"
* or '::' or ':=' or '!='? '=' might likely be used/useful in various arguments as well, and ':' alone is ambiguous because the use in namespaces
* or "/list -player blablubbabc -page 2" ? or "/list !player=blablubbabc !page=2"?
* or "/list --player blablubbabc --page 2"
* but: requires users to know the exact argument name: issue when arguments can have alises? (eg. for literals, especially if the command format uses an alias instead of the actual argument name)
* also: what about sub-arguments? Is it required to be able to explicitly specify which sub-argument an arg is meant to bind to? sub-arguments are usually an implementation detail
* or: "/list '' '2'" (quotes contain the arg(s) that get bound to specific arguments; including marking empty arguments explicitly)
* or: use some separator that marks which arg(s) get bound to which arguments, eg.: "/list | | 2"
* or: use special character/marker to explicitly mark missing arguments: "/list ! 2", or "/list _ 2"
* However: Does not allow resolving all kinds of ambiguities! eg. if arguments parse multiple args: "/cmd [multiple args (here] and there)" ('here' may be the last arg of the first argument, or the first arg of the second argument)
* -> Special character marks 'end of current argument': Allows marking missing arguments, as well as using it as argument delimiter
FallbackArgument delegates to another argument for its fallback implementation now. Moved some of the logic for determining which exception to use when handling fallbacks into Command and FallbackArgument itself. For the most part the behavior there is as before, but if a fallback failes due to missing an argument or requiring a player, the parsing exception of the original (root) argument is preferred. If the parsing context has changed (ag. if some of the following command arguments were able to parse something), the original argument may get reevaluated to determine the new error. Other command related changes: * Clarified CommandArgument#isOptional javadoc. * Added some notes and TODO entries on remaining issues with ambiguities. * Renamed FallbackArgumentException#getArgument to #getFallbackArgument. * Added FallbackArgument#getOriginalArgument. * Added MissingArgumentException and InvalidArgumentException (which ArgumentRejectedException extends) which allows specifically handling those types of exceptions. * Renamed CommandArgument#missingArgument and #invalidArgument to #missingArgumentError and #invalidArgumentError. * Added CommandArgument#requiresPlayerError, RequiresPlayerArgumentException and a corresponding default message (msg-command-argument-requires-player) for arguments that require a player as executor. * PostponedArgumentParseException wraps another exception now which gets thrown by the command in place of the postponed exception itself. This allows providing context about the type of the postponed exception. * Added PostponedArgumentParseException#getRootException. * Using an Optional to indicate whether an argument's parent has already been set vs whether it has no parent. * Making sure that the command argument parent isn't set to the argument itself. * Fixed parent validation in Command#addArgument. * Added: Validating that no arguments with the same name are added to the same command. Other changes: * Added Utils#concat() for arrays.
2019-10-17 06:28:53 -07:00
* Also allow for multi-part arguments: "/text text='some text'"
* And empty/missing argument: "/list player='' 1" (player argument is optional)
* Problem: FirstOf arguments with literals can internally have ambiguities as well, eg. "/list <'all'|player>" (with an existing player named 'all')
* This is only a problem if the sub arguments are joined to a single name in the argument format (which unusual if literal arguments are involved)
* Allow quoted string parsing "'some text'" (accepting "" and '' and ``)
* Also: "{some: map, like: data}"
2019-08-31 07:20:33 -07:00
* Prohibit multiple shopkeepers using the same chest (currently only possible by manually editing the save file)?
* Or make this an official 'feature'? (Allowing players to link multiple of their shops to the same chest.
* Maybe together with the ability to link shopkeepers to multiple chests (because otherwise this might not make much sense, since a single chest might be quite limiting if used by multiple shops)
* More types of block shops? -> clicking button to open shop
* Virtual shops (command to edit and open trade menu), would allow tons of possibilities for other plugins / server admins to use shopkeeper shops in other contexts
Major rewrite of the processing of trades: Overview over (unrelated) changes performed during the rewrite, which ended up in the same commit now: * Fix: All inventory operations are now limited to storage contents. This might fix issues with hiring cost items being removed from armor or extra slots. * Improvement: Normal player shops are now slightly smarter when figuring out whether the chest has enough space for the traded currency items, by allowing a variable ratio between high and low currency items. * Internal/Fix: All general inventory operations now work on content arrays (inventory snapshots) instead of the inventories directly. (This might even fix a few severe issues, which were unnoticed so far..) * Internal: Minor javadoc improvements. * Internal: Removed a few redundant final modifiers. * Internal: Added Settings#isHighCurrencyEnabled() and using that now everywhere. * Internal: Settings#isHighCurrencyItem() now returns false if high currency isn't enabled. * Internal: Minor refactoring related to setting up the editor window and handling of editor clicks. * Internal: Updated the TODO list and added a few more ideas. * Internal/Improvement: Buying shopkeepers are now slightly smarter when removing currency items from the chest: It prioritizes low currency items over high currency items, partial stacks over full stacks, and is able to split more than one large currency item and return the excess change to the chest (previously it only converted at most 1 high currency item). This should help keeping the chest more compact. * Internal: Added a debug message for every trade processed by Shopkeepers. * Fix: The purchase log was missing the header row since a previous commit (35bbfc0bc01ba41a6792449fecca3a664dd6f111). (Also forgot to mention on that previous commit: The logged data now contains the players uuid as well, in parentheses inside the 'PLAYER' column.) * Improvement: The collect-to-cursor inventory action is now allowed inside the trading window, as long as the result slot doesn't match the item on the cursor. Major rewrite of the processing of trades: Shopkeepers is now implementing the inventory actions that might trigger trades itself. Previously, trading with player shopkeepers only allowed simple left clicks of the result slot. We predicted the trading outcome (rather simple when only allowing left clicking..) and then let minecraft handle the click like normal. Any other inventory actions (ex. shift-clicks) were prevented, because so far we weren't able to predict the outcome of those. I finally took the time to dive into minecraft's and craftbukkit's internals to figure out, how it processes the various clicking types and resulting inventory actions, how those can trigger trades, and how minecraft processes those trades. Supporting more advanced inventory actions for trading requires at minimum being able to reliably predict the outcome of those inventory actions, and by that already requires re-implementing of large parts of the vanilla logic (which is one of the reasons why this wasn't done so far). The step to also apply those predicted inventory changes ourselves then isn't that much of an additional effort on top, but has a few advantages: For one, we can be quite sure that the predicted outcome of a trade actually matches what gets applied in the end. This should reduce the risk of inconsistencies between minecraft's behaviour and our predictions resulting in bugs (also with future minecraft updates in mind). Secondly, when relying on minecraft's implementation we are only able to allow or cancel the inventory action as a whole. The shift-clicking inventory action however is able to trigger multiple trades in a row (possibly even for different trades). By implementing this ourselves, we are able to apply as many trades as are possible with regards to available stock and chest capacity. Implementing the trading ourselves has a few advantages, but also comes with caveats: Caveats: * Increased maintenance and testing effort. * Increased possibility for inconsistencies between vanilla inventory/trading behaviour and our own implementation. * Minecraft performs some map initialization when certain map items are created by crafting or via trades: This seems to be responsible for updating the scaling (after crafting a larger map) and enabling tracking (this doesn't seem to be used anywhere inside minecraft though). Since this is only the case if the map item has some special internal nbt data (which shouldn't be obtainable in vanilla minecraft), we currently simply ignore this inside Shopkeepers. Neutral: * The usage count of the used trading recipes doesn't get increased anymore. But since these are only temporarily existing and aren't used for anything so far, this shouldn't be an issue. * The player's item crafting statistic doesn't get increased anymore for traded items. * The player's traded-with-villager statistic doesn't get increased anymore for shopkeeper trades. I could manually replicate some of these, but since these are custom trades anyways I don't see the need for it right now. But if there is a justified interest in this, let me know and I might add a config option for it. Advantages: * Support for more advanced inventory actions when trading with shopkeepers: * Currently supported are: Shift-clicking, and moving traded items to a hotbar slot. * Not (yet) supported are: * Dropping the traded item: Exactly reproducing the vanilla behaviour for dropping the traded item is actually rather tricky / impossible with pure bukkit API so far. * Trading by double-clicking an item in the player inventory: The behaviour of this inventory action is rather arbitrary in vanilla already (trades zero, one, or two times depending on the other items in the inventory), and it is affected by a bug (https://bugs.mojang.com/browse/MC-129515) * Simpler API: A single ShopkeeperTradeEvent is called for each trade (might be multiple per click depending on the used inventory action, ex. shift-clicking). The ShopkeeperTradeCompletedEvent was removed. (API breakage, sorry.) * Fixed: The logging of purchases should now actually be accurate for shift-clicks. It logs each trade individually. * Eventually this might allow for more options for customization of the trading behaviour in the future. Potential issues (that have existed before already): * If minecraft adds new inventory actions, those are initially not supported by shopkeepers: If those inventory actions involve clicking on the result slot, they will simply be prevented. If they however allow players to trigger trades by clicking on items inside the player's inventory (similar to the existing collect-to-cursor action by double-clicking), they might introduce bugs which players might be able to exploit. Other changes as consequence of the trade-handling changes: * Internal: Added a version-dependent function which matches items in the same way as minecraft does it: Minecraft allows the offered items to contain additional metadata and still accept them for the trade. * Internal/Fix: Minor refactoring during updating to the new trade handling. This might even fix a few small potential bugs/inconsistencies related to handling of high-currency items. * Internal: Due to the changes regarding the handling of trades, the trading count listener (added in a previous commit) is now solely used to detect trades with non-shopkeeper merchants (ex. vanilla villagers). This can be useful when comparing vanilla trading behaviour with shopkeeper's new trading behaviour. * The ShopkeeperTradeEvent now directly provides access to the items offered by the player and the order in which they were matched to the used trading recipe.
2018-05-17 15:26:05 -07:00
* Change amount-per-click from 10 to 8 when clicking items in the player shopkeeper editor? (feels more intuitive due to minecraft's stack sizes)
* Compress currency items in the chest (low currency to high currency)? To maximize available storage capacity. This would also mean that the usage of the high-currency-min-cost setting would be limited to creating the trading recipes, and not be used when adding currency items to the shop chests.
* Maybe prevent any unrecognised types of clicks if running in compatibility mode? To reduce the risk of minecraft updates with new clicking actions causing issues.
2018-06-18 10:49:03 -07:00
2018-06-18 09:12:15 -07:00
* introduce separate editor window to be able to add new player editing options
2019-08-31 07:20:33 -07:00
* add an option to reposition shops: button pressed > window closes + message > player clicks a block > runs new/updated/smarter placement logic there, checks distance to chest, option (default true) to not allow it for shops that are not directly placed on top of the shop chest (because those were probably created via command and it is unclear whether players are meant to be able to reposition those shops)
* Or allow shops to be picked up (with all their data) inside an item (possible now via bukkit API? Limit on amount of data that can be stored?)
2018-06-18 09:12:15 -07:00
* Remove AbstractType#matches with aliases
* remove AbstractType#isEnabled() and instead dynamically register and unregister enabled/disabled types?
2018-06-18 10:49:03 -07:00
* might change the order of the types dynamically though.. determine the order differently, via config?
2019-08-31 07:20:33 -07:00
* set entity attributes (subtypes/look) before spawning the entity (avoids short flicker)
2018-06-18 09:12:15 -07:00
* rename registry#getShopkeeperByBlock() to getShopkeeperBySignBlock or similar?
* properly separate loading/unloading from activation/deactivation in debug messages/method names/etc
* More editor options for mob shopkeepers
* Allow equipping (and unequipping again) items to mobs
2019-05-08 15:40:50 -07:00
* Mobs riding other mobs?
* Enderman: carrying block
* Phantom: size
* Puffer fish: puff up state.
* Shulker: color
* Snowman: derp
* Tropical fish: color, pattern, pattern color
* Vex: charged state?
* Add individual config options for the different editor options?
* Allow changing the editor option button items
Update for MC 1.14 (Spigot 1.14-pre5) * Dropped support for MC 1.13. This version only supports MC 1.14. * If you are upgrading, make sure that you have successfully updated shopkeepers to 1.13 first. Migration from older MC are not supported and might not work. Reverting to older versions isn't supported either. * Removed pre 1.13 sheep color migration. * Villager shopkeepers: * Changed default profession from farmer to 'none'. * Priest villagers get converted to 'cleric' and blacksmiths become 'armorer'. Previous regular farmer villagers stay farmers (but they look different now). * Changed the items representing the villager professions in the editor. * Note: Wandering traders are not a villager sub-variant, but a different mob type. * Ocelot shopkeepers of cat types have been converted to corresponding cat shopkeepers. You might have to adapt your players' permissions, since cat and ocelot shopkeepers require different permissions now. With this migration players can end up with cat shopkeepers even though they don't have the permission to freshly create those if they wanted. The different cat types are represented by colored leather armor inside the editor. * Any settings affecting regular villagers will affect wandering traders as well now. There are no settings (for now?) to handle wandering traders differently. When preventing regular villagers from spawning, the trader llamas of the wandering trader are prevented from spawning as well. When hiring a wandering trader its leashed llama should get unleashed due to the removal of the wandering trader. * Added: Sign shops can now switch between different wood types. TODO: * This has not yet been tested. Spigot 1.14-pre5 seems to still have some other issues which prevent me from testing this currently. * New entities have not yet been tested. * Support for changing between different villager biome types would be nice.
2019-04-24 18:18:01 -07:00
# 1.14:
* Allow changing the sign text color?
2019-04-25 12:43:22 -07:00
* Set villager trades and test if they display trade-able items when the player holds corresponding items
Update for MC 1.14 (Spigot 1.14-pre5) * Dropped support for MC 1.13. This version only supports MC 1.14. * If you are upgrading, make sure that you have successfully updated shopkeepers to 1.13 first. Migration from older MC are not supported and might not work. Reverting to older versions isn't supported either. * Removed pre 1.13 sheep color migration. * Villager shopkeepers: * Changed default profession from farmer to 'none'. * Priest villagers get converted to 'cleric' and blacksmiths become 'armorer'. Previous regular farmer villagers stay farmers (but they look different now). * Changed the items representing the villager professions in the editor. * Note: Wandering traders are not a villager sub-variant, but a different mob type. * Ocelot shopkeepers of cat types have been converted to corresponding cat shopkeepers. You might have to adapt your players' permissions, since cat and ocelot shopkeepers require different permissions now. With this migration players can end up with cat shopkeepers even though they don't have the permission to freshly create those if they wanted. The different cat types are represented by colored leather armor inside the editor. * Any settings affecting regular villagers will affect wandering traders as well now. There are no settings (for now?) to handle wandering traders differently. When preventing regular villagers from spawning, the trader llamas of the wandering trader are prevented from spawning as well. When hiring a wandering trader its leashed llama should get unleashed due to the removal of the wandering trader. * Added: Sign shops can now switch between different wood types. TODO: * This has not yet been tested. Spigot 1.14-pre5 seems to still have some other issues which prevent me from testing this currently. * New entities have not yet been tested. * Support for changing between different villager biome types would be nice.
2019-04-24 18:18:01 -07:00
Ideas:
* Per-Trade/Shopkeeper settings, maybe via written books:<br>
-> by adding another row to the shopkeeper-editor inventory window each trade option and shopkeeper could have a slot for a written-book<br>
-> which could contain additional meta-data, per-trade/shopkeeper settings, which could be used (ex. by other plugins) to trigger certain actions when a specific trade is used <br>
* Maybe move shop options (like currently name, profession, etc.) into a separate inventory view to have additional space there<br>
* Add message to default zero-currency items explaining how to increase/decrease costs.
* Add zero-cost items in trading shopkeeper, with lore which explains how to setup the trade.
2018-08-21 16:30:45 -07:00
* Store shopkeeper data (save.yml) in smaller chunks? Maybe 1 file per world?
2020-07-21 16:39:27 -07:00
* Makes only sense for very large numbers of shops, with many trades -> TODO benchmark