Slimefun4 icon indicating copy to clipboard operation
Slimefun4 copied to clipboard

New Gps Activation Device (Group) (Whitelist teleporter)

Open Toast732 opened this issue 2 years ago • 6 comments

Description

This would add a new item, the GPS Activation Device (Group) which the player can right click to add players to its whitelist, allowing for private teleporters for a team or a group, without having uninvited guests teleporting to your possibly secret bases

Proposed changes

Added the group teleporter activation plate to work as a teleporter activation plate, added some files (mostly based off of waypoints system) that allow the teleporter to save its whitelist in the files per user, and added the group teleporter activation plate itself

Related Issues (if applicable)

N/A

Checklist

  • [x] I have fully tested the proposed changes and promise that they will not break everything into chaos.
  • [x] I have also tested the proposed changes in combination with various popular addons and can confirm my changes do not break them.
  • [x] I followed the existing code standards and didn't mess up the formatting.
  • [x] I did my best to add documentation to any public classes or methods I added.
  • [x] I have added Nonnull and Nullable annotations to my methods to indicate their behaviour for null values
  • [ ] I added sufficient Unit Tests to cover my code.

Toast732 avatar Oct 18 '21 16:10 Toast732

Merge remote-tracking branch 'origin/master'

Ignore that lol, I once was attempting to add a feature in the origin mod, and for whatever reason my IDE keeps using that, it doesnt affect anything.

Toast732 avatar Oct 18 '21 16:10 Toast732

I appreciate the PR, there are definitely some improvements to be done in formatting. The way of storing this data also seems very awkward. Storing it in the block itself rather than attaching to some player and then having to read that player.

WalshyDev avatar Oct 18 '21 16:10 WalshyDev

I appreciate the PR, there are definitely some improvements to be done in formatting. The way of storing this data also seems very awkward. Storing it in the block itself rather than attaching to some player and then having to read that player.

hey! so if im not mistaken, only the tag of the owner is stored in the block, as how to the personal teleporter plate works, and the data storage is saved using the player's uuid, not a block for the whitelisted users, I did just find a few bugs so im gonna work on those

Toast732 avatar Oct 18 '21 16:10 Toast732

Haven't given this a too thorough look, but I a already starting to see quite a lot of stuff I don't particularly like. To be completely honest I cannot really say when or even if this will be merged.

On another note, your commit history is pretty messed up. You have 35 commits, out of which only 7 have proper commit names.

TheBusyBiscuit avatar Oct 21 '21 10:10 TheBusyBiscuit

Haven't given this a too thorough look, but I a already starting to see quite a lot of stuff I don't particularly like. To be completely honest I cannot really say when or even if this will be merged.

Hey sorry! this was my first time ever really working on something like this, so a lot of the code is based on how the waypoints work, I contacted walshy in dms about how they don't like how it saves in the player profile, and they didn't like the idea of it saving in BS either, so i'm not sure what to use haha. Most of the code for saving has been copied and pasted from the waypoints system, as I thought using a pre-existing system would be better than making my own which would be harder to maintain and possibly less stable.

On another note, your commit history is pretty messed up. You have 35 commits, out of which only 7 have proper commit names.

Sorry, I wasn't sure what to name the commits for updating the readme files as I thought they were self explanatory and didnt need much more context, sorry.

Toast732 avatar Oct 23 '21 16:10 Toast732

Haven't given this a too thorough look, but I a already starting to see quite a lot of stuff I don't particularly like. To be completely honest I cannot really say when or even if this will be merged.

Hey sorry! this was my first time ever really working on something like this, so a lot of the code is based on how the waypoints work, I contacted walshy in dms about how they don't like how it saves in the player profile, and they didn't like the idea of it saving in BS either, so i'm not sure what to use haha. Most of the code for saving has been copied and pasted from the waypoints system, as I thought using a pre-existing system would be better than making my own which would be harder to maintain and possibly less stable.

On another note, your commit history is pretty messed up. You have 35 commits, out of which only 7 have proper commit names.

Sorry, I wasn't sure what to name the commits for updating the readme files as I thought they were self explanatory and didnt need much more context, sorry.

sorry if this sounded rude at all, didnt mean to in any way, basically theres a bug where the owner must be online or it will not work, but I am fairly certain this is due to me using the player storage, and from this feedback I was thinking of rewriting the code to not need regex but I also thought if im gonna rewrite it, I should wait to see what storage solution they want me to use so theres no need for multiple rewrites, basically I'm just wondering what storage solution would be best to use for storing whos allowed to use the teleporter, thanks in advance!

Toast732 avatar Oct 23 '21 19:10 Toast732

Closing this because the branch is stale. Please reopen a new PR if you still want this in make sure to also do it from a different branch and not master next time :)

J3fftw1 avatar Jul 29 '23 16:07 J3fftw1