Resources File Handling

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

Resources File Handling

Post by lgromanowski » 13 Aug 2011, 21:35

Zini wrote: Quoting myself from another thread:
Actually, I think it does work on Windows. The problem is the difference in file systems. The best solution is to make a new Ogre Filesystem class (derived from the existing one). We probably should scrap the file finder component and re-route sound resources through the Ogre resources system. Also the situation where a file exists in a bsa and in the regular file system needs to be handled (the one in the file system takes precedence).
The class that needs to be subclassed is Ogre::FileSystemArchive:

http://www.ogre3d.org/docs/api/html/cla ... chive.html

At least the basic implementation would require only a few additions (we still need to cache the results for speed).

Handling the other parts of the issue (file both inside and outside of bsa and resources file types currently not handled by OGRE) will need a bit more work.
jhooks1 wrote: I just did a checkout of your master branch and made a new project. I had to add a locale to your tolower() calls in multidircollection.hpp

The first thing I will do is verify that loading separate files outside of bsas works in windows.
Zini wrote: Okay. Sounds reasonable. I'll write you up for the task.
gus wrote:
I just did a checkout of your master branch and made a new project. I had to add a locale to your tolower() calls in multidircollection.hpp
I fixed that in my master branch.
jhooks1 wrote: I am pretty sure that windows also shares this problem. I tried to insert a wizard hat nif mesh that is in my Data Files\Meshes folder into the scene. Openmw could not find it.
Zini wrote: Then we have a bigger problem. I suggest to first look how Nico implemented the bsa access and then to see what is the difference to regular file system access.
jhooks1 wrote: Zini, I got it working!!!!


http://www.ogre3d.org/addonforums/viewt ... =8&t=12710

Wizard hat shows in all its glory.

Just had to add a filesystem resource location.
Zini wrote: That is very strange, because the code already added all the required filesystem locations. See apps/openmw/engine.cpp: OMW::Engine::go()
jhooks1 wrote: Nevermind, that wasn't what fixed it.

I removed the \\Meshes from the file string, thats what made it work. So now I need to try to get this running in linux.
Zini wrote: Can you push your changes to github, please?
jhooks1 wrote: I just pushed them, but the changes aren't what fixed it. Removing the \\Meshes from the address is what allowed openmw to find the mesh
Zini wrote: You mean in an ESM/ESP? That is the wrong direction. We must adjust OpenMW to work with the existing files and not adjust these files to work with OpenMW.
jhooks1 wrote: No I did not modify any ESP's or ESM's

This is the line of code that loads the mesh
cellRender.insertMesh("bald_MJ_hat.NIF");

This mesh exists in my Data Files\Meshes\ folder
Zini wrote: I see. In other words the Windows-problems were false alarm. It worked all along, but your test code was wrong. Which means your most recent push didn't change anything related to the problem.

I suggest to not add this kind of random test code to our code base. If you have a bit experience with the TES-CS, it is trivial to add a new mesh. Since we don't have plugins yet, you will need to make a copy of Morrowind.esm, create a plugin with the changes and then merge the plugin into it (this requires changing a setting in Morrowind.ini: http://www.angelfire.com/clone2/shadowsong/How-To.html; the lines below "How to...").
jhooks1 wrote: Ok, yeah I haven't really changed anything yet. So do I need to get separate mesh files working in linux, or is that not an issue? What about other resource files?
Zini wrote: I think textures don't work either. We don't use video yet and sound is routed through a different resources channel. Ideally we want to integrate sound into the OGRE resources system, but that can wait until we have meshes and textures working.
jhooks1 wrote: So I need to setup a dev environment in ubuntu to test out and fix resource loading, correct? I already have ubuntu installed in a partition on my machine, so I just need to follow the dev environment setup guide.
Zini wrote: Correct.
jhooks1 wrote: Haven't worked any on this task lately, trying to start now. I have openmw built in ubuntu but when I start in Beshara or any other cell I only get approximately 1FPS.
safferli wrote:
jhooks1 wrote:Haven't worked any on this task lately, trying to start now. I have openmw built in ubuntu but when I start in Beshara or any other cell I only get approximately 1FPS.
Have you got hw 3d support activated in Linux? Depending on your graphics card, Ubuntu does not do that automatically.
jhooks1 wrote: Thanks for the tip. I installed my nvidia drivers and I am up to 15fps, which isn't great, but it is better than before and good enough to dev.

My wizard hat nifs are still loaded fine, just as they were in windows.
Zini wrote: Very strange. I just tested again and I am getting hundreds of "File not found" errors.
Try to change the case of your NIF file so it doesn't match anymore. If that does not fail either, something is wrong with your test setup.
Zini wrote: You could also try to move the NIF file into a subdirectory. Meshes/something/file.nif instead of Meshes/file.nif.
jhooks1 wrote: I just moved the mesh to Meshes/test

The line to display the mesh was
cellRender.insertMesh("test/bald_MJ_hat.NIF");

This works in ubuntu, but you have to use a forward slash. I would assume a \\ is need instead in Windows, but I will try and see later.

You have to leave out the "Meshes" part in the code, the code already starts inside that directory.
Zini wrote:
This works in ubuntu, but you have to use a forward slash. I would assume a \\ is need instead in Windows, but I will try and see later.
That is really the point (besides the case problem). Its \, not /. Always. The esp files don't care about operating system.
jhooks1 wrote:
Zini wrote:
This works in ubuntu, but you have to use a forward slash. I would assume a \\ is need instead in Windows, but I will try and see later.
That is really the point (besides the case problem). Its \, not /. Always. The esp files don't care about operating system.
Ok, Understood.
jhooks1 wrote: So
1) make a class that is similar in function to BSAArchive. This class shall inherit from FileSystemArchive

2)Make a new open function that, if(OGRE_PLATFORM != OGRE_PLATFORM_WINDOWS) takes the existing file string, searches for \'s and makes a new string with /'s

3) Make OpenMW use this new class to look for meshes and textures that are not in the BSA

Is this what you had in mind?
Zini wrote: Not exactly. We also need to handle case-folding. No way around re-implementing something like the file-finder. But the class you suggested is the right place.

There are a couple of other problems to consider (handling of resources currently not loaded through the OGRE resources system and the --fs-strict flag). But we can take care of that later. Important for now is, that the implementation is efficient. A previous implementation via file finder slowed us down awfully much.
jhooks1 wrote: Why did the file finder slow us down, and what should I do differently? Was it because it searches all subdirectories for a file?
Zini wrote: I think there was a problem with creating multiple instances of the file finder, which wasted a lot of resources. I guess you won't run into the same problem, but we must still pay attention to performance.

It is important, that you cache the results of a directory search. I suggest you do it just in time, i.e. only build a file list for a directory when a file from this directory is required. As for sub-directories, I think you only need to search the directory specified. MW resources finding does not work recursively (actually I am only 99% sure that this is also true for textures).
jhooks1 wrote: Worked on this a good bit today and committed to git. Made a DirArchive that inherits from the FileSystemArchive. Also implemented new respective factory functions.

Next things to implement are making files/directories case insensitive and converting "\"'s to "/"'s in unix.
jhooks1 wrote: Windows directories (\\) for meshes/resources should now work in unix, I pushed to github.


One thing that seems strange to me is some meshes are gold and white instead of having the proper texture. This was happening before I modified anything though, I am guessing the texture is in another file somewhere? I've never really made any morrowind mods so I dunno.


EDIT: Excuse the random insertMesh in npc.cpp
Zini wrote: Textures are separate files, if that was the question. Not sure what is going wrong there.

I tested your code and I see no difference when running it with Redemption. Might be the case problem that is getting in the way. Looks like you still have the hardest part of the task ahead of you.
Zini wrote: Okay. Found the problem. The meshes folder is called "Meshes". For some reason the CS uses "meshes" when referring to a mesh. I think we still need to handle the case folding for all parts of the file name, not only the top level folder.

I also see the problem with the missing textures now. That is definitely a case problem too. Texture references in the meshes file seem to start with "texture". But the texture root directory is called "Texture".

As a sidenote, I am getting tons of error messages, rendering errors and even one skeleton-related crash with Redemption. We are using models with more advanced features, that are normally not found in Morrowind (but are certainly used by some parts of the Morrowind community). There is little point in getting into it now, but we still have a long way ahead of us before the NIF-loader is working properly.
jhooks1 wrote: I think that there is going to need to be some interaction with boost to implement case insensitivity.
Zini wrote: Definitely. You need to read in and buffer the contents of directories. boost::filesystem is the right choice for that.
jhooks1 wrote: Committed today's progress to git. Made a map that maps string directories to a vector of all filenames in the respective directory. I think I am going to use the same comparator we used for case-insensitive cell changing.
jhooks1 wrote: The loader is now working, in any case. Textures are also being loaded, although I don't think they are being displayed perfectly. I committed the code to github.

I have not tested this in windows yet, only linux. When loading do not lead with a \\, I have not handled that scenario yet.

Example of how to load:

cellRender.insertMesh("MeSHes\\1\\red_MJ_hat_org.NIF");
Zini wrote: Tested it. Kinda works. I still get a lot of missing textures, but that looks more like a problem with the NIF loader. I think it is pointless to investigate this problem at this time.

The performance is not so good. Takes several seconds to go from a cell to another. Maybe all the logging is slowing us down. That should be tested again after the branch has been cleaned up. But it is also possible, that we need to look into optimising your code more.

I am getting a lot of warnings when compiling your code (unused variables, comparison between signed and unsigned integers and such). Not a big deal, I can clean it up after you are finished. However I get the feeling that the MSVC compiler settings in the cmake script are not quite right. You are not getting enough warnings and Star recently mentioned, that he gets lots of warnings where he should have got none.
jhooks1 wrote: When I switch between interior cells I don't seem to have a problem. I am going to change a few things that should make the process a little more efficient.
Zini wrote: I tested with Redemption (the main city which spans multiple cells and has lots of custom models) and it got pretty bad there. Obviously Redemption is a lot more demanding than MW, but with the original game engine cell changes in Redemption are almost instant.
jhooks1 wrote: Leading slashes (e.g. \\Meshes\\some.nif) now work. I also changed a few things, I think the loader should be a little more efficient now. Got rid of all related std::cout messages. Committed my changes to github.

Zini, could you also test it in an original morrowind cell?
Zini wrote: Better, but still a bit slow. Almost instant for MW, maybe 2 seconds for Redemption.

But I am still getting plenty of log messages, even with MW:
rrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrController's Rtype:-1939612013Stype: 0Ttype:3
Creating WholeThing21
AFTERThe target rec: Root Bone
Controller's Rtype:3Stype: 0Ttype:3
The target rec: Bone02
Controller's Rtype:3Stype: 0Ttype:2
The target rec: body1
Controller's Rtype:3Stype: 0Ttype:2
The target rec: body
Controller's Rtype:3Stype: 0Ttype:3
The target rec: Head
I am pretty sure I did not had them before.
jhooks1 wrote: Those messages are from the animation system, not the resource loader. Somehow the code from my animation attempts got mixed in with the resource loader. This is kind of weird, to make this branch I took my master and pulled your latest and merged them together.

I can suppress these message, or even, entirely get rid of the animation system. Just let me know what I should do.
Zini wrote: Made a minor performance optimisation and pushed it to github.

Regarding the animation system: Yes, please remove it. We should not mix up branches.
jhooks1 wrote: Its gone from my latest commit. There is still some related commented out code from engine.cpp, will remove this too.

I apologise for getting things mixed up.
jhooks1 wrote: Boost uses /'s for windows also. Just committed a minor change, the resource loader works in windows.
Zini wrote: Shit happens, but it works nicely now.

I am still getting a bit of logging: rrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr

Also, there are some warnings left:
/home/marc/OpenMW/openmw/components/bsa/bsa_archive.cpp: In member function â??void DirArchive::populateMap(boost::filesystem::path)â??:
/home/marc/OpenMW/openmw/components/bsa/bsa_archive.cpp:93: warning: comparison between signed and unsigned integer expressions
/home/marc/OpenMW/openmw/components/bsa/bsa_archive.cpp:104: warning: comparison between signed and unsigned integer expressions
/home/marc/OpenMW/openmw/components/bsa/bsa_archive.cpp: In member function â??virtual bool DirArchive::exists(const Ogre::String&)â??:
/home/marc/OpenMW/openmw/components/bsa/bsa_archive.cpp:131: warning: comparison between signed and unsigned integer expressions
/home/marc/OpenMW/openmw/components/bsa/bsa_archive.cpp: In member function â??virtual Ogre::DataStreamPtr DirArchive::open(const Ogre::String&, bool) constâ??:
/home/marc/OpenMW/openmw/components/
I think once that has cleaned up, we can merge.

Edit: Or not. Forgot one thing. We still need to implement the --fs-strict option (given on the command line and telling OpenMW not to fold case in filenames).

And forgot one more. *sigh*

The file finder is now almost redundant. We should get rid of it. But the sound system is still using it, so we must redirect it through the OGRE resources system.
jhooks1 wrote: Handled fs-strict, works properly in linux. In windows it can still find the files when enabled, Windows must be automatically case insensitive.
Zini wrote: It is.
jhooks1 wrote: It seems there is no Ogre::ResourceManager for Audio, maybe I could use one of the other Managers?

EDIT: Also a bigger problem, convert an Ogre::ResourcePtr into a boost SoundPtr
Zini wrote: I think the correct way of handling new resources types is to add a new resources manager. Not sure what you mean with boost SoundPtr. There is no such class in boost.

If the whole thing turns out to be too much work, we can stick with the file finder. It would be nice to have a unified solution, but since we have something that already works, it doesn't make sense to spend a large amount of developer resources on it.

If you keep the file finder, you should implement the --fs-strict mode there too.
jhooks1 wrote: typedef boost::shared_ptr<Sound> SoundPtr is what I mean, it's in playSound(), playSound3d(), and add().
jhooks1 wrote: I think this will be too much work, because of the shared ptr conversion.
Zini wrote: Okay, then add the --fs-strict mode to the file finder and we call the task finished.
jhooks1 wrote: SoundManager::StreamMusic() and SoundManager()::say() are the relevant functions. Do I need to also handle SoundManager::playSound() and other id functions, making the id case sensitive during fs-strict?
Zini wrote: No, IDs must stay case-insensitive. This is only about file names.
jhooks1 wrote: Running into some problems.

Right now streamMusic, takes an absolute path, so there is no slash replacing or case folding for it. Should this be changed? Right now --fs-strict is pretty much being followed without having the switch on.

On say() what is the proper way to set up a MWWorld::Ptr ptr? I cannot figure it out and am getting compiler errors.
Zini wrote:
Right now streamMusic, takes an absolute path, so there is no slash replacing or case folding for it. Should this be changed? Right now --fs-strict is pretty much being followed without having the switch on.
Does this mean streamMusic does not go through the file finder? In this case we have a working implementation for --fs-strict, but no implementation for --fs-strict=0.
On say() what is the proper way to set up a MWWorld::Ptr ptr? I cannot figure it out and am getting compiler errors.
What do you mean with set up? I don't understand the question.
Zini wrote: Oh, wait! I get it. You mean you want to get an instance of MWWorld::Ptr pointing to a particular reference.

For a 0-pointer you can construct the MWWorld::Ptr directly. For a pointer pointing to a reference use MWWorld::World::getPtr or MWWorld::World::getPtrViaHandle.

Edit: fixed the class name
jhooks1 wrote:
Zini wrote:
Right now streamMusic, takes an absolute path, so there is no slash replacing or case folding for it. Should this be changed? Right now --fs-strict is pretty much being followed without having the switch on.
Does this mean streamMusic does not go through the file finder? In this case we have a working implementation for --fs-strict, but no implementation for --fs-strict=0.
It does not go through the file finder. Should I implement the file finder in streamMusic()?
Zini wrote:
What do you mean with set up? I don't understand the question.
If I want to call say() from frameRenderingQueued()

MWWorld::Ptr ptr;
mEnvironment.mSoundManager->say(ptr, "Fx/bearsniff.wav");

What would I set ptr to?
Zini wrote:
It does not go through the file finder. Should I implement the file finder in streamMusic()?
Yes.
What would I set ptr to?
See my posting above yours.
Zini wrote: And now I also understand, why you want to call say. Don't! There is a script instruction available at the console that you can use:

Say "file"

or

ID_Of_NPC -> Say "file"

Edit: seems I have to correct every posting today. The first form would obviously only work from within a NPC-script.
jhooks1 wrote: Ok makes sense. So do I need to find an npc id and call say from the console on it? Is the command even implemented yet?
Zini wrote: Fully implemented. See here: http://openmw.org/wiki/index.php?title= ... tatus%29#S

Just make sure that the NPC is in an active cell. References outside the current interior cell or the current 3x3 exterior grid are not found currently.
Zini wrote: *sigh* Looks like I have to correct myself one more. I got the syntax of the say instruction wrong. It is actually:

NPC -> Say "filename", "text"
jhooks1 wrote: So just the npc name for the id? In the construction set that what it seems to be.

I still cannot get the say command working.

I am getting an Exception has been thrown: Unknown id laire


EDIT: Nevermind, got it working. It only works on npcs with lower case ids.
jhooks1 wrote: Say is now working with fsstrict. I think for streamMusic() there should be two functions, one that uses file finder and one that uses an absolute path.
Zini wrote: You mean as an implementation detail? Because exposing two functions to the outside doesn't not make any sense.
Zini wrote:
EDIT: Nevermind, got it working. It only works on npcs with lower case ids.
You sure about that? I thought we had this problem sorted out already.
jhooks1 wrote:
Zini wrote:
EDIT: Nevermind, got it working. It only works on npcs with lower case ids.
You sure about that? I thought we had this problem sorted out already.
Unless it was committed recently and I just haven't pulled it yet, the problem still exists.


Anyways, streammusic should work now, just pushed my changes.

Example syntax:
StreamMusic "Explore\Morrowind Title.mp3"
Zini wrote: Strange. I'll investigate the case problem.

Regarding your fork, can you please merge in my branch with the same name and then do a bit more cleanup? There are still some out-commented pieces of code lying around (and I think you killed the doxygen comments on the stream music function while moving it around).

And the indention of your code on github looks very odd (soundmanager). It didn't look great before, but now it is worse. Either you got very creative with indenting or you are mixing up tabs and spaces again.
Zini wrote: I can confirm the bug and I am also having an idea now what is going wrong. Here is the issue: http://bugs.openmw.org/issues/150
Zini wrote: I pushed a fix (casefix branch). Some more testing would be welcome before I merge it into master.

I think we will need to clean up the whole case handling code at some point (maybe even add an option to disable case folding/smashing all together; similar to the --fs-strict option). But that can wait until 1.0.0.
jhooks1 wrote: Just did some cleanup and a merge on the directory branch.
Zini wrote: Still a lot of cleanup to do. I take it from here.

But please check your editor settings again. As I suspected you used tabs instead of 4 spaces. Please don't! That screws up the formatting all over the place.
Zini wrote: Finished. I hope. There were too many remains of your new animation code. I think I got it right, but I don't understand the code well enough to be sure. Since we are very close to a release, I don't want to take a risk. Your branch will go into 0.12.0 instead of 0.11.0 so we have more time for testing.
best regards,
Lukasz

Locked