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

Improved identification of character names. #72

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

Conversation

aaronshenhao
Copy link

@aaronshenhao aaronshenhao commented May 26, 2024

Motivation

This aims to address LostRuins/koboldcpp#867, where certain lines of text are misidentified as character names.

Changes

Updated the regex expressions responsible for identifying character names to make sure that only listed characters (stored in localsettings.chatopponent) are matched.

The logic for this is done in 4 places in index.html. I renamed the variable names of one of the regex expressions to make them more searchable, but ideally this should be delegated to a function to prevent code duplication and inconsistencies.

To review before merging

  • Line 13353 (used for Messenger UI in Chat mode) does not have a \n before the opening bracket, but this seems inconsistent with the other regex expressions, so I added it.
    • Apparently, if you add a \n in front, like all other cases, it fails to format the character in Messenger UI.
  • Does line 13356 (permalink) need to change as well?

Potential issues

This will break characters which are not added to the characters list. This would affect users who added characters then deleted them, or for those who manually write custom characters into the story. However, I'm not sure if any users fall into these categories, and it seems like it has always been the intention to only search for added characters.

Scope of impact

It should only affect Chat mode (localsettings.opmode == 3).

@aaronshenhao
Copy link
Author

aaronshenhao commented May 26, 2024

Made it a draft, as I realized that there were two more bugs. The regex expression matches too much. For example, with Bob: X: Y, it'll think Bob: X is the character name. Also, it fails to recognize characters for the Messaging UI.

Edit: Fixed issue and squashed commits.

@henk717
Copy link
Collaborator

henk717 commented May 26, 2024

We have an auto mode and I am one of the users that will sometimes put a third character in. So you'd want to limit this PR to sessions where the character names are defined.

@aaronshenhao
Copy link
Author

We have an auto mode and I am one of the users that will sometimes put a third character in. So you'd want to limit this PR to sessions where the character names are defined.

@henk717 Sorry, what's the auto mode? And can you describe how you usually put a third character in?

@LostRuins
Copy link
Owner

You can create a group chat by specifying multiple character names on newlines.

@LostRuins LostRuins added the enhancement New feature or request label May 26, 2024
@LostRuins
Copy link
Owner

The issue with this approach is that people (myself included) like to change the chat opponent as the story progresses.

@LostRuins
Copy link
Owner

So i might start the chat with a name, halfway through I wanna change chat opponent and then the story would break.

@aaronshenhao
Copy link
Author

aaronshenhao commented May 26, 2024

We could keep track of the names of all characters which appear in the conversation of the save file. Character names are automatically added when they generate their first message.

If a user wants to use an old save file or to paste a conversation, they would have to update the character list manually. This character list should be separate from localsettings.chatopponent. If the list does not exist, the old behavior should be used instead. The list is only ever created on the creation of new files, or if manually created.

How does that sound?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants