Sky and Weather System

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

Sky and Weather System

Post by lgromanowski » 11 Aug 2011, 20:49

athile wrote: Hi all, I wanted to share some screenshots. In my local version of OpenMW I've integrated the OGRE plug-in Caelum. See the post attachments. It was fairly simple.

I chose Caelum because it was so simple to build and integrate. There's also the OGRE SkyX plug-in, which is possibly visually superior, but it is apparently less mature and also looks resource hungry (maybe OpenMW could use that later? OpenMW 1.1 or 2.0?). Anyway, for now, Caelum seems like a solid starting point that can always be improved later. I assume one of the project goals is to get a complete OpenMW as quickly as possible - and then focus on refinements.

A couple notes:
  • I realize it's a bit inefficient to have the weather system rendering for interior cells; but that can always be optimized out later once exteriors are supported fully.
  • Obviously at this point it's just a sky/weather renderer; there's no connection to the "Morrowind" time and weather. That too can be added later
  • Eventually, I'd like to introduce separate notions of something like a EnvironmentEngine and a EnvironmentRenderer. The "engine" would handle the internal Morrowind notions of time, weather, moon, stars, fog, etc. - and the "renderer" would be Caelum, SkyX, etc. I.e. keep the environment logic separate from the rendering. At this point, of course, it's just direct use of Caelum with Caelum's default settings.
And a question:
  • Caelum is LGPL. For simplicity, I'd like to create an directory "extern/caelum" with a CMakeLists.txt within the OpenMW source tree. I presume this is valid since any modifications made to the source would be publicly available. Does anyone more familiar with licensing know if this is an acceptable approach? Licensing aside, is creating a extern/caelum subdirectory approach something other developers would like or is there a preferred alternative?
Share any thoughts you have on the topic and I can get the code cleaned up to be pulled into the main branch.
sir_herrbatka wrote: clouds are 3d? It looks pretty, that's for sure.
Zini wrote: You probably will have to tweak Caelum pretty heavily. Possible problems are at least:

- Different date & time system
- two moons
- Probably different weather effects, that affect the sky too

Switching it of for interior cells shouldn't be a problem, but please note, that some interior cells actually have a regular sky (there is a special flag for it).

Regarding the LGPL: I see no problem here. LGPL v3 is compatible with GPL v3, but not with GPL v2. LGPL v2.1 is compatible with GPL v2 and v3. We should be able to make that work together.
Zini wrote: One point I forgot to add: It might make sense to modify Caelum not specially for Morrowind, but in a more generic way. Once we are past 1.0, we will eventually need a more configurable sky (more or less than two moons, other sky effects) and the option to use different sky and date system.

We could as well continue developing a generic Caelum fork (I think the project is abandoned). This would be done best as a separate project/repository, that could be incorporated into OpenMW as a sub-module.
athile wrote: I'm mostly interested in Caelum as a "good enough" sky / weather solution until all major pieces of OpenMW are in place. Right now OpenMW is effectively a viewer and not a game. Getting to the point where OpenMW is a playable strikes me as an order of magnitude higher priority than worrying about correct visual fidelity with Morrowind. Caelum v0.5 (which yes, the author is no longer maintaining) is a low-cost solution to get "a sky" even if it is not "the Morrowind sky."

In other words, I have trouble imagining a serious complaint about the wrong number of moons, when there are still huge pieces of non-existent code like sound, animated monsters, a battle system, the ability to load/save games, etc., etc. :)
Zini wrote: Well, if you see Caelum only as a stopgap, that will be replaced later with a different solution, then frankly I see little point in adding it at all. On the other hand, if we want to use Caelum as a base for a future sky system, then we can as well plan ahead.
athile wrote: Yes, I do see Caelum v0.5 potentially as a stopgap. But just because something is a stopgap doesn't mean it's not a step forward. I imagine it would be sufficient as is for a significant amount of time in the OpenMW development. (E.g. IMHO, one moon will be just fine until there's actual gameplay.)

When the limitations of Caelum v0.5 become the highest priority items on the OpenMW roadmap, yes, then I think that it's time for a discussion about whether (a) improving Caelum, (b) choosing a different plug-in, or (c) doing something entirely different is appropriate. Given the amount of code we're talking about for the initial integration, I don't think postponing that decision until necessity demands it is at all an issue.

I have to add that I've very wary of the attitude of doing nothing because it might have to be replaced eventually simply since the current solution is not perfect. That stance will keep this project just like 99% of all open source gaming projects: never finished.
nicolay wrote: I think this looks fantastic, even in an interior cell where it has no place whatsoever! :)

A couple of points:

First, licencing: As Zini said, OpenMW is GPL, using LGPL projects (both statically and dynamically linked) is no problem at all. We don't have to treat it separately from other libraries.

Secondly, whether we should use it at all: I think so. I'm all for easy, stopgap solutions - and this one will look great in screenshots and video. If we don't know whether we'll be using it permanently, then let's not spend any time tweaking it before we know, and just keep the default settings. I assume you can set time of day (eg. night or day sky) pretty easily? That's about the level of complexity we need at the moment I think.

I think it should be disabled by default for interior cells though. What about adding a --skytest command line option to enable it?

On a side note, imagine if sleeping outside in Morrowind looked something like this:
Greendogo wrote: Awesome!
Zini wrote:
I think it should be disabled by default for interior cells though. What about adding a --skytest command line option to enable it?
The cell entry has a flag for this purpose: Cell::QuasiEx (in loadcell.hpp). IMHO it would be the easiest solution to just use that one. If you want to test it, you can just use the --start option to select a cell, that has a sky (or wait for the exteriors to be implemented ;) ).

Edit: Oh, wait. IIRC original Morrowind didn't have any cells with this flag set. It was introduced with one of the addons. Well, either modify the code to make it accept the addon esms too or add the flag as suggested by Nico.
Zini wrote: Nico, I think by your modifications to athile's commits you broke the out-of-source build. The original configure_file stuff looked correct to me (haven't tried yet, though). The only improvement I can think of would be to me the whole resources directory, so we don't pollute the root directory with additional entries.
athile wrote: I believe Zini is correct. Nico, your commit comment says "Moved caelum resources into the right place in the repo, instead of having cmake make redundant copies" - but to do a proper out-of-source build, the copies are necessary, not redundant since no files in the source tree should be directly referenced :)
nicolay wrote: You are both right of course. I'm still not used to thinking in terms of out-of-source builds yet.

The latest commit should fix it back.
Zini wrote: Just a heads-up: I am currently doing some heavy refactoring on the engine class (required to properly integrate the script system). While doing that, I disabled the sky system.
Once I am finished, I would like to add it back in. But instead of the command line argument, I will implement a script function to toggle the sky on and off at runtime.

athile, I would like to ask you to change your SkyManager, so that it can be created as part of the construction sequence (but with disabled sky). Then add a set of functions to control it (IsEnabled, setEnable or something).

The changes can wait until I am finished. You hopefully will see then where you have to call the new sky manager functions.
Zini wrote: @athile: Finally done with reintegrating the sky manager. You can start to implement the additional SkyManager functions. Feel free to leave out anything, that you consider to be not worth it in the current stage. But we need at least the enable and the disable function. Please note, that the sky should be disabled for a newly created SkyManager.

A few comments regarding the existing code:

- You are catching exceptions inside the create function. There is little point in doing so. Exception handling is taken care of at higher level.

- You are returning a 0-pointer on failure. That is a big NO NO! Please throw an exception instead.

Edit. Obviously you will have to wait until Nico has merged my changes.
athile wrote: The intention of catching the exception was that the sky manager can fail gracefully and the rest of OpenMW still runs just fine. This does make me think that having a rough coding standards page on the wiki would be good. My opinion is that there's real benefit to keeping a code base consistent. I don't particularly care which standards are used (e.g. all errors are exceptions versus never use exceptions and make all errors return codes); lots of approaches work so just being consistent is, to me, what's most useful.

On a different note, I won't have much time to contribute for maybe the next couple weeks. I should have some spare time to get back into OMW afterward though.
Zini wrote:
The intention of catching the exception was that the sky manager can fail gracefully and the rest of OpenMW still runs just fine
That would mean whenever the rest of OpenMW is interacting with the sky system, it would have to check first, if it was set up correctly. No thanks! That is a headache we don't need.
Theoretically you could implement the graceful failure mode inside the sky manager, so it would simply ignore calls in case initialisation failed. But I see little point in it. If the sky manager fails, that most likely means we have a problem with the underlying graphics system (Ogre or video driver). In this case ignoring a sky manager failure won't save us, because the problem will most likely surface in other parts of OpenMW too.
This does make me think that having a rough coding standards page on the wiki would be good.
I agree, but we seem to get along without one reasonably well, so it doesn't have a high priority for me.
I don't particularly care which standards are used
This I don't agree with. A bad standard is worse than none.
On a different note, I won't have much time to contribute for maybe the next couple weeks. I should have some spare time to get back into OMW afterward though.
No problem. We are not much in a hurry. The sky is still pretty optional at this point.
athile wrote: Let me rephrase what I said then: "I don't particularly care which reasonable standard is used." I merely meant to imply there's usually more than one correct and maintainable way to accomplish something in C++. Debating which of those multiple approaches is the unqualified "right" approach usually degrades the conversation into style/opinion arguments that have little to do the objective correctness.
nicolay wrote: I think codifying a loose set of rules for the coding style on the Wiki isn't a bad idea. I have no idea what those rules would be though, I guess they have to grow out of threads such as this. In general I don't care how big your tabs are or where you put your braces, but I do care about dependency issues and not making the code a big spaghetti mess. Perhaps general repository rules (such as line endings) should be part of that too.

IMHO the easiest way to make such a page is just to wait for the questions to show up and then make a FAQ out of it.

BTW I noticed that the sky is now enabled for interior cells again, is this intentional?
Zini wrote: Yes. In the previous version I had the sky system completely disabled.
I re-enabled it, when I did the sky related script-instruction. I left out the test for interior cells for now, so we can test the ToggleSky command (since we can't render any cells yet, that are supposed to have a sky). We still need this missing bits of implementation from athile for that to work though.
Greendogo wrote: Well, if we could render Tribunal cells we could make use of it for Mournhold. This might be useful for taking screenshots for PR and recruitment.
Zini wrote: That is true and it would improve our testing options too. But unfortunately Tribunal is no option yet, because we can't load more than one master file.

I wouldn't use it for PR purpose though, since the sky is pretty much a placeholder at this point.
best regards,