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

Making TFM at least somewhat standardized #1

Closed

Conversation

DragonSlayer2189
Copy link

Hi yes, i just spent the last like 10 hours working on this, so here you go, I went through and corrected every inconsistency I could find and also made some other slight adjustments. I am about 80% sure that none of the edits made to this will break anything however you may want to look through everything just in case. I am also sorry for not doing it all in one commit because by the time i learned that was a thing i was already like 51 commits in.

so yeah, am i the official tfm nitpicker yet?

I want to make this bit not use partialname at all since it sometimes causes issues, but whatever
1) because the user has to manually accept the resource pack, the joke doesn't work too well
2) the file hasn't existed for more than 3 months
3) its just a bad command
maybe add -a flag just cuz, idk
this was added back??????
this was added back????????
@DragonSlayer2189
Copy link
Author

did... github break or something, it has been trying to check if the build is valid for the last 4 hours

autobuild fucked up so i added litterly nothing to see if that fixes it
Copy link
Member

@Wild1145 Wild1145 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've more or less reviewed everything now. There are 8 files I've looked at but need to check through again with a fresh head so will look at those tomorrow.

Also tagged @AtlasMediaGroup/totalfreedom-developers on a load of these that need them to look at properly.

@@ -27,7 +27,7 @@
{

private final List<String> BLOCKED_WORLDEDIT_COMMANDS = Arrays.asList(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WTF, why is this even here? Why is this entire thing not in config...?

CC @AtlasMediaGroup/totalfreedom-developers

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good question

Someone please review my aditions to /unban where i added -q to make it follow the same structure as unbanip and unbanname (even though unbanip and unbanname don't have the -r flag that unban does, but i don't think they need it)
I also changed /ro to not send the blocks removed message to everyone because that gets spammy sometimes

I don't remember who told me to do this but someone in vc said it was better with no periods on the staff actions
I am basicly just pushing this so that mabye the autobuild doens't fail
@DragonSlayer2189
Copy link
Author

if this build doesn't fail then I think I have fixed everything

Copy link
Member

@Wild1145 Wild1145 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also to save my e-mail inbox this time, could you not reply to every single comment with resolved? Just mark it as resolved so I don't get 2000000000 e-mails again.

Still need to review other files but I've done my best with the time I had spare tonight.

Admin when refering to the rank
admin when refering to anything else
"conflicting files" -_-
mbs couldn't use worledit it mbworld because of this
@DragonSlayer2189
Copy link
Author

I don't really feel like going through all of these commands that apparently now have conflicts (there was probably a refactor, but i haven't checked yet) but if i have to I will, for now, can a dev please look at the PLEASE FIX commit as the world restrictions are severely broken by brushes

@DragonSlayer2189
Copy link
Author

bruuuhh, the conflicts aren't resolved after doing any of those edits. whatever, I guess i willl let you guys deal with it

SupItsDillon added a commit that referenced this pull request Dec 14, 2020
@Wild1145
Copy link
Member

Wild1145 commented Jan 1, 2021

Given the nature of this PR and the fact it's ultimately just too large to reasonably review and the fact they are all minor changes, I'm going to close and decline this PR. I would suggest if you do want to make these changes to tackle them in smaller logical blocks rather than trying to change every single command and message in the entire plugin, because it's just not a sustainable way to make these changes.

@Wild1145 Wild1145 closed this Jan 1, 2021
@Wild1145 Wild1145 added the wontfix This will not be worked on label Jan 1, 2021
allinkdev pushed a commit that referenced this pull request Jun 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
2 participants