Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Iwd project type #66

Closed
wants to merge 4 commits into from
Closed

Iwd project type #66

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c4a527e
Add IWD project type.
JezuzLizard Dec 23, 2023
7fe01b2
Format using clang.
JezuzLizard Dec 23, 2023
6d116d9
Address review comments.
JezuzLizard Dec 23, 2023
b8397b5
Address review comments.
JezuzLizard Dec 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 33 additions & 6 deletions src/Linker/Linker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "LinkerSearchPaths.h"
#include "ObjContainer/IPak/IPakWriter.h"
#include "ObjContainer/IWD/IWD.h"
#include "ObjContainer/IWD/IWDWriter.h"
#include "ObjLoading.h"
#include "ObjWriting.h"
#include "SearchPath/SearchPaths.h"
Expand Down Expand Up @@ -45,15 +46,11 @@ enum class ProjectType
NONE,
FASTFILE,
IPAK,

IWD,
MAX
};

constexpr const char* PROJECT_TYPE_NAMES[static_cast<unsigned>(ProjectType::MAX)]{
"none",
"fastfile",
"ipak",
};
constexpr const char* PROJECT_TYPE_NAMES[static_cast<std::underlying_type_t<ProjectType>>(ProjectType::MAX)]{"none", "fastfile", "ipak", "iwd"};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately clang-format does not behave particularly cool when it comes to array initializer lists.

You can force it to make way less awkward formatting by adding a trailing comma , after the last entry which is what i do everywhere where i catch it until they introduce a better way to handle this 😅


class LinkerImpl final : public Linker
{
Expand Down Expand Up @@ -462,6 +459,32 @@ class LinkerImpl final : public Linker
return true;
}

bool BuildIWD(const std::string& projectName, const ZoneDefinition& zoneDefinition, SearchPaths& assetSearchPaths) const
{
const fs::path iwdFolderPath(m_args.GetOutputFolderPathForProject(projectName));
auto iwdFilePath(iwdFolderPath);
const auto iwdFileName = zoneDefinition.m_name + ".iwd";
iwdFilePath.append(iwdFileName);

fs::create_directories(iwdFolderPath);

const auto iwdWriter = IWDWriter::Create(iwdFilePath.string(), &assetSearchPaths);
for (const auto& assetEntry : zoneDefinition.m_assets)
{
iwdWriter->AddFile(assetEntry.m_asset_name);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that mean it does try to write every asset into the IWD with the asset name interpreted as the full path?

That would mean that you cannot just simply use the same zone files as for the fastfile, right?
Because in a zone file for a fastfile you would write:

image,test_image

and it would refer to images/test_image.iwi.

In the context of the IWD here it would probably throw a "File not found"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm that is true. Would that mean IWDs should have their own type of definition that is only paths to files? Because you can plausibly load any kind of asset from raw in games such as CoD4/T4. I suppose it could be handled by making it so each asset type that doesn't have the full path in its name has a folder: for example image asset to images folder this way you wouldn't need to specify the images folder in the file name.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean while it's true that the older games support a lot of assets from IWDs, they should be consistent in what assets they put in which files, right? For example MW2 does only use IWDs for iwi images and streamed sounds.

I think some older titles also have weapon files in IWDs.

The way i'd do it is just have as much as possible in fastfiles and not in IWDs. At least for the first iteration.

Or what do you think?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I should add: The idea i had in mind on how to handle these other project types in the future was not to write different zone files necessarly.

I added a thing called "assetlists" for IPaks previously for example.
In the future i thought it might be neat to be able to refer to an assetlist of the written fastfile to just "put everything from the fastfile to an IWD or IPak".

Like Linker would link a fastfile mod.ff and create an assetlists/mod.csv file which can then be refered to from the IWD zone file with assetlist,mod.

That would require the way you define a zone and the way you define an IWD to match though 😅

}

if (!iwdWriter->Write())
{
std::cerr << "Writing iwd failed." << std::endl;
return false;
}

std::cout << "Created iwd \"" << iwdFilePath.string() << "\"\n";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for mixing std::endl and \n? 😅

I know i did it but i try to stick to just \n atm since std::endl also flushes and that can cost a bit of performance.


return true;
}

bool BuildReferencedTargets(const std::string& projectName, const std::string& targetName, const ZoneDefinition& zoneDefinition)
{
return std::all_of(zoneDefinition.m_targets_to_build.begin(),
Expand Down Expand Up @@ -512,6 +535,10 @@ class LinkerImpl final : public Linker
result = BuildIPak(projectName, *zoneDefinition, assetSearchPaths);
break;

case ProjectType::IWD:
result = BuildIWD(projectName, *zoneDefinition, assetSearchPaths);
break;

default:
assert(false);
result = false;
Expand Down
121 changes: 121 additions & 0 deletions src/ObjWriting/ObjContainer/IWD/IWDWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
#include "IWDWriter.h"

#include "../minizip/zip.h"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't use relative paths in includes 😅 If minizip is currently not in the include paths for "ObjWriting" you may need to add it in the premake script "ObjWriting.lua"


#include <algorithm>
#include <iostream>

class IWDWriterImpl final : public IWDWriter
{
public:
explicit IWDWriterImpl(const std::string& iwdFileName, ISearchPath* assetSearchPath)
: m_filename(iwdFileName),
m_asset_search_path(assetSearchPath)
{
m_files.clear();
m_added_files.clear();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearing vectors in constructors is not necessary because the vector will be initialize by the automatically called default constructor of std::vector

m_zipfile = zipOpen(m_filename.c_str(), APPEND_STATUS_CREATE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure you want to append? What if it already exists? Files duplicate?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as i understood it, this does make exactly what you are expecting: It creates the file instead of appending.

The APPEND_STATUS_ is just the prefix and the CREATE is the actual chosen value.

Compared to the source:

#define APPEND_STATUS_CREATE        (0)
#define APPEND_STATUS_CREATEAFTER   (1)
#define APPEND_STATUS_ADDINZIP      (2)

if (!m_zipfile)
{
throw std::runtime_error("Error creating IWD file: \"" + m_filename + "\"");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to log this on std::cerr and have a graceful termination with non-zero exit code.
If you just throw an std::runtime_error this will unfortunately not be logged and will just crash.

And not being able to create a file may happen for example when having an invalid character in the path name.

}
}

void AddFile(std::string fileName) override
{
const auto existingEntry = std::find(m_added_files.cbegin(), m_added_files.cend(), fileName);
if (existingEntry != m_added_files.cend())
{
std::cerr << "Duplicate entry in " << m_filename << "'s zone definition; ignoring" << "\n";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can just make it ...ignoring\n"; you don't need the extra output command 😛

return;
}
m_files.emplace_back(fileName);
}

void AddFileToArchive(const std::string& fileName, const std::unique_ptr<char[]>& fileData, const size_t& fileSize)
{
zip_fileinfo zi = {};

if (zipOpenNewFileInZip(m_zipfile, fileName.c_str(), &zi, NULL, 0, NULL, 0, NULL, Z_DEFLATED, Z_DEFAULT_COMPRESSION) != ZIP_OK)
{
zipClose(m_zipfile, NULL);
throw std::runtime_error("Could not open new file in IWD \"" + fileName + "\"\n");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't use std::runtime_error 😅 This does not log anything and will just crash and not graceful exit

}

if (zipWriteInFileInZip(m_zipfile, fileData.get(), fileSize) != ZIP_OK)
{
zipClose(m_zipfile, NULL);
throw std::runtime_error("Could not write file data to IWD \"" + fileName + "\"\n");
}

if (zipCloseFileInZip(m_zipfile) != ZIP_OK)
{
zipClose(m_zipfile, NULL);
throw std::runtime_error("Could not close file in IWD \"" + fileName + "\"\n");
}
}

std::unique_ptr<char[]> ReadFileDataFromSearchPath(const std::string& fileName, size_t& fileSize) const
{
const auto openFile = m_asset_search_path->Open(fileName);
if (!openFile.IsOpen())
{
std::cerr << "Could not open file for writing to IWD \"" << fileName << "\"\n";
return nullptr;
}

fileSize = static_cast<size_t>(openFile.m_length);
auto fileData = std::make_unique<char[]>(fileSize);
openFile.m_stream->read(fileData.get(), fileSize);

return fileData;
}

bool WriteFileData(const std::string& fileName)
{
size_t fileSize;
const auto fileData = ReadFileDataFromSearchPath(fileName, fileSize);
if (!fileData)
return false;

AddFileToArchive(fileName, fileData, fileSize);

return true;
}

bool Write() override
{
auto result = false;
try
{
result = std::all_of(m_files.cbegin(),
m_files.cend(),
[this](const std::string& fileName)
{
return WriteFileData(fileName);
});
}
catch (const std::exception& e)
{
std::cerr << "Error while creating IWD " << m_filename << " : " << e.what();
result = false;
}

zipClose(m_zipfile, NULL);
return result;
}

private:
const std::string m_filename;

ISearchPath* m_asset_search_path;
std::vector<std::string> m_files;
std::vector<std::string> m_added_files;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use an std::unsorted_set for this, this will not have to iterate through every file and do a basic string comparison for each previously added file on adding a file.

Sets make use of hashing and are therefore magnitudes faster for this.


zipFile m_zipfile;
};

std::unique_ptr<IWDWriter> IWDWriter::Create(const std::string& iwdFileName, ISearchPath* assetSearchPath)
{
return std::make_unique<IWDWriterImpl>(iwdFileName, assetSearchPath);
}
24 changes: 24 additions & 0 deletions src/ObjWriting/ObjContainer/IWD/IWDWriter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#pragma once
#include "SearchPath/ISearchPath.h"

#include <memory>
#include <ostream>

class IWDWriter
{
public:
static constexpr auto USE_COMPRESSION = true;

IWDWriter() = default;
virtual ~IWDWriter() = default;

IWDWriter(const IWDWriter& other) = default;
IWDWriter(IWDWriter&& other) noexcept = default;
IWDWriter& operator=(const IWDWriter& other) = default;
IWDWriter& operator=(IWDWriter&& other) noexcept = default;

virtual void AddFile(std::string fileName) = 0;
virtual bool Write() = 0;

static std::unique_ptr<IWDWriter> Create(const std::string& iwdFileName, ISearchPath* assetSearchPath);
};
Loading