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

Proper /nldg confirmation #383

Merged
merged 10 commits into from
May 1, 2024
Merged

Proper /nldg confirmation #383

merged 10 commits into from
May 1, 2024

Conversation

drekamor
Copy link
Contributor

People still delete group by accident because the command is similar to /nlsdg and the confirmation request is green (which implies safety of the operation)

@okx-code
Copy link
Contributor

okx-code commented Mar 22, 2024

You can use NamedTextColor.RED instead of an RGB colour. It can also be provided as an argument in Component.text

@awoo-civ
Copy link
Contributor

🎉

@okx-code
Copy link
Contributor

Could you do the confirmation without an underscore?

@drekamor
Copy link
Contributor Author

Could you do the confirmation without an underscore?

Sure. I just wasn't sure whether the command uses greedy string or normal string arguments. I'll look into it

@drekamor
Copy link
Contributor Author

@okx-code I removed underscores from the message but I'm not sure whether the command takes a normal string argument or a greedy string

@okx-code
Copy link
Contributor

Have you tested it?

@drekamor
Copy link
Contributor Author

Not yet, need to build the plugin

@drekamor
Copy link
Contributor Author

@okx-code
image
tested, seems to be working

@awoo-civ
Copy link
Contributor

awoo-civ commented Mar 22, 2024

@drekamor Hmm, perhaps /nldg CONFIRM DELETION should be enclosed in quotes, i.e. Type '/nldg CONFIRM DELETION test' within 15 seconds. Should help disambiguate between the string that is to be typed and the rest of the sentence.

@drekamor
Copy link
Contributor Author

@drekamor Hmm, perhaps /nldg CONFIRM DELETION should be enclosed in quotes, i.e. Type '/nldg CONFIRM DELETION test' within 15 seconds.. Should help disambiguate between the string that is to be typed and the rest of the sentence.

Was thinking about that as well. I'll add them

@MrJeremyFisher
Copy link
Contributor

Imo the message shouldn't mention snitches and bastions as Citadel doesn't need to be run with Bastion and Jukealert and it won't necessarily be on other servers.

@awoo-civ
Copy link
Contributor

That's up to the admins I suppose, but personally I'm vehemently against supporting anything other than the needs of CivMC if it requires any sacrifices.

@AngrySoundTech AngrySoundTech added Category: Feature Priority: Low Meta: Needs Discussion Needs to be discussed more before moving further labels Mar 23, 2024
@AngrySoundTech AngrySoundTech added this to the 1.27.0 - Next Update milestone May 1, 2024
@RedDevel2 RedDevel2 merged commit fbc5552 into CivMC:main May 1, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Feature Meta: Needs Discussion Needs to be discussed more before moving further Priority: Low
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants