Everything about development and the OpenMW source code.
User avatar
Site Admin
Posts: 1193
Joined: 05 Aug 2011, 22:21
Location: Wroclaw, Poland


Post by lgromanowski » 11 Aug 2011, 21:16

athile wrote: There has been discussion on other threads about the desire for an in-game console. I'm working on a console but with a slightly different approach and wanted to share what I've been doing.

The core design...

OpenMW engine will have a single thread-safe queue of std::strings. These are the console commands. On each frame OpenMW will do something like this:

Code: Select all

std::string command;
while (tsQueue.pop_front(command)) {
Really simple. The only notable aspect of the above is that the queue is thread-safe. This basically means anyone can throw a command into the queue at any time and it should be safe. Boost.Thread is used for thread-safety.

Gui Console

Eventually, we'll need a OGRE GUI console that puts strings into this queue. I think this is pretty self-explanatory.

However, this requires writing some CEGUI code (or whatever specific GUI library is eventually chosen).

Interprocess Console

I wanted to get a simple console in place without all the overhead of learning/maintaining a specific GUI framework. As a result, I thought I'd give interprocess communication a chance so that the console could be written a as a simple command-line app using gets().

I used Boost.Asio to write a small TCP server (approx. 200 LOC) that listens for string commands from an external connection. When the connection sends a string to the server, the server places it in the thread-safe queue. The server is running in its own thread, so it's not slowing (or complicating) the code in the main thread. At only 200 LOC, it's straightforward code.

The client code is even simpler. Its a simple command-line app that sends the results of gets() calls to the server. It could easily be extended/modified to create some small, specific debugging tools to push/pull data to/from the OpenMW engine. I don't have any specific plans for any code beyond the trivial console, but it does allow for a lot of flexibility moving forward.


It's all working on my local copy of the code - and didn't involve having to write any GUI code. I was planning to clean up the server into an OMW::ConsoleServer namespace in the "components" directory and add a "console" command-line client to the "apps" directory.

Any ideas or feedback before I commit anything to my fork?
Zini wrote: Interesting approach. But I am not sure how well it would work in this environment. All the Morrowind consoel has to do is collecting the user input from a GUI window and send it to my script compiler/interpreter. What you are proposing sounds like a lot of overhead (and I don't mean performance here). I have some difficulties to see the benefit of an implementation via IP.
OpenMW engine will have a single thread-safe queue of std::strings. These are the console commands. On each frame OpenMW will do something like this:
Also, that looks like overkill. OpenMW can execute at most one line of console script code per frame, because that is as fast as the user can enter it (theoretically).

btw. we are planning to use MyGUI, not CEGUI.
Zini wrote: Oh, wait. You meant the external console tool as a stopgrap until the GUI is up? That actually might work, but there is a problem. On Linux you can't tab out of a window, while the window is in exclusive mode (which is definitely required for OpenMW). And yes, I agree, this is one of the few cases where Microcoft got it right and the Linux guys (actually the guys got it wrong.

We will have to find a way around it anyway (maybe leaving exclusive mode when the GUI is up?). But for now, your console would be unusable under Linux.

Edit: Lol! The more I think about it, the more I like it. At first I thought having an external console isn't very useful, but in fact I can come up with several cases where I would have wanted one in the past.

We still have to address the problem mentioned above, but it basically sounds good. Please push it to your fork (optionally to a separate branch), so that we can review your code.
nicolay wrote: I have to agree with Zini that this one might be a bit overkill. As the rest of the engine will be hard to make thread-safe, and the scripts will more than likely not run in a separate thread for that reason (as they interact too much with the game state), it's hard to see how a thread safe message server might come to use. But OTOH for use as an external way to feed console messages into the game (as was mentioned) it might make good sense. And having it probably can't hurt. I guess I have to see the code to understand what it does.

As for the GUI system: we already have quite a bit of GUI code and window layouts for MyGUI in the old codebase, so that is what we'll aim for. We even had a working console, so I guess some of that code can be salvaged and reused more or less directly.
Zini wrote:
scripts will more than likely not run in a separate thread
Not "more then likely not", but "absolutely not". Exactly for the reason you mentioned. Not doable without constantly locking/unlocking every single bit of the engine, which will most likely slow it down to a crawl.
Greendogo wrote: Will the scripts run in a separate thread from the graphics? What about the AI? How many threads could you conceivably make use of?
Zini wrote: Scripts will definitely not run in a separate thread. The overhead would be forbidding. We might be running the script compiler in a separate thread though, but that has no priority for me.
No idea about the AI yet. We are so far away from implementing it, that I haven't given it any thought yet.
Greendogo wrote: Scripts would not run in a separate thread from what? The graphics? Other scripts?
Zini wrote: Yes. We have one main thread. We might consider moving things like physics and AI partially to a separate thread. And Ogre can perform some operations in a background thread. But that's it basically.
Greendogo wrote: Ah, ok gotcha.
athile wrote: I committed the console code to my fork if anyone wants to comment on the changes: ... 1a918bfb65
Zini wrote: Well, I can't claim to understand the asio stuff (never worked with this part of boost).

The whole thing looks very asynchronous. That isn't really what you would want from a console. You enter a command, the console blocks, the engine does its work, you get some output and only then the console un-blocks and
you can enter the next command.
If we allow for new commands midway through command-execution, we will get the input/output of different commands mixed up.

Also, I can't see any way to get output back to the external console.


Code: Select all

     char buffer[1024];
Had to look that function up first, but from what I see it is terribly unsafe. Also, char-arrays for strings? ... no.

Code: Select all

std::string buffer;
std::getline (std::cin, buffer);
athile wrote:
If we allow for new commands midway through command-execution, we will get the input/output of different commands mixed up.
The commands (i.e. std::string objects) get pushed to the deque in-order. They get processed in-order. The output therefore can be collected in-order. There's nothing (that I'm aware of) about the current code that would prevents the input/output from being processed correctly.

Once the command processing actually returns a value (rather than being a dummy print statement), the synchronization code you are worried about can be added. It can't really be added now because the command processing doesn't generate any output to send back.

And yes, gets() is very unsafe. I put this code together quickly and will gladly change it to std::getline().
Zini wrote:
It can't really be added now because the command processing doesn't generate any output to send back.
Well, just send some dummy text back to the console (but note, that it could be several lines). I will change it later, when I integrate the script system.

Once that is sorted out, I suggest you send Nico a pull request.
athile wrote: ... 037aefd780

Added dummy output where the command string is echoed back in all uppercase.
Zini wrote: Looks good!
nicolay wrote: I pulled in both your master branches now. Not sure if there was some WIP code there, but I had to fix up a couple of small things to get it to compile and run.

Most notably I've commented out two cleanup lines in engine.cpp (for the command server and for caelum respectively - search for FIXME). Both crashed or hanged for me. I'm not quite sure what either of these do exactly so I didn't want to start debugging them until you've had a look at it.

All in all, things are looking good, great work guys!

PS: Noticed a bug in the console client: if I press ctrl-D (on linux) it enters an infinite loop.
Zini wrote: Looks like there went something wrong with merging. I have no idea about the crashes, but you re-inserted some code, that I took out. Will have a look.

My master branch was a WIP indeed, but it shouldn't cause any problems. You already fixed the only bug I still had (the mWorld (NULL) line).


At a first look: Console isn't working properly. I don't get any reply.
What ever you did with the mouse pointer settings, it made the game unplayable. You can't really navigate without mouse pointer exclusive mode.
The previous configuration was actually fine. Worked much better than all my tries to get Ogre/OIS to play nicely with Maybe we should revert this change?
Zini wrote: Pushed are partial fix. You can pull now, if you want.

I had to keep mspCommandServer->stop(); commented out though. The console is definitely not working on Linux.
nicolay wrote: Hmm, deleting the sky (even after you moved it to the destructor) still segfaults for me.

OTOH, the console seems to be working fine (except for the hanging at exit): I start a separate xterm, run clientconsole, and everything I type show up in the openmw xterm too. This is in linux.

About the exclusive input: I just disabled it while debugging, since a hang with exclusive input is pretty bothersome (can't even alt-tab out.) In the D version the --debug switch would disable exclusive input, perhaps we should put that back.
Zini wrote: Well, I can only say, that it doesn't work for me.

Oops, I see the SkyManager problem. Will fix. And take care of the --debug option too.
Zini wrote: Oh dear! The mix-up with the merge was even bigger than I thought. But it should work now.
--debug is in too.
athile wrote: Regarding the console problems and the sky manager crash, I'll do what I can to figure it out. Sorry about the troubles, I've seen neither problem on my Windows build. I'll attempt to setup a linux virtual machine to debug these.
Zini wrote: There is no skymanager crash. That was just Nico mixing up some code in the wrong way. The console problem is real unfortunately. Best wait for Nico to pull in my changes. I altered the engine class a lot.
Zini wrote: I guess, I should give an error description for the console problem.

I start OpenMW normally. Here is a part of the log:
Setting up cell rendering
Setting up input system
Starting command server on port 27917
Then I start the console client in a separate terminal and press some keys (including return):
OpenMW client console
Type 'quit' to exit.
Client> xxx
With the stop line commented out, OpenMW terminates normally when pressing q, but I get no output related to your console (in neither terminal).

Redaring the crash, I am not sure, if it was there in your original code. Could be another leftover from the merge. Or I broke something when refactoring the engine class.
athile wrote: Removing the call to stop() causes OpenMW to crash on shutdown on Windows (since the call to stop() ensures the threads clean themselves up).

At this point I'm tempted to remove the console code unless someone wants to argue that it needs to stay.
Zini wrote: Well, if we can't make it work reliably across all platforms, then there is little point in keeping it.
athile wrote: I disabled the command server by default in my fork, but did not delete from the code base. I'm currently seeing if I can get a Linux build running so I can debug the Linux problems (that'll likely take me a while since I'm used to Windows, not Linux).
athile wrote: I got OpenMW building and running (albiet very slowly) on a Slackware virtual machine and I can reproduce the console hang. The code definitely works on Windows and not on Linux :(

The problem seems to be something at least one other person has run into with Boost.Asio. To quote the first bullet from the link, "It is impossible to cancel synchronous operations" ( It seems to me that actually, it appears to be impossible to cancel synchronous operations on Linux. On Windows, mAcceptor.close() is canceling the synchronous accept() call. OpenMW hangs on Linux since it was relying on that behavior.

At this point, I'm not much of a fan of Boost.Asio and am not sure I want to spend much time restructuring the console code to avoid the synchronous calls. I used Asio since OpenMW was already using Boost. However, at this point, I'd rather spend my time trying to learn Bullet or something that might have more long-term value than working around the idiosyncrasies of the Asio API.
athile wrote: I believe I've fixed it. Tested OpenMW starting and shutting down on:

- Windows and Linux
- With no connections
- With an active connection
- With a connection that shutdown before OpenMW did

All six scenarios work fine on my Win7 build and the Slackware virtual machine.

I worked around the "no way to cancel a blocking accept()" problem by having the server flag that it was shutting down then send a dummy client connection/disconnect to get accept() to unblock. I fixed the hanging open client connections by using socket.shutdown() followed by socket.close(). Apparently both are needed for correct behavior on both OSes.

I haven't figured out git rebasing to collapse the fix into one commit, but the fix is in there as of the latest commit in my fork ( ... 321ddf2c53). I've sent a pull request.
nicolay wrote: Pulled.

The way I do rebasing in Git:

Code: Select all

git rebase -i HEAD~5
where 5 is the number of commits to squash (including the current one). That pops up an editor window (emacs for me) with a list of commits. Then I change all but the first one from 'pick' to 'squash' (or just 's').
best regards,