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

Feature: Whitelist #140

Merged
merged 40 commits into from
Dec 30, 2024
Merged

Conversation

WhaleFromMars
Copy link
Contributor

@WhaleFromMars WhaleFromMars commented Dec 11, 2024

adds whitelist support - same format as the vanilla file

I can look at adding tests afterwards for valid or not configs, all been manually tested so far

@0xnim
Copy link
Contributor

0xnim commented Dec 13, 2024

why not just use minecraft format

[
  {
    "uuid": "f430dbb6-5d9a-444e-b542-e47329b2c5a0",
    "name": "username"
  },
  {
    "uuid": "e5aa0f99-2727-4a11-981f-dded8b1cd032",
    "name": "username"
  }
]

@Mcrtin
Copy link

Mcrtin commented Dec 13, 2024

#140 (comment)

Using json seems kinda redundant. The data is not very complex, a newline/comma/space separated file format would suffice. Not a big deal tho since it should only be run once on start so performance isn't a major concern

@0xnim
Copy link
Contributor

0xnim commented Dec 13, 2024

Using JSON is better because we can use it directly without rewriting the format. It’s a standard, so other programs can work with it easily. JSON also supports more complex structures if we need them in the future(op level for example), and there are plenty of tools available for handling it. Overall, it’s more flexible and easier to work with than a custom format like comma seperated.

@0xnim
Copy link
Contributor

0xnim commented Dec 13, 2024

i'm fine doing it if you dont want to

@0xnim
Copy link
Contributor

0xnim commented Dec 13, 2024

Also can we move this to a feature branch so i can pr to it?

@Mcrtin
Copy link

Mcrtin commented Dec 13, 2024

If I recall correctly, it was already in json in the first place, but got changed because of that comment

@WhaleFromMars
Copy link
Contributor Author

This is for a whitelist, it was moved from vanilla file parity due to pr reviews, unless im missing something I see no need for a more complex structure in the future, I was planning on keeping the same rules as vanilla. if youre op (when implemented), you bypass it, simple as, theres no need to add a layer to configuration, I dont see why another program would need to use it, plugins will have access through whatever api gets defined in future, the goal is to make it as simple for people to use as possible, using the format in this discord message https://discord.com/channels/1277314213878173726/1302503481432604742/1317064474963873822

@ReCore-sys
Copy link
Collaborator

Using JSON is better because we can use it directly without rewriting the format. It’s a standard, so other programs can work with it easily. JSON also supports more complex structures if we need them in the future(op level for example), and there are plenty of tools available for handling it. Overall, it’s more flexible and easier to work with than a custom format like comma seperated.

I don't see why other programs need to interact with it? The reason we'd have it as as a file in the first place instead of storing in the database is so that it's easy for the end user to modify it by hand. I'm also not sure why we'd need a more complex data structure, a list of user names and uuids is fine.
Overall it's meant to be a simple format so that an average end user can just set up a quick server to play minecraft with their friends, having the whitelist be complicated to set up will deter people.

@0xnim
Copy link
Contributor

0xnim commented Dec 14, 2024

Server managment software uses a json as a standard. Having to write libraries just to interact with the whitelist does not sound good

@GStudiosX2 GStudiosX2 mentioned this pull request Dec 14, 2024
@WhaleFromMars WhaleFromMars marked this pull request as ready for review December 16, 2024 15:35
@0xnim
Copy link
Contributor

0xnim commented Dec 16, 2024

Just merge my pr

@0xnim
Copy link
Contributor

0xnim commented Dec 16, 2024

This is the exact reason we have feature branches. Dony make pra directly to main

@Mcrtin
Copy link

Mcrtin commented Dec 16, 2024

after the second hyphen, i believe there can only be an 8, 9, A, or B

@GStudiosX2
Copy link

GStudiosX2 commented Dec 17, 2024

This is the exact reason we have feature branches. Dony make pra directly to main

No one has access to make branches or push to branches other than Recore and Sweatty

@Skullians
Copy link
Contributor

clippy is not happy

@ReCore-sys ReCore-sys merged commit cc11874 into ferrumc-rs:master Dec 30, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants