[fixed]IFileSystem bugfixes and improvements.

You discovered a bug in the engine, and you are sure that it is not a problem of your code? Just post it in here. Please read the bug posting guidelines first.
Post Reply
Nalin
Posts: 194
Joined: Thu Mar 30, 2006 12:34 am
Location: Lacey, WA, USA
Contact:

[fixed]IFileSystem bugfixes and improvements.

Post by Nalin »

https://sourceforge.net/tracker/?func=d ... tid=540678
This patch fixes some bugs with CFileSystem. It adds the _IRR_IMPLEMENT_MANAGED_MARSHALLING_BUGFIX define to missing return statements.

This patch also improves CFileSystem in the following ways:
1) Adds an IFileArchive** parameter to the addFileArchive function. The function will return the archive it created through that pointer.
2) Adds another addFileArchive function, this one taking an IFileArchive*.
3) Adds a removeFileArchive function taking an IFileArchive*.

The purpose of this patch is to make it easier to work with file archives in memory. You can add/remove/manipulate IFileArchive's much easier. As an example of how this can be used, in my project I scan for valid archives that can be loaded. I call addFileArchive for each one, store the pointer to the archive, grab() it, and remove the archive from the IFileSystem. I can then manually enter each archive and search for a specific file with IFileArchive::createAndOpenFile(), using that file to display appropriate contents. Once I have found the archive I want, I can re-add it to the IFileSystem and drop() the rest.

I decided to create the patch using an IFileArchive** so it wouldn't interfere with existing code and would prevent the duplication of the addFileArchive() function. If you think it would be better to create two new functions that return an IFileArchive*, I would be up for that. I just figured you wouldn't want identical functions in the code.

Ideally, though, I would like to see addFileArchive() return an IFileArchive* instead of a bool. It looks like addFileArchive() only returns a false if it cannot open the file, which can also be achieved by returning a nullptr. addFileArchive() will also always make duplicate IFileArchive's if called twice with the same file. I don't know if that is intended behavior or not. I personally think it should always return an IFileArchive* with the archive it creates, and that it should not create duplicate archives. It should return the IFileArchive* that already exists in that case.
hybrid
Admin
Posts: 14143
Joined: Wed Apr 19, 2006 9:20 pm
Location: Oldenburg(Oldb), Germany
Contact:

Re: IFileSystem bugfixes and improvements.

Post by hybrid »

Ok, thanks for splitting. I have largely no complaints or questions about this patch. Just a slightly off-topic suggestions and question: Since we will drop support for MSVC 2003 for Irrlicht 1.9 (latest) I think we can remove the marshalling bugfix as well. Is it required anyway anymore? Maybe there was a patch that fixed this dumb behavior even for 2003? Does anyone have a link for the KB page for this problem?
EDIT: Ok, found the bug description and the fix. It is part of SP1 for MSVC 2003, so I guess we can already remove this whole stuff right now. Seems to be completely superfluous. If really someone wants to compile with this old and buggy compiler without SP installed, it's up to him to fix these return values. Any complaints (especially from the managed C++ users)?!
Nalin
Posts: 194
Joined: Thu Mar 30, 2006 12:34 am
Location: Lacey, WA, USA
Contact:

Re: IFileSystem bugfixes and improvements.

Post by Nalin »

hybrid wrote:Any complaints (especially from the managed C++ users)?!
None from me, but I'm not a managed C++ user. I just saw a bunch of instances where the marshalling bugfix wasn't applied and decided to fix the whole file for those who are affected by it. If it was fixed in a service pack, I would remove it. No point in supporting buggy software when patches exist.
Post Reply