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

Add support for article=True and format=fmt to $You(r) and $you(r) #3511

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chiizujin
Copy link
Contributor

Brief overview of PR changes/additions

This adds two kwargs to the funcparser functions $You()/$you() and $Your()/$your().

  • article=True

    This will use get_numbered_name instead of get_display_name to return, for example, "a mouse" rather than "mouse".

  • format=upper|lower|capital|title

    This enforces specific casing of the output. Note that this overrides the current behaviour of only capitalising "You/Your", making the upper- and lower-case variations of the functions interchangeable. If no format is specified then the current behaviour is used, with the separate functions working independently.

Motivation for adding to Evennia

More options for the output of $You(), etc.

Examples

Before:

Using:

attacker.location.msg_contents(
    "$You() $conj(attack) $you(target).",
     from_obj=attacker, mapping={"target": target}
)
rat attacks you.
You attack rat.

After:

Using capitalisation formatting. This forces the first noun to always be capitalised, even if it is not "you".:

attacker.location.msg_contents(
    "$You(format=capital) $conj(attack) $you(target).",
     from_obj=attacker, mapping={"target": target}
)
Rat attacks you.
You attack rat.

Including a request for the indefinite article:

attacker.location.msg_contents(
    "$You(article=True, format=capital) $conj(attack) $you(target, article=True).",
     from_obj=attacker, mapping={"target": target}
)
A rat attacks you.
You attack a rat.

Without any of the new formatting kwargs:

attacker.location.msg_contents(
    "$You() $conj(attack) $you(target).",
     from_obj=attacker, mapping={"target": target}
)

Output is the same as the current behaviour:

rat attacks you.
You attack rat.

An example using $your():

attacker.location.msg_contents(
    "$You(article=True, format=capital) $conj(strike) $your(target, article=True) leg.",
     from_obj=attacker, mapping={"target": target}
)
A rat strikes your leg.
You strike a rat's leg.

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 looks good except for one thing: As per your examples,

$You() would have to be supplied as $You(format=capital) for rat to become Rat. I think this is unintuitive.

  • Since we are now moving to modifying the object's display name anyway, I think it makes sense that $You()would return capitalizedRatby default, and you'd have to do$You(format=lower)to make it returnrat. Without this, there is no difference between $Youand$you`.
  • On the other hand $you() makes sense to just keep the native display name unless a format is given (it shouldn't default to lower() I don't think, but I'm open to discussion on that).

@chiizujin
Copy link
Contributor Author

chiizujin commented Apr 28, 2024

That sounds fair enough. The only thing I'd do differently is to not use capitalize() or title() for the $You/rat combination. Rather, I'd do s[:1].upper() + s[1:]

This is because the two functions above both change more than just the first character:

>>> s = 'The Knights of Ni'

>>> s.capitalize()
'The knights of ni'
# Not desired as it has lower-cased the rest of the string

>>> s.title()
'The Knights Of Ni'
# Not desired as it has upper-cased the first letter of every word
>>> s = 'the Knights of Ni'
>>> s[:1].upper() + s[1:]
'The Knights of Ni'
# This seems more acceptable

I have already added "first": lambda s: s[:1].upper() + s[1:] to _YOU_FORMAT_FUNCS in my local version as I have a use case for it, so I could add that here. I'm not sure about calling it first, though, as that doesn't really say what it does. I just thought upper_first was a bit long.

@InspectorCaracal
Copy link
Contributor

InspectorCaracal commented Apr 28, 2024

I know this is a bit of a nitpick, but I think it would be better if this change were implemented using a different keyword, such as case, rather than format. The keyword of format suggests that it would take a string matching Python's formatting syntax, so I think having it specified instead as e.g. case or capitalization (or an abbreviation thereof) would be more intuitive for the actual feature.

@chiizujin
Copy link
Contributor Author

case does sound better, yes. It's closer to what it's doing.

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