Plugin update #25

Merged
ShadowRanger merged 2 commits from master into master 2015-06-22 04:56:48 -04:00
ShadowRanger commented 2015-06-22 04:38:48 -04:00 (Migrated from github.com)

So I fixed and changed a few things in this pull request. Please let me know if you're not happy with anything in this pull request.

  • Added Maven support
  • Fixed potential memory leak with use of player objects in Maps
  • Use UUIDs instead of names for handling players
  • Removed support for old CraftBukkit versions
  • Minor code refactoring
So I fixed and changed a few things in this pull request. Please let me know if you're not happy with anything in this pull request. - Added Maven support - Fixed potential memory leak with use of player objects in Maps - Use UUIDs instead of names for handling players - Removed support for old CraftBukkit versions - Minor code refactoring
lishid commented 2015-06-22 04:56:01 -04:00 (Migrated from github.com)

Looks nice, great work! I didn't get a chance to test it though. I added you on the bukkitdev project as well, if you feel like uploading the jar there.

Looks nice, great work! I didn't get a chance to test it though. I added you on the bukkitdev project as well, if you feel like uploading the jar there.
ShadowRanger commented 2015-06-22 04:58:37 -04:00 (Migrated from github.com)

Sure, I'd be happy to do that. You added the wrong ShadowRanger though. Unfortunately someone took my name on BukkitDev so I had to use ShadowRanger24 instead.

Sure, I'd be happy to do that. You added the wrong ShadowRanger though. Unfortunately someone took my name on BukkitDev so I had to use ShadowRanger24 instead.
ShadowRanger commented 2015-06-22 04:59:56 -04:00 (Migrated from github.com)

I just sent you a PM on BukkitDev with my actual account.

I just sent you a PM on BukkitDev with my actual account.
lishid commented 2015-06-22 05:00:52 -04:00 (Migrated from github.com)

That's unfortunate... I corrected the permissions on bukkitdev. Thanks for helping out! (I also have to say that I'm quite embarrassed reading over my old code, seeing bad practices everywhere)

That's unfortunate... I corrected the permissions on bukkitdev. Thanks for helping out! (I also have to say that I'm quite embarrassed reading over my old code, seeing bad practices everywhere)
lishid commented 2015-06-22 05:04:07 -04:00 (Migrated from github.com)

I've added you as a github commiter as well, in case you want to oversee the repo anytime in the future.

I've added you as a github commiter as well, in case you want to oversee the repo anytime in the future.
ShadowRanger commented 2015-06-22 05:10:53 -04:00 (Migrated from github.com)

Alright sweet, thanks. I'll upload the updated jar of the plugin tonight :)

Alright sweet, thanks. I'll upload the updated jar of the plugin tonight :)
ShadowRanger commented 2015-06-22 09:15:09 -04:00 (Migrated from github.com)

Actually I have a few questions which I was hoping you could provide some input on.

Currently the plugin uses item id's for setting the item which is used to perform the openinv command when a player right clicks. Due to the fact that this is now deprecated with the Minecraft changes to how blocks and items are identified, I thought it might be worth updating the plugin so it uses names instead. The only thing with this is for people who are currently using id's for the item they want to use and how they might have to change their config to use the name of the item instead.

Another thing that I wanted to ask was in regards to the current method used for setting a player's status for silent/any chest into the config. Right now it uses player names when saving to config so I thought it might be worth changing them to UUID's instead. But then again, a similar problem to above would be how to approach old configs. I was thinking of maybe using the UUIDFetcher I added to retrieve the UUID's of any of the usernames people might have added and converting the usernames to them instead. But with that I'm not sure because I wouldn't want to have the plugin do that without them knowing and then when they happen to check wonder what happened to their config.

Please let me know what you think.

Actually I have a few questions which I was hoping you could provide some input on. Currently the plugin uses item id's for setting the item which is used to perform the openinv command when a player right clicks. Due to the fact that this is now deprecated with the Minecraft changes to how blocks and items are identified, I thought it might be worth updating the plugin so it uses names instead. The only thing with this is for people who are currently using id's for the item they want to use and how they might have to change their config to use the name of the item instead. Another thing that I wanted to ask was in regards to the current method used for setting a player's status for silent/any chest into the config. Right now it uses player names when saving to config so I thought it might be worth changing them to UUID's instead. But then again, a similar problem to above would be how to approach old configs. I was thinking of maybe using the UUIDFetcher I added to retrieve the UUID's of any of the usernames people might have added and converting the usernames to them instead. But with that I'm not sure because I wouldn't want to have the plugin do that without them knowing and then when they happen to check wonder what happened to their config. Please let me know what you think.
lishid commented 2015-06-22 16:03:58 -04:00 (Migrated from github.com)

Currently the plugin uses item id's for setting the item which is used to perform the openinv command when a player right clicks. Due to the fact that this is now deprecated with the Minecraft changes to how blocks and items are identified, I thought it might be worth updating the plugin so it uses names instead. The only thing with this is for people who are currently using id's for the item they want to use and how they might have to change their config to use the name of the item instead.

I'm thinking of running some kind of migration on plugin load that would check and read the old integer and write it back as a string corresponding to the item. Then we can internally use the item name instead.
I'd say add a "config version" field that will identify whether the config is from the old world (no config version field) or new world (with, say, version=1).

Another thing that I wanted to ask was in regards to the current method used for setting a player's status for silent/any chest into the config. Right now it uses player names when saving to config so I thought it might be worth changing them to UUID's instead. But then again, a similar problem to above would be how to approach old configs. I was thinking of maybe using the UUIDFetcher I added to retrieve the UUID's of any of the usernames people might have added and converting the usernames to them instead. But with that I'm not sure because I wouldn't want to have the plugin do that without them knowing and then when they happen to check wonder what happened to their config.

I don't think anyone would manually touch the config for that. It would have been probably better if I used some kind of DB (like sqlite) but the config option was lightweight so I chose that.
I'd say use the same method as the config upgrade I wrote above. Migrate everything to UUIDs and then put in a config version so you don't attempt to migrate again.

> Currently the plugin uses item id's for setting the item which is used to perform the openinv command when a player right clicks. Due to the fact that this is now deprecated with the Minecraft changes to how blocks and items are identified, I thought it might be worth updating the plugin so it uses names instead. The only thing with this is for people who are currently using id's for the item they want to use and how they might have to change their config to use the name of the item instead. I'm thinking of running some kind of migration on plugin load that would check and read the old integer and write it back as a string corresponding to the item. Then we can internally use the item name instead. I'd say add a "config version" field that will identify whether the config is from the old world (no config version field) or new world (with, say, version=1). > Another thing that I wanted to ask was in regards to the current method used for setting a player's status for silent/any chest into the config. Right now it uses player names when saving to config so I thought it might be worth changing them to UUID's instead. But then again, a similar problem to above would be how to approach old configs. I was thinking of maybe using the UUIDFetcher I added to retrieve the UUID's of any of the usernames people might have added and converting the usernames to them instead. But with that I'm not sure because I wouldn't want to have the plugin do that without them knowing and then when they happen to check wonder what happened to their config. I don't think anyone would manually touch the config for that. It would have been probably better if I used some kind of DB (like sqlite) but the config option was lightweight so I chose that. I'd say use the same method as the config upgrade I wrote above. Migrate everything to UUIDs and then put in a config version so you don't attempt to migrate again.
AlfieC commented 2016-08-14 11:04:20 -04:00 (Migrated from github.com)

@ShadowRanger worlds largest necro but -

I was looking over the commit - would it not be useful, for the sake of backwards compatibility (for plugins using the player methods) to just have a bunch of methods for Player but they just return the same method but using Player#getUniqueId()?

@ShadowRanger worlds largest necro but - I was looking over the commit - would it not be useful, for the sake of backwards compatibility (for plugins using the player methods) to just have a bunch of methods for Player but they just return the same method but using Player#getUniqueId()?
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: minster586/OpenInv#25
No description provided.