OOP Bukkit

Hey Bukkit, this is more of a general question,

Say I'm creating a town plugin where players have the ability to build up their own town. There is a class called "Town" so that when I want a new town created, I can do

Code:

Town town = new Town();

The town attributes have to be saved to a file for it to handle reloads, so that leads to my overall question..

Would it be more efficient to create instances of that class and save and load the objects on the onEnable and onDisable, or rather have a static class and have methods such as this

Code:

public static String getTownLeader(String townName){
          return mainClass.getConfig().getString(this.townName+".Leader");
      }

Although a static class seems simpler, having an instance class has it's advantages such as efficiency. I would imagine that getting a value from the config every time would be resource consuming, rather than load an object with all of those attributes pre-defined. But on the down side it would be resource consuming when it comes to reloads and restarts.

If you have a answer, please provide facts rather than saying "I think this". Back up your statements rather than making assumptions. Thanks

 

You can do it like

Code:java

import org.bukkit.Location;
 
import java.util.LinkedList;
 
public class Town {
  private String TownName;
  private String TownLeader;
  private Location TownHome;
  private LinkedList<String> TownPlayers = new LinkedList<String>();
 
  public Town(String Name, String Leader, Location Home ) {
    TownName = Name;
    TownLeader = Leader;
    TownHome = Home;
  }
 
  public String getName() {
    return TownName;
  }
 
  public String getLeader() {
    return TownLeader;
  }
 
  public Location getHome() {
    return Home;
  }
 
  public void setName( String Name ) {
    TownName = Name;
  }
 
  public void setLeader( String Leader ) {
    TownLeader = Leader;
  }
 
  public void setHome( Location Home ) {
    TownHome = Home;
  }
 
  public void addPlayer( String p)  {
    if( !TownPlayers.contains( p ) )
      TownPlayers.add( p );
  }
 
  public void removePlayer( String p ) {
    if( TownPlayers.contains( p ) )
      TownPlayers.remove( p ); 
  }
 
  public boolean hasPlayer( String p ) {
    return TownPlayers.contains( p );
  }
 
  public LinkedList<String> getPlayers() {
    return TownPlayers;
  }
 
}

TownManager

Code:java

 
 
class TownManager {
 
  private HashMap<String, Town> Towns = new HashMap<String, Town>();
 
  public Town getTown( String Name ) {
    if( Towns.containsKey ( Name )
      return Towns.get( Name );
    return null;
  }
 
  public boolean hasTown( String Name ) {
    return Towns.containsKey( Name );
  }
 
  public void addTown( String Name, String Leader, Location Home ) {
    if( !Towns.containsKey( Name ) )
      Towns.put( Name, new Town( Name, Leader, Home ) );
 }
 
  public Town getTownByPlayer( String p ) {
    for( Town t : Towns.values() ) {
      if( t.hasPlayer( p ) ) return t;
    }
    return null;
  }
 
  public HashMap<String, Town> getTowns() {
    return Towns;
  }
 
}
 
 

Put this in the top of your JavaPlugin class

Code:java

 
  private TownManager = new TownManager();
 

And to load towns from the config:

Code:java

 
ConfigurationSection Towns = getConfig().getConfigurationSection("towns");
 
for( String key : Towns.getKeys( false ) ) {
  ConfigurationSection TownSection = Towns.getConfigurationSection( key );
 
  String Name = TownSection.getString( "name" );
  String Leader = TownSection.getString( "leader" );
  double homeX = TownSection.getDouble( "home.x" );
  double homeY = TownSection.getDouble( "home.y" );
  double homeZ = TownSection.getDouble( "home.z" );
  double homeW = TownSection.getDouble( "home.w" );
  World homeWorld = getServer().getWorld( homeW );
  Location Home = new Location( homeWorld, homeX, homeY, homeZ );
 
  TownManager.addTown( Name, Leader, Home );
 
  for( String p : TownSection.getStringList("players") ) {
    TownManager.getTown( Name ).addPlayer( p );
  }
}
 

And the saving:

Code:java

 
getConfig().set( "towns", null );
 
for( Town t : TownManager.getTowns().values() ) {
  String SectionPath = "towns." + t.getName();
  getConfig().set( SectionPath + ".name", t.getName() );
  getConfig().set( SectionPath + ".leader", t.geLeader() );
  getConfig().set( SectionPath + ".home.x", t.getHome().getX() );
  getConfig().set( SectionPath + ".home.y", t.getHome().getY() );
  getConfig().set( SectionPath + ".home.z", t.getHome().getZ() );
  getConfig().set( SectionPath + ".home.w", t.getHome().getWorld().getName() );
}
  String[] players = t.getPlayers.toArray( String[0] );
  getConfig(SectionPath + ".players");
 

Wrote on iPhone, not sure if it works but should.

Took me around an hour to write on my iPhone #notepad

 

Was going to say the same exact thing AronTheGamer was.. And boy, I'm amazed he wrote all that on an iPhone

 

Loading and storing the resources will always be better, than always grabbing from the configuration each time. It keeps efficiency while it's running, but reloads will be a tiny bit slower. Overall, take the assumption that there are 100 players on the server. If they each keep using commands that call the getConfig() method, and they do this once every 10 seconds, the resources used to grab from the config, because of the file streams of loading, reading, and closing will always take a bit of time. Loading it on startup will only make you have to do this once, so overall loading the information on startup is better, and I recommend doing as AronTheGamer said.

 

AronTheGamer
Wouldn't you want to singleton your TownManager if your accessing it in a non-static way?

 

CoderMusgrove If you are referring to FileConfiguration/YamlConfiguration when you say getConfig(), from what I've researched, it doesn't actually do any reading from a file other then loading when its made), it uses a MemorySection (which is generally a Map<String, Object>), so unless you made a new YamlConfiguration everytime you called get(), you wouldn't have the lag that comes from the loading/reading/closing of a file stream.
It'd still be easier to just store the resources, rather then constantly deserialize/serialize the data from a FileConfiguration everytime you want something, but it'd just be personal preference at that point.

 

xTigerRebornx Mrawesomecookie CoderMusgrove joeygallegos AronTheGamer Thanks for all the comments, I'll show you how I approached it

Loading and saving

Code:java

 public void setUpDefaultConfig(){
FileConfiguration config = mainClass.getConfig();
try{
for(String key : config.getKeys(false)){
String townName = config.getString(key+".Name");
String townLeader = config.getString(key+".Leader");
double x = config.getDouble(key+".location.x");
double y = config.getDouble(key+".location.y");
double z = config.getDouble(key+".location.z");
World world = Bukkit.getWorld(config.getString(key+".location.World"));
Location townCenter = new Location(world,x,y,z);
boolean open = config.getBoolean(key+".Open");
int tier = config.getInt(key+".Tier");
Town town = new Town(townCenter, townName, townLeader, tier, mainClass);
towns.add(town);
town.setOpen(open);
for(String member: config.getStringList(key+".Members")){
town.addMember(member);
}
for(String member: config.getStringList(key+".Admins")){
town.addAdmin(member);
}
 
}
 
 
 
 
}catch(Exception ex){
ex.printStackTrace();
}
config.options().copyDefaults(true);
mainClass.saveConfig();
 
}

Instance Class

Code:java

public class Town {
 
public Main mainClass;
String townName;
List<String> members = new ArrayList<String>();
List<String> admins = new ArrayList<String>();
UpgradeChain townTier;
TownAccount townAccount;
Plans plans;
 
public Town(Location townLocation, String townName, String townLeader, int tier, Main mainClass){
//Set up variables
this.mainClass = mainClass;
this.townName = townName;
//Create starting Block
Block startingBlock = townLocation.getWorld().getBlockAt(townLocation);
if(!(startingBlock.getType().equals(Material.GOLD_BLOCK))){
startingBlock.setType(Material.GOLD_BLOCK);
}
 
 
mainClass.getConfig().addDefault(townName+".Name",townName);
mainClass.getConfig().addDefault(townName+".location."+"x",startingBlock.getLocation().getX());
mainClass.getConfig().addDefault(townName+".location."+"y",startingBlock.getLocation().getY());
mainClass.getConfig().addDefault(townName+".location."+"z",startingBlock.getLocation().getZ());
mainClass.getConfig().addDefault(townName+".location."+"World",startingBlock.getLocation().getWorld().getName().toString());
mainClass.getConfig().addDefault(townName+".Leader",townLeader);
mainClass.getConfig().addDefault(townName+".Open", false);
mainClass.getConfig().addDefault(townName+".Members", members);
mainClass.getConfig().addDefault(townName+".Admins", admins);
mainClass.getConfig().options().copyDefaults(true);
 
townTier = new UpgradeChain(townName, tier, mainClass);
townAccount= new TownAccount(townName, mainClass);
plans = new Plans(mainClass, townName);
 
}
public Location getTownCenter(){
return (new Location(
Bukkit.getWorld(mainClass.getConfig().getString(this.townName+".location."+"World")),
mainClass.getConfig().getDouble(this.townName+".location."+"x"),
mainClass.getConfig().getDouble(this.townName+".location."+"y"),
mainClass.getConfig().getDouble(this.townName+".location."+"z")
));
}
public UpgradeChain getTownTier(){
return townTier;
}
public TownAccount getAccount(){
return townAccount;
}
public Plans getPlans(){
return plans;
}
public String getTownLeader(){
return mainClass.getConfig().getString(this.townName+".Leader");
}
public String getTownName(){
return mainClass.getConfig().getString(this.townName+".Name");
}
public void setTownName(String townName){
mainClass.getConfig().set(this.townName+".Name", townName);
mainClass.saveConfig();
}
public void setTownLeader(String newLeader){
mainClass.getConfig().set(this.townName+".Leader", newLeader);
mainClass.saveConfig();
}
public void setOpen(boolean tf){
if(tf){
mainClass.getConfig().set(this.townName+".Open", true);
}else{
mainClass.getConfig().set(this.townName+".Open", false);
}
mainClass.saveConfig();
}
public boolean isOpen(){
return mainClass.getConfig().getBoolean(this.townName+".Open");
}
public void addMember(String player){
members.add(player);
mainClass.getConfig().set(this.townName+".Members", members);
mainClass.saveConfig();
}
public void addAdmin(String player){
admins.add(player);
mainClass.getConfig().set(this.townName+".Admins", admins);
mainClass.saveConfig();
}
public List<String> getMembers(){
return members;
}
public List<String> getAdmins(){
return admins;
}
public void remove(String player){
admins.remove(player);
members.remove(player);
mainClass.getConfig().set(this.townName+".Members", members);
mainClass.saveConfig();
}
public boolean isTownLeader(String player){
if(getTownLeader().equals(player)){
return true;
}
return false;
}
public void sendToAdmins(String message){
for(String player: mainClass.getConfig().getStringList(this.townName+".Admins")){
try{
Player players = Bukkit.getPlayer(player);
players.sendMessage(message);
}catch(Exception ex){
 
}
}
try{
Player player = Bukkit.getPlayer(getTownLeader());
player.sendMessage(message);
}catch(Exception ex){
 
}
}
public void sendToAll(String message){
for(String player: mainClass.getConfig().getStringList(this.townName+".Admins")){
try{
Player players = Bukkit.getPlayer(player);
players.sendMessage(message);
}catch(Exception ex){
 
}
}
try{
Player player = Bukkit.getPlayer(getTownLeader());
player.sendMessage(message);
}catch(Exception ex){
 
}
for(String player: mainClass.getConfig().getStringList(this.townName+".Members")){
try{
Player players = Bukkit.getPlayer(player);
players.sendMessage(message);
}catch(Exception ex){
 
}
}
}
 
public void del(){
File configFile = new File(mainClass.getDataFolder(), "config.yml");
FileConfiguration config = YamlConfiguration.loadConfiguration(configFile);
config.set(this.townName, null);
this.getTownCenter().getBlock().setType(Material.AIR);
TownCommands.towns.remove(this);
try{
config.save(configFile);
mainClass.reloadConfig();
}catch(IOException e){
Logger.getLogger("Minecraft").info("Error while reading config.yml");
}
}
 
public boolean containsLocation(Location location){
int townSize = this.getTownTier().getTierLevel()*20;
double maxX = this.getTownCenter().getX()+(townSize);
double maxZ = this.getTownCenter().getZ()+(townSize);
double minX = this.getTownCenter().getX()-(townSize);
double minZ = this.getTownCenter().getZ()-(townSize);
 
if(location.getX() <= maxX && location.getX() >= minX){
if(location.getZ() <= maxZ && location.getZ() >= minZ){
return true;
}
}
return false;
}
 
 
}
 

I guess it's exactly what he said

But as you can see, I did some sort of a mixture. I load up the objects on startup, but I save everything to the config rather than the values. I'm thinking about rewriting it, it's a little messy

Maybe someone could find a way to write some sort of mixture for better efficiency. If there are limited people on, I don't see a problem in just interacting with the config directly. But when it gets to the hundreds it's better to go with the objects load/save

 

Thank you for that helpful hint, it will be taken into consideration next time. I was most likely just thinking of the save in a FileConfiguration, how saving it at the server shutdown is more efficient than actually saving it each time that you do something. I may look into this more.

 

xTigerRebornx Now that I think about it, wouldn't it be possible to create some sort of library that allows you to save objects to the config.yml, and load them. It would take the object and all it's attributes and split it up to save it, then grab it again and load it back into that object.

I could find that very helpful. I'm thinking about making it..

Edit It looks like someone beat me to it https://forums.bukkit.org/threads/l...y-with-load-and-safe-for-your-project.210101/

 

AronTheGamer Now...I know you took the time to write all of that out. And on a mobile phone at that, so I really hate to say this...but please never give people code examples with badly named variables/methods/anything. Use the proper naming conventions to avoid teaching some new coder who happened upon this thread some terrible habits.

Again, I know you wrote it on a phone...but still. It can cause problems.

 

Garris0n It doesn't matter if the variables are named something or not with what he was giving me. It wasn't the actual code I was looking at, but the idea on how he accomplished this task. He was simply explaining the basic concept behind his code rather than the specifics

Again as the original question stated, i'm trying to decide the most efficient way to get information from the config, not code of a town class

 

Yes, but some new coder might look at it and say "oh, capital letters at the start of variables are ok". That is what I meant. And you underestimate the habit of some people to copy code without reading it.

As for the question, it depends. If you get it from the config, it's still stored in memory, but it's in a map. I would personally suggest the "Town" class.

 

Garris0n Well no matter what it's going to have a town class, the question is if it should be a singleton class or an instance class, or in other words, get it directly from the config or from an instance class.

But as we stated above I guess it's more of a preference

 

Instance class.

 

Don't bother, Bukkit has this feature built in with the ConfigurationSerializable interface:
http://wiki.bukkit.org/Configuration_API_Reference#Serializing_and_Deserializing_Objects

Simply implement that and add the methods required by the specification, i.e. public Map<String,Object> Serialize() and one of any: constructor that takes a Map<String,Object> as an argument, public deserialize(Map<String,Object>) etc.

I use these in a lot of my plugins, as long as the object you're adding to the map is a primitive, or implements ConfigurationSerializable itself, this works well.

EDIT: Strings work too.

 

mythbusterma Thanks that would work

 

AronTheGamer
An hour to write that!??

It takes me like an hour to write 20 lines of NMS code on a phone... Check and double check obfuscated methods, forget names and parameters, check what it is used for (hardest part), wait for net/minecraft/server to load (lots of time), research past implementation, watch framework design, check field access, check reflection, check conventions, check semicolons, check and double check every thing else... Yes, NMS is hard without an IDE

 

I just want to say though while the usage of singleton comes out in handy note don't make your class 90% static and then I mean places where things shouldn't be singleton, instead what you could do is this:

Code:

private static SomeConfig conf; //when bussy in a constructor you could make this final though

public static SomeConfig getConfig() {
    if(conf instanceof SomeConfig){return conf;
    conf = new SomeConfig();
    return conf;
}

this is called a factory method, for now this snippet doesn't look very much usefull but believe me it will.

a good bridge of knowledge I use for static is Immutable objects, these objects are the best objects to be static only if it is needed.
however if you use a non immutable object which could change things inside the internals such like a ItemStack for example then you can better not use it because it can cause unpredictable errors in separated threads.

now why are these factory methods good?
I use it to instance classes for just once from the main class and then make a getter and deleter to edit it.
the idea is sometimes objects aren't immutable in that case you can write a wrapper holding the ItemStack and put the wrapper in a singleton HashMap or HashSet this means you do a attempt of making it abstract in a sort of sandbox which makes you profit when you place this in a factory method.

I hope ive explained good, I'm on a iphone to so I can't reread.

 

@xTrollDudex It was like 3 oclock at night and o was very tired and made loads of spelling mistakes. So...

 

I'm sorry... (ElmentAnimation villager sound)
I know the conventions, and in my private/public work, I always use the right CamelCase, but it was late and didn't want to stay up till over 3:00pm to fix it