Dilvish wrote:I have mixed feelings about this proposal.
That is understandable, but I will try to answer to give you a better insight in the idea behind it.
Dilvish wrote:The actual benefit is much lower, because there are already structural flags/tokens in place that mark every translatable entry; those structural flags are simply more varied that "NOPUserString". You must have already gone through and identified all those flags in order for the patch to make the parser require the new syntax (or even be compatible with it). So in this case, there is a ready alternative -- rather then putting an additional syntax burden on every piece of scripted content, the translatable-string-identifier-script could be made more complex to check for these additional structures within the content directory. That alternative seems to substantially reduce the benefit of requiring this additional syntax.
I would agree if the identification of translatable items would be trivial. But the problem actually is that this isn't the case. Given the fact that the parser is token based the source semantics would allow to span simple assignments over several lines, so this case would need be handled. Also there are not so simple assignments of lists (tags for example) that would require a the translatable-string-identifier-script to understand the semantic of those lists. List assignments are optional for tags, so you need to handle this case to. As you see the translatable-string-identifier-script gets more and more complex, it needs to understand the scripts semantic for a simple case, that could be handled by some simple regular expression. And finally as you said there is the need to correctly identify the parameters that need translation. Even that isn't possible. For example: The 'category' parameter for encyclopedia articles are translatable strings, whereas the 'category' parameter of Techs is in fact a reference to (Tech)Category objects. Now we need to evaluate the context where the parameter exists. And with that we are at a point where need a full blown fo-script parser.
Dilvish wrote:On the cost side, it seems to me that there is a greater cost as well. I expect Geoff and others have gone through a lot of effort to make the scripting syntax as streamlined as possible, to facilitate content creation by nonprogrammers.
Bigjoe5 wrote:IThe intention is that users can modify FO freely and easily. Anything that complicates the scripting language, with no benefit to the language itself is not beneficial, IMO, even if it makes things a bit better on the code side.
Geoff had said on serveral occations that this is the case so I think he and the other devs want to keep it that way, but I think the 'make it easy for non-programmers' is pretty moot. Please keep the following in mind:
- Contributors, regardless of programming knowledge, need to learn the syntax to realize their idea
- When stepping back and looking at the fo-script documentation I realize that the fo-script language is already complex and non intuitive. You need to have or to gain programming knowledge to implement something in fo-script. Adding additional syntax wouldn't make a difference as long as it is documented, behaves deterministic and is easily debuggable.
Dilvish wrote:Additionally, it seems to me that the additional syntax clutters/degrades readability of the content scripts much more than it did the c++ code.
That's maybe a point I could agree to somewhat. But choosing another syntax isn't that easy. [ ] os already used for grouping/lists further overloading it just makes the language harder to understand, using something like _"IDENTIFIER" would reduce cluttering but isn't as bold as UserString("IDENTIFER"). But I'm open for ideas on that. Point is that to prevent implementing a full fledged parser an exclusive for translations token identifier would be needed. I did choose UserString("") because it is already in place anywhere else.
Dilvish wrote:At least a portion of the work you did to make this patch would still be applicable to augmenting the translatable-string-identifier-script, so that work wouldn't be entirely lost if you did take the alternate approach.
It's would not be a problem to abandon the patch, but I can hardly think of a way to implement it in a better way. But of couse I'm open for suggestions.
Dilvish wrote:Geoff and I had previously discusses exposing the "UserString" call to the python AI, and having it use that for all strings visible to the user. I'll bump that up on my priority list, and add NOPUserString as well.
That should be UserStringNop in that case, to keep the variants minimal.
But thanks for your input on that matter.