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

Support full capitalization in GMCP commands #3521

Merged
merged 1 commit into from Apr 27, 2024

Conversation

InspectorCaracal
Copy link
Contributor

@InspectorCaracal InspectorCaracal commented Apr 27, 2024

Brief overview of PR changes/additions

Changes the GMCP command encoding to only run capitalize() on command parts that are not fully uppercase.

Motivation for adding to Evennia

A previous conversation on Discord identified the current GMCP encoding's command capitalization as somewhat lacking, based on their own previous use of GMCP in other MUD libraries. Additionally, today a specific use-case arose that is currently impossible: sending a Mudlet package install command, as per the Mudlet docs - it requires the ability to send a fully capitalized command section, Client.GUI.

Other info (issues closed, discussion etc)

I'm not extremely familiar with the specifications of the GMCP protocol, so I wrote this under the assumption that mixed-case command parts are not allowed.

Copy link
Member

@Griatch Griatch left a comment

Choose a reason for hiding this comment

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

This is not clear from the GMCP specs, hence different clients implement this differently. My interpretation is that GMCP commands are always on the form Word.Word and not Word.WORD but that may also be because the IRE reference implementation looked like that (but I could be wrong too, it's been a while). Either way, if people find the current implementation hinders them, I don't see a problem with making it more lenient.

@Griatch Griatch merged commit a9d77d0 into evennia:main Apr 27, 2024
6 of 9 checks passed
@MorquinDevlar
Copy link

I would still advocate for not enforcing any kind of capitalisation on GMCP output. Let the developer set it like they want.

Take a look at this list from IRE:

_Room.WrongDir
Sent if the player attempts to move in a non-existant direction using the standard movement commands
Upon receiving this message, the client can safely assume that the specified direction does not lead anywhere at this time
Message body is a string with the name if the non-existant exit
Example: Room.WrongDir "ne"

Room.Players
Object containing player details, each key is the short name of the player and each value is the full name including titles for the player
Example: Room.Players [{ "name": "Tecton", "fullname": "Tecton the Terraformer" }, {"name": "Cardan", "fullname": "Cardan, the Curious" }]

Room.AddPlayer
Message body has the same object structure as Room.Players except that it only contains the one player being added to the room.
Example: Room.AddPlayer { "name": "Cardan", "fullname": "(Cardan, the Curious)" }

Room.RemovePlayer
Message body has the same object structure as Room.Players except that it only contains the one player being removed from the room.
Example: Room.RemovePlayer "Cardan"_

They have a lot of commands sent with SnakeCase, which would not be compatible with an enforced capitalisation.

@InspectorCaracal
Copy link
Contributor Author

@MorquinDevlar Since this PR has already been merged and closed, you'd be best off creating a feature request or second PR for your desired outcome.

@MorquinDevlar
Copy link

@InspectorCaracal You are absolutely right, and I actually also meant CamelCase not snake_case. My only excuse was the lateness of the hour, as I was writing.

I'll push in a PR, with my suggested change.

I really appreciate you taking the time to make the first change. Makes it easier to put in my own suggestion.

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.

None yet

3 participants