Rewrite for 1.13

[Cheaterman]

Nobody talks about this? I don't get it. Spent a whole day trying to make Citadel work on 1.14.4 before realizing the new block format and the numeric ID deprecation. Reverted my server to 1.12.2 which is more suitable for my server anyway because reasons - but really, even the sources of NameLayer + Citadel as they are presented on DevotedMC's repos are incompatible with each other (GroupManager doesn't have the setTimestamp() method that's used in Citadel...).

Fortunately I'm a dev, unfortunately I'm not familiar with all the Java crud (at least I kept my sanity ), so it took a bunch of time for me to fix everything, from the ebean server bullcrap to the different deps (spigot vs bukkit muddying the waters) and outdated repo links or versions - NameLayer even has a dependency on spigot instead of spigot-api in its pom.xml which cost me two hours or something... Many things were ABSOLUTELY NOT in a working state "as-is" and I guess most people trying to build them (or make them work together) would FAIL.

I got it working - admittedly the two biggest hurdles were: 1°) trying to make Citadel work in 1.14 (fool's errand, when I realized the block format changed I knew I was not going to fix compatibility on my own in a single evening, especially since I hate Java and would probably kill myself before succeeding) 2°) trying to make the PythonPluginLoader work in 1.14 (totally unrelated to Citadel) - surprisingly successful, the biggest issue was broken onTabComplete, but the fix was VEEERY easy (it involved more Python knowledge than Java) - also the ebean dependency that wasn't in the pom.xml, some API-breaking Jython upgrades, and a pair of Java details I absolutely do not understand (at least one of them should never have worked before I fixed it???) :

diff --git a/src/main/java/net/lahwran/bukkit/jython/PythonPlugin.java b/src/main/java/net/lahwran/bukkit/jython/PythonPlugin.java
index 5d80ef1..8889a67 100644
--- a/src/main/java/net/lahwran/bukkit/jython/PythonPlugin.java
+++ b/src/main/java/net/lahwran/bukkit/jython/PythonPlugin.java
@@ -7,7 +7,9 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.InputStreamReader;
 import java.io.OutputStream;
+import java.io.Reader;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.logging.Level;
@@ -377,9 +379,9 @@ private boolean isEnabled = false;
     public void reloadConfig() {
         newConfig = YamlConfiguration.loadConfiguration(configFile);

-        InputStream defConfigStream = getResource("config.yml");
-        if (defConfigStream != null) {
-            YamlConfiguration defConfig = YamlConfiguration.loadConfiguration(defConfigStream);
+        Reader defConfigReader = new InputStreamReader(getResource("config.yml"));
+        if (defConfigReader != null) {
+            YamlConfiguration defConfig = YamlConfiguration.loadConfiguration(defConfigReader);

             newConfig.setDefaults(defConfig);
         }
diff --git a/src/main/java/net/lahwran/bukkit/jython/PythonPluginLoader.java b/src/main/java/net/lahwran/bukkit/jython/PythonPluginLoader.java
index e78a837..93b3f17 100644
--- a/src/main/java/net/lahwran/bukkit/jython/PythonPluginLoader.java
+++ b/src/main/java/net/lahwran/bukkit/jython/PythonPluginLoader.java
@@ -171,7 +171,7 @@ public class PythonPluginLoader implements PluginLoader {
         ArrayList<String> depend;

         try {
-            depend = (ArrayList<String>) description.getDepend();
+            depend = new ArrayList<String>(description.getDepend());
             if (depend == null) {
                 depend = new ArrayList<String>();
             }

Anyway sorry about the random ramblings, I'm just saying, I'm very surprised this doesn't get more visibility.

[Maxopoly]

Sorry you had to go through this. Usually no one but us uses this and even we are a lot fewer than we used to be, so documentation and similar are very sparse to put it lightly.

At the moment 1.12.2 is the latest version supported by the civ plugin suite. The MC 1.13 changes required major changes in all plugins using block data, which ended up culminating in entire rewrites for multiple of the core plugins. If you are really interested you can find them each on my repos under the civ13 branch, but they are unfinished and still require a lot of work, I'd recommend against touching them.

The setTimestamp() method is intended, the current NameLayer master is the 1.13.2 version, which does not need that anymore as part of other changes. It works with the latest Citadel (this one) which is not merged yet, but pretty much done. Rollback to the latest 1.12.2 commit or use an old build from our build server for each plugin to get a working version.

[Maxopoly]

Also real life happened and my work on updating all the plugins to 1.13.2 is paused atm. I don't think anyone else is willing to take that over and my schedule still looks busy, so don't expect a working set of 1.13+ plugins any time soon.

[ProgrammerDan]

I keep trying to convince myself to sit down and work on these things. Got closest last night of any night in recent history, but wound up cleaning instead. So yeah. Progress will be slow although I'm generally responsive to merge completed changes (this change is not afaik).

On Mon, Jul 29, 2019 at 4:44 PM Maxopoly notifications@github.com wrote:

Also real life happened and my work on updating all the plugins to 1.13.2 is paused atm. I don't think anyone else is willing to take that over and my schedule still looks busy, so don't expect a working set of 1.13+ plugins any time soon.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/DevotedMC/Citadel/pull/59?email_source=notifications&email_token=AADD5DW3SN7SIQS6PFHUG7TQB5JBBA5CNFSM4HJTRBJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3B6K3Y#issuecomment-516154735, or mute the thread https://github.com/notifications/unsubscribe-auth/AADD5DWL7HZTSGZSAGW7PLTQB5JBBANCNFSM4HJTRBJA .

[Cheaterman]

Alright! I didn't realize your work on 1.13+ wasn't finished @Maxopoly but I'm glad it's been started. Even if you need to pause your own work on this, I'm fairly sure having done as much work as you already did will motivate someone to pick up where you left. On the other hand, just as you suggested, I'll stick to 1.12.2 for now.

The comments you made about setTimestamp() on NameLayer are interesting, "current master" (on DevotedMC's repos) didn't seem to work for me - but I think I know why, I think I mistakenly assumed the version numbers were somehow related to Minecraft, and linked against (not sure if proper Java linguo here) nms-v1_12_R1 instead of nms-v1_13_R2. My bad :-) as I already said, I'm very much out of my element in Java territory, hehe. Or even in MC modding territory, really.

@ProgrammerDan it's pretty cool that you're still around! Don't worry about the motivation thing, I know how it is - I myself didn't touch my MC server since 1.9, until last weekend basically. And yeah, TBH I thought this PR was more "completed" than it is, seeing how @Maxopoly recommends against me using it.

Really, thanks to both of your for taking the time to respond to my rant, and generally for your hard work on these projects!!!

@Maxopoly If there's pressure from my MC community (which I highly doubt right now), it's possible that I could try and host your 1.13+ version and work on trying to make it relatively usable. For better or worse though - as I said I'm no Java programmer :-) - it's fairly unlikely my players will care anyway.

[Maxopoly]

it's possible that I could try and host your 1.13+ version and work on trying to make it relatively usable

Appreciate the offer, but I got enough testing environments if needed. Problem is rather that there is still quite a bit of code that needs to be written before it can be tested.