Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default Items Support #346

Closed
LiruMouse opened this issue Nov 15, 2018 · 9 comments
Closed

Default Items Support #346

LiruMouse opened this issue Nov 15, 2018 · 9 comments
Assignees
Labels
Type: Enhancement Improvement or modification which is usually a new feature.

Comments

@LiruMouse
Copy link

LiruMouse commented Nov 15, 2018

Is your feature request related to a problem? Please describe.
On our bungeecord servers, we have some items we automatically give players so they can navigate from place to place, and back to the spawn. We've also recreated the control panel from ASkyBlock using CreateYourOwnMenus. We have these given out using ItemJoin, on the join, respawn, and world-change events. The problem is that on reset with clear inventory on, these disappear.
The reason we need reset to clear inventory on reset is because players can exploit resets to get an endless supply of whatever they had in their inventories otherwise (is this a bug? The setting leads me to believe this is intentional).

Describe the solution you'd like
There could be many solutions to this, perhaps some already exist:

  • An API that gives events that another plugin (like a tweaked ItemJoin) could listen for, and then give the items.
  • A config file where default custom inventory could be configured (perhaps even define custom chest items).
  • Simply killing the player when the reset is done to trigger respawn event, reset is close enough to killing the player anyhow, could even be based on a setting.
    (I've done enough research on this issue that I'd feel comfortable writing the last option myself if requested... although it feels like a dirtier solution, it's probably the one most interoperable with other plugins).

Additional context
I suppose I'll provide my test case for what probably isn't a bug, just to be verbose, if anything else is needed from me, I'll be available to answer things whenever I'm awake. Also, I apologize if I overlooked an already existing feature, I didn't see this anywhere when I searched the repository and issues, nor in the config file.

  1. Create your island /ai or /is
  2. Grab anything out of the chest and put it in your hotbar (just so you can see it immediately)
  3. /ai reset or /is reset twice
  4. If island.reset.on-leave.inventory and island.reset.on-join.inventory are false, items from the chest stick around.

Edits
Edit: There's apparently me.RockinChaos.itemjoin.ItemJoinAPI.getAllItems(player) which could be used to hook into ItemJoin, too, but I'm not sure if this project would be okay doing such a thing. I'm currently looking into using this.
Edit 2: I got that working by using islandManager::homeTeleport's newIsland block. I see this comment there TODO add command running, this issue could perhaps be fixed by that TODO being completed (/ij get <item name> for those who end up here in the future when that's a thing). So technically this issue is solved, unless you'd like to go with one of the other options instead of just waiting until using commands is a thing.
Edit 3: As a note, your TODO is before the clearing options, I think that this should be after so that those items won't be wiped if these settings are enabled, unless there's a reason why the user would want a command to know the possessions they had just before creating a new island?

@Poslovitch
Copy link
Member

Did not read the issue completely right now, will do later.

Wondering if you checked the Javadocs, you may find something that would help you: https://ci.codemc.org/job/BentoBoxWorld/job/bentobox/javadoc/world/bentobox/bentobox/api/events/island/package-summary.html

I'll have a look into adding a hook for ItemJoin, if it gets to be possible.

@Poslovitch Poslovitch added Status: Under investigation Investigating the interest and the feasability of the issue. Status: Need answer Waiting for more information to be provided by the issue's author. labels Nov 15, 2018
@LiruMouse
Copy link
Author

Indeed, I did see the events, I wasn't sure when they fired though. (Has inventory been wiped by both settings by the time it fires?) The two options say "When an island is going to be created" and "when an island is created" but there's no "after"/"has been" so I was unsure.
I could totally look into adding the support right into ItemJoin or a plugin that consumes both, though, when I have more time to look this over. (Instead of patching it as fast as I could before someone figured out how to get endless items or got stranded on the bentobox server)

@robert-beckley
Copy link

Not sure how your items work for your navigation. But on my bungee we have a item that players receive on login that opens a menu with a right click event that lets them move around the network between servers / worlds. We use have a command that opens that same gui menu of they have lost or dont want to use the item.

Now we dont give this item to them after they make an island and have thier inventory reset but we could give it to them if we placed it in the starting chest. Items in the starting chest will retain lore when created by BBOX so they would see its the standard movement item you use. Our movement item does not need to be given from the movement plugin to work so a player could craft one or take out of creative and it would still work. So the fact that BBOX is making it would not make a difference.

"A config file where default custom inventory could be configured (perhaps even define custom chest items)."

A config option to define what items a player is given upon island creation / reset would be a option i would like to see added. But for now you can just change the chest items in the schematic.

So you could have the items given to them when they join the sever so they can get around if they want swap between BSB AI or a Skyhub you might have. Then when they make the island there inventory is wiped. But as soon as they get on there island they will open thier chest and see the items they just used to get around. If a player is resetting the island then by that point they should know about the items enough.

Only issue i can see with this method is with teams. As both players will loose the items but will only get one set in the chest.

@LiruMouse
Copy link
Author

Our menu works similarly to yours.
Indeed, that schematic thing could work for sure, aside from the teams issue.
Ultimately, I will probably create an interop plugin using both APIs as I suggested early, and I'll probably publish the source somewhere if that's wanted. Maybe it will get reuse.

@LiruMouse
Copy link
Author

First off, lemme say now, thank you for all the work you've done on this project, and the speedy reply to my issue. I know people must complain and ask for things all the time, so genuinely, thank you for this and all you do.

So this is what I have so far, I'm gonna call it a night on this, but I figured I'd share first:

public class IslandItemJoin extends JavaPlugin implements Listener {
    public void onEnable() {
        PluginManager pm = getServer().getPluginManager();
        if (pm.getPlugin("ItemJoin") != null && pm.getPlugin("BentoBox") != null) {
            pm.registerEvents(this, this);
        }
    }

    void giveItems(UUID player_id) {
        Player p = getServer().getPlayer(player_id);
        ItemJoinAPI.getAllItems(p);
    }

    // TODO: Most Island events should be supported and optional based on config file.
    @EventHandler
    void onIslandResetted(IslandResettedEvent e) { giveItems(e.getPlayerUUID()); }

    @EventHandler
    void onIslandCreated(IslandCreatedEvent e) { giveItems(e.getPlayerUUID()); }
}

I have a few comments/questions on using this API:
When I tried using world.bentobox.bentobox.api.events.IslandBaseEvent as my parameter for giveItems, even though I compiled it against the same jar I use on our bento box server, it said that class didn't exist... My Java is a little rusty so maybe that's me doing something wrong?

It'd be great if I could just consume a central event and use the Reason enum, instead of listening to almost all the events in individual functions... I tried using IslandBaseEvent and IslandEvent, but this didn't appear to be exposed. Is this kinda thing planned, did I overlook something?

I noticed that, at least for Acid Island (didn't test SkyBlock), onIslandCreated doesn't fire when resetting, does it only fire when they create the very first?

@Poslovitch
Copy link
Member

Hi.

I still haven't had the time to properly read what your request is, however I've seen a few questions I can answer to:

  • IslandBaseEvent provides a default implementation of PremadeEvent that contains a few generic fields and methods which are common to TeamEvents and IslandEvents. The fact that it tells you the class doesn't exist is weird though;
  • IslandCreatedEvent might be fired only when the "very first" island of the player gets created. That's how it worked in ASB and I think we, up to now, just copied over from ASB.

There are a few things I definitely need to investigate though, some of the issues that are being raised here are definitely worth a check.

@Poslovitch Poslovitch removed the Status: Need answer Waiting for more information to be provided by the issue's author. label Nov 17, 2018
@tastybento
Copy link
Member

I added documentation around the events and when they are fired. For you, I also added a generic IslandEvent that is called in addition to the other more specific events and holds the enum Reason for the event being called. This should give devs flexibility to use either the generic event or individual events.

tastybento added a commit that referenced this issue Nov 19, 2018
Adds JavaDocs on the event reasons and when they are called.
Completed island Lock event. As locking is a rank and not a binary the
UNLOCK event is not really useful and can probably be removed.

#346
@tastybento
Copy link
Member

@Lirusaito Does this work for you?

@LiruMouse
Copy link
Author

Sorry, busy weekend. Yeah, that's awesome, I'll have a look when I have a spare moment but it sounds amazing.

@Poslovitch Poslovitch added Type: Enhancement Improvement or modification which is usually a new feature. and removed Status: Under investigation Investigating the interest and the feasability of the issue. labels Dec 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Improvement or modification which is usually a new feature.
Projects
None yet
Development

No branches or pull requests

4 participants