Modifying OP2Decompress to allow returning a file in memory instead of to disk

[Vagabond]

I'm looking through the OP2Decompress code. Currently, it only allows unpacking a file from an archive (either .vol or .clm) to disk.

For full code, see: https://svn.outpostuniverse.org:8443/!/#outpost2/view/head/GameResources/VolDecompress

The virtual method to accomplish this from Class CArchiveUnpacker:

Code: [Select]

virtual int ExtractFile(int index, const char *fileName) = 0;

From Class CVolFile, the implication of ExtractFile for .vol archive.

Code: [Select]

// Extracts the internal file at the given index to the file 
// filename. Returns nonzero if successful.
int CVolFile::ExtractFile(int index, const char *filename)
{
    HANDLE outFile;
    unsigned long bytesWritten;
    char *offset;
    int length;
    int retVal;

    outFile = CreateFileA(filename,	// filename
			GENERIC_WRITE,	// access mode
			0,		// share mode
			NULL,		// security attributes
			CREATE_ALWAYS,	// creation disposition
			FILE_ATTRIBUTE_NORMAL,	// file attributes
			0);		// template

    // Check for errors opening file
    if (m_FileHandle == INVALID_HANDLE_VALUE)
    {
	// Error opening file
	return false;
    }

    // Get offset and length of file to extract
    offset = (char*)m_BaseOfFile + m_Index[index].dataBlockOffset;
    length = *(int*)(offset + 4) & 0x00FFFFFF;
    offset += 8;

    // Check if the file is compressed
    if (m_Index[index].compressionTag == 0x100)
    {
	// Write the file (uncompressed)
	retVal = WriteFile(outFile, offset, length, &bytesWritten, NULL);
    }
    else
    {
	// The file is compressed
	if (m_Index[index].compressionTag == 0x103)
        {
	    // Decompress the file
	    // Construct the decompressor
	    CHuffLZ decomp(length, offset);
	    const char *buff = 0;

	    do
	    {
		buff = decomp.GetInternalBuffer(&length);
		retVal = WriteFile(outFile, buff, length, &bytesWritten, NULL);
	    } while (length);
	}
	else
	    retVal = 0;
    }

    // Close the file
    CloseHandle(outFile);

    return retVal;
}

What I would like to do is create a virtual function:

Code: [Select]

virtual int ExtractFileToMemory(int index, const char *fileName) = 0;

Then change the .vol file and .clm implementation of ExtractFile to read something like:
* Call ExtractFileToMemory
* Then actually save the file.

This way I can extract a file for use in the OP2MapImager without actually creating a copy in directory, polluting the user's file system.

My knowledge of the windows API is about zero right now though. Anyways, I don't know if anyone feels like hand walking me through the process. If not, that is okay too.

[Hooman]

Ahh. If you look at CVolReader.h from the OP2Editor project, you'll see an alternate implementation. That project uses custom stream classes (perhaps something your XFile part might provide). You can open a stream directly from the VOL file, and read data through that stream object.

From OP2Editor:

Code: [Select]

    HRESULT STDMETHODCALLTYPE OpenStreamRead( 
            /* [in] */ BSTR fileName,
            /* [retval][out] */ StreamReader __RPC_FAR *__RPC_FAR *stream);

Without all the COM junk, that would look more like:

Code: [Select]

StreamReader* OpenStreamRead(string fileName);

Another option, closer to the project you are modifying, is to use the internal index number, rather than search the index for a filename:

Code: [Select]

StreamReader* OpenStreamRead(int index);

Implementing the first method in terms of the second one would just require one method call to adapt the inputs:

Code: [Select]

StreamReader* OpenStreamRead(string fileName)
{
        return OpenStreamRead(GetFileIndex(fileName));
}

For the VolDecompress utility, it was useful to iterate over the whole index and extract each file. For the OP2Editor utility, it was useful to lookup specific named files, and extract just that data. It's very easy to support both use cases by implementing one function in terms of the other.

Side note: The GetFileIndex method in OP2Editor uses a binary search to find the desired index entry. This is more efficient than a linear search, and is possible because the index entries are sorted alphabetically.


All of the loading code in the OP2Editor project (tile sets, maps, etc.) uses generic read methods from the StreamReader interface. Hence, just passing the returned StreamReader object to those load methods would allow them to load directly from the VOL file.

Note that the StreamReader interface used in the OP2Editor project is custom to that project, while you're using C++'s standard stream classes in your project. They are not equivalent. You would need to adapt to using one way or the other, possibly through a wrapper interface.

The stream classes in OP2Editor support loading directly from files on disk using the CFileStreamReader class, or loading from memory using the CMemoryStreamReader class. Both classes implement the same StreamReader interface, which only includes 2 virtual functions (Read, and Status). Consumers of stream data (map loaders, bitmap loaders) use only these 2 methods. Most of the difference is in object construction. You pass CFileStreamReader a filename, while you pass CMemoryStreamReader a pointer to a memory buffer, and it's length.

The file searching code checks the directory for lose files and, if found, constructs a CFileStreamReader object using the file name. If it's not found, it defers to the CVolReader objects (there are more than one) to search their contents and, if found, return a CMemoryStreamReader object, otherwise they return NULL. The search method then returns the StreamReader object (which is an instance of CFileStreamReader for lose files, or CMemoryStreamReader for files packed in a VOL), or NULL if no file could be found. At this point, the distinction between the sources is effectively lost. You now just have a pointer to a StreamReader interface, which allows you to call those 2 virtual methods, or a NULL pointer indicating the file couldn't be found.

Code: [Select]

HRESULT CResourceManager::OpenStreamRead( 
            /* [in] */ BSTR fileName,
            /* [retval][out] */ StreamReader **stream)
{
  // Details omitted...
}

Without all the COM junk, and with some implementation details, that would be:

Code: [Select]

StreamReader* CResourceManager::OpenStreamRead(string fileName)
{
  StreamReader* stream;
  // ...
  try {
    stream = new CFileStreamReader(pathAndFile);
  }
  if (stream) return stream;

  // ...
  while (currentArchive) {
    stream = currentArchive->OpenStreamRead(fileName);
    if (stream) return stream;
    // ...
  }
  return NULL;
}

In your case, you can split the MapData constructor code. You'll need to extract the code that creates the file stream and put it elsewhere. The constructor then accepts a stream reader object, and uses that to parse and interpret the file. That would allow the constructor to load data from either a file, or from memory, consuming data directly from a VOL reader object. It probably makes the most sense to have a file search method return a stream reader of the appropriate type.

Alternatively, the MapData class could have two constructors, where one just wraps and calls the other. That would allow a constructor to take a file name, create a stream object to read that file, then pass the stream reader to the other constructor method. It might be a useful convenience method for a library. Though considering you have multiple object types (tile sets, maps, saved games) that need to load either directly from disk, or from a VOL file, it probably makes more sense to have the file search code create an appropriate stream, which is then fed into the appropriate object constructor.

It might also make sense to have the file search code throw an exception when a file can't be found, rather than return NULL. Let the runtime handle problems, rather than putting NULL checks at each point of call.

[Vagabond]

Thanks Hooman,

I spent some time reviewing the OP2Editor code.

I was looking at the CMemoryStreamReader. There are some function declarations in the header file under IUnknown. Are we saying these functions fulfill an interface contract, but the rest of the interface is unknown?

Code: [Select]

    // IUnknown
    // ********
    ULONG __stdcall AddRef();
    ULONG __stdcall Release();
    HRESULT __stdcall QueryInterface(REFIID riid, void** ppv);

The CMemoryStreamReader class inherits from SeekableStreamReader which in turn inherits from StreamReader. The declaration of both SeekableStreamReader and StreamReader are provided in OP2Editor.h, with all functions being virtual. The top of OP2Editor.h states:

Code: [Select]

/* this ALWAYS GENERATED file contains the definitions for the interfaces */


/* File created by MIDL compiler version 5.01.0164 */
/* at Fri Nov 11 16:06:01 2005
 */
/* Compiler settings for E:\op2\OP2Editor\OP2Editor.idl:
    Oicf (OptLev=i2), W1, Zp8, env=Win32, ms_ext, c_ext
    error checks: allocation ref bounds_check enum stub_data 
*/
//@@MIDL_FILE_HEADING(  )

I did an internet search on MIDL and it is a Microsoft developed compiler. So if I want to implement the features the StreamReader, does this mean that I have to support the MIDL compiler? It looks like Visual Studio somehow auto-creates the code in OP2Editor.h. I'm guessing it is related to the COM interoperability?

Code: [Select]

    MIDL_INTERFACE("57B5FDD0-8E6E-4134-94DE-6949BD2EB3CB")
    SeekableStreamReader : public StreamReader
    {
    public:
        virtual /* [helpstring][propget] */ HRESULT STDMETHODCALLTYPE get_ReadOffset( 
            /* [retval][out] */ int __RPC_FAR *readOffset) = 0;
        
        virtual /* [helpstring][propget] */ HRESULT STDMETHODCALLTYPE get_StreamSize( 
            /* [retval][out] */ int __RPC_FAR *streamSize) = 0;
        
        virtual /* [helpstring] */ HRESULT STDMETHODCALLTYPE Seek( 
            /* [in] */ int offset) = 0;
        
    };

If all of this is COM specific code, can I then just leave it out of a new implementation. IE drop the SeekableStreamReader inheritance from CMemoryStreamReader and delete the functions under the IUnkown header?

[Hooman]

Ahh. Just drop anything COM related. IUnknown is a COM thing.

COM is a specific implementation of the more general concept of interfaced based programming. It contains a lot of complexity to deal with problems we don't care about. For example (assuming I remember the details correctly):

Distributed COM (DCOM): DCOM allows you can make function calls from one computer to another. The COM system will marshal those parameters from one computer to another, and marshal back the return value. This of course requires marshalling code and appropriate bindings (which the MIDL compiler helps with), and the ability to handle network errors during a function call (the HRESULT return values).

Object Lifecycle: COM objects are expected to be reference counted. This adds a bit of memory safety, with only a minor bit of overhead. It's now fully general though, and you can still get memory leaks if you have cyclic references. Think of a self referencing object that holds itself in memory. It can also happen if you have some family of objects that contains a cycle of references. A common example is a circularly linked list.

Object Creation: Rather than import code, either statically or dynamically, you can create registered COM objects using a Universally Unique Identifer (UUID). You may also hear of Globally Unique Identifiers (GUIDs), which is just Microsoft's implementation of the UUID standard. COM will do a lookup in the system registry for the UUID, find out which DLL has the code for the object with that UUID, load the DLL if it's not already loaded, create an object using the code in the DLL, and return the new object. The COM creation method required COM DLLs to be registered when they were installed on a user's system, which created the appropriate registry entries. This is an overly complicated system that many people using COM didn't want to bother with. You might even notice that DirectX, which used COM, had exported functions to create initial root objects, rather than forcing people to go through the COM creation hoops.


IUnknown is the base class of all COM objects. It contains the 3 virtual functions that all COM objects must implement. QueryInterface is used to ask for and get various interfaces that an object might support. It's quite possible an object will support more than one interface, and this method is how you obtain those different interfaces. Internally it just casts the object pointer to interface pointers. The other two functions are AddRef and Release. These are to implement the reference based counting.

Inside OP2Editor.idl, you'll find the IDL source code. This file contains the interface definitions, along with the UUID for each COM interface, and each COM class. The MIDL compiler processes this file and generates the type library file OP2Editor.tlb, which helps with bindings to various languages, such as VB. It also generates OP2Editor.h, and can produce marshalling related code. Our main interest was the type library so VB projects, such as the map editor front end, could easily use the code.


Discarding the complexity of COM, you can still get many benefits from Interface based programming. This is something supported by the core of C++, even without the standard library, and has absolutely nothing to do with Windows or COM. Remember, COM is just a specific implementation of interfaced based programming, and it was built on top of what C++ provides. I highly recommend learning and making use of interfaces.


To do what you want to do easily, it would be nice to re-use the loading code for maps and tilesets whether they come directly from the file system, or from the contents of a VOL file. This is what interface based programming lets you do. To do this, you'll want loading code that uses a single interface, which can be implemented by both an object that reads directly from the file system, as well as an object the reads the VOL contents from memory. The loading code only makes assumptions based on the interface, not on the specific object implementing that interface.

To do this, like in OP2Editor, but without COM:
 - define the StreamReader interface (a class with all pure virtual functions)
   - Provide a Read method
 - define a FileStreamReader class, which inherits the StreamReader interface
   - Have the constructor take a file name and open the file using the standard C++ library
   - Implement the Read method, making use of the standard C++ library to do the reading
 - define a MemoryStreamReader class, which inherits from the StreamReader interface
   - Have a constructor take the memory region to serve (a pointer and length)
   - Implement the Read method, which returns successive chunks from the memory buffer
 - Write your loading code to take a StreamReader*, and use the Read method to load stuff (it won't care whether it's a FileStreamReader or a MemoryStreamReader).

For this to work, you need to have the common StreamReader interface as a base class. This is how the compiler figures out which function to call at runtime. The Read method is looked up in the virtual function table (vtbl) of the passed object, which is different for each class. This table is just an array of function pointers. Hence the Read method must have the same signature in all derived classes to make sure the calls are binary compatible at run time. The compiler does all the work for you. As long as you define the common base class with the appropriate shared method signature, it should all just work.

I'm sure an example would be helpful. Perhaps try something similar to this (untested, simplified, no error checking):

Code: [Select]

/* Stream support code */

class StreamReader {
  virtual void Read(void* buffer, int size) = 0;
};

// Just defer calls to C++ standard library methods
class FileStreamReader : public StreamReader {
  FileStreamReader(string fileName) {
    file.open(fileName)
  };
  ~FileStreamReader() {
    file.close();
  };

  void Read(void* buffer, int size) {
    file.read(buffer, size);
  };

private:
  stream file;
};

class MemoryStreamReader {
  MemoryStreamReader(void* buffer, int size) {
    streamBuffer = buffer;
    streamSize = size;
    offset = 0;
  };

  void Read(void* buffer, int size) {
    // Copy streamBuffer[offset .. (offset + size)] to buffer
    offset += size;
  };

private:
  void* streamBuffer;
  int streamSize;
  int offset;
};


/* Application specific code */

class Map {
  // Note: We take a pointer to the base class (the interface)
  Map(StreamReader& streamReader) {
    // This calls either FileStreamReader::Read or MemoryStreamReader::Read
    streamReader.Read(&header, sizeof(header));
    // ...
  };
};

void SomeFunction() {
  FileStreamReader fileStreamReader("TestFile.map");
  Map testFileMap(fileStreamReader);  // Calls go to FileStreamReader::Read

  MemoryStreamReader memStreamReader = volFile.open("TestVol.map");
  Map testVolMap(memStreamReader);  // Calls go to MemoryStreamReader::Read
}

[Vagabond]

Hooman,

Thank you for the detailed background info on COM. I think it is a subject I'd have to study and read a book on to become comfortable even starting to understand... I would have never thought to call a base interface for all objects IUnkown.

This is what interface based programming lets you do. To do this, you'll want loading code that uses a single interface, which can be implemented by both an object that reads directly from the file system, as well as an object the reads the VOL contents from memory. The loading code only makes assumptions based on the interface, not on the specific object implementing that interface.

I'm pretty familiar with interfaces from C#, although I hadn't created one yet in C++. So no problem here. Good to actually learn how in C++.

Implement the Read method, which returns successive chunks from the memory buffer

I was more looking for help on designing the part that actually takes the data and stuffs it into the supplied structure. This is something I would always rely on the .NET Framework to do for me in the past. I should have been more specific in my previous post. I'll try googling it some more when I have some more time. So far I am just getting a bunch of posts on how to use iostreams from files. I probably need to refine what I'm asking a little better.

The information you provided made it easy to implement the interface and the FileStreamReader. I worked the StreamReader Interface into the project without too much difficulty. I added another virtual method called ignore. This way it can read a saved game and ignore the first part of the file. The code compiles and works properly taking a filename. The MemoryStreamReader isn't finished since I need to figure out how to actually read stuff into the buffer and then shove it into a data structure.

Code: [Select]

class StreamReader {
public:
	virtual void Read(char* buffer, int size) = 0;
	virtual void Ignore(int size) = 0;
};

FileStreamReader Header

Code: [Select]

class FileStreamReader : public StreamReader {
public:
	FileStreamReader(std::string fileName);
	~FileStreamReader();
	void Read(char* buffer, int size);
	void Ignore(int size);

private:
	std::ifstream file;
};

MemoryStreamReader Header

Code: [Select]

class MemoryStreamReader {
public:
	MemoryStreamReader(char* buffer, int size);
	void Read(char* buffer, int size);
	void Ignore(int size);

private:
	void* streamBuffer;
	int streamSize;
	int offset;
};

Below are the 2 constructors for MapData. I had to do a little research since this is the first time I've done constructor chaining in C++.

Code: [Select]

MapData(StreamReader& mapStream, bool saveGame = false);
MapData(string filename, bool saveGame = false) : MapData(FileStreamReader(filename), saveGame) {}

So you can either give it the StreamReader directly or provide a filename and it will create a StreamReader for you.

I plan on implementing the code to sort through the vol archives outside of the MapData class. I think it makes more sense to keep vol archive related code outside the scope of the MapData.

The size variables for the StreamReader might make sense to be some sort of unsigned long int (not sure that is an actual type). I haven't done the math on the size of data in a large file. It looks like the built in fstream uses a typedef long long streamsize;

[Hooman]

You can safely ignore COM at this point. It's pretty much dead.

Though in answer to your comment. IUnknown represents the methods you can call on any arbitrary unknown COM object. Given an unknown object, it allows you to get access to the interfaces it supports, and manage the object's life cycle.

For implementing a memory stream buffer, you can memcpy from the internal stream buffer to the one passed as a parameter to read, and then update the tracking info for the internal stream position. Remember to bounds check things first.

Also, keep in mind I left out all error checking in my examples. It might make sense to return the number of bytes actually transferred; a very common and generic API design that pushes error checking onto the client. Or you might thrown an exception if not enough bytes are available to do the full requested transfer; a very easy and safe to use API design. You can of course implement one in terms of the other. Though that might not be ideal if you want to make certain guarantees about stream state in the event of an error. For instance, does a partial read that raises an exception update the internal stream state, or leave it where it started?

I see you've found a use where you need to seek within your stream. Might I suggest an additional interface layer for the seek capability? Consider the following inheritance possibility:

Code: [Select]

StreamReader : SeekableStreamReader : FileStreamReader
StreamReader : SeekableStreamReader : MemoryStreamReader
StreamReader : SocketStreamReader
StreamReader : ZipStreamReader

Not all streams support seeking in a clean way. It would be a cleaner design to not include seeking in the base interface.


Hmm. I had some misgivings about the design of the two constructor option I suggested before. Now that I'm looking at it, I like it even less. My original issue was it seemed like it would be redundant code that was better written elsewhere. My second concern now is about memory safety. It's not immediately obvious looking at this that it's safe to do. It does work here, because the entire stream is read in the constructor, but is perhaps a poor pattern that could get copied elsewhere that isn't so suitable. Consider memory management for a moment. When and how is the FileStreamReader class cleaned up?

Code: [Select]

MapData(string filename, bool saveGame = false) : MapData(FileStreamReader(filename), saveGame) {}

Here the stream is a temporary object that gets destroyed at the end of the second constructor. This is fine, since the entire stream will be read in the nested call to the first constructor, so the stream isn't needed anymore by the end of the second constructor.


If we consider a different object, like say VOL files, where a passed in stream might be stored, and later used to read further chunks of data, this design fails in unexpected ways. It fails because the temporary stream object would be destroyed at the end of the outer constructor call. The stored pointer is now a dangling pointer. Potentially future calls will still work using the ghost of the destroyed stream object. Until one day it suddenly doesn't, and everything blows up because that memory of the deleted stream object got reused.

Another option is to new the stream object using dynamically allocated memory. Slightly less efficient if you don't need dynamic memory, but the real issue is ownership. Who cleans up the stream object?

Assume the object doesn't own the stream, it's just temporarily using the stream object. A constructor can't return a value. At the end of the constructor, the stream pointer silently goes out of scope, and is lost to the outside program. As the object doesn't own the stream, it's not going to try and clean it up. It now becomes a memory leak, and the program leaves files open after it's done using them. Not good.

Now assume the object does own the passed stream. Ok, now things work, and when the object is done with the stream, it can delete it, closing files and freeing memory. That works great so long as you only ever pass streams that will be used and discarded. You can no longer pass in a re-usable stream. Consider processing the contents of a Zip file, create objects as you unpack each internal file. If the objects claimed ownership of the stream and closed/deleted the stream after they were done reading their section, there would be no way to continue reading and loading the rest of the objects. It also doesn't work for streams stored in local variables on the stack, since you can't delete those. This is very restrictive.


Again though, this isn't an issue here because your map object reads the whole stream in the constructor, and doesn't store a pointer to the stream internally. This let's the point of call deal with cleaning up the stream. That might be letting locally declared streams on the stack go out of scope and get cleaned up automatically, such as your outer constructor, or it might mean dynamically allocated streams get deleted after use in some calling function. The pattern is fine, but it's not clear that it's safe unless you know the stream pointer isn't stored and used later. An assumption that should be documented somewhere.


In terms of ease of use and being generic, I think it's best to have a utility function that finds resources, either loosely in a folder, or packed in a VOL file, and returns a stream. A loading function (possibly a second constructor) can call the utility function to get an appropriate stream, whatever the source, pass that stream in to the Map constructor, and then later discard the stream. The utility function can then be reused for loading any other type of data.

It also seems likely that utility function will be some sort of class member function, such as:

Code: [Select]

StreamReader& ResourceManager.Open(string resourceName);

The reason for making it a member function, is that it must search some list of VOL files, which implies some kind of internal state. The resource manager needs to know what VOL files, and for efficiency probably wants to keep them open, with the headers and index already parsed, but probably not do a full load of the contents.


The size variable should probably be a larger type, such as size_t. Since Read copies data into memory, the read size is limited by the memory size. This makes size_t appropriate, which is defined as being large enough to span the memory address space. For 32-bit that means 4GB max, for 64-bit that's much much larger. Regardless of platform size, files greater than 4GB will be supported, so file/stream size and seek positions should always allow at least 64 bits. I believe fstream uses a "pos_type" typedef for an appropriately sized type.

[Vagabond]

Spent some time on the MapImager today. Grumble Grumble about C++ refactoring woes. Creating classes requires building the header file, building the .cpp file, and then appending all the functions with the class name. Not friendly at all for refactoring. and VS17 decided it needed to blow up when I tried to use the auto-rename everywhere feature. I didn't upload the code to the repo since the ArchiveHelper is a bit broke and I ran out of time. I'll try to get it uploaded tomorrow.

I think it's best to have a utility function that finds resources, either loosely in a folder, or packed in a VOL file, and returns a stream.

I had already thought of this and have a static class called ArchiveHelper. It currently handles searching in the directory for the file, gathering the names of all the VOL files in the directory, searching the VOL files for the required file, and extracting it. I basically just need to change the end state to passing a stream instead of extracting. ResourceManager seems too vague to me, but it is probably better than ArchiveHelper.

Currently working on de-static-afying the class and adding the ability to pass streams.

The size variable should probably be a larger type, such as size_t.

Done, all offsets and sizes in MemoryStream are now size_t. I'm finally starting to understand the use of size_t as well...

Might I suggest an additional interface layer for the seek capability? Consider the following inheritance possibility:

I separated out a new interface for SeekableStreamReader. I'm guessing that since files can be compressed within the vol file that we will eventually need a MemoryStream that can figure this out. I checked and one of the maps.vol is not compressed, so it isn't a problem right now.

Code: [Select]

class StreamReader {
public:
	virtual void Read(char* buffer, size_t size) = 0;
};

class SeekableStreamReader : public StreamReader {
public:
	virtual void Ignore(size_t size) = 0;
};

Below is the initial stab at the MemoryStreamReader. I won't be able to test it until finish the modifications to GameResources (previously ArchiveHelper). It throws errors whenever someone tries to read past the provided buffer size. I'm betting it doesn't work yet though.

Code: [Select]

MemoryStreamReader::MemoryStreamReader(char* buffer, size_t size) {
	streamBuffer = buffer;
	streamSize = size;
	offset = 0;
}

void MemoryStreamReader::Read(char* buffer, size_t size) {
	if (offset + size > streamSize)
		throw exception("Size to read exceeds remaining size of buffer.");

	memcpy(buffer, streamBuffer, size);
	//Copy streamBuffer[offset .. (offset + size)] to buffer
	offset += size;
}

void MemoryStreamReader::Ignore(size_t size)
{
	if (offset + size > streamSize)
		throw exception("Size to ignore exceeds remaining size of buffer.");

	offset += size;
}

I modified CArchiveFile to include 3 new functions to facilitate getting a memoryStream out and a simple helper function to find if a filename exists within the archive.

CArchiveFile now contains the following virtual functions

Code: [Select]

// Returns -1 if internalFileName is not present in archive.
virtual int GetInternalFileIndex(const char *internalFileName) = 0;

virtual void OpenMemoryStreamReader(MemoryStreamReader& memoryStreamReaderOut, const char *internalFileName) = 0;
virtual void OpenMemoryStreamReader(MemoryStreamReader& memoryStreamReaderOut, int fileIndex) = 0;

CVolFile.cpp contains the following line:

Code: [Select]

		
// The file is compressed
if (m_Index[index].compressionTag == 0x103)
{
    ....
}

I'd like to create some sort of enum to make this a little clearer. Something like below. Thoughts?

enum class CompressionType
{
    None = 0,
    HuffmanTree = 0x103
};

[Vagabond]

I'm having some trouble with the ResourceManager.

I want to create a SeekableStreamReader within the ResourceManager and pass it out. Since SeekableStreamReader is a virtual class, I cannot instantiate it in memory before calling getResourceStream.

Code: [Select]

bool ResourceManager::getResourceStream(SeekableStreamReader& seekableStreamReaderOut, const string& filename)

I could just use the return value as the SeekableStreamReader. But my plan was to use the return value to be a bool that indicated if a file/archived file was found to return in the stream.

[Hooman]

Very nice progress.

Grumble Grumble about C++ refactoring woes. Creating classes requires building the header file, building the .cpp file, and then appending all the functions with the class name.

I know! It's terrible! Why should a class have to be split between two files, and the implementation be declared separate from the definition? This is something a compiler should be able to manage. Yes, I know only the header information is needed for users of the class, but that header information should be extracted automatically by the compiler, in an already compiled form for efficiency, and used that way. There's no reason to put this burden on the programmer in this day and age.

Another small nuisance, why do constructors take the name of the class? If you change the name of the class, you have to go renaming all the constructors as well. Having to repeat yourself is bad for code maintenance. Same with the class name prefix on all the member functions in the implementation file.

I'm guessing that since files can be compressed within the vol file that we will eventually need a MemoryStream that can figure this out.

You're on the right idea. We'll need a StreamReader that can figure this out. We can write a new class that handles decompression on the fly with each call to Read.

Expect to have a new StreamReader derived class for each decompression method supported. The game supports 4 compression codes, though only 2 are actually used. One of them of course being uncompressed. The only compressed files were the sheets, and they should be re-packed uncompressed now, so you could conveniently skip over the decompression part if you wanted.

There should be an enum somewhere with the 4 compression methods. Maybe copy that. They were:

• Uncompressed
• RLE (Run-Length Encoded)
• LZ (Lempel-Ziv, the two creators)
• LZH (Lempel-Ziv, with adaptive Huffman encoding)

Below is the initial stab at the MemoryStreamReader. I won't be able to test it until finish the modifications to GameResources (previously ArchiveHelper). It throws errors whenever someone tries to read past the provided buffer size. I'm betting it doesn't work yet though.

Looked like a pretty good implementation to me.

CArchiveFile now contains the following virtual functions

Code: [Select]

// Returns -1 if internalFileName is not present in archive.
virtual int GetInternalFileIndex(const char *internalFileName) = 0;

virtual void OpenMemoryStreamReader(MemoryStreamReader& memoryStreamReaderOut, const char *internalFileName) = 0;
virtual void OpenMemoryStreamReader(MemoryStreamReader& memoryStreamReaderOut, int fileIndex) = 0;

The MemoryStream part of "OpenMemoryStreamReader" is an implementation detail. I'd say remove it. Especially since if you support decompression on the fly, they might be using something other than MemoryStream.

For similar reasons, passing a stream in by reference to become your out value is never going to work. A MemoryStreamReader will not be the same as a FileStreamReader. They will take different sizes in memory. As the method may return either one, you will not know ahead of time which type of object the parameter should point to. Better to pass back as a return value. If the stream could not be opened, a null value can be returned.

If you really wanted to hack it, which I don't recommend, passing a double pointer for your out value could work, and would be similar to a single pointer return value. The function is returning a pointer to an object. Either it does that with the return value, which gets copied to a pointer variable in the caller, it is takes an input parameter which is the address of a pointer variable (a double pointer), so again the function can "return" the pointer value into a pointer variable. Again though, I don't recommend this. Since the bool is already a sentinel value, why not just use null as the sentinel value.

[Vagabond]

There should be an enum somewhere with the 4 compression methods. Maybe copy that.

Thanks for the info. This enum isn't in the VolDecompress code and I don't see it in a cursory glance of the OP2 Map Editor backend code either. If everything has been resaved as uncompressed and that is up to date with the current OP2 release, I'm thinking of just leaving compression out. I'm having enough trouble getting the StreamReaders to work properly as things are.


I reworked everything to return StreamReaders as suggested instead of trying to use out variables. I implemented nullptr. Nullptr is something I hadn't really used yet since it was confusing to me in the past when first learning, but it makes more sense now. I am getting the following warning from the function returning a SeekableStreamReader pointer.

Code: [Select]

Warning	C4172	returning address of local variable or temporary	OP2Utility	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\archives\cvolfile.cpp	79

I'm assuming there is a way to get the SeekableStreamReader to remain in scope when the function finishes running, but not sure how? It would have made sense that returning a pointer to the class would have kept SeekableStreamReader in focus??? I'll work on this more and test it when the code is actually compiling again...

Code: [Select]

SeekableStreamReader* CVolFile::OpenSeekableStreamReader(int fileIndex)
{
	// Get offset and length of file to extract
	char* offset = (char*)m_BaseOfFile + m_Index[fileIndex].dataBlockOffset;
	size_t length = *(int*)(offset + 4) & 0x00FFFFFF;
	offset += 8;

	return &MemoryStreamReader(offset, length);
}

Unfortunately, somewhere in the middle of rebuilding all the code, I started getting the following large batch of error codes. All of them originate from the file CClmFile.h. It looks like half are generated from the OP2Utility library project and an identical half by the OP2MapImager application project. I think it started occurring when I manually deleted all the debug and release directories from both projects to troubleshoot some of what I thought weird behavior of the abstract archive classes. (It turned out that weird behavior was normal for C++ code after more research and I changed the coding style to match. Subtle differences from C#.)

It occurs on both release and debug builds with x86 configuration set (Earlier similar things happened when I had x64 set for configuration).

Until it is solved, I'm pretty much at a stand still and I have no idea how to proceed. The code is uploaded to the repository in its broken state.        

Code: [Select]

Severity	Code	Description	Project	File	Line	Suppression State
Error	C2061	syntax error: identifier 'WAVEFORMATEX'	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	48	
Error	C2061	syntax error: identifier 'WAVEFORMATEX'	OP2Utility	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	48	
Error	C2061	syntax error: identifier 'WAVEFORMATEX'	OP2Utility	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	50	
Error	C2061	syntax error: identifier 'WAVEFORMATEX'	OP2Utility	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	51	
Error	C2061	syntax error: identifier 'WAVEFORMATEX'	OP2Utility	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	53	
Error	C3646	'm_WaveFormat': unknown override specifier	OP2Utility	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	56	
Error	C4430	missing type specifier - int assumed. Note: C++ does not support default-int	OP2Utility	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	56	
Error	C2061	syntax error: identifier 'WAVEFORMATEX'	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	50	
Error	C2061	syntax error: identifier 'WAVEFORMATEX'	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	51	
Error	C2061	syntax error: identifier 'WAVEFORMATEX'	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	53	
Error	C3646	'm_WaveFormat': unknown override specifier	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	56	
Error	C4430	missing type specifier - int assumed. Note: C++ does not support default-int	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	56	
Error	C2061	syntax error: identifier 'WAVEFORMATEX'	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	48	
Error	C2061	syntax error: identifier 'WAVEFORMATEX'	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	50	
Error	C2061	syntax error: identifier 'WAVEFORMATEX'	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	51	
Error	C2061	syntax error: identifier 'WAVEFORMATEX'	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	53	
Error	C3646	'm_WaveFormat': unknown override specifier	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	56	
Error	C4430	missing type specifier - int assumed. Note: C++ does not support default-int	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	56	
Error	C2061	syntax error: identifier 'WAVEFORMATEX'	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	48	
Error	C2061	syntax error: identifier 'WAVEFORMATEX'	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	50	
Error	C2061	syntax error: identifier 'WAVEFORMATEX'	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	51	
Error	C2061	syntax error: identifier 'WAVEFORMATEX'	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	53	
Error	C3646	'm_WaveFormat': unknown override specifier	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	56	
Error	C4430	missing type specifier - int assumed. Note: C++ does not support default-int	OP2MapImager	c:\users\brett\documents\outpost2svn\gameresources\op2utility\op2utility\Archives\CClmFile.h	56	

   

[Hooman]

Perhaps I'm mistaken. I don't see an enum anywhere either.

Returning a pointer won't keep a variable around. Remember that C++ is not garbage collected. It uses explicit memory management. Or I suppose implicit, in the case to local variables on the stack being cleaned up automatically when the function goes out of scope.

To get around the problem about returning a local variable, you can new the object instead. That will allocate it on the heap rather than on the stack. Anything on the stack is cleaned up at function exit. Things on the heap are not. This does mean you'll have to explicitly delete the object afterwards though.

Code: [Select]

return new MemoryStreamReader(offset, length);

I also noticed your bitmask was a bit off. The size is all but the highest bit (0x7FFFFFFF), rather than all but the highest byte (0x00FFFFFF):

Code: [Select]

size_t length = *(int*)(offset + 4) & 0x7FFFFFFF;

Small note: Since the file format uses a fixed size on disk, and it's always limited to 32-bits (actually 31-bits of length). This means the VOL files can't pack individual files larger than 2GB. Whether you use int or size_t here is irrelevant, though I suspect the size_t type will play nicer interacting with the MemoryStreamReader class. I suspect it may also produce more efficient opcodes at the assembly level. I like how you did that here.


The WAVEFORMATEX structure is from the Windows header files. If you've removed that include, you'll need to manually paste in the structure. That's probably a good idea, especially if you want the code to remain cross platform. As per Microsoft the WAVEFORMATEX struct is defined as follows:

Code: [Select]

typedef struct {
  WORD  wFormatTag;
  WORD  nChannels;
  DWORD nSamplesPerSec;
  DWORD nAvgBytesPerSec;
  WORD  nBlockAlign;
  WORD  wBitsPerSample;
  WORD  cbSize;
} WAVEFORMATEX;

You can remove the typedef, and move the name up to the struct line. The reason for the typedef is to do with C compatibility, where C has separate namespaces for structs. In C you always need to write "struct structName" instead of just "structName", unless of course you include the typedef. This is irrelevant in C++, which doesn't have a separate namespace for structs. You may also want to remove the Windows typedef names on each field for basic unsigned integer types. Typically WORD is defined as "unsigned short", and DWORD is defined as "unsigned long". However, the exact size of short and long are compiler and platform dependent. This is bad for file formats. In practice, WORD is an unsigned 16-bit quantity, while DWORD is an unsigned 32-bit quantity. Better to use types from stdint.h.

Code: [Select]

struct WAVEFORMATEX {
  uint16_t wFormatTag;
  uint16_t nChannels;
  uint32_t nSamplesPerSec;
  uint32_t nAvgBytesPerSec;
  uint16_t nBlockAlign;
  uint16_t wBitsPerSample;
  uint16_t cbSize;
};

[Vagabond]

Getting this VolFile stuff incorporated is getting really involved, but I seem to be learning a fair amount in the process...

Good news! The code is successfully compiling again. Bad News! I broke CclmFile replacing WAVEFORMATEX typedef with a struct (or and earlier change broke it).

I replaced the built in Windows.h WAVEFORMATEX with a struct call WaveFormat as suggested. I first tried using the original name WAVEFORMATEX for the struct, but the compiler was angry that the name was already defined. I'm confused here because the compiler knows that WAVEFORMATEX exists within Windows.h, but it is unable to fetch the definition.

Using this new struct as defined by Hooman made the code compile. I tried loading op2.clm file into the ResourceManager as a test. Unfortunately it threw an error when attempting to read the file's header. I went back and tried using the console application OP2Decomp to extract from a .clm file and it worked fine. So somewhere along the line, I've broken CClmFile. I doubt adding functions to open a memory stream did any harm, so it may be related to the WaveFormat struct. This isn't really a problem for OP2MapImager, but I wanted to keep all the code working.

OP2MapImager is now able to receive the custom built StreamReaders. I'm using maps.vol as a test subject by loading the map eden04.map. The program seems to read some of the data correctly and some incorrectly.

I also noticed your bitmask was a bit off. The size is all but the highest bit (0x7FFFFFFF), rather than all but the highest byte (0x00FFFFFF)

I made this change to the MemoryStreamReader.

It successfully reads the width, height, and number of tile sets. But the length of a tile set string is being set at 4113 instead of 8 or less. This is causing an exception because the MapData class knows the length must be 8 or less. It looks like maybe some of the TileData is reading incorrectly as well since I saw a negative celltype and I think celltype must be positive (haven't checked yet for sure). I'll spend some time looking at this tomorrow and if I don't get anywhere try to post something that will help others troubleshoot.


Memory Management

Since I don't do memory management without the garbage collector often, I wanted to post my methods here for review.

The ResourceManager's constructor finds all archive files (.vol and .clm) files in the directory, loads them into memory, then stuffs them into a vector. Then destructor then iterates through the vector and deletes all CVolFile and CClmFile classes.

Hopefully this is a good practice?

Code: [Select]

ResourceManager::ResourceManager(const string& archiveDirectory)
{
	vector<string> filenames;
	XFile::getFilesFromDirectory(filenames, archiveDirectory, ".vol");

	for each (const string& volFilename in filenames)
		archiveFiles.push_back(new CVolFile(volFilename.c_str()));

	filenames.clear();
	XFile::getFilesFromDirectory(filenames, archiveDirectory, ".clm");

	for each (const string& clmFilename in filenames)
		archiveFiles.push_back(new CClmFile(clmFilename.c_str()));
}

ResourceManager::~ResourceManager()
{
	for each (CArchiveFile* archiveFile in archiveFiles)
		delete archiveFile;
}

And the second hack at memory management.

Below is an excerpt from a function that gets the SeekableStreamReader for a map, then passes it to the MapData constructor for creating a Map, and then deletes the SeekableStreamReader. Hmmm, good memory management, meh memory management or bad?

Code: [Select]

SeekableStreamReader* seekableStreamReader = resourceManager.getResourceStream(filename);

if (seekableStreamReader == nullptr)
	throw std::exception("Unable to find specified map or save file.");

MapData mapData(seekableStreamReader, saveGame);

delete seekableStreamReader;

[Hooman]

Check sizeof WAVEFORMATEX. If the size has changed, that will offset things in memory and cause loading errors.

Sounds like the Map data is also experiencing offset errors.

If you're using wrapped or nested streams, pay attention to any unexpected interaction between the streams. (... "There's something very important I forgot to tell you." ... "Don't cross the streams" ).


The memory management looks fine. If the null check and exception raising is common, it may be worth creating a checked version of the getResourceStream method that raises an exception rather than returns null.

[Vagabond]

I managed to get clm files loading into memory properly again. I added mmeapi.h to CClmFile.h. It had to be added after windows.h to compile though. Really unsure why this suddenly became necessary. I'm interested in removing the Windows header eventually, but for now am just happy it is working again.

Code: [Select]

#include <windows.h>
#include <mmeapi.h>

I went ahead and added a compression enum:

Code: [Select]

/*Represents compression flags used by Outpost 2 .vol & .clm files to compress files. See FileFormat Archive Vol.txt for more details*/
enum class CompressionType
{
	Uncompressed = 0x100,
	RLE = 0x101,  //Run - Length Encoded. Currently not supported.
	LZ = 0x102,  //Lempel - Ziv, named after the two creators. Currently not supported.
	LZH = 0x103, //Lempel - Ziv, with adaptive Huffman encoding.
};

Below is the enum in use in CVolFile.cpp. I removed some comments since the enum is sort of self documenting what is going on now. The function the code is from is probably still longer than it could be.

Code: [Select]

	if (m_Index[index].compressionTag == (int)CompressionType::Uncompressed)
	{
		retVal = WriteFile(outFile, offset, length, &bytesWritten, NULL);
	}
	else
	{
		if (m_Index[index].compressionTag == (int)CompressionType::LZH)
		{
			// Decompress the file
			// Construct the decompressor
			CHuffLZ decomp(length, offset);
			const char *buff = 0;
                        . . .

I was not able to get the MemoryStreamReader to properly read a .map file from an archive yet. The MemoryStreamReader seems able to read the MapHeader properly. Then when it moves on to reading the TileData vector, it breaks down on the first value. So, I'm guessing there is one of 2 things going on:

1. Something is going wrong when calling MemoryStreamReader.Read twice. Between reading in the Header file and then starting the read call for the TileData vector.
2. The MemoryStreamReader is unable to properly read in TileData because it is a bitfield structure.

Below is the notes I took on reading it in. I can try testing reading in some sort of file that doesn't have a bitfield structure in it and separate the read into at least 2 different calls to the MemoryStreamReader. This might narrow it down to being the bitfield that is the problem. Any insight is appreciated.

The TileData bitfield structure:

Code: [Select]

// Bitfield structure (32 bits total) containing metadata for a tile placed on the map.
struct TileData
{
	// Determines movement speed of tile, or if tile is bulldozed, walled, tubed, or has rubble.
	int cellType : 5;

	// The tile's associated index in the TileInfo array. 
	// TileInfo lists general properties associated with a tile.
	unsigned int tileIndex : 11;

	// Index of the unit that occupies the tile.
	int unitIndex : 11;

	// True if lava is present on tile.
	int bLava : 1;

	// True if it is possible for lava to enter the tile.
	int bLavaPossible : 1;

	// Used in controlling Lava and Microbe spread. exact workings UNKNOWN.
	int bExpansion : 1;

	// True if the Blight is present on tile.
	int bMicrobe : 1;

	// True if a wall or a building has been built on the tile.
	int bWallOrBuilding : 1;
};

Notes comparing the bad MemoryStreamReader from archive copy to a good FileStreamReader copy.

Code: [Select]

MapHeader
    int VersionTag = 4113 (checks)
    int bSavedGame = 0 (checks)
    unsigned int lgMapTileWidth = 6 (checks)
    unsigned int mapTileHeight = 64 (checks)
    unsigned int numTileSets = 512 (checks)

tileDataVector
tileData[0] 
    cellType = -15 (should be 2)
	tileIndex = 128 (should be 589)
	unitIndex = 0 (Checks) 
	bLava = 0 (Checks)
	bLavaPossible = 0 (Checks)
	bExpansion = 0 (Checks)
	bMicrobe = 0 (Checks)
	bWallOrBuilding = 0 (Checks)
tileData[1]
    everything is 0 (cellType should be 2 & tileIndex 566, everything else 0)
tileData[2]
    cellType = 6
	Everything else is 0 (cellType should be 2 & tileIndex 573, everything else 0)
tileData[3]
    tileIndex = 2 (cellType should be 2 & tileIndex 602, everything else 0)
	Everything else is 0
tileData[4]
    tileIndex = 16 (cellType should be 4 & tileIndex 1789, everything else 0)
	Everything else is 0
...
ClipRect
    x1 = 4113 (should be 32)
    y1 = 0 (Checks)
    x2 = 6 (should be 95)
    y2 = 64 (should be 62)
tileSetSources
tileSetSource[0]
    stringlength = 4113 (Should be 8)
        Throws error because stringlength must be equal to or less than 8.

[Hooman]

You're getting 4113 over again, which is suggesting the stream isn't advancing after each read. You probably didn't update the internal stream position. That would be why it fails on the second and subsequent calls. It keeps reading the same data at the start of the stream over and over.


For the enum, you can declare its size after it's name.

Code: [Select]

enum class CompressionType : unsigned short
{
  // ...
}

Then in the struct, declare the compressionTag field to be of the enum type. Then you can get rid of the nasty int cast.

It may also be better to use a switch statement than nested if statements.


As for the windows header include, maybe #define WIN32_LEAN_AND_MEAN is being defined before the include, and that's causing it to cut out needed definitions. That could happen if you re-order header includes.

[Vagabond]

Hooman,

Thank you for analyzing the Data! You are spot on. I just fixed the MemoryStreamReader offset to actually offset the position in the stream and it read eden04.map out of the maps.vol without any issue!

I need to spend some time cleaning the code next. It will involve looking into the enum update you gave. I was a little wary of changing more of the volFile code guts to push in the enum, hence the cast, but I'll look at it.

WindowsLeanAndMean is defined elsewhere in the project, but not at the top of CClmFile.h. I may just rip out WindowsLeanAndMean altogether. I don't think we need to reduce the size of the application we are getting by calling WindowsLeanAndMean unless it is significant. Especially if it isn't playing nice with the .clm specific code.

Well, after cleaning the code, we should be ready to release her 1.0.0, unless we want to pursue the custom BMP loader first. Unless there are further vol archive questions, I'll plan to switch back over to the main thread.

Thanks for all the help so far!

[leeor_net]

Code rot is an amazing thing. 

[Hooman]

WindowsLeanAndMean is defined elsewhere in the project, but not at the top of CClmFile.h. I may just rip out WindowsLeanAndMean altogether. I don't think we need to reduce the size of the application we are getting by calling WindowsLeanAndMean unless it is significant. Especially if it isn't playing nice with the .clm specific code.

Keep in mind how #include works. It's handled by the pre-processor. It basically reads the contents on the included file, and pastes it into your source code. Same for any nested includes. What results is one big long file, which is then passed on to the compiler. If you have a #define in there somewhere, possibly from some nested include, it will affect everything that comes after it in the pasted results.

It pays to keep #includes clean and simplified as much as possible. It keeps the code easier to reason about. It also drastically impacts the compile time, as deep long chains of #includes can bring in a lot of code. That's one of the reasons why WIN32_LEAN_AND_MEAN exists. It cuts out a lot of sub-includes. This has a noticeable affect on compile times. Remember short compile times are good for iterative development. It takes a lot of the pain out of a compile cycle.

If you want to know how much this affects what the compiler sees, pass the compiler the flag to pre-process only, and spit out the resulting file that would have been passed to the compiler proper. It can be shocking just how much code a one line hello world program can expand to. Also a good way to see how much WIN32_LEAN_AND_MEAN affects things.

Well, after cleaning the code, we should be ready to release her 1.0.0, unless we want to pursue the custom BMP loader first.

I do want custom BMP loading. :)

[Vagabond]

I understand the basic concept of #include pasting all the code from the include into my source code. What I don't understand is the subtleties of WIN32_LEAN_AND_MEAN. CClmFile.h does not define WIN32_LEAN_AND_MEAN before including windows.h but CVolFile.h does include WIN32_LEAN_AND_MEAN before including windows.h.

Since both files have #pragma once at top (I believe this means if the file has already been included by another include statement, don't re-include it), does this mean it is basically a race between whether CClmFile.h or CVolFile.h is loaded first whether or not windows.h is loaded with WIN32_LEAN_AND_MEAN or not? Or am I loading 2 separate versions of windows.h, one streamlined and one including everything?

I tried defining WIN32_LEAN_AND_MEAN to the top of CClmFile.h. It compiles, but then when running the code, it throws an error during the CClmFile class constructor. at the following line:

Code: [Select]

if (ReadHeader() == false) throw "Invalid header";

Anyways, since it is confusing me and not necessary, I was thinking it better to just cut it out. Maybe not the right answer...

[Hooman]

Since both files have #pragma once at top (I believe this means if the file has already been included by another include statement, don't re-include it), does this mean it is basically a race between whether CClmFile.h or CVolFile.h is loaded first whether or not windows.h is loaded with WIN32_LEAN_AND_MEAN or not? Or am I loading 2 separate versions of windows.h, one streamlined and one including everything?

I think you get the idea. What happens depends on windows.h. Does it use #pragma once? Does it use an include guard? I don't know if you get two copies of it. Though if the #define WIN32_LEAN_AND_MEAN comes before both copies, they will both be abbreviated, unless of course you explicitly undefined WIN32_LEAN_AND_MEAN before the second #include. Not so easy to reason about. I can see why there's a push to become less reliant on a pre-processor to compile C++ code.