Skip to content

Improve config comments & ordering #6108

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

Open
wants to merge 15 commits into
base: 2.x
Choose a base branch
from

Conversation

JasonHorkles
Copy link
Contributor

@JasonHorkles JasonHorkles commented Mar 24, 2025

This PR significantly overhauls the comments of the EssentialsX configuration files, ensuring consistency, clarity, and overall improved grammar (validated by Grammarly and LLMs). There are still a few things that must be completed before this is ready to be merged, but no further significant changes are expected. A couple of options have been moved entirely to make more logical sense, but this will not affect any compatibility.

Remaining tasks:

  • Ensure permission consistency when surrounded by apostrophes
  • Make quotation and apostrophes consistent (values, permissions, and config references surrounded by apostrophes. Names/examples surrounded by quotes)
  • Make 'permissions plugin' be consistent (vs 'permission plugin')
  • Ensure consistent indentation
  • Lowercase mob names (if any remain)
  • Replace 'users' with 'players'
  • Ensure consistent 'above' and 'below' formatting (either make all or none of them surrounded by parentheses)
  • Pull new changes and repeat above steps
  • Separate commit: Add 'default' comment to every option (decided this is unnecessary and instead added a link to the default config)

This makes a bit more logical sense in the ordering
This adds slightly more logical ordering to the sign list, keeping them in related groups while sorting them alphabetically. Full alphabetical feels a bit messy, but could still be a valid option.
Includes permissions/files/keys surrounded by apostrophes, 'user' references replaced with 'player' (where appropriate), and additional grammatical fixes.
Almost there! Includes consistency with apostrophes and quotation marks, fixes some capitalization, and changes references of 'permission plugin' to 'permissions plugin' (naming verified by LuckPerms)
@JasonHorkles JasonHorkles marked this pull request as ready for review March 27, 2025 00:21
@JasonHorkles JasonHorkles requested a review from Copilot March 27, 2025 00:21
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the configuration file comments for EssentialsX, enhancing clarity, consistency, and punctuation across several resource files.

  • Enhanced phrasing and punctuation in kit configuration.
  • Standardized comment styles in custom item, worth, and teleport configurations.
  • Updated URLs and command syntax for clarity.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

File Description
Essentials/src/main/resources/kits.yml Improved comment phrasing and ordering
Essentials/src/main/resources/custom_items.yml Updated comment clarity on alias limitations
Essentials/src/main/resources/worth.yml Refined wording of worth configuration comments
Essentials/src/main/resources/tpr.yml Clarified command syntax in the teleport config

@JasonHorkles JasonHorkles marked this pull request as draft March 31, 2025 18:33
@JasonHorkles JasonHorkles marked this pull request as ready for review March 31, 2025 18:41
@mdcfe mdcfe modified the milestones: 2.21.2, 2.21.1 Apr 19, 2025

# Items not listed in this file will not be sellable on the server
# Setting the worth to 0 will sell items for free, delete the item or set to -1 to disable.
# Determines the value of items on the server.
Copy link
Member

Choose a reason for hiding this comment

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

This file could do with updating to only use 1.13+ names. Pre-flattening examples could be limited to a comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, I'd have to learn how the file actually works first though 😂

change-displayname: true

# This option will cause Essentials to show players' displaynames instead of usernames when tab completing Essentials commands.
# This option will cause Essentials to show players' display names instead of usernames when tab completing Essentials commands.
Copy link
Member

Choose a reason for hiding this comment

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

I'd be tempted to remove this option - it breaks things more often than it's useful. (I think this used to tab complete nicknames without prefixes/suffixes, but now includes those - so maybe that's a separate bug?)

spawnmob-limit: 10

# Shall we notify users when using /lightning?
# Shall Essentials notify smitten players when /lightning is used?
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Shall Essentials notify smitten players when /lightning is used?
# Should Essentials notify smitten players when /lightning is used?

# Commands in this list, will tell Essentials to 'not give up' the command to other plugins.
# In this state, which plugin 'wins' appears to be almost random.
# When a command conflicts with another plugin, Essentials will, by default, try to force the OTHER plugin to take priority.
# Commands in this list will tell Essentials not to "give up" the command to other plugins.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Commands in this list will tell Essentials not to "give up" the command to other plugins.
# Adding commands to this list will tell Essentials not to "give up" the command to other plugins.

Comment on lines +368 to +369
# Essentials' colored sign support will be enabled when any sign type is enabled.
# 'color' is not an actual sign type; it enables using color codes on signs when the correct permissions are given.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Essentials' colored sign support will be enabled when any sign type is enabled.
# 'color' is not an actual sign type; it enables using color codes on signs when the correct permissions are given.
# 'color' is not an actual sign type; it enables using color codes on signs when the correct permissions are given.
# Essentials' colored sign support can only be enabled when any other sign type is enabled.

I'm not sure if this order makes it clearer, or if it'd be better the original way around?

# Don't forget to remove the # in front of the line.
# For more information, visit https://essentialsx.net/wiki/Locale.html
#locale: en

# Should EssentialsX use player's language instead of the server's when sending messages?
# Should Essentials use player's language instead of the server's when sending messages?
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Should Essentials use player's language instead of the server's when sending messages?
# Should Essentials use the player's language instead of the server's when sending messages?

sleep-ignores-vanished-player: true

# Set the player's list name when they are AFK. This is none by default which specifies that Essentials
# Change the player's list (not tab) name when they are AFK. This is none by default, which specifies that Essentials
Copy link
Member

Choose a reason for hiding this comment

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

Unclear what "not tab" is referring to - tab completion or tab list?

# Give someone permission to teleport to a world with essentials.worlds.<worldname>
# This does not affect the /home command, there is a separate toggle below for this.
# Set to true to enable per-world permissions for teleporting between worlds with Essentials commands.
# This applies to /world, /back, /tp[a|o|here|all] but not warps.
Copy link
Member

Choose a reason for hiding this comment

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

The previous version of this line was probably slightly more accurate :p

# Users with essentials.chat.shout can override this by prefixing their message with an exclamation mark (!)
# Users with essentials.chat.question can override this by prefixing their message with a question mark (?)
# You can add command costs for shout/question by adding chat-shout and chat-question to the command costs section.
# If Essentials Chat is installed, this sets how many blocks a player's chat will travel. Set to 0 for global chat.
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to keep the EssentialsX name in any references to module names, to prevent someone looking for module jars from upstream.

(I'm a little mixed on removing the EssentialsX branding in general, but that's probably a separate discussion)

# You can specify a format for each group.
# You can also specify a format for each group.
# If using group formats, remove the # to activate the setting.
# Note: Group names are case-sensitive, so you must match them up with your permissions plugin.
Copy link
Member

Choose a reason for hiding this comment

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

Possibly worth a note on LuckPerms group aliases - aliases change the Vault group name

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.

2 participants