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

Jump to bottom

Add getResourceFiles #3603

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Add getResourceFiles #3603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
4f3acf9
Add `getLoadedFiles`
TracerDS Jul 23, 2024
da7617f
Updated CLuaResourceDefs.cpp
TracerDS Jul 23, 2024
adde333
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS Jul 24, 2024
8289dcb
Updated code
TracerDS Jul 28, 2024
59052f4
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS Jul 28, 2024
17e0c76
Fix MacOS/Linux builds
TracerDS Jul 28, 2024
452dde7
Updated code according to reviews
TracerDS Jul 28, 2024
5e3d05a
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS Jul 28, 2024
6f6a8b3
Update CResourceFile.h
TracerDS Jul 31, 2024
6127d68
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS Jul 31, 2024
7b78449
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS Aug 1, 2024
eb831e0
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS Aug 3, 2024
543e6cd
Update CLuaResourceDefs.cpp
TracerDS Aug 4, 2024
df5275e
Update CLuaResourceDefs.cpp
TracerDS Aug 4, 2024
0d5e2e9
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS Aug 15, 2024
859f7f4
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS Aug 20, 2024
e794557
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS Aug 23, 2024
979d2ae
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS Aug 24, 2024
04a49af
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS Oct 17, 2024
a9c3768
Merge branch 'master' into 230724_Add_getLoadedFiles
TracerDS Nov 6, 2024
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
15 changes: 15 additions & 0 deletions Client/mods/deathmatch/logic/CDownloadableResource.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,21 @@
CDownloadableResource::CDownloadableResource(CResource* pResource, eResourceType resourceType, const char* szName, const char* szNameShort, uint uiDownloadSize,
CChecksum serverChecksum, bool bAutoDownload)
{
switch (resourceType)
{
case eResourceType::RESOURCE_FILE_TYPE_SCRIPT:
case eResourceType::RESOURCE_FILE_TYPE_CLIENT_SCRIPT:
m_resourceCategoryType = eResourceCategory::SCRIPTS;
case eResourceType::RESOURCE_FILE_TYPE_MAP:
m_resourceCategoryType = eResourceCategory::MAPS;
case eResourceType::RESOURCE_FILE_TYPE_CONFIG:
case eResourceType::RESOURCE_FILE_TYPE_CLIENT_CONFIG:
m_resourceCategoryType = eResourceCategory::CONFIGS;
case eResourceType::RESOURCE_FILE_TYPE_CLIENT_FILE:
case eResourceType::RESOURCE_FILE_TYPE_HTML:
m_resourceCategoryType = eResourceCategory::FILES;
}

Comment on lines +17 to +31
Copy link
Member

Choose a reason for hiding this comment

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

You can filter it in CLuaResourceDefs::GetResourceFiles. You don't need GetResourceCategoryType

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its better this way

Copy link
Member

Choose a reason for hiding this comment

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

Hmm. You can use original types. I think client-scripts and server-scripts have some sense for developers. So you shouldn't map eResourceType into custom enum for one Lua function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd have to do string manipulation in both functions just to get values.
Its better that way

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean?

Copy link
Member

Choose a reason for hiding this comment

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

What is wrong with eResourceType ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

too much noise

Copy link
Member

@TheNormalnij TheNormalnij Aug 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It looks like using existing enum is much better than creating a new one with mapping from eResourceType and adding boilerplate code in client and server.

too much noise

What do you mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean?

Exactly what I said. Its easier to have 3 categories: scripts, maps, files instead of: client/server scripts, client/server files, (client/server?) maps, etc.

Copy link
Member

Choose a reason for hiding this comment

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

I see no problem here. It's much useful.

m_pResource = pResource;
m_resourceType = resourceType;
m_strName = szName;
Expand Down
11 changes: 11 additions & 0 deletions Client/mods/deathmatch/logic/CDownloadableResource.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ class CDownloadableResource
RESOURCE_FILE_TYPE_CLIENT_FILE,
};

enum class eResourceCategory
{
ALL,
SCRIPTS,
MAPS,
CONFIGS,
FILES,
};

public:
CDownloadableResource(CResource* pResource, eResourceType resourceType, const char* szName, const char* szNameShort, uint uiDownloadSize,
CChecksum serverChecksum, bool bAutoDownload);
Expand All @@ -43,6 +52,7 @@ class CDownloadableResource
bool DoesClientAndServerChecksumMatch();

eResourceType GetResourceType() { return m_resourceType; };
eResourceCategory GetResourceCategoryType() const noexcept { return m_resourceCategoryType; }
const char* GetName() { return m_strName; };
const char* GetShortName() { return m_strNameShort; };
CResource* GetResource() { return m_pResource; }
Expand All @@ -66,6 +76,7 @@ class CDownloadableResource
protected:
CResource* m_pResource;
eResourceType m_resourceType;
eResourceCategory m_resourceCategoryType;

SString m_strName;
SString m_strNameShort;
Expand Down
2 changes: 2 additions & 0 deletions Client/mods/deathmatch/logic/CResource.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ class CResource

const auto& GetExportedFunctions() const noexcept { return m_exportedFunctions; }

std::list<CResourceFile*>& GetResourceFiles() noexcept { return m_ResourceFiles; }
const std::list<CResourceFile*>& GetResourceFiles() const noexcept { return m_ResourceFiles; }
std::list<CResourceFile*>::iterator IterBeginResourceFiles() { return m_ResourceFiles.begin(); }
std::list<CResourceFile*>::iterator IterEndResourceFiles() { return m_ResourceFiles.end(); }

Expand Down
8 changes: 8 additions & 0 deletions Client/mods/deathmatch/logic/lua/CLuaFunctionParseHelpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -896,6 +896,14 @@ ADD_ENUM(WEATHER_SANDSTORM, "Sandstorm")
ADD_ENUM(WEATHER_RAINBOW, "Rainbow")
IMPLEMENT_ENUM_END("world-property")

IMPLEMENT_ENUM_CLASS_BEGIN(CResourceFile::eResourceCategory)
ADD_ENUM(CResourceFile::eResourceCategory::ALL, "all")
ADD_ENUM(CResourceFile::eResourceCategory::CONFIGS, "config")
ADD_ENUM(CResourceFile::eResourceCategory::FILES, "file")
ADD_ENUM(CResourceFile::eResourceCategory::MAPS, "map")
ADD_ENUM(CResourceFile::eResourceCategory::SCRIPTS, "script")
IMPLEMENT_ENUM_CLASS_END("resource-category-type")

IMPLEMENT_ENUM_CLASS_BEGIN(eModelLoadState)
ADD_ENUM(eModelLoadState::LOADSTATE_NOT_LOADED, "unloaded")
ADD_ENUM(eModelLoadState::LOADSTATE_LOADED, "loaded")
Expand Down
2 changes: 2 additions & 0 deletions Client/mods/deathmatch/logic/lua/CLuaFunctionParseHelpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <game/CHud.h>
#include <game/CStreaming.h>
#include <type_traits>
#include "CResourceFile.h"

enum eLuaType
{
Expand Down Expand Up @@ -86,6 +87,7 @@ DECLARE_ENUM_CLASS(eRenderStage);
DECLARE_ENUM_CLASS(eFxParticleSystems);
DECLARE_ENUM(ePools);
DECLARE_ENUM(eWorldProperty);
DECLARE_ENUM_CLASS(CResourceFile::eResourceCategory);
DECLARE_ENUM_CLASS(eModelLoadState);
DECLARE_ENUM_CLASS(PreloadAreaOption);

Expand Down
35 changes: 34 additions & 1 deletion Client/mods/deathmatch/logic/luadefs/CLuaResourceDefs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ using std::list;

void CLuaResourceDefs::LoadFunctions()
{
constexpr static const std::pair<const char*, lua_CFunction> functions[]{
constexpr static const std::pair<const char*, lua_CFunction> functions[]
{
{"call", Call},
{"getThisResource", GetThisResource},
{"getResourceConfig", GetResourceConfig},
Expand All @@ -29,6 +30,7 @@ void CLuaResourceDefs::LoadFunctions()
{"getResourceState", GetResourceState},
{"loadstring", LoadString},
{"load", Load},
{"getResourceFiles", ArgumentParser<GetResourceFiles>},
TracerDS marked this conversation as resolved.
Show resolved Hide resolved
};

// Add functions
Expand All @@ -52,6 +54,7 @@ void CLuaResourceDefs::AddClass(lua_State* luaVM)
lua_classfunction(luaVM, "getDynamicElementRoot", "getResourceDynamicElementRoot");
lua_classfunction(luaVM, "getExportedFunctions", "getResourceExportedFunctions");
lua_classfunction(luaVM, "getState", "getResourceState");
lua_classfunction(luaVM, "getFiles", "getResourceFiles");

lua_classvariable(luaVM, "config", NULL, "getResourceConfig");
lua_classvariable(luaVM, "dynamicElementRoot", NULL, "getResourceDynamicElementRoot");
Expand All @@ -60,6 +63,7 @@ void CLuaResourceDefs::AddClass(lua_State* luaVM)
lua_classvariable(luaVM, "state", NULL, "getResourceState");
lua_classvariable(luaVM, "name", NULL, "getResourceName");
lua_classvariable(luaVM, "rootElement", NULL, "getResourceRootElement");
lua_classvariable(luaVM, "files", nullptr, "getResourceFiles");

lua_registerclass(luaVM, "Resource");
}
Expand Down Expand Up @@ -545,3 +549,32 @@ int CLuaResourceDefs::Load(lua_State* luaVM)
lua_pushboolean(luaVM, false);
return 1;
}

std::vector<std::string> CLuaResourceDefs::GetResourceFiles(lua_State* luaVM, std::optional<CResourceFile::eResourceCategory> type,
std::optional<CResource*> resource) noexcept
{
using eResourceCategory = CResourceFile::eResourceCategory;

if (!type)
type = eResourceCategory::ALL;

if (!resource)
resource = &lua_getownerresource(luaVM);

const auto resourceFiles = (*resource)->GetResourceFiles();

std::unordered_set<std::string> files;
for (const auto& file : resourceFiles)
{
if (file->GetResourceCategoryType() != type.value() && type.value() != eResourceCategory::ALL)
continue;
files.insert(file->GetName());
}

// TODO: Upgrade ArgumentParser so it would accept
// std::set and std::unordered_set as return values
return std::vector<std::string>(
std::make_move_iterator(files.begin()),
std::make_move_iterator(files.end())
);
}
2 changes: 2 additions & 0 deletions Client/mods/deathmatch/logic/luadefs/CLuaResourceDefs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,6 @@ class CLuaResourceDefs : public CLuaDefs
LUA_DECLARE(Load);

static std::string GetResourceName(lua_State* luaVM, std::optional<CResource*> resourceElement);
static std::vector<std::string> GetResourceFiles(lua_State* luaVM, std::optional<CResourceFile::eResourceCategory> type,
std::optional<CResource*> resource) noexcept;
};
4 changes: 2 additions & 2 deletions Server/mods/deathmatch/logic/CResource.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,8 @@ class CResource : public EHS
const std::string& GetResourceDirectoryPath() const { return m_strResourceDirectoryPath; }
const std::string& GetResourceCacheDirectoryPath() const { return m_strResourceCachePath; }

std::list<CResourceFile*>& GetFiles() { return m_ResourceFiles; }
size_t GetFileCount() const noexcept { return m_ResourceFiles.size(); }
std::list<CResourceFile*>& GetResourceFiles() noexcept { return m_ResourceFiles; }
TracerDS marked this conversation as resolved.
Show resolved Hide resolved
const std::list<CResourceFile*>& GetResourceFiles() const noexcept { return m_ResourceFiles; }

time_t GetTimeStarted() const noexcept { return m_timeStarted; }
time_t GetTimeLoaded() const noexcept { return m_timeLoaded; }
Expand Down
2 changes: 2 additions & 0 deletions Server/mods/deathmatch/logic/CResourceClientConfigItem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ CResourceClientConfigItem::CResourceClientConfigItem(CResource* resource, const
m_pXMLRootNode = NULL;
m_bInvalid = false;
m_type = RESOURCE_FILE_TYPE_CLIENT_CONFIG;
m_resourceCategoryType = eResourceCategory::CONFIGS;

}

CResourceClientConfigItem::~CResourceClientConfigItem()
Expand Down
1 change: 1 addition & 0 deletions Server/mods/deathmatch/logic/CResourceClientFileItem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ CResourceClientFileItem::CResourceClientFileItem(CResource* resource, const char
: CResourceFile(resource, szShortName, szResourceFileName, xmlAttributes)
{
m_type = RESOURCE_FILE_TYPE_CLIENT_FILE;
m_resourceCategoryType = eResourceCategory::FILES;
m_bClientAutoDownload = bClientAutoDownload;
}

Expand Down
1 change: 1 addition & 0 deletions Server/mods/deathmatch/logic/CResourceClientScriptItem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ CResourceClientScriptItem::CResourceClientScriptItem(CResource* resource, const
: CResourceFile(resource, szShortName, szResourceFileName, xmlAttributes)
{
m_type = RESOURCE_FILE_TYPE_CLIENT_SCRIPT;
m_resourceCategoryType = eResourceCategory::SCRIPTS;

// Check if this file should be cached by the client
if (MapGet(m_attributeMap, "protected") == "true" || MapGet(m_attributeMap, "cache") == "false")
Expand Down
1 change: 1 addition & 0 deletions Server/mods/deathmatch/logic/CResourceConfigItem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ CResourceConfigItem::CResourceConfigItem(CResource* resource, const char* szShor
m_bInvalid = true;

m_type = RESOURCE_FILE_TYPE_CONFIG;
m_resourceCategoryType = eResourceCategory::CONFIGS;
}

CResourceConfigItem::~CResourceConfigItem()
Expand Down
11 changes: 11 additions & 0 deletions Server/mods/deathmatch/logic/CResourceFile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,22 @@ class CResourceFile
RESOURCE_FILE_TYPE_NONE,
}; // TODO: sort all client-side enums and use >= (instead of each individual type) on comparisons that use this enum?

enum class eResourceCategory
{
ALL,
SCRIPTS,
MAPS,
CONFIGS,
FILES,
};

protected:
class CResource* m_resource;
std::string m_strResourceFileName; // full path
std::string m_strShortName; // just the filename
std::string m_strWindowsName; // the name with backwards slashes
eResourceType m_type;
eResourceCategory m_resourceCategoryType;
class CLuaMain* m_pVM;
CChecksum m_checksum; // Checksum last time this was loaded, generated by GenerateChecksum()
uint m_uiFileSize;
Expand All @@ -59,6 +69,7 @@ class CResourceFile
virtual bool IsNoClientCache() const { return false; }

eResourceType GetType() { return m_type; }
eResourceCategory GetResourceCategoryType() const noexcept { return m_resourceCategoryType; }
const char* GetName() { return m_strShortName.c_str(); }
const char* GetFullName() { return m_strResourceFileName.c_str(); }
const char* GetWindowsName() { return m_strWindowsName.c_str(); }
Expand Down
1 change: 1 addition & 0 deletions Server/mods/deathmatch/logic/CResourceHTMLItem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ CResourceHTMLItem::CResourceHTMLItem(CResource* resource, const char* szShortNam
{
m_bIsRaw = bIsRaw;
m_type = RESOURCE_FILE_TYPE_HTML;
m_resourceCategoryType = eResourceCategory::FILES;
m_bDefault = bIsDefault;
m_pVM = NULL;
m_bIsBeingRequested = false;
Expand Down
2 changes: 1 addition & 1 deletion Server/mods/deathmatch/logic/CResourceManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ void CResourceManager::ListResourcesLoaded(const SString& strListType)
else
{
if (strListType == "stopped" || strListType == "all")
CLogger::LogPrintf("%-20.20s STOPPED (%d files)\n", res->GetName().c_str(), res->GetFileCount());
CLogger::LogPrintf("%-20.20s STOPPED (%d files)\n", res->GetName().c_str(), res->GetResourceFiles().size());
}
uiCount++;
}
Expand Down
1 change: 1 addition & 0 deletions Server/mods/deathmatch/logic/CResourceMapItem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ CResourceMapItem::CResourceMapItem(CResource* pResource, const char* szShortName
m_pElementGroup = nullptr;
m_iDimension = iDimension;
m_type = RESOURCE_FILE_TYPE_MAP;
m_resourceCategoryType = eResourceCategory::MAPS;
m_pMapElement = nullptr;
}

Expand Down
1 change: 1 addition & 0 deletions Server/mods/deathmatch/logic/CResourceScriptItem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ CResourceScriptItem::CResourceScriptItem(CResource* resource, const char* szShor
: CResourceFile(resource, szShortName, szResourceFileName, xmlAttributes)
{
m_type = RESOURCE_FILE_TYPE_SCRIPT;
m_resourceCategoryType = eResourceCategory::SCRIPTS;
}

CResourceScriptItem::~CResourceScriptItem()
Expand Down
8 changes: 8 additions & 0 deletions Server/mods/deathmatch/logic/lua/CLuaFunctionParseHelpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,14 @@ ADD_ENUM(ESyncType::LOCAL, "local")
ADD_ENUM(ESyncType::SUBSCRIBE, "subscribe")
IMPLEMENT_ENUM_CLASS_END("sync-mode")

IMPLEMENT_ENUM_CLASS_BEGIN(CResourceFile::eResourceCategory)
ADD_ENUM(CResourceFile::eResourceCategory::ALL, "all")
ADD_ENUM(CResourceFile::eResourceCategory::CONFIGS, "config")
ADD_ENUM(CResourceFile::eResourceCategory::FILES, "file")
ADD_ENUM(CResourceFile::eResourceCategory::MAPS, "map")
ADD_ENUM(CResourceFile::eResourceCategory::SCRIPTS, "script")
IMPLEMENT_ENUM_CLASS_END("resource-category-type")

//
// CResource from userdata
//
Expand Down
4 changes: 3 additions & 1 deletion Server/mods/deathmatch/logic/lua/CLuaFunctionParseHelpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@

#pragma once

#include <type_traits>
#include "LuaCommon.h"
#include "CElementIDs.h"
#include "CConsoleClient.h"
#include "CAccount.h"
#include "CEasingCurve.h"
#include "CAccessControlListRight.h"
#include <type_traits>
#include "CResourceFile.h"

class CLuaVector2D;
class CLuaVector3D;
Expand All @@ -38,6 +39,7 @@ DECLARE_ENUM(CAccessControlListRight::ERightType);
DECLARE_ENUM(CElement::EElementType);
DECLARE_ENUM(CAccountPassword::EAccountPasswordType);
DECLARE_ENUM_CLASS(ESyncType);
DECLARE_ENUM_CLASS(CResourceFile::eResourceCategory);

enum eHudComponent
{
Expand Down
35 changes: 34 additions & 1 deletion Server/mods/deathmatch/logic/luadefs/CLuaResourceDefs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ void CLuaResourceDefs::LoadFunctions()
{"getResourceACLRequests", getResourceACLRequests},
{"loadstring", LoadString},
{"load", Load},

{"getResourceFiles", ArgumentParser<GetResourceFiles>},
};

// Add functions
Expand Down Expand Up @@ -129,6 +131,7 @@ void CLuaResourceDefs::AddClass(lua_State* luaVM)
lua_classfunction(luaVM, "getACLRequests", "getResourceACLRequests");
lua_classfunction(luaVM, "isArchived", "isResourceArchived");
lua_classfunction(luaVM, "isProtected", "isResourceProtected");
lua_classfunction(luaVM, "getFiles", "getResourceFiles");

lua_classvariable(luaVM, "dynamicElementRoot", NULL, "getResourceDynamicElementRoot");
lua_classvariable(luaVM, "exportedFunctions", NULL, "getResourceExportedFunctions");
Expand All @@ -142,6 +145,7 @@ void CLuaResourceDefs::AddClass(lua_State* luaVM)
lua_classvariable(luaVM, "archived", NULL, "isResourceArchived");
lua_classvariable(luaVM, "protected", nullptr, "isResourceProtected");
lua_classvariable(luaVM, "loadFailureReason", NULL, "getResourceLoadFailureReason");
lua_classvariable(luaVM, "files", nullptr, "getResourceFiles");

lua_registerclass(luaVM, "Resource");
}
Expand Down Expand Up @@ -804,7 +808,7 @@ int CLuaResourceDefs::getResourceConfig(lua_State* luaVM)
CheckCanModifyOtherResource(argStream, pThisResource, pResource);
if (!argStream.HasErrors())
{
for (CResourceFile* pResourceFile : pResource->GetFiles())
for (CResourceFile* pResourceFile : pResource->GetResourceFiles())
{
if (pResourceFile->GetType() != CResourceFile::RESOURCE_FILE_TYPE_CONFIG)
continue;
Expand Down Expand Up @@ -1468,3 +1472,32 @@ bool CLuaResourceDefs::isResourceProtected(CResource* const resource)
{
return resource->IsProtected();
}

std::vector<std::string> CLuaResourceDefs::GetResourceFiles(lua_State* luaVM, std::optional<CResourceFile::eResourceCategory> type,
std::optional<CResource*> resource) noexcept
{
using eResourceCategory = CResourceFile::eResourceCategory;

if (!type)
type = eResourceCategory::ALL;

if (!resource)
resource = &lua_getownerresource(luaVM);

const auto resourceFiles = (*resource)->GetResourceFiles();

std::unordered_set<std::string> files;
for (const auto& file : resourceFiles)
{
if (file->GetResourceCategoryType() != type.value() && type.value() != eResourceCategory::ALL)
continue;
files.insert(file->GetName());
Comment on lines +1492 to +1494
Copy link
Member

Choose a reason for hiding this comment

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

Previous varian was more readable. Why did you change it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

previous variant was basically the same.
This leaves room for more conditions to come.

Copy link
Member

Choose a reason for hiding this comment

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

There is no 'early continue' rule. You obfuscate the code with overusing 'continue' in loops. And you don't really need to use early return for small code blocks.
You did the code less readable and maintainable.

if (file->GetResourceCategoryType() == type.value() || type.value() == eResourceCategory::ALL)
    files.insert(file->GetName());

Is fine condition, that describes the function logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is early-return which is basically the same but whatever.
I'd rather go with continue rather than indentation

Copy link
Member

Choose a reason for hiding this comment

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

Your continue creates indentation too. Please, keep it simple

There is early-return which is basically the same but whatever.

Not really. Multiple 'continue' conditions can make a loop harder for reading.
There is no goal to use minimal indentation. The code should primarily describe the logic.
You're overusing 'early return'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again?

No one was interested in this convo for 2 weeks, give it a break.
No contributors were opposed against this so Im leaving it as it is.

Copy link
Member

Choose a reason for hiding this comment

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

I also see the early return overuse here.

Copy link
Member

Choose a reason for hiding this comment

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

No contributors were opposed against this so Im leaving it as it is.

And nobody confirm your position. Don't hide requested changes. Stale conversations should be dismissed by another members.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And nobody confirm your position

Im following the guidelines is'all.

Copy link
Member

Choose a reason for hiding this comment

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

you interpreter these guidelines very differently

}

// TODO: Upgrade ArgumentParser so it would accept
// std::set and std::unordered_set as return values
return std::vector<std::string>(
std::make_move_iterator(files.begin()),
std::make_move_iterator(files.end())
);
}
3 changes: 3 additions & 0 deletions Server/mods/deathmatch/logic/luadefs/CLuaResourceDefs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,7 @@ class CLuaResourceDefs : public CLuaDefs

LUA_DECLARE(LoadString);
LUA_DECLARE(Load);

static std::vector<std::string> GetResourceFiles(lua_State* luaVM, std::optional<CResourceFile::eResourceCategory> type,
std::optional<CResource*> resource) noexcept;
};
Loading