Solved How to always open a level 1 entchanting table?

Discussion in 'Plugin Development' started by Kicksy, Jul 22, 2018.

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

    Kicksy

    I would like to open an Enchanting table in the Bukkit 1.12.2 version. The table should never pay attention to the bookshelf. So that you can only enchant at the minimum level.

    I tried it that way:
    Code:
        @EventHandler
        public void onClick(InventoryOpenEvent e) {
            if (e.getInventory().getType() == InventoryType.ENCHANTING) {
                e.getPlayer().openInventory(Bukkit.createInventory(e.getPlayer(), InventoryType.ENCHANTING));
            }
    
        }
    And I get this Error:
    https://pastebin.com/xJvizcht

    What am I doing wrong?
     
  2. Offline

    KarimAKL

    @Kicksy I don't know what "StackOverflowError" means exactly and i haven't been able to find any answers so i'm sorry if this sounds dumb but maybe delay opening the inventory by a tick or two? (Sometimes works)
    If anyone could answer what "StackOverflowError" means that would be great.
     
  3. StackOverflowError is when something repeats over and over again until the stack runs out of memory. Usually it's when some method calls another which then calls the previous one, so it gets stuck in a permanent loop. The problem is, when someone opens an Inventory this then opens another one calling InventoryOpenEvent again, which opens another inventory, and so on. Now I'm not too experienced with Enchanting stuff, so I don't know how you could fix this, but the problem is your code repeats itself forever. Hope this helps.

    Sent from my SM-G903F using Tapatalk
     
    TheKingElessar, Kicksy and KarimAKL like this.
  4. Offline

    KarimAKL

    @TheMinecraftKnight I see, thanks alot for explaining that. :) If that's the case then making a boolean and checking if it's true/false before opening the inventory should work, right?
    EDIT: (Of course setting the boolean after opening the inventory)
     
  5. That could work, although if lots of people are opening at once it *could* overwrite the Boolean from someone else. What I'd suggest doing (albeit not 100% sure you can do it in Bukkit easily) is give the Enchanting inventory you create to open some sort of custom name, then you can check that they haven't opened a custom one. If you don't want to completely change the name, just add something like a colour code (can't type on mobile) or an Invisible Unicode Character (may break in Minecraft font, Google it) to the end of the "Enchanting". This may break localisation though (even if your language is set to something else it will still be in English).
    Another alternative that shouldn't break localisation is using a Boolean HashMap just so it's player independent. I'll leave it up to you to decide which to use and which works best for your situation.

    Hope this helps.

    Sent from my SM-G903F using Tapatalk
     
  6. Offline

    KarimAKL

    @TheMinecraftKnight What i meant by "boolean" was actually a HashSet or something and then you could check for each player using their UUID. (From what i've heard a HashMap is overkill for a boolean(true/false))
     
  7. Not sure a HashSet is what you're looking for. That's just an indexed list of non-duplicate (so it could only store at most 2 things AFAIK). You're probably thinking of a HashMap<UUID, Boolean> which uses the UUID as a key, so then you don't have different players overriding a single Boolean. Hopefully you understand what I mean.

    Sent from my SM-G903F using Tapatalk
     
  8. Offline

    KarimAKL

    @TheMinecraftKnight I think i understand but i've always used "HashSet<UUID>" for boolean per player and i haven't had any errors as of yet, tho you are probably right.
    Example of what i've been doing:
    Code:Java
    1. HashSet<UUID> whateverName = new Set<UUID>();

    Either that or this:
    Code:Java
    1. Set<UUID> whateverName = new HashSet<UUID>();

    And then:
    Code:Java
    1. if (whateverName.contains(player.getUniqueId)) { /*do something*/ }
     
  9. Offline

    Kicksy

    Thanks for all the answers. How fix it:
    Code:
        @EventHandler
        public void onClick(PlayerInteractEvent e) {
            if (e.getAction() == Action.RIGHT_CLICK_BLOCK && e.getClickedBlock().getType() == Material.ENCHANTMENT_TABLE
                    && !e.getPlayer().isSneaking()) {
                e.setCancelled(true);
                Location loc = new Location(e.getPlayer().getWorld(), 0, -10, 0);
                e.getPlayer().openEnchanting(loc, true);
    
            }
        }
     
  10. Ahhh, I see what you're doing now. I was thinking of a Set<UUID> (make the variable type the interface not the implementation, it doesn't affect functionality and is good practice for Abstraction. Yeah, that would work fine (just make sure to remove all the values onDisable() if it's static). Sorry for the misunderstanding.

    Sent from my SM-G903F using Tapatalk
     
  11. Offline

    KarimAKL

    @Kicksy Nice, good to know it's solved. :)

    @TheMinecraftKnight Np, glad to know i didn't do something that was bad practice or didn't work. :p Also i don't use static, i make the HashSet<UUID> private and then make public methods to get/set/remove the uuids.
    I'm not sure but i think the name for it is "getter and setter" or something like that, example of what i do below.
    Example:
    Code:Java
    1.  
    2. private Set<UUID> whateverName = new HashSet<UUID>();
    3.  
    4. public boolean containsUUID(Player player) {
    5. return this.whateverName.contains(player.getUniqueId());
    6. }
    7.  
    8. public void setUUID(Player player) {
    9. this.whateverName.add(player.getUniqueId());
    10. }
    11.  
    12. public void removeUUID(Player player) {
    13. this.whateverName.remove(player.getUniqueId());
    14. }

    That's how i usually do, tho i might have made some mistakes in the code as i'm not at my main pc at the moment.
     
  12. Looks good :)
    Getters and setters are part of a concept called Encapsulation (only giving access to bits of code that need to be accessed), so making it private and delegating methods is good, although I'd rename setPlayer to addPlayer for clarity.

    Good luck coding :)

    Sent from my SM-G903F using Tapatalk
     
  13. Offline

    KarimAKL

    @TheMinecraftKnight Yeah, i just wrote that right now to show you an example of what i'm doing. :p Anyway thanks and good luck to you too. :)
     
Thread Status:
Not open for further replies.

Share This Page