Add the use of grammar processors #2667
Conversation
Create two scripts with new classes into Game/Localization and add calls to grammar processor methods into nine existing scripts that display text to screen. See issue Interkarma#2656.
Daneel53
changed the title
New method SetGenreNPC()
New method SetGenreNPC()
| public abstract void SetGenreHero(string Genre); | ||
| public abstract void SetGenreNPC(string Genre); |
There was a problem hiding this comment.
- Maybe something more English-like?
| public abstract void SetGenreHero(string Genre); | |
| public abstract void SetGenreNPC(string Genre); | |
| public abstract void SetHeroGender(string Gender); | |
| public abstract void SetNPCGender(string Gender); |
- Just curious. Why string? Can we use built-in
Genders.MaleandGenders.Female?
There was a problem hiding this comment.
1 - You're perfectly right! I realize that I almost always use in English the word "genre" (as in French) instead of "gender", and because the word "genre" exists also in English, the spellcheckers never says anything. Oups!
So I agree, I will change for SetHeroGender() and SetNPCGender().
2 - Why string? That is because the grammar processor may be used with another game and I don't want to link its code to the code of DFU. There is no "using..." of whatever is DFU definition into the code of the French grammar processor. So I prefer to use the classic and neutral string values "M" and "F" as parameter. Moreover, it is always possible that into another game there may be more than two genders, a kind of "Neutral" or something like that. I saw recently, on the discord of the Bannerlord mod Banner Kings, a player asking to introduce "non-binary" characters with the pronouns "their" and "them". So we must be ready...
3 - I'm not fully sure either to have covered all places where a call to ProcessGrammar() would be useful, but for the moment I don't see a place where a grammar token is not interpreted. What is coming very soon is the use of the new method SetNPCGender() that I've added two days ago. It shall be used into the quests where we need to know the gender of NPCs as cousin or healer. We can handle the last NPC gender when macros as %g2 are interpreted, but it seems also possible with values as cousin or _cousin or __cousin. I'm working on it, it already works with macros as %gx, should have a definitive answer today or tomorrow.
There was a problem hiding this comment.
That is because the grammar processor may be used with another game and I don't want to link its code to the code of DFU
If that's the case, then you probably shouldn't put the code in the DFU code folders.
The simple place would be under Assets/Scripts/External, but it would be nice if you could find how to place it under Game/Addons, like the CSharpCompiler or the UnityConsole plugins
If you don't do that, I think you should fully respect the DFU namespace and DFU coding convention. But we do already have external modules, so I think it would be fine for you to add yours there
There was a problem hiding this comment.
OK, I will use genders.Male and genders.Female...
There was a problem hiding this comment.
If you go that direction, you should change the namespace to DaggerfallWorkshop.Localization rather than GrammarProcessor.
I still think you can just move the file to Assets/Scripts/External, to make it clear this is being contributed as an external module
There was a problem hiding this comment.
I already made (and tested) all the changes needed with the new names of the methods and the use of the enum Genders. I just not yet uploaded them to the PR but finally it's a good thing.
I'm ready to keep on changing things so that it is convenient for you. My goal is that this PR is inside DFU 1.2, so I will do what is needed.
If I change the namespace from GrammarModule to DaggerfallWorkshop.localisation, I will have to change the using GrammarModule that are in the code where the grammar methods are called. OK, I can do.
If I move grammar.cs and DefaultGrammaRules.cs to Assets/Scripts/External, I suppose that I can keep the namespace GrammarModule.
What is your preference? I think that this is the second one, but tell me, and I will do.
There was a problem hiding this comment.
Latter. It's gonna be easier to update your scripts as you copy-paste them from your other games if you don't have to port them to DFU standards each time
There was a problem hiding this comment.
Sometimes I would like to be able to talk quietly with other people in my language, I would probably be more clear...
My script is not yet included into another game, but my goal is that it can be usable for another game. But when I look carefully to the details, obviously I will have to modifiy it a little in order to be able to insert it properly in each game. The core of the script is the method ProcessGrammar() that will be into FrenchGrammarRules.cs, and this one is enough isolated from any game so that I can reuse it with very little effort in another game. Even with DFU, this script and method will be into the French mod, and not into DFU code, so that's fine.
What I'm inserting today into DFU with this PR is the backbone needed to be able to plug a grammar processor to DFU : the basic abstract class GrammarRules and the GrammarManager into Grammar.cs, the DefaultGrammarRules class into DefaultGrammarRules.cs, and all the calls to ProcessGrammar() where needed into the DFU code so that a real grammar processor can make its job. If there is no grammar processor plugged through a mod, no matter, the game will act as before due to the default grammar processor. But if you plug a specific language processor via a mod, it will do its job without modifying anything to DFU code once this PR is inserted.
In my point of view, this PR add a functionality into the heart of DFU that, I agree, was not existing in the original Daggerfall. But its the same for all function linked to localization that Interkarma added to DFU. That's why I thought that Scripts\Localization was the right place. But if you think that Scripts\External is a better place, no problem. Because no matter for me: this will change nothing to the heart code of my grammar processor that will be enclosed into the French mod, except some using and the enum Genders that can be changed easily for another game.
We just need to find an agreement: Grammar.cs and DefaultGrammarRules.cs into Localization or External?
If into Localization, namespace DaggerfallWorkshop.Localization (seems more logical, I agree) or GrammarModule?
And that's it. The rest is on my side. ;)
And many thanks anyway to take my request into consideration.
There was a problem hiding this comment.
I've taken the following decisions:
- Grammar.cs and DefaultGrammarRules.cs stay into Localization.
- Namespace changed for DaggerfallWorkshop.Localization for coherency with other scripts into Localization.
- Names of the methods and parameters are now fully in English.
- Parameter for both methods SetxxxGender() is now coming from DFU enum Genders.
The 11 scripts impacted by this update have been pushed into the current PR that is now ready to be approved if nobody have other remarks on it.
The only thing that is still on my desk is the way to find and use properly the gender of the NPCs into the quests so that the localized text is correct ("mon cousin" or "ma cousine" for _cousin_ for example). If I cannot find a correct way to realize that before 1.2 is out, it will be for 1.3.
- namespace changed for DaggerfallWorkshop.Localization. - Change name of methods. - Change gender parameter.
|
Would you be willing to post a sample script of how you would use this in your French localization for testing? Doesn't have to be the full thing if you don't want to, just an example of how you're using this. |
|
I will post a package with the current 3 "tokenized" French translation files that I'm using into Unity Editor with current state of the DFU Github.... and the scripts modified by this PR, for sure. |
Permits to get the gender of the last NPC referenced in quests and use it properly inside a gammar processor.
|
Found the right place to get the NPC gender in quests and send it to the grammar processor. The PR is complete for 1.2. |
Code update as recommended by Pango.
Create two scripts with new classes into Game/Localization and add calls to grammar processor methods into nine existing scripts that display text to screen.
See issue #2656 for explanation.