SonyWWS / LevelEditor

The ATF LevelEditor is a powerful tool for constructing and assembling game levels. It provides a WYSIWYG interface and allows you to place objects, edit properties, edit terrain, and build game levels.
Apache License 2.0
1.32k stars 229 forks source link

Bug: changing skin makes editor unusable (machine 'culture' dependent) #11

Closed LogicalError closed 10 years ago

LogicalError commented 10 years ago

I changed the skin to the dark skin and after that all I saw was a giant empty toolstrip with a bigger size than my window, blocking everything including the menus. On restart I got an error message, but couldn't read it because the error text had the same color as the background in the dialog box, same with it's buttons. So far I haven't been able to find out where the settings are stored for the level editor, making it impossible for me to reset this without diving deep in it's code ...

LogicalError commented 10 years ago

I managed to be able to see something again by commenting out ApplyNewPropertyValues(control, skinnedControls); in ApplyActiveSkin in SkinService.cs / Atf.Gui.Windorms.vs2010

Looking at how things are layout I suspect that the main menu got a ridiculously large height, perhaps the font that was set was really large?

LogicalError commented 10 years ago

Okay, I think I figured this out. The Dark.skn, for it's font, has

the Light.skn file, which does work properly, has

What I suspect is happening is that the 8.25 float value is parsed using Single.Parse(someString) (or Single.TryParse, float.Parse / float.TryParse, FromString etc); Problem with that is that C# makes the dangerous assumption that you want to use the current numeric localization on the machine you're running this on. Now in some cultures "." and "," are swapped. This means that it'll actually convert 8.25 into 825. The solution is to use Single.Parse(myString, CultureInfo.InvariantCulture.NumberFormat); instead, that way the behavior is always consistent everywhere

LogicalError commented 10 years ago

I traced this specific problem back to the line: instance = valueInfo.Converter.ConvertTo(null, valueInfo.Value, valueInfo.Type); in GetInstance in SkinService.cs/Atf.Gui.Windorms.vs2010. Changing it to instance = valueInfo.Converter.ConvertTo(null, System.Globalization.CultureInfo.InvariantCulture, valueInfo.Value, valueInfo.Type); doesn't fix it though, which makes me think that there are custom TypeConverters in play that doesn't respect the given CultureInfo ... sigh

if this was a .net 4.5 app the culture could be forced application wide .. :-/

LogicalError commented 10 years ago

Yikes. There are several locations that use CultureInfo.CurrentCulture when converting floats .. that's like forcing the wrong thing to do.

LogicalError commented 10 years ago

Yes, found it. It's caused by the ConvertTo method in DefaultTypeConverter in SkinService.cs/Atf.Gui.Windorms.vs2010. It itself calls SingleConverter().ConvertTo without even specifying a culture.

The source code seems to be full of conversions between values without respecting cultures or dangerously forcing the current culture when loading/saving file formats .. scary

abeckus commented 10 years ago

Hi Sander, Thanks for finding those culture related conversion bugs.

Alan

abeckus commented 10 years ago

Made the required changes. However, I don't have a proper setup to test the changes. Please let me know if the changes fixed the issue.

Thanks Alan

gameengineer commented 10 years ago

I also want the dark theme. Where did you find the skin? Is this skin related bug going to be fixed in an upcoming release?

abeckus commented 10 years ago

The dark skin located at: ..\LevelEditor\ATF\Framework\Atf.Gui.WinForms\Applications\SkinService\Skins\ The bug is already fixed. Alan

gameengineer commented 10 years ago

Great! That was the ticket, thanks.