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 better alternative names for certain custom Cottle functions #2147

Open
richardbuckle opened this issue Mar 23, 2021 · 19 comments
Open

Add better alternative names for certain custom Cottle functions #2147

richardbuckle opened this issue Mar 23, 2021 · 19 comments
Assignees
Labels
9. enhancement The behaviour is as specified, but we would like to modify or extend the spec. debatable For when developers disagree. needs thought For when you need to slow down and consider things.

Comments

@richardbuckle
Copy link
Member

richardbuckle commented Mar 23, 2021

What happens now

The Cottle functions F(), Humanise(), P() and Spacialise() are unintuitively named and we are constantly asked what they do.

What I'd like to happen

Keep the existing names for backwards compatibility but add the following (suggested) aliases as the preferred form going forward:

  • F() -> InvokeScript()
  • Humanise() -> Approximate()
  • P() -> FixPronunciation() (not entirely sure about this name)
  • P() -> PronounceForContext(text, [context]) (liking this much more)
  • Spacialise() -> SpellOut()

How it can happen

I think this is a rare case where I would favour duplicating the very short implementation stubs in EddiSpeechResponder.CustomFunctions over evolving the underlying mechanism to support alternative names, but am open to persuasion.

Edit: this would also allow for separate documentation of the new and old names, allowing us to document the old names as deprecated.

EDDI Version

3.7.2

@richardbuckle richardbuckle added the 9. enhancement The behaviour is as specified, but we would like to modify or extend the spec. label Mar 23, 2021
@richardbuckle
Copy link
Member Author

richardbuckle commented Mar 23, 2021

@alterNERDtive
Copy link

  • P() -> FixPronunciation() (not entirely sure about this name)

Maybe just call it “Pronounce”?

@richardbuckle
Copy link
Member Author

I considered a few options. Naming things well is a bit of a difficult art.

Pronounce() was my first thought but then I wondered whether it conveyed the purpose of the function adequately. After all, everything is being pronounced, so does that name help scripters? But if enough scripters favor it, that would allay my concern.

PronounceUsingGameSpecificDictionary() is accurate but clumsy, and also too verbose.

PronounceNicely() is ... too vague.

PronounceJargon() could work but it's not really jargon, it's proper names like "Aisling Duval" and all the classical star names.

Keep the thoughts coming!

@jcdick1
Copy link

jcdick1 commented Mar 23, 2021

I considered a few options. Naming things well is a bit of a difficult art.

PronounceUsingGameSpecificDictionary() is accurate but clumsy, and also too verbose.

Keep the thoughts coming!

'GamePronounce()' ?

@Darkcyde13
Copy link

I thought maybe TryPronouncing() or PronounceBetter(), but how about Enunciate()? Or would that be too obscure?

@richardbuckle
Copy link
Member Author

richardbuckle commented Mar 23, 2021

Ooh, here's a thought, inspired by the optional second argument: PronounceForContext(text, [context])

  • You are in the {PronounceForContext(system.name, "starsystem")} system.
  • The controlling faction is {PronounceForContext(faction, "faction")}
  • You have been promoted by {PronounceForContext(powerplayFigurehead, "power")}

... etc

PronounceForContext(text) with the context omitted still works because the context is understood to be the game.

I think I like this one.

@richardbuckle
Copy link
Member Author

I chose PronounceForContext(text, [context]) over PronounceInContext(text, [context]) consciously, because I am currently helping with the Russian localisation, where the context in which a noun (or number) finds itself can determine its inflection.

So PronounceInContext(text, [context]) might be misleading to any scripter in an inflected language such as Russian, Polish, etc...

@richardbuckle
Copy link
Member Author

Yes, I am liking PronounceForContext(text, [context]) a lot.

@Tkael
Copy link
Member

Tkael commented Mar 23, 2021

Perhaps P() -> Phonetic() but that may not be sufficiently intuitive.

@richardbuckle
Copy link
Member Author

The more I think about it, the more I think that the second, optional, argument should be recognized in the function's name.

@Tkael
Copy link
Member

Tkael commented Mar 23, 2021

Will having multiple command names for the same action potentially make commands more confusing and more difficult to learn?

@richardbuckle
Copy link
Member Author

richardbuckle commented Mar 23, 2021

I don't see why if we cleanly document along these lines:

PronounceForContext(text, [context]) 
This function will attempt to provide phonetic pronunciation for the supplied text. This function uses SSML tags.
...
The older name for this function was P(); that name is now deprecated.
P(text)
An older name for PronounceForContext(text, [context]) <--hyperlink

@richardbuckle
Copy link
Member Author

Perhaps P() -> Phonetic() but that may not be sufficiently intuitive.

I was aiming for active verbs as I find them more intuitive in a scripting context.

@Tkael
Copy link
Member

Tkael commented Mar 24, 2021

People forget that we even have a Help file documenting these functions. 😉
I wonder if it might be more helpful to embed some additional info into the Script Editor UI (perhaps with tooltips or by prompting users with lists of available commands when users type specific characters?)

@alterNERDtive
Copy link

The more I think about it, the more I think that the second, optional, argument should be recognized in the function's name.

The way I see it it’s only optional for backwards compatibility anyway.

@Darkcyde13
Copy link

The more I think about it, the more I think that the second, optional, argument should be recognized in the function's name.

The way I see it it’s only optional for backwards compatibility anyway.

I believe Richard was referring to the 'context' part of PronounceForContext(text, [context]) as being the second, optional argument.

People forget that we even have a Help file documenting these functions. 😉

I think that's how this whole thing started, with a question on the forum that could have easily been answered by looking in the help file. 😉

Personally, I prefer the shorter command names. I feel that the longer ones take up too much space in a script and make it a little more difficult to read. In this case, where some scripts have a few P() all close together, this will look like a 'wall-of-text'. I feel keeping it short is more user-friendly, especially for new users. I'm reminded of the old computer adage of keeping things simple. :)

@richardbuckle
Copy link
Member Author

richardbuckle commented Mar 24, 2021

People forget that we even have a Help file documenting these functions. 😉

Here's the thing: there are two kinds of people: (a) those who will inhale any and all documentation available and (b) those who won't read it even it is shoved right under their noses with bright red flashing lights.

Devs are by nature always of type (a) and simply do not grok the type (b) people, but watch any Let's Play of a tutorial level to gasp in horror as they almost reflexively dismiss anything that requires stopping to read the effing text.

That's why I've learned that an ounce of improved naming far outweighs a ton of documentation. And let's face it, the names F() and P() are just embarrassingly bad.

@Tkael Tkael added debatable For when developers disagree. needs thought For when you need to slow down and consider things. labels Mar 26, 2021
@Tkael
Copy link
Member

Tkael commented Mar 26, 2021

Honestly, I believe that type (b) often don't use EDDI's speech responder at all. They're more likely to disable all or most of the speech responder and to use EDDI to create commands for VoiceAttack.

Speaking honestly again, it took me a while using EDDI before I found the documentation under Help. Not for lack of interest in the subject but just because I didn't know what Help contained and didn't click on it for a long time.

When a user creates a copy of the default personality for the first time, we could pop up a message box giving people an option to watch a video tutorial explaining how to use the speech responder and where to find useful information within the UI. Thoughts?

@richardbuckle
Copy link
Member Author

If you're prepared to create such a video tutorial, sure. I for my part am not.

I merely suggested adding better names for four badly-named functions: a very simple fix for a very obvious problem. I am not sure what is debatable about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
9. enhancement The behaviour is as specified, but we would like to modify or extend the spec. debatable For when developers disagree. needs thought For when you need to slow down and consider things.
Projects
None yet
Development

No branches or pull requests

5 participants