[feature] Add chat message when Ai script fails

[Cjkjvfnby]

I add code to send a chat message to host if AI script function ended with error.

It that type of feater will be OK for release? IMHO Yes, if user has a error he should know about it.

[Dilvish]

Hmm, this chat patch wouldn't apply for me properly due to line endings mismatch, and then I noticed that you had intended line endings to get cleaned up in the blank lines patch. I double checked that one, using both svn patch and regular patch to apply it, but it didn't seem to affect any line endings, so I'm not sure what you really intended there. I think one of the common options for mixing git and svn is to ignore whitespace when making diffs and perhaps that interfered with your intent.

I went ahead and cleaned up the line endings in all the AI files using the line-ending checker/fixer script in the default/AI/charting folder (there's no real reason for it to be located there, it's just where I tucked it out of the way). I had made that for clearing up issues I ran into with other FO files, so it's a little amusing there were still mismatches in the AI files. I guess I never checked the AI files because I haven't had call to be applying anyone else's patches to them until now.

However, even after clearing up those line endings, I am still getting hunks 1 and 5 from this chat patch failing to apply. Could you regenerate the patch against the current repository?

Whether or not we'd want to make these chat messages after release, they certainly seem like a good idea for now. Though could you change the message to be

Code: Select all

message = 'AI script error in "%s" with "%s"' % (callable.__name__, e)

[Geoff the Medio]

I feel I should remind that stuff like chat messages that the player will see need to be localized. Currently little or none of it is looked up in stringtables before sending...

[Dilvish]

I feel I should remind that stuff like chat messages that the player will see need to be localized. Currently little or none of it is looked up in stringtables before sending...

None of the small handful of current chat messages are intended to stay through to release, though, sure, as long as they're in place it wouldn't hurt for them to be translatable. The other AI text currently presented to users are the home planet names, which are looked up in the stringtables, and the ship design category names ("Lynx", "Griffon", etc.), which I'm reluctant to make translatable because the design category names have some functionality for the AI & if a translator messed up and assigned the same translation to two different design categories, for example, AI planning would be impaired. I suppose that risk is fairly small and manageable, though, so if you'd prefer the AI design category names could be translatable as well.

As for this current chat message, I really see as a debugging aid, like our log files which we don't bother to translate. But, Cjkjvfnby, if you want to make this more user friendly, you could make this message translatable by adding an entry to default/stringtables/en.txt (the 'master' stringtable from which all others get derived). The structure for each entry is simply a first line label (which we do as all caps though I forget if that is a technical requirement), followed on the next line by the text translation. The translation can be multi-lined if it is enclosed in triple single quotes. A suitable entry for this might be

Code: Select all

AI_ERROR_MSG
AI script error in

and then having the respective code line be something like

Code: Select all

message = '%s "%s" : "%s"' % (fo.userString('AI_ERROR_MSG'), callable.__name__, e)

[Cjkjvfnby]

I feel I should remind that stuff like chat messages that the player will see need to be localized. Currently little or none of it is looked up in stringtables before sending...

Little offtopick, why you use custom localization, why not gettext (i18n)?

[Cjkjvfnby]

I went ahead and cleaned up the line endings in all the AI files using the line-ending checker/fixer script in the default/AI/charting folder

Why not whole project? (there is 205 more files over project)

[Dilvish]

I add code to send a chat message to host if AI script function ended with error.

Committed r7229

Little offtopick, why you use custom localization, why not gettext (i18n)?

Recall from another thread that the game content, some of which is in python but much of which is written in a simpler custom scripting language, is meant to be user-editable by the playerbase. I suspect that the special tools and workflow used for gettext were deemed a bit too arcane.

Why not whole project? (there is 205 more files over project)

I was a bit reluctant to force recompiles just for clearing up line endings, but I guess it's good to get them all back in the in right form. r7230

[Cjkjvfnby]

Committed r7229

Now we have notification to user about AI error. May be we should remove try...except around big blocks?

[Dilvish]

Now we have notification to user about AI error. May be we should remove try...except around big blocks?

We'll need to remove them or adjust them to reraise since your chat wrapper itself relies on a try...except; removing them sounds best **edit: at first, but see below**. I added the stringtable entry for the error message,

Code: Select all

AI_ERROR_MSG
AI_Error: AI script error in

and also modified the chat_on_error wrapper to print/log the message in addition to sending it to the user -- I often find it very helpful to test out the AI scipts via a multiuser game with the human client in observer or moderator mode, and in observer mode particularly, the turns fly by very fast and the chat message would be easy to miss. So I just periodically grep the logs, now for "AI_Error".

[adrian_broher]

I feel I should remind that stuff like chat messages that the player will see need to be localized. Currently little or none of it is looked up in stringtables before sending...

This idea will waste translator effort and time. Translators are (most of the time) not programmers, most of them haven't even the slightest clue what is the meaning of the messages. This is a guarranteed loss of information.
Same goes for the users. They won't understand programming issues even if it is in his native language. He won't understand what 'obect x isn't Callable' means, that's even true most developers who aren't familiar with python.
And in case a user comes here and brings a localized error message on the table who should fix it? No hablo español, et ni français. English is a development baseline I can deal with, everything else is a waste of effort of support time. If the user needs to translate back messages there will be another loss of information just prolonging the support time.

Recall from another thread that the game content, some of which is in python but much of which is written in a simpler custom scripting language, is meant to be user-editable by the playerbase. I suspect that the special tools and workflow used for gettext were deemed a bit too arcane.

Users who edit the game content won't bother with translations. They will just add content in the language they deem fit for their purpose.

Also the whole translation mechanism needs to be replaced with something sane. But this is a discussion for another time.

[Dilvish]

On thinking about it some more, I'm more inclined to adjust the inner try...excepts to reraise rather than taking them out altogether, and to also remove the reraise from the chat_on_error wrapper. It looks to me that taking them out altogether simply means that if an AI does run into some odd case somewhere that causes an exception, it will most likely cease to operate altogether. If it's just you and I testing things out that would be good since it might call greater attention to the problem if we miss the chat message, but we'll be scanning the logs anyways, and I think that most of the playtesters would prefer that the AI continue to hobble along as best it can in such a situation (depending on circumstances it might not be significantly impaired).

[Geoff the Medio]

...why you use custom localization, why not gettext (i18n)?

I suspect that the special tools and workflow used for gettext were deemed a bit too arcane.

gettext is a very awkward and complicated workaround to a problem that a stringtable handles better.

...the whole translation mechanism needs to be replaced with something sane.

I'm not aware of anything better than a stringtable... Both content scripts and source code need to reference user-visible strings, and those strings should be in one place and editable outside of the code. That pretty much means using a stringtable. The strings used just in content could be coded into the content scripts, but that would split strings into multiple files and make localizing and maintaining much more complicated. The english versions of strings could be added as (effectively) comments near their in-code or in-script use, but I think the rather verbose string names already give a reasonable idea of what the string contains, and that would also make maintenance more complicated.

...chat messages that the player will see need to be localized...

This idea will waste translator effort and time.

If it's user visible, it needs to be localized, at least somewhat. I don't want to translate the actual python error messages; those can and should be shown in whatever language they are generated in. But there needs to be a bit of localized text before them that the player can understand in their own language to give some context about the following blob of computer gibberish, along the lines of "AI Script Error:". And I was also talking about the various AI chat messages, such as used for diplomacy, which are only intended for users to read, so need to be localized.

[adrian_broher]

gettext is a very awkward and complicated workaround to a problem that a stringtable handles better.

Stringtables are a poor reinvention of gettext.

• It is unable to handle singular and plural cases properly in way that respects the needs of the language we translate into
• It doesn't have any toolchain, applications and infrastructure to aid developers and translators with documentation transition (format placeholder), keeping track of updated, deleted or moved strings, finding change deltas, dirty translations(translations that need a check because the source literal has changed)
• It encourages string duplicates
• It encourages plain wrong and outdated translations because not all string tables are maintained and the id mechanism just pulls out whatever it wants
• It breaks code if format strings in different languages differ unless you take special care of it (FlexibleFormat is just that 'special care' implementation)
• It moves the literals in a far away file, away from the point where it describes content and formatting. It will break if the developer forgets to update the string table and even than if can break at other places where the same key is used.
• It has no useful fallback mechanism, if the key is missing completely in the translation tables is missing we get gibberish 'Missing key: FOO_BAR', which makes the application unusable
• I know you said something else but the string names are NOT good indicators for translators. They can be missleading without context. When written at certain place it maybe needs a different writing (upper case, lower case at the beginning of a sentence), the semantic of the content behind the key can change during the lifetime of the project and the key is still the same. For example there was OBJ_BUILDING, which was translated as 'Struttura' AND 'Edefico' in the it.txt file. Backtranslation that 'struttura' and 'edificio' could be both right depending on context and in fact both were used for same semantic object at different places in the application.
• Translators know gettext, whereas Stringtables are undocumented NIH, which actually DETERS translators from contributing

If it's user visible, it needs to be localized, at least somewhat. I don't want to translate the actual python error messages; those can and should be shown in whatever language they are generated in. But there needs to be a bit of localized text before them that the player can understand in their own language to give some context about the following blob of computer gibberish, along the lines of "AI Script Error:". And I was also talking about the various AI chat messages, such as used for diplomacy, which are only intended for users to read, so need to be localized.

Well, I missunderstood your interjection in that case. Sorry about that.

Could we also split off the stringtables vs gettext discussion? In my opinion it doesn't belong here.