Skip to content

Dynamic Phrase Loading, User Settings Error Handling, and Documentation Enhancements#5

Open
Kapparina wants to merge 6 commits intojwebmeister:mainfrom
Kapparina:dynamic_phrases
Open

Dynamic Phrase Loading, User Settings Error Handling, and Documentation Enhancements#5
Kapparina wants to merge 6 commits intojwebmeister:mainfrom
Kapparina:dynamic_phrases

Conversation

@Kapparina
Copy link
Copy Markdown

This pull request introduces several key improvements, feature enhancements, and documentation extension efforts:

  1. Dynamic Phrase Loading:

    • Implemented dynamic loading of phrase mappings from phrases.toml file across several commands in tacspeak/grammar/_readyornot.py, replacing the previous hardcoded mapping dictionaries. This effort was facilitated by the addition of two new TOML files - phrases.toml and phrases_wip.toml.
    • Added tomllib and pathlib.Path imports to support this change.
  2. Improved User Settings Error Handling:

    • Improved error handling while loading tacspeak/user_settings.py in tacspeak/main.py. Now, for each attribute in user settings, specific error messages are shown, facilitating quicker identification and rectification of errors.
  3. Enhancements in Door Handling Functionality:

    • Introduced a new attribute 'trapped' in the cmd_door_options() function in tacspeak/grammar/_readyornot.py to enable door trapping options during gameplay.
  4. Documentation and Other Updates:

    • The README.md document was extended to include a download section for GitHub, Nexus Mods, and a link to join the project's Discord server. Detailed extraction instructions for Tacspeak and Kaldi model were added, and Microsoft Visual C++ Redistributable was added as a prerequisite.
    • Tacspeak's version update from 0.1.1 to 0.1.2 was reflected in tacspeak/__init__.py.
    • An unnecessary blank line was removed from setup.py.
    • A print statement to display the Tacspeak version was included in cli.py.

Updated

Module: tacspeak/user_settings.py
- Several variables redefined with explicit type hinting.
- Refactored variable initialization lines to use type hinting and be more readable.

Module: cli.py
- Reformatted and enhanced script with explicit type hinting.
- Improved readability of argument parser setup.
- Arguments are now added using an extended syntax and providing better code readability.
- Replaced os library usage with pathlib for handling directories.
- Refactored different condition clauses to better readability.

Module: tacspeak/__main__.py
- Replaced os.path use with pathlib for handling file paths.

Module: tacspeak/grammar/_readyornot.py
- Added `from __future__ import annotations` for Postponed Evaluation of Type Annotations.
- Refactored `DEBUG_MODE` conditional code blocks to leverage better structure and readability.
- Refactored `map_ingame_key_bindings` dictionary construction to be more readable.
- Declared new_binding as a local variable for clearer code.
- Added TODO comments about moving dictionary definitions to a separate TOML file.

Added
- `.idea/` entry added to `.gitignore` to ignore any IDE-based settings files.
- Added:
  - `tacspeak/grammar/`:
    - Added 2 new TOML files : `phrases.toml` and `phrases_wip.toml`

- Modified:
  - `tacspeak/grammar/`:
    - Modified `_readyornot.py`:
       - Added imports: `tomllib`, `pathlib.Path`
       - Replaced static phrase mapping dictionaries with dynamic loading from `phrases.toml`
       - Adjusted the print output format for key-bindings
- Modified:
   - `_readyornot.py`:
     - Removed multiple hardcoded mapping dictionaries.
     - Implemented dynamic loading from the `map_phrases` dictionary for several commands and added respective `Choice()` instances.
     - Enhanced readability by refactoring `Choice()` parameters into name-value pairs.
     - Included type hinting for several variables and parameters for better code clarity.
     - Maintained consistent formatting and arrangement of the code.
- Modified:
    - `cli.py`:
        - Added print statement to display Tacspeak version when the main function is called.
    - `tacspeak/grammar/_readyornot.py`:
        - Added a new argument 'trapped' in the `cmd_door_options()` function and accordingly updated the related code for processing speech recognition.
        - Introduced a new Choice instance for "trapped" conditions in the specifications string.
    - `README.md`:
        - Added a section displaying the download button for GitHub, Nexus Mods, and Discord server link.
        - Updated the Prerequisites section by adding Microsoft Visual C++ Redistributable as a requirement.
        - Added detailed hints for extracting the Tacspeak application .zip and the Kaldi model .zip.
    - `tacspeak/__init__.py`:
        - Updated Tacspeak version from 0.1.1 to 0.1.2.
    - `tacspeak/__main__.py`:
        - Improved the error handling for loading `tacspeak/user_settings.py`.
        - Added specific error messages for each attribute in user settings.
    - `tacspeak/grammar/phrases.toml`:
        - Introduced new phrases for door options - 'disarm', 'trapped' and 'safe'.
    - `setup.py`:
        - Removed an unnecessary blank line.
- Modified:
  - `tacspeak/grammar/_readyornot.py`:
    - Replaced block comments with docstrings at the beginning of the module for better adherence to Python's standard style guide.
    - Reorganized and reformatted multiple imports for better readability and conformance to Python's standard style guide.
# Conflicts:
#	tacspeak/__init__.py
#	tacspeak/grammar/_readyornot.py
@jwebmeister
Copy link
Copy Markdown
Owner

I'm unsure if we should incorporate "Dynamic Phrase Loading" for the grammar modules with just the current scope.

Pros:

  • Separate data from logic (good in general!)
  • Makes it easier for user to customise parts of the module, with less chance of user error
  • Makes it possible to develop a more user friendly interface for users to customise the module (do I dare dream of a GUI!?)

Cons:

  • Only partial separation of data, in terms of things the user would likely want to customise. Examples of what I think also needs to be considered (not an exhaustive list):
    • the "spec" (i.e. full sentence structure, or the full recognised spoken phrase structure), of an existing command
    • within the "spec", which "extras" (i.e. choices, or variables) are optional and/or included
    • the default values of "extras"
    • adding in a new command or function (well obviously, but I'm listing it here anyway!)
  • Added complexity for certain types of user customisation
    • User has to go to and modify multiple files for certain types of customisation
    • Divergence from Dragonfly docs

Currently I'm failing to be imaginative (or be actually helpful, sorry) and think it's better to keep it as a simple single file, for now.
But, what are your thoughts on the points I've raised?
Is it actually simple / possible / worth it to expand the scope and extract everything (within reason) that the user would want to customise into a data only file?

P.S. user_settings.py should definitely go (behind the woodshed and be shot, at some point) and be replaced with a data file.

P.P.S. there are a few more changes to the mappings / phrases that I made on "main" branch on the basis of improving speech recognition. One important one is "hold" being removed from the "hold" command (ironic right?), so it's now only "on my command" (or variations).

@Kapparina
Copy link
Copy Markdown
Author

I'm unsure if we should incorporate "Dynamic Phrase Loading" for the grammar modules with just the current scope.

Pros:

  • Separate data from logic (good in general!)

I'm glad we're aligned on this.

  • Makes it easier for user to customise parts of the module, with less chance of user error
  • Makes it possible to develop a more user friendly interface for users to customise the module (do I dare dream of a GUI!?)

That was my goal: to strike a balance between easy to use and easy to customise - on the topic of GUIs, I have ample experience with libraries like PyQt/PySide6, so that's not off of the table as far as I'm concerned.

Cons:

  • Only partial separation of data, in terms of things the user would likely want to customise. Examples of what I think also needs to be considered (not an exhaustive list):

    • the "spec" (i.e. full sentence structure, or the full recognised spoken phrase structure), of an existing command
    • within the "spec", which "extras" (i.e. choices, or variables) are optional and/or included
    • the default values of "extras"
    • adding in a new command or function (well obviously, but I'm listing it here anyway!)

You make some valid points here. Let's give it some thought and see what we can come up with. My first thought is perhaps having generic filler words ("the", "a", "that", "this", etc.) be accounted for by the grammar model, and having keywords be the only thing a user needs to specify. Allowing users to specify which keywords are optional and which aren't is obviously important too.

  • Added complexity for certain types of user customisation

    • User has to go to and modify multiple files for certain types of customisation
    • Divergence from Dragonfly docs

As it stands, having everything in one file is definitely simpler from one perspective, but it raises the barrier for entry somewhat. I think separating functions into different files (e.g. user_settings.toml and config.py for loading user preferences, grammar.py for loading the grammar model, etc.) could work; I'm thinking we could leverage Pydantic for this.

Regarding divergence from Dragonfly's documentation - this project already uses a custom variant of Dragonfly, no? Whilst I believe Dragonfly's documentation is a good starting point, divergence is nothing documentation can't solve.

Currently I'm failing to be imaginative (or be actually helpful, sorry) and think it's better to keep it as a simple single file, for now. But, what are your thoughts on the points I've raised? Is it actually simple / possible / worth it to expand the scope and extract everything (within reason) that the user would want to customise into a data only file?

Someone has to play devil's advocate, and unwillingness to challenge something is tantamount to negligence. Let's continue exploring our options, and see where we can take this.

P.S. user_settings.py should definitely go (behind the woodshed and be shot, at some point) and be replaced with a data file.

I'll show Lenny the rabbits.

P.P.S. there are a few more changes to the mappings / phrases that I made on "main" branch on the basis of improving speech recognition. One important one is "hold" being removed from the "hold" command (ironic right?), so it's now only "on my command" (or variations).

I'd noticed some changes had been made, I'll update my fork accordingly.

Dynamic phrase loading will be moved to another branch for now, as it will require a large refactor, especially if we're considering a GUI/abstracting customisation away from the source code. In the interim, I'll put some focus into readability and maintainability and create a separate PR for that.

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