#Creating a skill system
calm flare
I think you are. Let me draft you a quick proof of concept as to how I would approach this
Creating a skill system
You don't have to match the proof of concept 1:1, maybe you think your approach makes more sense. But I want to give you an idea of how maybe you could alternatively approach it and take some inspiration from that, maybe simplify your code a little bit
winged niche
Bit of an abstraction lesson too?
round magnet
no some inspiration would be good, this has been stumping me lol
calm flare
Yeah!
winged niche
Awesome I will stay tuned for sure
calm flare
public class SkillType {
private static final Collection<SkillType> ALL_SKILL_TYPES = new ArrayList<>();
public static final SkillType SWORDS = new SkillType("Swords", "Your ability to inflict damage with swords.");
public static final MiningSkillType MINING = new MiningSkillType("Mining", "Your ability to destroy blocks while mining for ores.");
private final String name;
private final String description;
public SkillType(String name, String description) {
this.name = name;
this.description = description;
ALL_SKILL_TYPES.add(this);
}
public String getName() {
return name;
}
public String getDescription() {
return description;
}
public void loadDataFromConfig(JavaPlugin plugin, ConfigurationSection config) {
// By default, do nothing!
}
public static Collection<SkillType> getAllSkillTypes() {
return Collections.unmodifiableCollection(ALL_SKILL_TYPES); // So it's not mutable
}
}
public final class MiningSkillType extends SkillType {
private final Map<Material, Integer> blockBreakExperience = new HashMap<>();
public MiningSkillType(String name, String description) {
super(name, description);
}
public void setBlockBreakExperience(Material material, int experience) {
this.blockBreakExperience.put(material, experience);
}
public int getBlockBreakExperience(Material material) {
return blockBreakExperience.getOrDefault(material, 0);
}
@Override
public void loadDataFromConfig(JavaPlugin plugin, ConfigurationSection config) {
ConfigurationSection materialsConfig = config.getConfigurationSection("Level settings.Materials");
if (materialsConfig == null) {
return;
}
for (String materialKey : materialsConfig.getKeys(false)) {
Material material = Material.matchMaterial(materialKey);
if (material == null) {
// Unknown material. Maybe send a logger message here
continue;
}
int experience = materialsConfig.getInt(materialKey);
if (experience > 0) {
this.setBlockBreakExperience(material, experience);
plugin.getLogger().info(String.format("Added material: %s xp: %s", material, experience));
}
}
}
}
public final class PlayerSkillData {
private static final class SkillData {
private int level;
private int experience;
public SkillData(int level, int experience) {
this.level = level;
this.experience = experience;
}
public static SkillData empty() {
return new SkillData(0, 0);
}
}
private final Map<SkillType, SkillData> skillData = new HashMap<>();
private final UUID playerUUID;
public PlayerSkillData(UUID playerUUID) {
// You may want to initialize some SkillData from a file or something... how you want to do this is up to you
this.playerUUID = playerUUID;
}
public UUID getPlayerUUID() {
return playerUUID;
}
public void setLevel(SkillType skill, int level) {
this.getSkillData(skill).level = Math.max(level, 0);
}
public int getLevel(SkillType skill) {
return getSkillData(skill).level;
}
public void setExperience(SkillType skill, int experience) {
this.getSkillData(skill).experience = Math.max(experience, 0);
}
public int getExperience(SkillType skill) {
return getSkillData(skill).experience;
}
private SkillData getSkillData(SkillType skill) {
return skillData.computeIfAbsent(skill, ignore -> SkillData.empty());
}
}
public final class TestPlugin extends JavaPlugin {
// This can either be static inside of PlayerSkillData, or make a SkillPlayer object, or make a PlayerSkillDataManager, whatever you want!
private final Map<UUID, PlayerSkillData> playerSkillData = new HashMap<>();
@Override
public void onEnable() {
// Load all your skill types on enable once
for (SkillType skillType : SkillType.getAllSkillTypes()) {
skillType.loadDataFromConfig(this, getConfig());
}
// Now you can access your skill data like this:
Player player = Bukkit.getPlayer("2008Choco");
// State is managed in PlayerSkillData, you can access SkillType statically because that's all constant and has no state
int miningLevel = this.getPlayerSkillData(player).getLevel(SkillType.MINING);
this.getPlayerSkillData(player).setExperience(SkillType.SWORDS, 69);
// Or you can get some stateless information about a SkillType statically
int experienceForStone = SkillType.MINING.getBlockBreakExperience(Material.STONE);
String skillName = SkillType.SWORDS.getName();
}
public PlayerSkillData getPlayerSkillData(Player player) {
return playerSkillData.computeIfAbsent(player.getUniqueId(), PlayerSkillData::new);
}
}
Now, keep in mind that there are some low-effort implementations here (like holding the PlayerSkillData in the plugin class, and also getting a Player in the onEnable() method but I'm doing this for demonstration purposes), and maybe there's a better way to load in the data from the config into the MiningSkillType, but I hope this sort of gives you an idea of how I would approach things
Your skills are constant types. You'll notice that skill types themselves aren't holding information about the amount of experience a player has or what level they have. That's the responsibility of a separate data object, PlayerSkillData. The SkillType is responsible solely for providing and holding information about the skill, not about the players to which the skill belongs
Also what's important to note is that it's possible that not all skills need an implementation. Maybe you can implement things more abstractly in SkillType and you have no further configuration to do. You'll notice I did that with a very simple SWORDS skill type where I created a new instance of SkillType instead of creating a SwordsSkillType class that extended SkillType. It had no configuration associated with it so creating a subtype just wasn't necessary.
I highly recommend taking a look at how Block is implemented in Minecraft (not Bukkit, but NMS' Block and its subtypes, then looking at the Blocks class to see all the constants). It uses a similar design paradigm as what I've written here and has methods like onInteract(EntityPlayer, World, Hand, BlockPos) that each subclass of Block can optionally override.
public final class MiningSkillListener implements Listener {
private final TestPlugin plugin;
public MiningSkillListener(TestPlugin plugin) {
this.plugin = plugin;
}
@EventHandler
private void onBreakBlock(BlockBreakEvent event) {
int experience = SkillType.MINING.getBlockBreakExperience(event.getBlock().getType());
if (experience <= 0) {
return;
}
Player player = event.getPlayer();
this.plugin.getPlayerSkillData(player).addExperience(experience); // addExperience() doesn't exist but it would be easy to add
}
}
You could implement your mining experience listener like this super super easily with the system above ^
winged niche
Could I send my current impl?
calm flare
Yeah for sure. My way isn't perfect. There are different ways to approach any problem. This is just how I would do it 🙂
winged niche
Well what I have now is a bit meh I still have magic numbers for ability id's haha
Just being lazy tbf
calm flare
Yeah I mean you could easily replace that with an enum or something if you'd like but having a useAbility() method is a good example of what I was detailing with Block and the onInteract() method
Passing in stated information into a stateless class' methods
Maybe a SkillType has a method called activatePrimaryAbility(Player, PlayerSkillData) that you can override in MiningSkillType, implement the logic there for activating an ability, then you can call that method anywhere with SkillType.MINING.activatePrimaryAbility(player, skillData)
The SkillType still stores no state about the player but the implementation for its ability (which is relevant to the skill!) can be implemented in the type
winged niche
I've always had a hard time grasping state, for example in my Berserk class, I would say it holds a lot of state considering I am running a task and changing player attributes
calm flare
Yes it definitely does. It has some Maps that hold a player's experience and levels, as well as some cooldown stuff but tbh that's less of an issue, maybe just not relevant to that class is all
But the single responsibility principle is a whole other issue tbf
winged niche
Yeah I'm still in the process of navigating to a cooldown manager system
Perhaps refactor the class level & exp / ability level & exp to player data
calm flare
And don't get me wrong. There's nothing inherently wrong with classes like that holding state either. Maybe that's a design paradigm you want to go for. You can definitely create multiple instances of Warrior if you wanted. One for each player. But the thing you need to ask yourself is... do you need to? 👀 What differs between instances and are you able to maybe shift out that responsibility from the class into individual methods? Another class? What's relevant and what's not?
There are always a dozen different ways to solve a programming problem and each has its own drawbacks and benefits
winged niche
What I have right now helps me interact a bit better with the classes and their abilities during runtime (sort of the design I was going for but could still use some abstractions and whatnot) as well as just keeping track of the data I need to
calm flare
My approach maybe suffers from complexity and boilerplate, but it benefits from a pretty low memory footprint, ease of use, and some code cleanliness
Could you imagine if for every block in Minecraft they had to make a new instance? Just having like 100,000 GrassBlock instances sitting around in the JVM all representing the exact same thing?
Instead they can just represent each occurrence of a block as an integer value and cross-reference that int to one class instance (via the registry)
winged niche
So if I'm understanding this correctly, a stateless class with helper methods that can be passed stated objects in order to manipulate whatever?
calm flare
Yeah pretty much. Pass in information into methods that you need for that method
There are situations where maybe that doesn't make sense. Like the PlayerSkillData object obviously needs a new instance for each player. Making just one instance and passing in a Player to its methods doesn't really make sense because a PlayerSkillData object can only exist if it has a Player
winged niche
Right yeah
calm flare
Might as well give it a Player to begin with (or a UUID whatever) then simplify your methods instead of passing a Player to every single method you invoke
😄
winged niche
As far as persistence of this data what's your recommended approach? yml sounds reasonable to me considering there's not all that much going to be in there, also most cases for plugins are somewhat low playerbase servers
calm flare
YAML is more of a configuration file format so when you load that up then you have to read the whole file into memory, then you can pick and choose what data you want. If you want to save the file, you have to write it all at once. If it's a low player count server and you're not storing a lot of data, it's probably going to be negligible but if you're expecting a large amount of players, that's where a proper database architecture would come into play
calm flare
You could also opt to create individual files for each player to remedy the "reading all at once" issue but then you have a messiness issue of having potentially hundreds of files in a directory
but maybe there's some data in that file you don't care about but you have to read it anyways and blah blah blah
Binary files or your own file format could help too so you could maybe put some data indices in the file header, but like 
at that point use a database lol
winged niche
You think yt is a good place to learn about db (setup, configuration, security, impl, structure, etc)?
calm flare
If you find the right tutorial on it then yeah I don't see why not. There's a guy I remember seeing a while ago who has some great MySQL query/table optimization videos but I can't remember his channel for the life of me
Ah it was PlanetScale but they don't have a proper tutorial series, just some optimization tips
winged niche
man trooper is hanging out with his family too early, I was gonna get some db lessons from him
calm flare
Anyways, I've gotta get to sleep, but I hope this thread was useful for you @round magnet. Let me know if you have any questions and I can respond in the morning
winged niche
Sleep well choco thanks for the advice!
calm flare
You as well NK o/ 😄
round magnet
I passed out last night, I will check this out when I get home from work, thank you.
round magnet
nn mjb
round magnet
wouldn't using static that often in skillType be static abuse?
calm flare
No, and I think people sort of throw around the term "static abuse" to beginners a bit too liberally without explaining why it's not good to use it the way that they are
static isn't bad, especially if you're using it where it's meant to be used. static just means that the member belongs to the class rather than to an instance of that class, which when you're working with stateless objects that's totally fine
In this case specifically they're constants. You could argue the ALL_SKILL_TYPES Collection maybe can be improved a little, but I was just writing a quick proof of concept anyways. A proper skill registry object might be worth implementing as well. That way third party plugins could maybe register their own skills!
The SWORDS and MINING fields though are constants so static makes perfect sense. It's akin to doing public static final int MAX_HEALTH = 20;. You're just defining something that will be constant throughout the lifetime of the JVM. Even if you were to have a SkillRegistry, it's fine to have these constants and just reference them when doing registration of your base skills.