CEGUI (Crazy Eddie's GUI) Forums

Hi,
When Typing [window=''] or [font=''] in a EditBox, it may crash, since the window or the font does not exist.
If it doesn't occur, try to delete some of the string and retype.
Is this a bug? Or we should try-catch in our client code?
The demos shipped with 0.7.0 have the same problem.
Here is the callstack information taken from the VS debugger.

Hi,

Thanks for (another!) report

This is confirmed as a bug; while the parsed content is never used (currently) in editbox widgets, we should not be parsing it in the first place. I'll have to do some work in this area because the current mechanisms are not good enough to allow a simple disabling of the parsing (you can force no parsing my assigning a custom DefaultRenderedStringParser, though this should not be forced on users) - a fix for this should go in tomorrow or Monday.

CE.

It's not tomorrow or Monday, but this is now fixed

I have added new functions, a property and an event related to setting the text parsing functionality to enabled or disabled. Editbox type widgets now have parsing disabled by default.

CE.

That's so cool, I'll try the version newly committed to SVN.

zhaosili

Hi,

I have tested the new version, it may still crash
if the text with tags in the EditBox is submitted to change other widget's Text Property.

E.g. I change the Sample_FalagardDemo1's layout with the following first:


Then I type [font='']a or [window='']a and click the submit button, it crashes.

I think the solution is patching the constructor of RenderedStringTextComponent

and RenderedStringWidgetComponent with try-catch.


Perhaps patches are also needed for the following:

Hi,

Yes, it may still crash, but the key fact is that the issue is now that the string from the editbox is being passed to another component unchecked - this is the responsibility of the user of the library to do this. The demos should be updated in this regard in order to show good practice - whether that be by cleaning the entered strings or by simply adding a try/catch block somewhere (there are ways of forcing the immediate parse of the assigned text, so the user code is always able to do it's own validation via some means or other - thus preventing the throw from occurring later on in the renderGUI call).

I certainly don't agree that we should be protecting against this in the library itself - what happens in some theoretical scenario where the users code needs to hear about those exceptions for some reason? If we catch those internally we deny the user certain powers - there are lots of other places where we allow the user to potentially shoot themselves in the foot also

CE.

Hi,

Yeah, if the library itself protects the scenario, the user'll lose the power to check whether the text is valid.
But for users that are porting from 0.6.x, there maybe tens of dozens of setText or similar things in the code without try-catch for this.
E.g. In a listbox, typically it's normal for the users using previous verisons' CEGUI like this:

If We get exception with the first item, the listbox'll be blank. Since for the new version ,we need to deal with the situation like this:

This change will cost a lot in Porting for a large project.

I wonder if the library could provide some interface to set GLOBAL string parser e.g. something like d_basicStringParser (currently, the d_customStringParser is just for one window instance), and this'll give user power to control whether the exceptions can be all ignored.

zhaosili wrote:I wonder if the library could provide some interface to set GLOBAL string parser e.g. something like d_basicStringParser (currently, the d_customStringParser is just for one window instance), and this'll give user power to control whether the exceptions can be all ignored.

Yes, it should be possible to add this. The priorities used when choosing the parser to use will be as follows:

1) Window has parsing disabled - use built-in DefaultRenderedStringParser to render string verbatim (i.e no parsing).
2) Window has parsing enabled and custom parser set on Window - use the Window instance's custom parser.
3) Window has parsing enabled, no custom parser set on Window - use the globally set custom parser.
4) Window has parsing enabled, no custom parser set on Window, no global parser set - use the built-in BasicRenderedStringParser.

CE.

This feature is so cool that we can port code from 0.6.x to 0.7.x easily

Thanks CE.

zhaosili

Ok, this has now been added. You can set a system-wide custom parser via the a RSI inducing function named: System::setDefaultCustomRenderedStringParser and passing in a pointer to the desired parser.

HTH

CE.

Thank you very much, CE.
I'll try it.
BTW: I found the code of the ListboxTextItem and TreeItem also includes basicStringParser, does the global parser affect this?

The situation with ListboxTextItem and TreeItem is a bit sticky. Currently these are not affected by the global parser. What I really would like to do is have those item classes use a settable custom parser, but otherwise defer to the owning window (and so to the global parser where appropriate). The issue now is whether we consider this quite a major change in behaviour, and will 0.7.0 users be mad should the behaviour change when making an upgrade to 0.7.1? I'm very tempted to say f*** it and make the change (since it's the Right Thing™ to do for the lib).

CE.

Yeah, the problem is sticky. Currently I hack CEGUI with this:


Then I use 'MyListBoxTextItem:public ListboxTextItem' and 'MyTreeItem:public TreeItem',
override the 'parseTextString()' method to work around.

I think it might be a good idea for us to add a new version of these item classes that behave the way we would now prefer, and mark the existing versions deprecated (or in the case of TreeItem, double-deprecated ). This way we can get the better, more consistent behaviour in for 0.7.1 while maintaining the previous implementation for those that might have started using it already.

CE.

I agree with you, that's a good idea for users who want to porting from 0.7.0 to 0.7.1.

zhaosili