[Too hard to explain in title - Please read | Config related issues]

Discussion in 'Plugin Development' started by OTF Catastrophe, Dec 9, 2016.

Thread Status:
Not open for further replies.
  1. Offline

    OTF Catastrophe

    **THIS IS A PLUGIN I AM FIXING TO BETTER SUIT MY SERVER, MOST OF THE CODE ISN'T MINE**
    Alrighty so the issue that I'm having is with a block collection plugin somewhat like the plugin that TheHive or Hypixel uses to collect skulls and get a prize. I'm trying out an alternative towards using MySQL that I know if possible through config files and boolean values.

    So my issue is, I have an if statement that runs if the player has 0 blocks left to collect, and if the users data has the string "finished" set to false. By default it is set to false until the player count hits 0. The code for this is as shown:

    Code:
                                    blocksLeft = this.getConfig()
                                            .getStringList("blocks").size()
                                            - ((List<?>) this.blocksss.get(e
                                                    .getPlayer().getName())).size();
                                    if ((blocksLeft == 0) && (this.data.getConfig().getBoolean("data." + e.getPlayer().getUniqueId() + ".finished") != true)) {
    I'm not having an issue with that code, I just figured it's important. So this runs absolutely fine and it does check it. But the issue is I need the config to update after specific commands are run that are defined in the code as well that comes from config. All of this runs perfectly normal. But after I have the dispatch of commands occur, I have it set to save the file and update "finished" to true in config. The config updates and it goes through fine.

    THE ISSUE:
    When running this code the part of the code that saves the config:

    Code:
                                    if (this.data.getConfig().getBoolean(
                                            "data." + e.getPlayer().getUniqueId() + ".finished") == false) {
                                        this.data.getConfig().set(
                                            "data." + e.getPlayer().getUniqueId() + ".finished", true);
                                        this.data.saveConfig();
                                        }
                                    }
    updates the config while running the if statement causing some command issues. I've honestly never seen this happen but I've tested almost every possibility to try and make it save config AFTER all the commands have run, but to no avail. The commands that are server are titles and theres two different string lists that run when the error occurs when it's only supposed to be one. I know it happens because the main title runs for when all the blocks are found, but the subtitle that runs is getting how many blocks are left, aka the block found subtitle. These two titles pop up together and I know it's a timing kind of issue I just have no clue how to fix it.

    What I am looking for is a way for the config to update only AFTER every command was run from the config.

    ONE LAST THING: There may be small insignificant things that you want to change about the code, I know someone is gonna do this because it happens quite literally every single time I make a post. But please keep comments to finding a fix for the issue only. Thank you for reading the post and thank you for anyone who leaves a helpful comment! :)

    ARE OF CODE ENTIRELY:
    Code:
                            if (((List<?>) this.blocksss.get(e.getPlayer().getName()))
                                    .size() >= this.getConfig()
                                    .getStringList("blocks").size()) {
                                lst1 = this.getConfig()
                                        .getStringList("all-blocks-found-commands")
                                        .iterator();
    
                                while (lst1.hasNext()) {
                                    s = (String) lst1.next();
                                    blocksLeft = this.getConfig()
                                            .getStringList("blocks").size()
                                            - ((List<?>) this.blocksss.get(e
                                                    .getPlayer().getName())).size();
                                    if ((blocksLeft == 0) && (this.data.getConfig().getBoolean("data." + e.getPlayer().getUniqueId() + ".finished") != true)) {
                                    Bukkit.dispatchCommand(
                                            Bukkit.getConsoleSender(),
                                            s.replace("%player%",
                                                    e.getPlayer().getName())
                                                    .replace(
                                                            "%pLocX%",
                                                            ""
                                                                    + e.getPlayer()
                                                                            .getLocation()
                                                                            .getX())
                                                    .replace(
                                                            "%pLocY%",
                                                            ""
                                                                    + e.getPlayer()
                                                                            .getLocation()
                                                                            .getY())
                                                    .replace(
                                                            "%pLocZ%",
                                                            ""
                                                                    + e.getPlayer()
                                                                            .getLocation()
                                                                            .getZ())
                                                    .replace(
                                                            "%locX5%",
                                                            ""
                                                                    + (e.getClickedBlock()
                                                                            .getLocation()
                                                                            .getX() + 0.5D))
                                                    .replace(
                                                            "%locY5%",
                                                            ""
                                                                    + (e.getClickedBlock()
                                                                            .getLocation()
                                                                            .getY() + 0.5D))
                                                    .replace(
                                                            "%locZ5%",
                                                            ""
                                                                    + (e.getClickedBlock()
                                                                            .getLocation()
                                                                            .getZ() + 0.5D))
                                                    .replace(
                                                            "%locX%",
                                                            ""
                                                                    + e.getClickedBlock()
                                                                            .getLocation()
                                                                            .getX())
                                                    .replace(
                                                            "%locY%",
                                                            ""
                                                                    + e.getClickedBlock()
                                                                            .getLocation()
                                                                            .getY())
                                                    .replace(
                                                            "%locZ%",
                                                            ""
                                                                    + e.getClickedBlock()
                                                                            .getLocation()
                                                                            .getZ())
                                                    .replace("%blockLeft%",
                                                            "" + blocksLeft)
                                                    .replace("%blocksLeft%",
                                                            "" + blocksLeft));
                                    this.data.saveConfig();
                                    if (this.data.getConfig().getBoolean(
                                            "data." + e.getPlayer().getUniqueId() + ".finished") == false) {
                                        this.data.getConfig().set(
                                            "data." + e.getPlayer().getUniqueId() + ".finished", true);
                                        this.data.saveConfig();
                                        }
                                    }
                                }
                            }
     
  2. Offline

    Zombie_Striker

    @OTF Catastrophe
    Have you tried using schedulers? Could you update the config in a delayed task?
     
  3. Offline

    mythbusterma

    @OTF Catastrophe

    Perhaps if people are leaving comments about how to make your code better, instead of rejecting them outright and saying that they are unhelpful, you could follow them and make your code more maintainable. Every single one of the things we point out is an issue, just as much as the unintended behaviour of the code is, and you're going to realise that sooner or later.

    As Zombie said, you could update this a tick or two after you're done, but you might run into issues where something is done the next tick before it is saved and you'd be out of sync with the file. For what it's worth, why do you need to save this every time? Saving a file (relative to the normal operation of the server) is extremely slow. You should avoid saving a file at all costs. If you're using this file to transfer data between parts of your plugin, don't. It's a bad idea and is extremely fragile. Instead, store the information you need in memory, and only save out to a file when the server is shutting down (or every couple of minutes if you're worried about data loss due to crashes), this also has the added benefit of making your code simpler.

    Also, you don't need to compare boolean values to "true" or "false," and you may want to introduce more local variables to make this look nicer.
     
    MrGeneralQ likes this.
  4. Offline

    OTF Catastrophe

    @Zombie_Striker I'm new to schedulers and I got this scheduler working but for some reason it keeps throwing multiple NPE's in console when trying to access the data config, could you possibly take a look at these two classes and see what I did wrong?
    Main class: http://pastebin.com/TZ5Qs294
    Config class: http://pastebin.com/0sVDxnPi
    (You don't have to spoonfeed code don't worry, just like to know why this doesn't work when I try to access the data.yml config.)

    @mythbusterma I only added the "But please keep comments to finding a fix for the issue only." part because every single time I make a post on here there always has to be one or two people who leave a comment saying something that doesn't even revolve around the issue itself, they just comment to comment. I'd rather just get an answer to the problem i'm having and not "Hey your capitalization in your class file is off". Though I get why that could lead to a problem in the future, it has never happened and most likely won't be an issue ever. I'm thankful for any comments that could lead to better code but I'd prefer to get my problem fixed rather then get excited over thinking someone has helped me, when they've commented something totally unneeded.
     
  5. Offline

    mythbusterma

    @OTF Catastrophe

    Did you look at the rest of my post? I suggested you redo the way you're handling this.
     
  6. Offline

    Zombie_Striker

    The thing is, they will be a problem in the future and there will be no way to contact you outside of the thread to tell you about these problems. We post all these issues because they will cause a problem, if not for this project then it will be another one where you make the same mistake. We want to make sure we cover everything we can so that you and other members do not have to come back creating new threads when the issues may just be capitalization.

    "this.data" in a scheduler is not the same as "this.data" outside of the scheduler. Currently, you are accessing that "config" field that you have not set.

    I would recommend removing that "data" field in the scheduler and removing the "this".
     
  7. Offline

    OTF Catastrophe

    Thank you so much, I figured from the start a scheduler was needed but honestly I know almost nothing about making them. It works perfect thank you! :) Just one more quick question, I have my scheduler set to:

    Code:
    this.getServer().getScheduler().scheduleSyncDelayedTask(this, new Runnable() {
                                        public void run() {
                                              data.getConfig().set("data." + e.getPlayer().getUniqueId() + ".finished", true);
                                          }
                                        }, 40L);
    And for some reason when this code runs after 2 seconds(40L), then it seems to run the code 8 times. I tried this with separate times and it either does more times or less. Is that common with scheduleSyncDelayedTasks? If so, anything I could read on how to fix this issue to only run once?
     
  8. @OTF Catastrophe
    Copying that exact code and running it runs only once for me, so the issue must be somewhere else in your code. Mind providing the full class that this code is in so we can get a bit more overview?
     
  9. Offline

    mythbusterma

    @OTF Catastrophe

    It's likely that you're scheduling the task more than once.

    Again, I'm telling you that you really should not be using the config file to pass data amongst various parts of your plugin. You're just going to cause yourself grief, as you can see here.

    Store the information in memory and write it out only when you need to like when the server is stopping or periodic saves if you're worried about crashes.
     
  10. Offline

    OTF Catastrophe

    The data.yml file that I'm using only is used when a player finds a block and immediately saves. What I'm trying to achieve here is changing a players data string "finished" to "true" then it'll save and after that the players data never changes unless I manually do it. So the file is pretty much a storage file.


    http://pastebin.com/TZ5Qs294
    I had posted it a few comments before aha. Thank you for taking a look ^-^

    EDIT: Due to the scheduler only running the data.yml edit I cannot tell if it does it multiple times by that. But when I add in something like a broadcast to see what it does, it broadcasts 8 times.
     
  11. Offline

    mythbusterma

    @OTF Catastrophe

    How often do they find a block, and how are you querying if they have the block?
     
  12. @OTF Catastrophe
    Well, that piece of code is inside this iterator:
    Code:java
    1. while (lst1.hasNext()) {
    so it should become quite apparent to you as to why it runs multiple times..
     
  13. Offline

    OTF Catastrophe

    Well in total all the blocks(15 total) are scattered around the spawn. It's quite difficult and it usually takes around 30-45 minutes to actually find all the skulls for most players. So the data file doesn't update often which won't cause any lag or any performance issues with the low player count that I have now. Also when a block is found it puts the blocks location under the users data and compares it to the config where the block locations are. So if a player clicks on a block that's on the specified coordinates in config then it'll run and say they found a skull and when it's logged into the data.yml file then if they do it again it'll tell you that you've already found it.

    I figured that must be an issue with it since it's looping multiple times but it's required for the use of skull finding and data logging. Is there any way around this?
     
  14. @OTF Catastrophe
    Well, move it out of the iterator while loop?

    Also, regarding the config usage, I used to think that saving configs directly was better too, but the truth is, it becomes way easier to manage if you store everything in memory, and only reading the config when it needs to be saved.
     
Thread Status:
Not open for further replies.

Share This Page