Question Recipes, reflection, testing; questions for the old ones

Discussion in 'Plugin Help/Development/Requests' started by sciolizer, May 23, 2015.

Thread Status:
Not open for further replies.
  1. Offline

    sciolizer

    Hi all,

    I just published my first plugin, and I wanted to share some questions that came up during development.

    http://dev.bukkit.org/bukkit-plugins/craftinomicon/

    The plugin iterates through all recipes and indexes them. The indexing is a somewhat expensive operation, about 1/4 of a tick, so it makes sense to build the index only once and then re-use it for the rest of the server's lifetime. So my first question is:

    1) When should I build the index?

    If I build the index when the plugin is enabled, then there's the possibility that the index will not include recipes from plugins loaded after mine. My current solution is to build the index when a player opens their craftinomicon, which is almost certainly going to happen after all of the plugins have been loaded. I can invalidate the cached index whenever a plugin is enabled or disabled, though I haven't implemented that yet. However, if there are plugins that add or remove recipes after they have initialized themselves (say, as the result of a player's action), my plugin will not catch that. Are there any plugins that do this? If so, is there something like a RecipeAddedEvent that I can hook into?

    When a player activates their craftinomicon, they are presented with a custom inventory screen, filled with all of the game's craftables and crafting ingredients. Clicking on an item shows recipes related to that item. I saw two obvious ways of capturing the InventoryClickEvent: either register a new listener every time the craftinomicon interface was opened, or register a single global listener which kept a map of all of the inventories. In both cases, I have to worry about memory leaks. In the first case, InventoryClickEvent.getHandlerList() could get arbitrarily large. In the second case, my map of all inventories could get arbitrarily large. There's a simple solution that works in both cases: subscribe to InventoryCloseEvent and cleanup after yourself. But the second way has an alternative solution: make the map of inventories be a weak hash map. My question for this one is pretty open-ended:

    2) How do you feel about weak hash maps? Good idea? Bad idea? How do you feel about constantly registering and un-registering event listeners? Good idea? Bad idea? Have an idea for a totally different design?

    I went for the weak hash map, although I keyed off of HumanEntity instead of the inventories themselves. So the inventories are longer lived, but there are fewer of them. Code is in MenuRegistry if you're interested.

    I like the weak hash map approach because I think it's harder to mess up (in a way that produces a memory leak). You may think that un-registering an event listener on InventoryCloseEvent is also pretty hard to mess up, but consider the following scenario: My plugin is installed on a server with another plugin. The other plugin also has an InventoryCloseEvent listener, priority MONITOR, but it breaks the rules about MONITOR and sometimes un-cancels the event. What happens if the other plugin's listener fires after mine?

    I glanced at the code for registering and un-registering event handlers. Registering uses reflection, and un-registering makes a linear pass through all of the handlers. Never bench-marked it, but I'm pretty sure my weak hash map implementation is faster. Probably doesn't matter, though, since inventory open and close events are relatively infrequent.

    On an unrelated note, when I was browsing through the sponge modding api, I found out they generate bytecode at runtime to call the event handlers instead of using the more expensive Method.invoke. Pretty cool! Except it completely screwed up my reloader plugin (WIP), which was using class loader magic at the time. Did the code generation design come from bukkit, or is that new?

    When the player mouse-overs an item in the craftinomicon, the lore of the item shows how many recipes that item has and how many recipes that item is used in. I want to put the same lore on the items in the player's personal inventory, but doing so makes me nervous, because I need to make sure that on InventoryCloseEvent I restore the metadata of all the items to exactly their prior state. That isn't necessarily hard (ItemMeta is Cloneable), but what happens if the server crashes while a player has the craftinomicon open? So I have a couple questions around this issue:

    3) Is there a way to show recipe counts to the player without manipulating their inventory?

    or

    4) Is there a safe way to temporarily manipulate a player's inventory?

    Currently the craftinomicon only does crafting and furnace recipes. Those two were easy to do, thanks to recipeIterator(). I'd like to index brewing recipes as well, though.

    5) Is there a way to iterate over all of the brewing recipes, either in the bukkit api or using a 3rd party plugin?

    I wandered a bit through the decompiled minecraft code, and I think I found the function that maps old potion damage values to new potion damage values. Unfortunately Class.forName("net.minecraft.server....") didn't work.

    6) What's the best way to get a reference to minecraft classes? Any gotchas to watch out for?

    I found a thread on abstracting with maven, but it seems like overkill for a single function.

    7) What's the best way to write unit and functional tests?

    I gave up on unit testing, because the bukkit api is full of static methods, and mocking static methods is annoying. For functional testing, I made a simple test runner that runs the tests on plugin enable, but only if the server is running on my laptop.

    Lastly, I encountered a few oddities in the recipeIterator(), which I had to make manually exclude from my index.

    - Purple dye can be crafted into 0 black banners?
    - Purple dye can be crafted into 0 leather caps?
    - Book and quill can be crafted into 0 enchanted books?
    - Leather cap can be crafted into 1 leather cap?
    - Gunpowder (with no icon?) can be crafted into 0 fireworks.

    8) Any idea what's going on with these strange recipes?

    Wow that was a mouthful. Maybe I should have been asking these questions as I was developing. :) Don't feel like you need to answer all of them in a single post. Or take it as a challenge, idk.

    Thanks,
    sciolizer
     
  2. Online

    timtower Administrator Administrator Moderator

    @sciolizer You could make a task on the main thread, will run after the server is done booting
     
  3. Offline

    sciolizer

    I didn't think of that! Of course, there remains the possibility that another plugin will try the same thing, in which case I have no guarantee that mine will run after theirs. Though it's definitely less likely that recipes will be created in a scheduled task. I'll give it a try. It should definitely simplify things a bit.
     
  4. Online

    timtower Administrator Administrator Moderator

Thread Status:
Not open for further replies.

Share This Page