This is a discussion topic for the Ore project, PlaceholderAPI. View the full project on Ore for downloads and more information.
PlaceholderAPI
If you were sent here from another plugin, simply download this plugin and install it in your mods folder. After the first start of this plugin, you will need to enable the default expansions in the config file to be able to use those placeholders. Example if you want Player placeholders, open the config and set Player to True. Now you can use %player_name% as a placeholder. This same method applies for every built in expansion.
There is a chance that a plugin may update and cause one of the internal placeholder hooks to break. If this happens, please report the issue immediately so we can update the placeholder hook to the latest version of the plugin that broke.
FEATURES
No lag
Fast updates
Easy contribution
Robust placeholder replacing with support for TextTemplates and Strings
bStats support (You can opt out of this in the /config/bStats/config.conf file if you want)
Planned
Vast plugin support (I’ll try to create PR’s to help the process)
USAGE
Server Owners
Just drop this plugin into your mods folder and enable any default expansions you want to use in the config file.
Doing a bit of a @pie_flavor here, but I’ve seen something in this plugin that I’ve seen in others and I really want to ask now. I realise this might not be your original structure!
It’s about your Configurables and Configs classes. It’s a structure that doesn’t add anything or tries to reinvent the wheel. For example:
I don’t think you need to use Files.createFile(configFile); in your “setup” method. In fact, surely it would be better to either pull a default config file using the Asset API, or to use Configurate’s configNode.mergeValuesFrom(CommentedConfigurationNode defaults) to create your initial defaults - these are much better solutions. You could then do away with setup and populate, and either (with the second being my preference):
Check if the file exists, if not copy from the Asset API, or;
Load like normal, but have a CommentedConfigurationNode that holds your default config, so you could do (with try/catch omitted for brevity):
public void load() {
configNode = configLoader.load();
configNode.mergeValuesFrom(defaultConfig);
configLoader.save(configNode);
}
In this way, this would allow you to drop both setup() and populate(). You could also just then use Sponge’s injected ConfigurationLoader<CommentedConfigurationNode> to allow Sponge to select the correct file so you don’t have to do the legwork and pass that into this file.
(I think that code would just about work, though it’s more the point I’m trying to get across)
If you get an exception on setup() or load(), you swallow the exception, but then the node is null if this is the first access of the class, so by accessing the config, you’ll end up creating a load of NullPointerExceptions.
What does having a static Configs class give you? All it does it operate on a Configurable class that you have control over (and must possess). It breaks the Object Oriented paradigm.
getConfig(Configurable) and saveConfig() just call public methods on your Configurable that you should just call directly.
All the other methods could (and probably should) just go on the Configurable, I can see “set value and save” being useful, though I would argue that it doesn’t allow you to interact with more advanced features of Configurate.
However, rather than using strings throughout your plugin, which will be a nightmare to change if you update the config, why not have methods on your Config class that get and set the values you use in your plugin? That way, if you change your config file, you just need to change the one class, limiting the work you have to do.
Also, not that it matters so much, but HOCON convention is that config keys are lowercase and dash separated, so ConfigVersion should be config-version, but that is up to you.
Please note, this is not otherwise intended as a critique of your plugin itself, it’s just on the config file handling - again, I know it’s mostly copy/paste code, because I’ve seen it on other plugins (I think EssentialCmds had it). I’ve never understood what it offers, other than a few static methods that should just be on the Configurable itself, and I’m interested to know what it does offer.
@KingGoesGaming I copied the post from TitleMOTD, thank you for pointing that out!
@dualspiral Yes, I copied the Config setup from Pollis back when I was still teaching myself Sponge and I just haven’t gotten around to changing it. These tips and methods you talk about sound much easier in the long term use. I’ll try to set that up and learn how you used it in Nucleus.
Do have a look, but I’d be careful about copying Nucleus! Nucleus is a big plugin that has some more advanced stuff in so that I can focus on writing commands, listeners etc. and not worry about boilerplate code. Don’t copy the main config, that’s setup to work with the modular structure, same with the commands.conf file, and so I do some things that might not be necessary for simpler plugins! You might find that the Service classes might give you some insight as to how something a little simpler might work, though!
I would always take my designs with a pinch of salt too. Some of my designs come from when I worked as a professional software engineer, I worked with someone who made massive use of reflection and other tricks for things like this so that while there was a big cost (in time) up front, things would just snap in later - and I now have a similar mindset! So, make sure you keep that in mind too, my way isn’t necessarily the best way for you.
You may want to look at for instance how I handle configs. Check out my project Boop for an example. This can be plucked from the config with a single call to .get(Config.type), and then you never have to worry about parsing, or type problems, or anything - the ObjectMappingException handles everything.
Also, adding on to what @dualspiral said, if you create the loader yourself, you miss out on being able to map stuff like ItemTypes or Texts automatically.
That configurable was added by me in essentialcmds and hassan used it in his other projects from there on… Looking back on it I agree with @dualspiral on this.
@pie_flavor I’m using your Config setup from boop. It looks like it’ll work perfectly for what I need, but I’m running into an issue.
Do you know what’s wrong? Latest code on GitHub
EDIT I don’t REALLY know what I did, but I fixed it.
[Sponge]: Could not pass FMLPreInitializationEvent to Plugin{id=placeholderapi, name=placeholderapi, source=D:\Minecraft Plugins\Sponge Plugins\PlaceholderAPI\bin}
java.util.NoSuchElementException: No value present
at java.util.Optional.get(Unknown Source) ~[?:1.8.0_74]
at me.rojo8399.placeholderapi.PlaceholderAPIPlugin.onGamePreInitializationEvent(PlaceholderAPIPlugin.java:65) ~[PlaceholderAPIPlugin.class:?]
at org.spongepowered.common.event.listener.GamePreInitializationEventListener_PlaceholderAPIPlugin_onGamePreInitializationEvent6.handle(Unknown Source) ~[?:?]
at org.spongepowered.common.event.RegisteredListener.handle(RegisteredListener.java:95) ~[RegisteredListener.class:?]
at org.spongepowered.mod.event.SpongeModEventManager.post(SpongeModEventManager.java:301) [SpongeModEventManager.class:?]
at org.spongepowered.mod.event.SpongeModEventManager.post(SpongeModEventManager.java:330) [SpongeModEventManager.class:?]
at org.spongepowered.mod.SpongeMod.onStateEvent(SpongeMod.java:201) [SpongeMod.class:?]
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_74]
at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_74]
at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_74]
at java.lang.reflect.Method.invoke(Unknown Source) ~[?:1.8.0_74]
at com.google.common.eventbus.EventSubscriber.handleEvent(EventSubscriber.java:74) [guava-17.0.jar:?]
at com.google.common.eventbus.SynchronizedEventSubscriber.handleEvent(SynchronizedEventSubscriber.java:47) [guava-17.0.jar:?]
at com.google.common.eventbus.EventBus.dispatch(EventBus.java:322) [guava-17.0.jar:?]
at com.google.common.eventbus.EventBus.dispatchQueuedEvents(EventBus.java:304) [guava-17.0.jar:?]
at com.google.common.eventbus.EventBus.post(EventBus.java:275) [guava-17.0.jar:?]
at net.minecraftforge.fml.common.LoadController.sendEventToModContainer(LoadController.java:243) [LoadController.class:?]
at net.minecraftforge.fml.common.LoadController.propogateStateMessage(LoadController.java:221) [LoadController.class:?]
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_74]
at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_74]
at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_74]
at java.lang.reflect.Method.invoke(Unknown Source) ~[?:1.8.0_74]
at com.google.common.eventbus.EventSubscriber.handleEvent(EventSubscriber.java:74) [guava-17.0.jar:?]
at com.google.common.eventbus.SynchronizedEventSubscriber.handleEvent(SynchronizedEventSubscriber.java:47) [guava-17.0.jar:?]
at com.google.common.eventbus.EventBus.dispatch(EventBus.java:322) [guava-17.0.jar:?]
at com.google.common.eventbus.EventBus.dispatchQueuedEvents(EventBus.java:304) [guava-17.0.jar:?]
at com.google.common.eventbus.EventBus.post(EventBus.java:275) [guava-17.0.jar:?]
at net.minecraftforge.fml.common.LoadController.redirect$onPost$zzd000(LoadController.java:53) [LoadController.class:?]
at net.minecraftforge.fml.common.LoadController.distributeStateMessage(LoadController.java:145) [LoadController.class:?]
at net.minecraftforge.fml.common.Loader.preinitializeMods(Loader.java:615) [Loader.class:?]
at net.minecraftforge.fml.server.FMLServerHandler.beginServerLoading(FMLServerHandler.java:99) [FMLServerHandler.class:?]
at net.minecraftforge.fml.common.FMLCommonHandler.onServerStart(FMLCommonHandler.java:329) [FMLCommonHandler.class:?]
at net.minecraft.server.dedicated.DedicatedServer.init(DedicatedServer.java:124) [DedicatedServer.class:?]
at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:507) [MinecraftServer.class:?]
at java.lang.Thread.run(Unknown Source) [?:1.8.0_74]
@dualspiral@pie_flavor It took about 4 hours of debugging because I’ve never done a config like this, but I got it to work like @pie_flavor’s plugin “Boop”. Now, what are the advantages of doing it like this?
Automatic type-safety, automatic parsing, never having to interact with the actual file, small code, etc. You didn’t have to do the whole mapDefault thing, the point of the system is the extracting of the config to a class.
Ahh thank you.
I don’t know how the code work yet, but I at least know how to use it. I’ll keep it because I’m sure it must be better than @KingGoesGaming’s method I was using.
I don’t find myself generally changing configs, but you can setValue with a TypeToken the same as you can getValue with one. And of course you can always change it manually (for example, I never put my config version in the Config class).
How would I be able to use this for dynamic settings.
Say, from the code below I would want to get on of the biomes by names and then be able to get the settings for each biome.
I believe you misunderstand; the static field names are not the IDs. What you’re doing could be taken in a @Setting as a Map<BiomeType, BiomeSettings> where BiomeSettings is another @ConfigSerializable class with the correct fields. However, again, the static field names aren’t the IDs. The ID for every CatalogType is in fact the ID that is used in commands, or a debug screen, or whatever. So it’s cold_beach, desert, mutated_forest, etc. And icon can be an ItemType, but again, it’s the Minecraft item ID, not the Sponge static field name. So tallgrass, log, sapling, etc. The minecraft: on the name is assumed.
This may be slightly confusing; I’m guessing you come from Bukkit, where ItemType is an enum and therefore does not distinguish between field name and string ID. Take another look at Boop’s config, for example. The SoundType that I pull is entity.experience_orb.pickup. While the field name is SoundTypes.ENTITY_EXPERIENCE_ORB_PICKUP, the actual ID for retrieval is the Minecraft ID, created from resource pack structure and used in the /playsound command.
Would it be possible to allow plugins to insert their own placeholders into the plugin? After perusing your code, I noticed that you only operate on a switch statement for your internal “expansions”, however I think making that support any expansion, whether it is made by you or another author, would be better going forwards. Of course, this becomes problematic for configuration as you are not aware prior to loading as to what expansions the plugin will use, however you could always offer a configuration node to each expansion for loading purposes.
Another thing I would like to see is turning PlaceholderAPI into a service instead. That way, it can be retrieved through the service manager, and then can be used by plugins without making static calls (this method is a better practice and having people use the service API would help to set a good precedent as well). Your service could offer both an equivalent to setPlaceholders and a method to add your own placeholder.
One last suggestion, though this breaks your current API functionality, is supporting relational placeholders; for example, a sender and a recipient for a private message, or a message channel that uses different colors per recipient for certain parts of the message. Unfortunately, this would introduce the idea of nullable player arguments, which is likely something that is not going to be nice to deal with, but it could benefit plugins in the long run. This suggestion is more of an idea than a recommendation, however it is something I think developers could use for more dynamic plugins.
The way I have achieved iteration using this config setup would be using this: node.getChildrenMap()
The method returns a map of Object → ? extends ConfigurationNode. You can iterate over each with the entry set in the map.
For your example, using the root node of the file, you would call getNode for “biomes”, then get the map, then iterate. Your Object in the map would be the name of the biome; the first in the list being “COLD_BEACH”. Your node would then hold the information such as icon, friendlyname, description, etc. For the optional items in the configuration, make sure you use one of three options: check if the node is virtual, check if the returned object is null, or provide a default value when you call the getValue method.
One last little note: Even if you setValue on a config, there is no guarantee that it will be pushed to the file; The only way to ensure that the changes are saved to disk is to use the ConfigurationLoader you loaded the config from to save, using the “save” method. This is probably something you wouldn’t use on a real configuration file, since you can load defaults which is much more efficient.
He is in fact referring to my method of object mapping using a Config object instead of manually parsing values; if you read my post directly under his, I explain exactly what to do here.
Your first idea I want to implement. I was going to have that functionality in the plugin to begin with, but I couldn’t figure out how to do it.
I like your idea for making it a service instead. I’ve never messed with this, though. So I need to do some testing and tinkering to get it to work right.
I don’t think I get what you’re saying with this idea.
