[fixed]Is there a bug in the scene saving commands?

[Mel]

I don't know what could be causing it, but i've tumbled upon a bug which can't be but from the engine because it only uses irr commands to work, concretely the save scene and load scene commands from the scene manager.

code:

Code: Select all

 
bool onLoadScene::OnEvent(const irr::SEvent& event)
    {
        bool result = false;
        if (event.EventType == EET_GUI_EVENT)
            if (event.GUIEvent.EventType == gui::EGET_MENU_ITEM_SELECTED)
            {
                gui::IGUIContextMenu* menu = (gui::IGUIContextMenu*)event.GUIEvent.Caller;
                if (menu->getItemCommandId(menu->getSelectedItem()) == EGUIMENUFILELOADSCENE)
                {
                    gui->addFileOpenDialog(langPack::fileloadscene.c_str(),true,0,EGUIMENUFILELOADSCENEDIALOG,true);
                    result = true;
                }
            }
            else
            {
                if (event.GUIEvent.EventType == gui::EGET_FILE_SELECTED)
                {
                    gui::IGUIFileOpenDialog* dlg = (gui::IGUIFileOpenDialog*)event.GUIEvent.Caller;
                    if (dlg->getID() == EGUIMENUFILELOADSCENEDIALOG)
                    {
                        scene::ICameraSceneNode* cam = manager->getActiveCamera();
                        core::vector3df camPos, camTarget;
                        camPos = cam->getPosition();
                        camTarget = cam->getTarget();
                        manager->clear();
                        manager->loadScene(dlg->getFileNameP());
                        cam = manager->addCameraSceneNodeMaya(0, -700.0f, 400, 90, EGUIMENUVIEWCAMERA, 40);
                        cam->setPosition(camPos);
                        cam->setTarget(camTarget);
                    }
                }
            }
        return result;
    }
 
 
    bool onSaveSceneAs::OnEvent(const irr::SEvent& event)
    {
        bool result = false;
        gui::IGUIButton* btn;
        gui::IGUIListBox* lbx;
        core::stringw textConvert;
        gui::IGUISkin* skn;
 
        if (event.EventType == EET_GUI_EVENT)
        {
            if (event.GUIEvent.EventType == gui::EGET_MENU_ITEM_SELECTED)
            {
                gui::IGUIContextMenu* menu = (gui::IGUIContextMenu*)event.GUIEvent.Caller;
                if (menu->getItemCommandId(menu->getSelectedItem()) == EGUIMENUFILESAVEAS)
                {
                    s32 width = 400;
                    s32 height = 310;
                    core::recti rec = gui->getRootGUIElement()->getAbsoluteClippingRect();
                    core::dimension2di resolution;
                    resolution.Width = rec.LowerRightCorner.X - rec.UpperLeftCorner.X;
                    resolution.Height = rec.LowerRightCorner.Y - rec.UpperLeftCorner.Y;
 
                    win = gui->addWindow(
                        core::recti(
                        (resolution.Width - width) / 2,
                        (resolution.Height - height) / 2,
                        (resolution.Width + width) / 2,
                        (resolution.Height + height) / 2),
                        true, langPack::filesaveas.c_str(), NULL, EGUIMENUFILESAVEAS);
 
 
                    filename = gui->addEditBox(0, core::recti(10, 30, width - 90, 50), true, win, EGUIFILESAVEASEDITFILENAME);
                    filelist = gui->addListBox(core::recti(10, 55, width - 90, 280), win, -1, true);
                    Accept = gui->addButton(core::recti(width - 80, 30, width - 10, 50), win, EGUIFILESAVEASACCEPTBUTTON, L"Accept");
                    Cancel = gui->addButton(core::recti(width - 80, 55, width - 10, 75), win, EGUIFILESAVEASCANCELBUTTON, L"Cancel");
                    gui->addStaticText(langPack::filesaveaswarning.c_str(), core::recti(10, height - 25, width - 10, height - 5), false, true, win);
 
                    io::IFileSystem* fs = gui->getFileSystem();
                    skn = gui->getSkin();
 
                    currentDirectory = fs->getWorkingDirectory();
                    fl = fs->createFileList();
                    for (u32 i = 0; i < fl->getFileCount(); ++i)
                    {
                        textConvert = core::stringw(fl->getFileName(i));
                        filelist->addItem(textConvert.c_str(), skn->getIcon(fl->isDirectory(i) ? gui::EGDI_DIRECTORY : gui::EGDI_FILE));
                    }
                    fl->drop();
                    result = true;
                }
            }
 
            switch (event.GUIEvent.EventType)
            {
            case gui::EGET_BUTTON_CLICKED:
                btn = (gui::IGUIButton*)event.GUIEvent.Caller;
                if (btn==Accept)
                { 
                    io::path savefilename;
                    savefilename = gui->getFileSystem()->getWorkingDirectory();
                    savefilename.append('/');
                    savefilename += filename->getText();
                    manager->saveScene(savefilename);
                    win->remove();
                    result = true;
                }
 
                if (btn == Cancel)
                {
                    win->remove();
                    gui->getFileSystem()->changeWorkingDirectoryTo(currentDirectory);
                    result = true;
                }
                break;
            case gui::EGET_LISTBOX_SELECTED_AGAIN:
                lbx = (gui::IGUIListBox*)event.GUIEvent.Caller;
                if (lbx == filelist)
                {
                    io::IFileSystem* fs = gui->getFileSystem();
                    if (fs->changeWorkingDirectoryTo(lbx->getListItem(lbx->getSelected())))
                    {
                        filelist->clear();
                        io::IFileList* fl = fs->createFileList();
                        skn = gui->getSkin();
                        for (u32 i = 0; i < fl->getFileCount(); ++i)
                        {
                            textConvert = core::stringw(fl->getFileName(i));
                            filelist->addItem(textConvert.c_str(), skn->getIcon(fl->isDirectory(i) ? gui::EGDI_DIRECTORY : gui::EGDI_FILE));
                        }
                        fl->drop();
                    }
                    else
                    {
                        filename->setText(lbx->getListItem(lbx->getSelected()));
                    }
 
                    result = true;
                }
 
                break;
            }
        }
 
        return result;
    }

I am trying to work on a custom irrlicht scene editor (pretty much like the irrEdit, but simpler, and tweakable for my own needs) and to my surprise, i've found that the animated meshes get squashed and flat when they're loaded from a scene file. I can't point to a posible cause, i can give away the whole project code if you want to take a look because i use a custom node (an upgraded editor grid)

http://omegasection.g0dsoft.com/wp-content/uploads/2016/08/simpleEd.zip

This is the result of it. It is the dwarf, but it looks like its height is reduced to 0



A posible way to repeat the process is to load normally some animated meshes, save the resulting scene via saveScene

[CuteAlien]

Sorry, can't use your code to test as it's nothing I can compile. So wrote my own.. but this one works:

Code: Select all

 
// Testing serializing with animated models
 
#include <irrlicht.h>
 
#ifdef _MSC_VER
#pragma comment(lib, "Irrlicht.lib")
#endif
 
using namespace irr;
 
int main(int argc, char *argv[])
{
    IrrlichtDevice * Device = createDevice(video::EDT_OPENGL, core::dimension2d<u32>(640,480) );
    if (!Device)
        return false;
    
    scene::ISceneManager* smgr = Device->getSceneManager();
    video::IVideoDriver* videoDriver =  Device->getVideoDriver();   
 
    smgr->addCameraSceneNode (0, core::vector3df(30, 30, 100),
                                core::vector3df(0, 0, 0),
                                -1);
 
    smgr->addCubeSceneNode (10.0f, 0, -1);
    smgr->addLightSceneNode(0, core::vector3df(0, 50, 0),
                            video::SColorf(1.0f, 1.0f, 1.0f),
                            1000.0f);
    
    scene::IAnimatedMeshSceneNode* anms = smgr->addAnimatedMeshSceneNode(smgr->getMesh("../../media/dwarf.x"));
    //scene::IAnimatedMeshSceneNode* anms = smgr->addAnimatedMeshSceneNode(smgr->getMesh("../../media/ninja.b3d"));
    if (anms)
    {
        //anms->setScale(core::vector3df(5.f, 5.f, 5.f));   // ninja needs to be larger
        anms->setMaterialFlag(video::EMF_LIGHTING, false);
    }   
    
    const bool reloadScene = true;
    if ( reloadScene )
    {
        smgr->saveScene("scenetest.xml");
        smgr->clear();
        smgr->loadScene("scenetest.xml");
    }
 
    while ( Device->run() )
    {
        if ( Device->isWindowActive() )
        {
            videoDriver->beginScene(true, true);
            smgr->drawAll();
            videoDriver->endScene();
        }
        Device->sleep( 1 );
    }
    
    Device->closeDevice();
    Device->drop();
    
    return 0;
}
 

So... can't help unless I get a test-case which allows me to reproduce the bug. Maybe it only happens with specific models/formats. You can adapt my code easily to try with your own model.

[Mel]

Well, eventhough i myself haven't been able to reproduce, externally, the bug, i've traced it to its source. The file "CAttributeImpl.h", here

Code: Select all

    virtual core::vector3df getVector() _IRR_OVERRIDE_
    {
        core::vector3df v;
 
        if (IsFloat)
        {
            v.X = Count > 0 ? ValueF[0] : 0;
            v.Y = Count > 1 ? ValueF[1] : 0; //When this value is set, it gives odd values... 0 or very high values. The exported XML files are right, though, so the failure is somewhere else
            v.Z = Count > 2 ? ValueF[2] : 0;
        }
        else
        {
            v.X = (f32)(Count > 0 ? ValueI[0] : 0);
            v.Y = (f32)(Count > 1 ? ValueI[1] : 0);
            v.Z = (f32)(Count > 2 ? ValueI[2] : 0);
        }
 
        return v;
    }

Does this give any idea where i might have kicked the code the wrong way? :/

[CuteAlien]

Hm, not really, that should be used a lot. Can you post the scene-file? Maybe something in there can give a hint.

[Mel]

I've fed this xml to the sample application you posted and gives the same error

http://omegasection.g0dsoft.com/wp-cont ... netest.zip

[CuteAlien]

Hm, all float numbers written with comma instead of point. Probably fails then reading them again. Will have to dig into sources to figure out how that could happen. It shouldn't use language settings when writing (and even if it would - which it shouldn't - it should at least read/write the same...). I remember where was some strange setlocale in Irrlicht 1.8 (no longer in trunk) when using Irrlicht's file-open dialog. Which Irrlicht version are you currently using?

[Mel]

The latest SVN version. I also found odd that it used ',' instead of '.' for the floating points, but i thought it could be any sort of token for irrlicht's internals, so i didn't pay more attention to it. But it didn't seem to fail in your example.

Edit: In your example, the output uses points instead of commas. To open the file i use the file open dialog, but to save, i use a custom gui. But i don't touch locales of any sort.
Edit: I've found something... this function seems to preserve the locale setting

Code: Select all

void CGUIFileOpenDialog::pathToStringW(irr::core::stringw& result, const irr::io::path& p)
{
#ifndef _IRR_WCHAR_FILESYSTEM
    char* oldLocale = setlocale(LC_ALL, NULL);
    setlocale(LC_ALL,"");   // multibyteToWString is affected by LC_CTYPE. Filenames seem to need the system-locale.
    core::multibyteToWString(result, p);
    setlocale(LC_ALL, oldLocale);
#else
    result = p.c_str();
#endif
}

But to convert from io::path to core::stringw, i use a core::stringw constructor:

Code: Select all

bool onSaveSceneAs::OnEvent(const irr::SEvent& event)
    {
        bool result = false;
        gui::IGUIButton* btn;
        gui::IGUIListBox* lbx;
        core::stringw textConvert;
        gui::IGUISkin* skn;
 
        if (event.EventType == EET_GUI_EVENT)
        {
            if (event.GUIEvent.EventType == gui::EGET_MENU_ITEM_SELECTED)
            {
                gui::IGUIContextMenu* menu = (gui::IGUIContextMenu*)event.GUIEvent.Caller;
                if (menu->getItemCommandId(menu->getSelectedItem()) == EGUIMENUFILESAVEAS)
                {
                    s32 width = 400;
                    s32 height = 310;
                    core::recti rec = gui->getRootGUIElement()->getAbsoluteClippingRect();
                    core::dimension2di resolution;
                    resolution.Width = rec.LowerRightCorner.X - rec.UpperLeftCorner.X;
                    resolution.Height = rec.LowerRightCorner.Y - rec.UpperLeftCorner.Y;
 
                    win = gui->addWindow(
                        core::recti(
                        (resolution.Width - width) / 2,
                        (resolution.Height - height) / 2,
                        (resolution.Width + width) / 2,
                        (resolution.Height + height) / 2),
                        true, langPack::filesaveas.c_str(), NULL, EGUIMENUFILESAVEAS);
 
 
                    filename = gui->addEditBox(0, core::recti(10, 30, width - 90, 50), true, win, EGUIFILESAVEASEDITFILENAME);
                    filelist = gui->addListBox(core::recti(10, 55, width - 90, 280), win, -1, true);
                    Accept = gui->addButton(core::recti(width - 80, 30, width - 10, 50), win, EGUIFILESAVEASACCEPTBUTTON, L"Accept");
                    Cancel = gui->addButton(core::recti(width - 80, 55, width - 10, 75), win, EGUIFILESAVEASCANCELBUTTON, L"Cancel");
                    gui->addStaticText(langPack::filesaveaswarning.c_str(), core::recti(10, height - 25, width - 10, height - 5), false, true, win);
 
                    io::IFileSystem* fs = gui->getFileSystem();
                    skn = gui->getSkin();
 
                    currentDirectory = fs->getWorkingDirectory();
                    fl = fs->createFileList();
                    for (u32 i = 0; i < fl->getFileCount(); ++i)
                    {
                        textConvert = core::stringw(fl->getFileName(i)); //here
                        filelist->addItem(textConvert.c_str(), skn->getIcon(fl->isDirectory(i) ? gui::EGDI_DIRECTORY : gui::EGDI_FILE));
                    }
                    fl->drop();
                    result = true;
                }
            }
 
            switch (event.GUIEvent.EventType)
            {
            case gui::EGET_BUTTON_CLICKED:
                btn = (gui::IGUIButton*)event.GUIEvent.Caller;
                if (btn==Accept)
                {
                    io::path savefilename;
                    savefilename = gui->getFileSystem()->getWorkingDirectory();
                    savefilename.append('/');
                    savefilename += filename->getText(); //Here there is a conversion from stringw to io::path
                    manager->saveScene(savefilename);
                    win->remove();
                    result = true;
                }
 
                if (btn == Cancel)
                {
                    win->remove();
                    gui->getFileSystem()->changeWorkingDirectoryTo(currentDirectory);
                    result = true;
                }
                break;
            case gui::EGET_LISTBOX_SELECTED_AGAIN:
                lbx = (gui::IGUIListBox*)event.GUIEvent.Caller;
                if (lbx == filelist)
                {
                    io::IFileSystem* fs = gui->getFileSystem();
                    if (fs->changeWorkingDirectoryTo(lbx->getListItem(lbx->getSelected())))
                    {
                        filelist->clear();
                        io::IFileList* fl = fs->createFileList();
                        skn = gui->getSkin();
                        for (u32 i = 0; i < fl->getFileCount(); ++i)
                        {
                            textConvert = core::stringw(fl->getFileName(i)); //Here
                            filelist->addItem(textConvert.c_str(), skn->getIcon(fl->isDirectory(i) ? gui::EGDI_DIRECTORY : gui::EGDI_FILE));
                        }
                        fl->drop();
                    }
                    else
                    {
                        filename->setText(lbx->getListItem(lbx->getSelected()));
                    }
 
                    result = true;
                }
 
                break;
            }
        }
 
        return result;
    }

If the locales are touched somehow, there are conversions both forth and back from stringw to path, maybe that is the origin of the issues.

[CuteAlien]

Locals should only matter when writing float's with some printf function somewhere. Which we probably do inside xml-writer, but I think we shouldn't call setlocale before every write call (that would be very, very slow). pathToStringW restores the old locale, so the function doesn't really change it (it didn't do that in Irrlicht 1.8, that's why I asked). There might be another workaround - I added some function to Irrlicht once to read float's with comma and point - have to check why that's not used here. Or maybe it's not enabled by default. I'll check it this evening.

[CuteAlien]

You can work around it by adding the following line in your code:

Code: Select all

 
irr::core::LOCALE_DECIMAL_POINTS = ".,";
 

I'm not sure - maybe we should make that default.

edit: *sigh* can't test on my debian system, for some reason it doesn't install german locales and I can't get it to use comma... will experiment some more on Windows another day. I guess we can enforce it on saveScene, but have to test.

[Mel]

pathToStringW restores the old locale

That's the point, converting between io::path to core::stringw in the stringw constructors doesn't seem to do it. As i build my own dialog to save the file, maybe they're modified there when i build the paths somehow. Also, I noticed the country that the Irrlicht dll generates is Germany, so perhaps, at some point, it is necesary inside the engine to reference this. Maybe changing that to an international configuration that worked everywhere in the Windows builds could solve it. Linux shouldn't have this problem, as the wchat file system is only handled on Windows.

[CuteAlien]

No, that's not it. Basically the problem is that a locale is set that has a lc_numberic which uses comma instead of point. And saveScene doesn't reset that - which it probably should (I'll work on that). But the workaround with LOCALE_DECIMAL_POINTS mentioned above will also work (it allows the conversion routine used there to accept comma as well).

[AReichl]

be careful with "." and "," when having both decimal separator AND thousand separator, e.g. the number 1000:

1,000.00 or 1.000,00 or 1'000.00 or ... ?

Normally this should not be a problem, i only wanted to mention it ( right now i have to deal with it, and the
best solution i found was to use Boost - i know i know, not usable for Irrlicht, only mentioning it ).

If i remember it right, there is one country on this earth that uses "," for EVERYTHING - decimal separator AND
thousand separator AND list separator. Wonder how they can work with it; probably they will never fly to the moon.

[CuteAlien]

Hm, good point, so maybe we keep LOCALE_DECIMAL_POINTS as it is by default. Though probably printf doesn't do that mix (or does it?). But enforcing the point on saveScene should be fine.

[Mel]

You can work around it by adding the following line in your code:

Code: Select all

 
irr::core::LOCALE_DECIMAL_POINTS = ".,";
 

I'm not sure - maybe we should make that default.

edit: *sigh* can't test on my debian system, for some reason it doesn't install german locales and I can't get it to use comma... will experiment some more on Windows another day. I guess we can enforce it on saveScene, but have to test.

Okay, i've tried this, Works fine with the produced files, with commas, but i am not sure if it would break the engine somewhere else. I think the decimal points should use points, not commas. Feels like a clumsy hack.

[CuteAlien]

Shouldn't break - but I'll change saveScene on weekend (or next week if I don't find time).