Apparently, the /give command will drop an item "as" the target player, resulting in the player picking up the item if there is room in their inventory. This triggers a PlayerDropItemEvent, which MobArena catches and tries to cancel if the target player is in an arena. Furthermore, if the player is a spectator outside of the arena region, MobArena will force the player to leave the arena. This triggers the reward granting logic, so using the /give command as a reward has the potential of causing an infinite loop.
This commit introduces the idea of "leaving players", i.e. players in the process of leaving the arena. In the event of a dropped item from a player who is currently leaving an arena, the PlayerDropItemEvent is ignored, because it is assumed to be from the /give command. Note that this doesn't actually prevent normal PlayerDropItemEvents from causing a forced leave, since the player won't be in the process of leaving at that specific point.
This commit changes the naive behavior of the ConfigUtils methods addIfEmpty() and addMissingRemoveObsolete(). Instead of an almost guaranteed config-file write on every invocation of either method, writes only happen if the loaded configuration changes. The excessive writes result in long config reload times, and this change fixes that, effectively fixing the second part of #435.
To further the performance boost, the resource reads are cached in a map - this turns out to not be an issue for server plugin reloads, as a new ClassLoader instance is used to load the new set of plugins.
This means that the cleanup code will only run if there are players in the lobby, arena, or spectator area. This fixes the first part of #435 where the cleanup code is taking too long for large/many arenas.
Originally, the force end command was meant as a way to circumvent any condition keeping players from leaving the arena and cleaning it up. In retrospect, the main reason for using force end is to "get people out of there", but since there's plenty of stuff that can go wrong when a player leaves, this isn't really that helpful, as exceptions will just cause the command to break at the same point anyway.
With CommandThing and its parser, it is now possible to use commands anywhere a thing can be used. Commands are invoked as the console/server, and they support a single variable, the name of the recipient player.
Commands are give-only, meaning they will fail to be "taken" from players, and they cannot be "held" either. The idea of commands as things basically only makes sense in the context of rewards.
While it doesn't really make sense to have class prices be a Thing (or maybe it does?), this does get rid of the dirty "ItemStacks wit ID -29" hack for economy money, which will eventually break, when the upstream int-based ID API breaks.
With this commit, the Things API is introduced to the code base, but it is not yet used. It introduces the building blocks for an extensible architecture that supports custom parsers and custom "things". This should allow other developers simple, yet powerful hooks into the way MobArena handles class "equipment" and rewards. The basic skeleton includes parsers for ItemStacks and economy money, so it should be interchangeable with the current inner workings of the plugin.
The commit also adds an overload to the ItemParser that allows for a the method to fail silently. This is necessary to avoid false negatives in the log in case the ItemStackThingParser fails but a different parser succeeds.
The purpose of this is to actually commit to version control the jar(s) necessary to compile. It's not a best practice whatsoever, but if we can commit gradle wrappers, we can commit hard-to-incorporate binary dependencies.
As a nice side effect, it should be possible to run some basic CI tooling on the repository.
Since the announce() methods always required an Arena argument, and since the Messenger instance used was always acquired from an Arena instance, there really was no reason for the methods to exist on Messenger.
- Messenger no longer has a static nature and must be instantiated to be used. The prefix is provided in the constructor.
- MobArena instantiates a global Messenger in onEnable() with a prefix string from global-settings. This instance is used by anything that isn't arena-specific, as well as for arenas with no prefix.
- ArenaImpl instantiates a local Messenger in its constructor if, and only if, its arena-specific prefix setting is non-empty. Otherwise it uses the plugin's global instance.
This removes the logging capabilities from Messenger and replaces all references to them with proper logging via Bukkit's PluginLogger that all plugins have.
The blockList() call returns the actual List<Block> object in the explode event, which means it'll be shared between the the fake event and the original event. As a result, the call to blockList().clear() will clear the shared list, and the following call to blockList().addAll(fake.blockList()) results in trying to add the empty list to itself.
This commit makes sure to copy the original event's block list before sending it to the fake event.
Okay, "Fixes" would be lying, and that's terrible, but here's something to help you shrug it off: The timer module is a bit of a mess, and since there isn't actually anything "dangerous" about this, let's just calm our tits and focus on other stuff.
Before, these redundant calls didn't really do anything. Now they mess everything up, because setContents() doesn't set inventory contents only, but the entire inventory instead, because that's so super useful. Well done, boys!
#notsalty
Because it is clearly impossible to change JavaDocs to correctly describe what a method does, the method setContents() has now been changed to a useless, over-aggressive, inventory-wiping piece of junk that must be invoked before anything else. This commit ensures that armor contents are set after the setContents() method is called, overwriting the nulls that the setContents() method creates.
This changes the format from:
<effect>:<duration>:<amplifier>
to a (pressumably) more user-friendly format of:
<effect>:<amplifier>:<duration>
The reason for this swap is that the duration is more likely to be
wanted left at default than the amplifier. From previous user input
on the matter, it sounds like users generally want the bosses to have
"permanent" or "infinite duration" effects, and this change makes it
a little easier on them, not having to specify a duration.
Some of the config-file resets were fixed in a previous commit:
4330885d46
The resets fixed previously were ones where snakeyaml would throw a fit
due to tabs. Because an exception is thrown, the call to saveConfig()
is never reached, which means the config-file is never reset.
However, errors resulting from other illegal characters were not caught
because the built-in reloadConfig() method catches, prints and then
simply does nothing about any exceptions thrown, which means the actual
YamlConfiguration would be empty. Saving this empty file would result
in a reset, because MobArena, during initialization, sees missing bits
and pieces and tries to insert them. Thus, the resets were not an issue
with MobArena's attempt to keep the config-file tidy, but the reckless
handling of errors in the built-in config methods in JavaPlugin.
This commit completely overrides the methods in JavaPlugin that would
result in these resets, and the MobArena plugin class stores its own
reference to the config file and an associated FileConfiguration. When
the reloadConfig() and saveConfig() methods are called, the checked
exceptions are caught and rethrown as unchecked exceptions, causing the
plugin to crash when it needs to do so.
Andreas Troelsen