Still need a simple plugin [Need it quick!]

Discussion in 'Archived: Plugin Requests' started by TheOnlyRealTGS, Jul 2, 2013.

  1. Offline

    TheOnlyRealTGS

    Okay so since this bug exists, i need a plugin that will prevent people from opening their inventory in the first 1 second after a teleport. It's that simple. Not sure if i can give you money for it due to bukkits rules but if you do it i would be SO happy!
     
  2. Offline

    DemmyDemon

    Hmm...

    1) Monitor PlayerTeleportEvent, make a note of when the event happened. HashMap<Player,Date>
    2) Listen for InventoryOpenEvent and cancel it if that player teleported less than [configurable] seconds ago.

    If I had my test and build environments set up I could do this in about a minute, but seeing as I don't, it might be -days- before I have the time to make this.
    Pretty much anyone with any experience in Bukkit plugins could probably make this very easily, however.

    If InventoryOpenEvent cancellation is respected, that is. Never used it, so I don't know for sure.
     
  3. Offline

    TheOnlyRealTGS

    Well it dosn't matter if it take some days, asap is preferred, ty for you post :)
     
  4. Offline

    DemmyDemon

    I threw together this ENTIRELY UNTESTED code.
    I've packaged in this JAR:
    UNTESTED CODE MAY EXPLODE WITHOUT WARNING AND WIPE OUT ALL OF EXISTANCE (open)


    Edit: Did I mention that I don't have my test environment up, and as such I have no idea if this plugin loads as it should and works, or causes Bukkit to implode into a black hole?
     
  5. Offline

    timtower Administrator Administrator Moderator

    Lol
     
  6. Offline

    daboross

    Just a note on this, you were probably already planning on doing this, but I am going to mention it anyways. You are going to store the player's username in the HashMap not the player, right?

    On another note if you won't have your build env set up for a while I can create this pretty quickly as well.
     
  7. Offline

    DemmyDemon

    ...did you read the whole thread?
    I did end up using Player and Calendar for the hashmap, as I fail to see what difference it makes what key I store it under as long as it doesn't change for that player. AFAIK you will be handed the same player object for a player forever until he/she disconnects.
    --trivial-- to change, however. See the source code I linked to.
     
  8. Offline

    Henzz

    DemmyDemon
    Store the Player's username, some things can go wrong if you store the Player object when you think about it.
     
  9. Offline

    DemmyDemon

    Like what?

    I've been storing trivial stuff like this keyed on Player for a long time, and I've never had a bad side-effect.
    Please tell me what can go wrong so I can learn.

    [edit]
    BTW, when am I storing the Player object?
    I'm only storing a reference to it as far as I can see.
     
  10. Offline

    Henzz

  11. Offline

    DemmyDemon

    Yeah, the Player object does seem to have a much deeper reftree than I realized. *shrug*, change it to String and .getName(), then. Like I've noted before, that's a trivial change to make.
    Gotta love how nobody has even checked if the plugin will load :-P

    [edit]
    Did I miss any instances?
    Yeah, rebuilt the JAR, too.
     
  12. Offline

    timtower Administrator Administrator Moderator

    LOL, developer wars
     
  13. Offline

    DemmyDemon

    No, grasshopper, this is called "collaboration" and is a Good Thing ™
     
    zack6849 and MasculineBulldog like this.
  14. Offline

    TheOnlyRealTGS

    Cool I'll check it out and see if it works thank you for your help. :)
     
  15. Offline

    DemmyDemon

    Don't thank me until you've confirmed it doesn't catch your dog on fire, but sure, you're very welcome.
     
  16. Offline

    RROD

    Ran some tests after creating one for you; I know it's been done but I got bored. I'll release this under the public domain. Done in a slightly different way to the other one - I use Bukkit's Metadata instead of HashMaps. I kinda feel the system is underused. Complete overkill for a plugin so small xD
    Source code
    Plugin Jar

    As this isn't hosted on BukkitDev, I will not provide support - but it should be good to use and bug free for a very long time :3
    It has a permission node to override the wait time too: "invwait.override"
     
  17. Offline

    DemmyDemon

    WOW, we have -radically- different approaches to this.
    Why are you using ignoreCancelled, and why the EventPriority.LOWEST on the teleport event?
    Runnables, timers and metadata sets? Any particular reason you selected the different parts to build this from, or just old habits?

    Not trying to be critical here, just genuinely interested in why our code turned out so VASTLY different.
    I'd love to get into it deeper if we both had the time some day.
     
  18. Offline

    RROD

    I like to experiment with what would work in an attempt to find the fastest code. It's a habit of mine to utilize the most efficient resources available.
    • EventPriority.LOWEST - so other plugins can take priority over this one. It only writes metadata to a store, so doesn't need priority over everything else.
    • I ignore events cancelled by other plugins, because they're more likely going to need to cancel the event. Say for example, if LogBlock had an InventoryOpenEvent to check logs. If I didn't ignoreCancelled, then the person doing the event would have to wait. You don't want that ;)
    • The runnable updates everyone who has metadata, and ignores those who don't. The reason behind using a runnable is that it works at the server's speed, and takes up very little time on the thread. Calendar will likely slow down its timings if the server was to freeze, but I'm not sure.
    • If you delve deeper into the Metadata API like I have, then you'll find that in essence it's just a combination of HashMap and WeakHashMap. Metadata is also async thread safe - HashMaps tend to have the ability to throw concurrency exceptions if not done correctly.
    If you can give people the best experience without causing more issues than you need to, why not? Plus, it's a good way to learn the API :p
     
  19. Offline

    DemmyDemon

    Hmm, I'm clearly misunderstanding some stuff here.

    EventPriority.LOWEST runs absolutely first, meaning your actions and result have the lowest priority, right?
    Quoth the javadocs, "Event call is of very low importance and should be ran first, to allow other plugins to further customise the outcome"
    In the opposite end, EventPriority.MONITOR means you get informed so late you're not even supposed to change the outcome.
    That's why I do .MONITOR whenever I'm not going to change or cancel anything, and just want to ... urr ... monitor the event.

    IgnoreCanceled, of course, makes a pile of sense in it self. I mean, you wouldn't care about a teleport event if it was cancelled. Had this been an active project of mine I'd hop right in and add that!

    However, paired with EventPriority.LOWEST, how would you know if it was canceled further up the priority chain? It's in this specific context I must have missed something about the EventPriority system, I think.

    While I see that the runnable updates only those with MetaData, why use it at all? I mean, in my solution I've replaced the whole reoccurring runnable with a simple hashmap check. I'd love to hear what I missed on this, as I can't see how your solution has any potential to be faster.
    You still have to iterate over every single player and look up their metadata once every N ticks, right?
    And when one is found, there is a pretty serious chunk of code to extract the metadata, again including an iteration over a potentially large heap/stack (no idea how Java does this) of objects.
    Still, you're right that if the server is running at crappy speeds the Calendar will still only do one second per second, while using a runnable at ticked intervals will run every N ticks regardless of actual time passed. If you're having that kind of problems, however, I think a runnable for tiny trivial tasks is the last thing you need, hence my reliance on the "real time" Calendar.
    Am I wrong in this? What am I missing?
    The middle ground, I guess, is to store server ticks-since-launch and not Calendar instances.

    One thing I don't like about using a timer for this is that if wait-time-seconds is configured too low (one second, as the original plugin request called for), and the teleport happens just before your 20L timer hits, the restriction will get cleared before the second is up.
    One clear omission on my part is the lack of information to the player as to why the inventory didn't open. Ooops.
    My FileConfiguration stuff is also only the most basic support for configuration possible, with no saving of defaults or anything.
    As for permissions support, I think that while the support is nice, it just needlessly complicates this ever so basic concept. I mean, why would you have a burning need to open your inventory before the time expires.

    All in all, I find it fascinating to compare our approaches to the problem. I hope you don't mind.
     
  20. Offline

    RROD

    Yeah, I see what you mean. You're right about EventPriority.MONITOR, I'll fix that. And you're only Iterating over one possible key; the only instance of it iterating over more than one key is if two plugins share the same one.
    I suppose to me, it's more "this way" or "that way" in designing it. To be honest, it never crossed my mind to use Calendar, and I had already written it before seeing your code. Checking the time difference live, without the need of a runnable is certainly better for this sort of thing. Give me 5 mins and I'll change some stuff around :)

    Edit: Updated the original links.
     
  21. Offline

    Janmm14

    Well, why you are using a calendar object and not System.currentTimeMillis() ? That would save a bit memory, I think.
     
  22. Offline

    DemmyDemon

    Code:
    for (Player player : Bukkit.getOnlinePlayers()) {
    This iterates over every player, and happens every 20 ticks. Not at all a resource hog by any stretch of the imagination, but if there are 200 players it will still have to do a check for player.hasMetadata("inwaitTime") for each of them once every 20 ticks.
    **shrug** I don't think this is even significant, just interested in poking at the differences in our thinking to see what I can learn from it.
    And yeah, it's clearly just a design preference. I don't think either solution is much superior (...if I add IgnoreCanceled, that is!), so this is all academical for me.

    One thing that strikes me is that if the demands on the plugin grows, your design choice is much better suited to raise to the occasion. I'd have to do some design changes to accommodate a larger feature set, while you could just add the features.
    Yes, that's oversimplified, but I'm sure you get what I mean.

    The Calendar object is very very very very light-weight, so you would be saving something in the order of 100 bytes per teleported player, I think. As I don't have my test setup for Java installed yet (recently reinstalled my workbox and all the work I've done since then hasn't been Java-related), I can't profile it to see what the difference is.
    If you're concerned about saving 1MB of RAM on a server with a hundred players, then perhaps Calendar-vs-raw-long is not the best place to start optimizing.
    The only reason I used it at all is the very convenient..
    Code:
    compare.set(Calendar.SECOND,compare.get(Calendar.SECOND) - delayTime);
    ...that makes it very easy to see exactly what is being changed. Sure, storing the raw Long of system milliseconds time isn't that much harder to read/maintain, and it DOES save some RAM, but in the real world I don't think it makes any difference.
    Same thing as with the iterate-over-players vs. check-relevant-players-when-it-happens constructs: The difference is largely design choice and academical.
     
    RROD likes this.
  23. Offline

    TheOnlyRealTGS

    I find it kinda funny how i don't understand the most of what you guys are talking about xD
    Anyway, i can open my inventory right after using the warp command from essentials using both of your plugins :/
     
  24. Offline

    DemmyDemon

    Haha, no worries, if you had all this buttoned down yourself you wouldn't be requesting plugins in the first place ;-)


    Well, crap. That means warping doesn't trigger a teleportation event.
    I'd file that as a bug against Essentials and/or Bukkit.
    I have no idea where the bug might be, but I sort of assume Essentials uses the standard Bukkit API for teleportation, meaning the bug is likely in (Craft)Bukkit.
    Previous experience with Essentials, however, means I won't rule it out as the source of the problem.
     
  25. Offline

    TheOnlyRealTGS

    Tried with /mv tp Lobby (multiverse) works too
    Tried with /tppos -1571 151 -1306 works too (essentials cmd)
    EDIT: Tried with /tp 1 1 1 (default vanilla cmd) works too

    RROD
     
  26. Offline

    doubleboss00

    Just wondering what is this bug?
     
  27. Offline

    Garris0n

     
  28. Offline

    TheOnlyRealTGS

  29. Offline

    DemmyDemon

    Very odd that the Entity.teleport methods don't cause a teleport event.
    That's the only way I can see that neither method would work.

    Either that, of course, or InventoryOpenEvent's cancellation is not respected.
    Thinking about it, that is the most likely culprit.

    If I had my testing setup ready I would simply attach a debugger to bukkit and see which it is, but alas, no such luck :-P
    Anyone feel like compressing their CraftBukkit-and-IntelliJ setup and uploading somewhere for me to mooch off of?
     
  30. Offline

    Gamecube762

    MultiInv, Never had a problem related to per world invs and per gamemode invs =P
     

Share This Page