Solved SQL Database Not Updating?

Hello everybody. I have worked DAYS trying to get this to work... I am trying to work on an SQL storage system for an economy and ammo system, and to start I am simply counting logins per player. But, although I think my code is VERY correct, the table will not update. I have all my database information included in the code below BESIDES the password. (That is correct, though)

My table follows as:
PLAYER(Playername, will switch to UUIDs once I get this working), LOGINS (INT), AMMO (INT)

I've tried debugging, and the only message I get back is that the plugin is being enabled. Any help would be appreciated.

Code:

package com.themadmc.sql;

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.logging.Logger;

import org.bukkit.Bukkit;
import org.bukkit.entity.Player;
import org.bukkit.event.Listener;
import org.bukkit.event.player.PlayerLoginEvent;
import org.bukkit.plugin.java.JavaPlugin;

public class Main extends JavaPlugin implements Listener {

    private static Connection connection;
   
   
    public void onEnable() {
        Bukkit.getLogger().info("MySQLLobby enabled...");
        getServer().getPluginManager().registerEvents(this, this);
    }
   
    public void onDisable () {
        try {
            if(connection != null && !connection.isClosed()) {
                closeConnection();
                Bukkit.getLogger().info("MYSQLLobby disabled...");
            }
        } catch (SQLException e) {
            e.printStackTrace();
        }
    }
   
    public synchronized static void openConnection () {
        try {
            Bukkit.getLogger().info("Opened connection.");
            connection = DriverManager.getConnection("jdbc:mysql://198.100.144.199:3306/mc_2", "mc_2", "Password Removed");
        } catch(Exception e) {
            e.printStackTrace();
        }
    }
   
    public synchronized static void closeConnection () {
        try {
            Bukkit.getLogger().info("Closed connection");
            connection.close();
        } catch(Exception e) {
            e.printStackTrace();
        }
    }
   
    public synchronized static boolean playerDataContainsPlayer(Player player) {
        try{
            PreparedStatement sql = connection.prepareStatement("SELECT * FROM `lobby_data` WHERE player=?;");
            sql.setString(1, player.getName());
            ResultSet resultSet = sql.executeQuery();
            boolean containsPlayer = resultSet.next();
            Bukkit.getLogger().info("Player data from table recieved.");
            sql.close();
            resultSet.close();
           
            return containsPlayer;
        } catch(Exception e){
            e.printStackTrace();
            return false;
        }
    }
   
    public void onPlayerLogin(PlayerLoginEvent e) {
        openConnection();
        try{
            int previousLogins = 0;
            if(playerDataContainsPlayer(e.getPlayer())) {
                PreparedStatement sql = connection.prepareStatement("SELECT logins FROM `lobby_data` WHERE player=?;");
                sql.setString(1, e.getPlayer().getName());
               
                ResultSet result = sql.executeQuery();
                Bukkit.getLogger().info("Successfully retrieved player " + e.getPlayer().getName() + ".");
                result.next();
               
                previousLogins = result.getInt("logins");
                Bukkit.getLogger().info("Successfully retrieved player " + e.getPlayer().getName() + "'s logins: " + previousLogins);
                PreparedStatement moneyUpdate = connection.prepareStatement("UPDATE `lobby_data` SET logins=? WHERE player=?;");
                moneyUpdate.setInt(1, previousLogins + 1);
                moneyUpdate.setString(2, e.getPlayer().getName());
                Bukkit.getLogger().info("Successfully updated player " + e.getPlayer().getName() + "'s logins to: " + previousLogins);
                moneyUpdate.executeUpdate();
               
                moneyUpdate.close();
                sql.close();
                result.close();
            } else {
                PreparedStatement newPlayer = connection.prepareStatement("INSERT INTO `lobby_data` values(?,0,0);");
                newPlayer.setString(1, e.getPlayer().getName());
                newPlayer.execute();
                Bukkit.getLogger().info("Successfully added new column for " + e.getPlayer().getName() + ".");
                newPlayer.close();
            }
        } catch(Exception ex) {
            ex.printStackTrace();
        } finally {
            closeConnection();
        }
       
       
    }
   
   
   
}

 

Instead of using PlayerLoginEvent, try

Code:

@EventHandler
public void onJoin(PlayerJoinEvent e){
// do stuff
}

 

@Meecles Nothing...I don't get any debug messages either...

 

@AppleBabies

Quite a few questions I have for you, actually.

1. Why is your connection static? It's a state variable, and should be treated as such.
2. Why are all the methods static? None of them should be
3. Why are you blindly catching Exception? There's no reason to just toss any exceptions on the floor, that's a terrible practise. Catch only the relevant Exceptions and handle them properly when you do.
   
4. Why are you stealing Bukkit's logger? Your plugin has its own logger, use that (accessed via JavaPlugin#getLogger()).
5. Why do you have the "synchronized" keyword in this? Like at all? I don't think you understand how threading works, and none of these methods will be called asynchronously, anyway (not that it was synchronized properly in the first place).
6. Why are you making calls to a MySQL server on the main thread of the server? The queries on the database take an indeterminate amount of time and you can hang your server up for quite some time waiting on them.
7. You reopen a connection to the server every time a player logs into the server. This is not good. Cache your connection.

Once you've addressed these, maybe we can figure out what's wrong.

 

@mythbusterma Oh boy. You again....
I have my methods static and synchronized because I do not want two connections to be open at once for one player.
I can also log however I want to, thank you very much.
Every exception I have set in place is necessary, actually.
I make calls from the main thread because then it is prioritized.
And finally, I reopen the connection each time, yes. Why do you think that the methods are synchronized...? There's no need to cache an already monitored connection.

 

@AppleBabies I think by 'blindly cathcing exception' means that for your catch statement you have Exception. Most of your code will throw SQLException's from what I know, correct me if I'm wrong. Just catching 'exception' is just not the right thing.
Check for the specific exceptions each method will throw and put those in your catch. Not just exception.

 

@mug561 @mythbusterma Okay, fine, yeah I fixed that. But I noticed that actually, the problem is that when the player joins, the event doesn't ever fire.

My am I stupid. I forgot the EventHandler...sorry everyone! Marking as solved.

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

@AppleBabies

Yes, it's me. One of the few people who actually knows what they're talking about.

I'm not sure what you think "static" does, but it does not mean "one per player" (nor does synchronized, for that matter). It means this method can only be entered by one thread per a given instance of a class. It is a mutex on the method. This is completely meaningless, however, as all of the methods in this class will always be called on the same thread. So absolutely pointless, not to mention done incorrectly even if there were to be multiple threads accessing this class.
But a logging manager plugin would handle your plugin's output incorrectly, should you choose to do it this way.
No, it's never, ever necessary. See here: The Java™ Tutorials
I'm not sure why you think the other threads on the server are not "prioritised," but I'll make sure to log into your server to make it lag as much as possible.
I have no clue why the methods are synchronized, frankly I think it's because you don't have the first clue about what that keyword actually does. There's no need to re-establish a connection, caching the connection is perfectly fine. This is especially true since you're doing it on the main thread, wasting precious time the server needs to tick.

In short, I'd recommend a beginner's book on Java, and in particular:
https://docs.oracle.com/javase/tutorial/essential/exceptions/advantages.html
https://docs.oracle.com/javase/tutorial/essential/concurrency/syncmeth.html
https://docs.oracle.com/javase/tutorial/java/javaOO/classvars.html
http://www.vogella.com/tutorials/JavaConcurrency/article.html

As an aside:

This is the sort of dangerous hubris that makes plugins the trash they usually are.

@mug561

You're exactly right. Only catch what you need to.

 

@mythbusterma 1. As you can see, this thread is marked as solved. Your changes aren't necessary, and nothing lags at all. It works perfectly fine.

And 2. Please write me a letter when you are perfect. Nobody is. I don't understand why you must always be so negative. When a person asks for help, lend them a hand. Berating a person for something they're possibly not-so-knowledgable in doesn't help them. It teaches them that it's not okay to go to the Bukkit forums to ask a question. It's people like YOU that makes the world like it is - negative, neglectful, and boring.

 

@AppleBabies

I did, I told you as much as I could find wrong with your code. I wasn't negative, and I asked you to justify the mistakes you made. I was negative when you decided to become defensive and get an attitude with me for no reason.

Where did I even imply I was perfect? I know I'm not, but I also know that I know far better what I'm talking about than you, and that any time I give you advise, your response is to get defensive and say I'm wrong when I'm not.

It's perfectly okay to come here and ask a question, by all means. What you shouldn't be doing is responding to legitimate criticism of code with hubris. I didn't berate you for not being knowledgeable, I berated you for ignoring advise of those who know better than you with "I can do whatever I want and you can't stop me."

Also, I know for a fact it's lagging your server, you just can't tell if you're the only player online.