Spigot FaucetAPI 0.4.2

A API that expands the functionality of Spigot.

  1. Sources submitted a new resource:

    FaucetAPI - Just a another API.

    Read more about this resource...
     
  2. Hi,

    if you have javadocs, you should also add javadocs for every method you have there and any benefits if you compare to default bukkit / spigot api.

    If any of these methods are just present to hide deprecation, its not okay, as you can either just ignore the deprecation or suppress it with an annotation.

    FaucetMath is not in any way kind of efficient. Object creation is a heavily task in java. As the java math class, make the methods static and provide two arguments instead of constructor arguments.

    FaucetBukkit does not need the methods getOnlineMode(), createInventory(), getPlayerExact(), getOfflinePlayer(), getWorld(), as Bukkit itself contains that method.

    Using the Logger by FaucetBukkit.getLogger() is totally not useful or in any kind being accepted, as you should use your plugins logger or if you don't want to, just use System.out.println() (however also not good)

    FaucetBukkit.explode() is also nothing special, you can already do world.createExplosion() with various arguments and settings.

    FaucetBukkit.getPluginsFolder() is also simply done by plugin.getDataFolder().

    Lightnings of any kind can be done through the spigot api world.spigot().strikeLightning[Effect](), while also basic lightning are possible with world.strikeLightning[Effect]()

    FaucetInventory seems to also provide no benefits like FaucetPlayer.

    FaucetPluginInfo provides some useful, but little shorthands, however no ability to get the original plugin from.

    Does FaucetPluginManager use the javadoc since and author tag? (Additionally such information missing on any other plugins class).
     
    • Agree Agree x 1
  3. @Janman14
    Hi,
    I will be providing javadocs for every single method in the release of 0.4 javadocs.

    Why not have methods that hide deprecation? I don't see any issues with doing that, because the methods I'm hiding the deprecation for are methods that aren't broken, they don't also risk the server's security.

    I feel the FaucetMath is a simple java math class, it is intended for users who are beginners in plugin development. I don't intend of using the method myself. I can just do:
    Code (Text):
    int myMathProblem = 5 * 5;
    . Probably in the future another class will be implemented for more efficient and advance calculations.

    The methods specified are methods that I put, so developers can use the FaucetBukkit class instead of the Bukkit class. I don't plan to remove these methods.

    I might remove FaucetBukkit.getLogger(), not sure. I'll wait a few releases first.

    The lightning and explode methods are just fun methods that I just put in there for aesthetics.
    Code (Text):
    FaucetBukkit.launchLightning(Bukkit.getPlayer("t").getLocation(), Bukkit.getPlayer("t").getWorld());
    instead of
    Code (Text):
    world.strikeLightning(location, world)
    I couldn't think of anything for FaucetInventory, that is why I need suggestions for methods.

    FaucetPluginManager hasn't been documented.

    Thanks, your feedback was essential for the release of 0.4.
     
  4. It's basically allows to warn your users for security vulnerabilities, such as online-mode is false.
     
  5. TigerHix

    Supporter

    There are quite a few problems that worth mentioning.

    org.faucet.FaucetBukkit
    • createInventory is redundant.
    • getLogger is redundant.
    • getOfflinePlayer is redundant.
    • getOnlineMode is redundant.
    • getPlayerExact is redundant.
    • getWorld is redundant.
    • searchForPlayersWithName is redundant. Bukkit.matchPlayer does totally the same.
    • setTimeInWorld is redundant. If you have a World object already, you certainly use world.setTime(long). Why bother importing and using an external class to do literally the same work? Breaks the SRP of OOP as well.
    • launchLightning breaks the SRP.
    • createFaucetInventory seems like a factory method for creating a FaucetInventory. But FaucetInventory, at the same time, has the exactly same constructor. I think it is rather redundant. And again, breaks the SRP.
    org.faucet.FaucetInventory
    • I don't see any reason to use your wrapper class. Your wrapper class is just copying methods from the original one. Not to mention how expensive it is for object creation, even this class is served as a static class, I still do not understand the objective of this class.
    org.faucet.FaucetPluginManager
    • Same problem as FaucetInventory.
    org.faucet.FaucetServer
    • This class only contains a method, getBukkitServer, which returns probably Bukkit.getServer. I am totally confused.
    org.faucet.Security
    • I just tried to run Security.runSecurityOperations(Security.SecurityType.MOJANGAUTH) in offline mode, and the logger prints:
    "Mojang Authentication is not enabled for your server!"​
    • What is the purpose of that? Does printing a message make the server more secure? Not to mention that the caller of the method cannot get any result. Result is printed in console to the end-user.
    • What is Security.SecurityType.INVALID for?
    org.faucet.entities.FaucetEntity
    • None since there are no methods contained in this class.
    org.faucet.entities.FaucetPlayer
    • It seems to me that forceFlight is just a redundant replacement for Player.setAllowFlight?
    • getLocation is redundant.
    • getPlayerWorldName() is redundant, and breaks the SRP.
    • setHealth is redundant.
    • setMaxHealth is redundant.
    • It seems to me that setPlayerMaxHealth is another redundant replacement for Player.setHealth(Player.getMaxHealth())?
    org.faucet.entities.FaucetMath
    • Sorry, but this class is beyond my comprehension.
    • 100% of beginners learn operators first. Even though they are not familiar with some operators such as bitwise operators, they know about +-*/.
    • Object creation is expensive. For every calculation, you need to create an Object.
    org.faucet.entities.FaucetPluginInfo
    • getPluginDescriptionFile is redundant. It returns Plugin.getDescription, right? Why use your wrapper at all?
    • getVersion is redundant and breaks the SRP.
    • getName is redundant and breaks the SRP.
    • getMainFilePath is redundant and breaks the SRP.
    • getWebsite is redundant and breaks the SRP.
    Miscellaneous
    • An API stands for Application Programming Interface. There is no standalone application here, therefore this is not an API at all. A better name would be a library, although I highly doubt such library's practicability.
    • A library should be open-source. This is your choice, though.
    • Providing a Maven dependency would be useful for most developers.
    • Most methods in this library is redundant. In fact, speaking in inverse, I didn't find any method that is useful for a developer. There are many possibilities of making a library. You can make a library for creating inventory UI. Or a library for creating custom mobs. Or a library for making a minigame. If you are not on that level yet, even a library with fluent Bukkit methods would be nice.
    • Methods that are deprecated do not affect performance, readability or development. For most of the time, you are simply creating wrapper classes to wrap a Bukkit object, and provide the same methods that just simply removed the @Deprecation annotation. Apart from the issue of being useless, most importantly, as I have mentioned above for many times, object creation does impact on your performance. And if you are having code like new FaucetInventory(bukkitInventory) everywhere, though you are unlikely to run into a problem as that is still neglectable to most modern computers, I would strongly recommend you to stop using it. Make a static class if you really need to wrap certain Bukkit classes.
    I can see the effort you have put into making this plugin. But for now, I don't think people will be using it if it is just a collection of redundant classes.

    EDIT: For a more simple explanation on what is SRP, it stands for Single Responsibility Principle, a concept of object-oriented programming. A class should only have one responsibility. If your class handles file I/O, networking, messaging, calculus and graphing at the same time, you are violating the principle. For example, FaucetServer class, as its name stated, should only be responsible for handing server related functions. Launching a lightning somewhere is definitely not the responsibility of this class.
     
    #10 TigerHix, Oct 19, 2015
    Last edited: Oct 19, 2015
  6. Sources updated FaucetAPI with a new update entry:

    API Patches 0.4.2

    Read the rest of this update entry...
     
  7. Why don't you just make pull requests and contribute to spigot itself? There's no need for this.
     
    • Agree Agree x 2
  8. Your example code is a bit inefficient: You call a method to get a player, when you already have that player..