Submitted by matthiaskrgr
The file ./dialogs/StandardBotAfterTakeover_sub.dialog is not validated by the dialog validator "freedroidRPG -b dialog".

rev. 2574

Thank you for your fine work!
Matthias Kr\u00fcger

Posted by hakzsam
hi,

bug fixed

StandardBotAfterTalkeover_sub wasn't in a list of NPCs. see npc.c, line 118 :)

http://www.quickfilepost.com/download.do?get=4f0f0c6f315325de181bae01ea583e06

Posted by stedevil
Arthur, are we supposed to specifically list each and every _sub dialog in npc.c? We never did that before, but OTOH we in the past didnt bother much with dialog validation.

But to me it feels like a bad way to do it. The proper behavior would IMO be to simply test ALL .dialog files in dialogs/ subfolder. That way we never have files accidentally not checked because someone forgot or didnt know it needed to be added to npc.c.

PS hakzsam  "StandardBotAfterTalkeover_sub_not_tested"?
If you didnt even test it, we don't want to see it. ;)
Only send in patches for things you have tested and it actually works. If you can't reproduce a problem, ask for help/instructions so you can test it. :)

Posted by ahuillet
Stefan: we are not supposed to list subdialogs in npc.c.

npc.c is for NPCs, not subdialogs. I am currently thinking whether putting subdialogs as NPCs will work, our current run_subdialog() function won't pay attention to it, so it shouldn't be here in order not to confuse people, but it won't break stuff either.

I do agree that all dialog files should perhaps be tested.

Regarding the name of the patch: I'm sure it doesn't mean the patch wasn't tested, but that the patch addresses the problem of subdialogs not being tested.

Posted by ahuillet
In order to be clearer: NACK on the patch, but don't destroy it because it might still be the final solution.

Posted by stedevil
In other words, hakzsam or anyone else, feel free to see if you can solve this "the proper way" without too much work i.e. make the validator check all .dialog files in the dialog folder without them being specifically listed somewhere. 

Posted by ahuillet
I realize I haven't laid out the final solution to this problem.

We need a generic "file lister" function that would be based on our load game menu code. This function could be used for this particular feature request (it's not a bug, by the way), and the "don't overwrite existing character without warning"FR as well.

Posted by salimiles
Matthias has a patch that addresses this: http://rb.freedroid.org/r/839/

Posted by salimiles
Closed duplicate bug 2950941.

Moving this to feature requests and bumping prio.

Posted by ahuillet
The solution is laid out below - someone has to pick it up and implement it.