New wiki article: Basic Ogre Framework

[spacegaier]

Hello toghether!

Normally I wouldn't post here just because I wrote a new wiki article, but this time it's a bit different. I wanted to announce this glory development a bit wider, because this could be a link that helps the newbees that just want to get rid of the ExampleApplcation. How to accomplish this, is a rather commonly asked qustion here, that could now be answered by linking to this article, if you want to.

Wiki URL: BasicOgreFramework

Mercurial Repostiroy URL: BitBucket BasicOgreFramework Repository
Note: There are separate branches for the different Ogre versions.

I hope that this will help the beginners with creating their first advanced application.

Regards - spacegaier

PS: Of course, I'm open for any suggestions and criticism.

BOF_Screenshot_04142010_213925843.jpg

Update 2014/03/01:

• ported to Ogre 1.10
• added VS2012 solution (VS2008 still remains, but no guarantees that it works)

Update 2013/02/10:

• ported to Ogre 1.9 (v1.8 remains as an own branch)

Update 2011/12/30:

• ported to Ogre 1.8
• hWnd now uses size_t to prevent issues with x64 builds
• framework now hosted on BitBucket

Update 2011/07/18:

• all platforms now use Ogre::WindowEventUtilities::messagePump()

Update 2011/02/10:

• removed cg.dll dependency
• added some Linux specific code bits

Update 2010/10/22:

• removed memory leak caused by m_pTrayMgr

Update 2010/04/14:
Reorganized and renewed the media as I previously used media from older SDK version, resulting in crashes when trying to run with Ogre 1.7 media.

• old ogre head replaced
• alloverlays removed and replaced by SdkTrays

Update 2010/03/03:

• reuploaded a "new" version built against Ogre 1.7 Final

Update 2010/01/17:

• pressing "Escape" in the config dialog now results in the quitting the application

Update 2010/01/11: I updated the Basic Ogre Framework to work with Ogre 1.7 [Cthugha]. Changes (not only related to the version change) are:

• removed all the CEGUI artefacts (which were in fact never needed before, I just never took them completely out)
• removed deprecated build command
• removed camera rotation via keyboard, as the mouse already handles that
• altered destructor to prevent the new OIS to crash on exit
• explicitly marked floats as such to pretend conversion warnings

[Zini]

After just browsing the article, I noticed the following:

1. You are using "using namespace Ogre" in the header. Don't! That's bad practice.

2. You are using "#include <Ogre.h>". Again, don't. This can potentially cause very slow compilation, unless you use precompiled header.

3. OgreFramework::isOgreToBeShutDown is missing a const.

4. The try-catch-block in OgreFramework::initOgre is pointless, since the function isn't exception-safe anyway.

5. In the destructor you are using this construct

Code: Select all

	if(m_pKeyboard)
	{
		delete m_pKeyboard;
		m_pKeyboard = 0;
	}

Again, that is pointless. You don't need to check if m_pKeyboard is 0. And you don't need to set it to 0 after the delete either, because m_pKeyboard doesn't exist any more after the destructor has finished.

6. You should disable the automatically generated copy constructor and assignment operator for OgreFramework, by declaring them private.

7. DemoApp::initDemo looks odd. Usually you do the "init" part in the constructor.

8. Don't use Sleep. It isn't portable.

[jacmoe]

Looks extremely similar to my article here:
http://www.ogre3d.org/wiki/index.php/Ex ... emystified

But, I approve of it, because it's up to date.

Put a link to it in the Editors Picks box, here:
http://www.ogre3d.org/wiki/index.php/Ogre_Tutorials

I will definitely link to your article - this question pops up all the time.
Am a bit surprised that people find it that hard to make their own applications.

Great work!
And nice layout too.

[jacmoe]

One more thing:
Consider removing the 'I' part of the tutorial - rewrite it to be universal - that way you encourage other users to contribute to it.

[Speedo]

Looks like generally a nice starting point for people to reference. Just a few questions/critiques

-Any reason you handle messages yourself instead of using WindowEventUtilities::messagePump()?
-In OgreFramework::initOgre(), when you catch and rethrow the OIS Exception, you are throwing an Ogre::Exception*, but your main function is catching an Ogre::Exception&, meaning your rethrow won't be caught. Not sure why you'd need/want to throw a pointer, TBH.
-Also, if anything in OgreFramework::initOgre() throws, you may get segfaults in the destructor because you haven't initialized your various pointers to 0/NULL.
-And a nitpick - if you're going to dynamically allocate DemoApp in the main function, you should properly delete it, being that this is the kind of thing that newer programmers are likely to be reading and it's just good practice. I don't really see any reason you can't just let the DemoApp instance reside on the stack though.

Overall good stuff though.

[Zini]

One more:

Don't catch Ogre::Exception. Catch std::exception instead.

[iigor]

this code unneded, bacause it will be never used.

Code: Select all

if(timeSinceLastFrame == 0)
	{
		m_MoveScale = 1;
		m_RotScale  = 0.1;
	}

[spacegaier]

First of all: Thanks for checking the article and making so many good proposals and corrections! I have changed everything now.

But there are some questions left: The most impotant one:
When leaving the application, I now get a runtime error when trying to delete m_pRoot. Any suggestions?

6. You should disable the automatically generated copy constructor and assignment operator for OgreFramework, by declaring them private.

Not totally sure, whether my changes where what you suggested...

8. Don't use Sleep. It isn't portable.

What can I use instead of it?

If I should have overseen some proposals, please tell me again!

@jacmoe: You're right. It should not be written out of my view. Will change that. I also added the spot in OgreTutorials.

[betajaen]

I hate to be a killjoy, but any chance you can replace the cube.1m.mesh with something else? Like the OgreHead. It's just the yellow cube is more associated with NxOgre than Ogre, and people may get confused.

Otherwise, bang up work!

[spacegaier]

You're right. Originally, this once was a NxOgre demo for myself, of course with the yellow cube. Will change that.

[betajaen]

Cheers!

Now finish those BloodyMess tutorials for me.

[spacegaier]

I certainly will. I have the next two days off, so I guees that I could write some more. But remember: The more features are in, the more tutorials I can write.

-> Finish BloodyMess for me

[Zini]

6. If you write a class without a copy constructor or assignment operator, the compiler will create one for you. With your class, these won't do unfortunately. Therefore you should disable them, just to make sure, that no one calls a broken function by accidentially copying or assigning an OgreFramework.

Code: Select all

class OgreFramework
{
    private:

        OgreFramework (const OgreFramework&);
        OgreFramework& operator= (const OgreFramework&);

    ... 
};

(no implementation required for these functions). Actually this is a rather widely used trick.

8. I am not aware of any simple portable solution. I usually use boost threads here, but introducing a boost dependency would probably a bad decision. You might be able to get away with #ifdef-ing it for different plattforms, but are you really sure, that you need a sleep at all?

[Zini]

Regarding the crash: Do not delete the render window. The root is doing that for you already, so your code is deleting it twice.

Edit: And you shouldn't delete Node and Entity in the DemoApp destructor either.

Edit2: This line

Code: Select all

demo->~DemoApp();

should be remove as quickly as possible.

Edit3: std::exception does not have a function named getFullDescription(). Use what() instead.

Edit4. This

Code: Select all

		DemoApp* demo = new DemoApp();
		demo->initDemo();
		delete demo;

could be easily replaced by this:

Code: Select all

		DemoApp demo;
		demo.initDemo();

and why are we using a this-> here:

Code: Select all

this->setupDemoScene();

?

Bit of Java background, eh?

Edit5 (*sigh*): The DemoApp is more complex, than it needs to be. Why allocate the OgreFramework dynamically? Instead use:

Code: Select all

class DemoApp : public OIS::KeyListener
{
public:
	DemoApp();

	void initDemo();
	void setupDemoScene();
	void runDemo();

	bool keyPressed(const OIS::KeyEvent &keyEventRef);
	bool keyReleased(const OIS::KeyEvent &keyEventRef);

private:
	OgreFramework				m_pOgreFramework;

	Ogre::SceneNode*			m_pCubeNode;
	Ogre::Entity*				m_pCubeEntity;

	bool					m_bShutdown;
};

Edit6: The whole keyboard listener stuff looks fishy to me. If I am understanding it right, you register the OgreFramework with OIS for callbacks (which is correct). Then you have another listener in DemoApp, which is not registered and calls the OgreFramework functions too. This can not work.

Edit7: Make setupDemoScene and runDemo private. And think about renaming initDemo. The name just does not fit.

[Kentamanos]

6. If you write a class without a copy constructor or assignment operator, the compiler will create one for you. With your class, these won't do unfortunately. Therefore you should disable them, just to make sure, that no one calls a broken function by accidentially copying or assigning an OgreFramework.

Code: Select all

class OgreFramework
{
    private:

        OgreFramework (const OgreFramework&);
        OgreFramework& operator= (const OgreFramework&);

    ... 
};

(no implementation required for these functions). Actually this is a rather widely used trick.

Please correctly if I'm wrong, but the copy constructor would also get called if you passed an object instance to a function by-value as opposed to passing it as a reference, correct?

[Speedo]

Please correctly if I'm wrong, but the copy constructor would also get called if you passed an object instance to a function by-value as opposed to passing it as a reference, correct?

Correct. The point is, the OgreFramework class shouldn't be copied. A, the compiler generated copy ctor would not function correctly if it were called. B, it makes no sense to allow copies of it (multiple instances of Ogre::Root, for example?). Any passing of the class to functions must by done by reference or via a pointer. You declare the copy ctor and copy assignment operator private to enforce that - the compiler will complain if anyone accidentally writes code that would lead to the class being copied.

[spacegaier]

Edit5 (*sigh*): The DemoApp is more complex, than it needs to be. Why allocate the OgreFramework dynamically? Instead use:

Code: Select all

class DemoApp : public OIS::KeyListener
{
public:
	DemoApp();

	void initDemo();
	void setupDemoScene();
	void runDemo();

	bool keyPressed(const OIS::KeyEvent &keyEventRef);
	bool keyReleased(const OIS::KeyEvent &keyEventRef);

private:
	OgreFramework				m_pOgreFramework;

	Ogre::SceneNode*			m_pCubeNode;
	Ogre::Entity*				m_pCubeEntity;

	bool					m_bShutdown;
};

Edit6: The whole keyboard listener stuff looks fishy to me. If I am understanding it right, you register the OgreFramework with OIS for callbacks (which is correct). Then you have another listener in DemoApp, which is not registered and calls the OgreFramework functions too. This can not work.

What would be the benefit from useing your EDIT5?

I also don't understand your EDIT6: It works perfectly. If the user only wants to use the input handling of OgreFramework, he doesn't make DemoApp a input listener and doesn't pass it to the framework. Otherwise, he does.

The rest is changed. If sohuld have overseen something, please tell me.

[Zini]

Edit5: It is generally considered good practice to avoid new and delete, if you don't need the additional flexibility that they offer. If you do not allocate the OgreFramework dynamically, you don't need to destroy it either, which means you don't even need a destructor for DemoApp. It just makes the whole thing shorter and more simple.

Edit6: Unless I am missing something, DemoApp::keyPressed and DemoApp::keyReleased will never be called.

And a new edit : the const for isOgreToBeShutDown should be placed after the function, not at the return type.

[spacegaier]

Well, you're probably right, but I guees EDIT5 is nothing that important (the destructor might be neede anyway by the things added by the user).

EDIT6: Of course they get called. That is the sense of them, that you can define your own additional input handling.

The const thing is corrected.

[Zini]

Ah, now I see. You register the DemoApp's KeyListener with the OgreFramework, which in turn registers it with OIS. And because it is doing so you need to call OgreFramwork's keyPressed/keyReleased from the DemoApp (since OgreFramework can't register its own KeyListener with OIS in this case).

Not exactly very intuitive. Maybe it would be a good idea to explicitly mention, that you are disabling the OgreFramework's functions here and you must call them from the DemoApp. Something like:

Note: By inherting from OIS::KeyListener and passing our demo class as a parameter to the initOgre() method, we can extend the input handling from the OgreFramework class by reacting to the inputs events in the keyPressed() and keyReleased() functions. The keyPressed and keyReleased function of OgreFramework are not called by OIS in this case, so we must call them from keyPressed/keyReleased in the DemoApp instead.

?

[spacegaier]

Good point! I changed it according to your suggestion.

[spacegaier]

Okay, there are two more points just comming into my mind. I'd like to have some feedback on them:

1. Wouldn't it be a good idea to make this OgreFramework class a singleton (via the possibilities offered by Ogre)?
2. How severe is it that I declared all the Ogre member variables (Root, SceneManager, Camera, ...) public instead of private and giving them a getter method? Should I change this (as it is an entry point for mostly not so advanced programmer, so they learn it the good way right away)?

Curious, what you think about it. Thanks in advance for your thoughts!

[Speedo]

1. Wouldn't it be a good idea to make this OgreFramework class a singleton (via the possibilities offered by Ogre)?

Yeah, I would say so.

2. How severe is it that I declared all the Ogre member variables (Root, SceneManager, Camera, ...) public instead of private and giving them a getter method? Should I change this (as it is an entry point for mostly not so advanced programmer, so they learn it the good way right away)?

I would make them private unless there's some critical reason to have them public. Considering that Ogre::Root is a singleton and accessible that way (and the other components can be obtained through it), I don't think there's any reason to have them public here. If you don't like looking them up through the singletons, just go the route the example framework uses and pass pointers to them where they're needed.

[spacegaier]

Point No. 1 will be changed, but point 2...

You cannot access everything via the Root such as the Log or the SceneManager (if you don't have the name). So I don't like this way that much. Passing them through would only make the whole thing more complex which is not that good for a beginner example. But that's just my opinion. Curious what the others think...

EDIT: I removed the OgreFramework pointer from the DemoApp class and access now everything via the singleton. Should I have left this member thereand just use the singleton pattern to ensure that there is only one instance, but not use its mechanisms to retrieve it?

[Speedo]

You cannot access everything via the Root such as the Log

Ogre::LogManager::getSingleton().getLog()

or the SceneManager (if you don't have the name).

If you don't have the names of your own scenemanagers, something is seriously wrong...