Page MenuHomeWildfire Games
Differential D4751 Diff 20721 source/ps/TemplateLoader.cpp

Changeset View

Standalone View

View Options

source/ps/TemplateLoader.cpp

Show First 20 Lines • Show All 78 Lines • ▼ Show 20 Lines if (!parentName.empty() && !LoadTemplateFile(node, parentName, compositing, depth + 1))
return false; return false;
// Load the new file into the template data (overriding parent values). // Load the new file into the template data (overriding parent values).
// TODO: error handling. // TODO: error handling.
CParamNode::LoadXML(node, xero); CParamNode::LoadXML(node, xero);
return true; return true;
} }
static Status AddToTemplates(const VfsPath& pathname, const CFileInfo& UNUSED(fileInfo), const uintptr_t cbData) struct ListTemplatesState
{ {
std::vector<std::string>& templates = *(std::vector<std::string>*)cbData; std::vector<std::string> templates;
std::vector<std::string> templates_to_ignore;
vladislavbelovUnsubmitted
Not Done
Inline Actions

Wrong CC for the name.

vladislavbelov: Wrong CC for the name.
// Strip the .xml extension
VfsPath pathstem = pathname.ChangeExtension(L"");
// Strip the root from the path
std::string name = pathstem.string8().substr(ARRAY_SIZE(TEMPLATE_ROOT)-1);
// We want to ignore template_*.xml templates, since they should never be built in the editor
if (name.substr(0, 9) == "template_")
return INFO::OK;
// Also ignore some subfolders.
if (name.substr(0, 8) == "special/" || name.substr(0, 7) == "mixins/")
return INFO::OK;
templates.push_back(name); ListTemplatesState(const std::vector<std::string> templates_to_ignore)
return INFO::OK; : templates_to_ignore(templates_to_ignore),
} templates()
{}
};
phositUnsubmitted
Not Done
Inline Actions
PS::span<const std::string_view> templates_to_ignore;
- ListTemplatesState(const PS::span<const std::string_view>& templates_to_ignore)
+ ListTemplatesState(const PS::span<const std::string_view> templates_to_ignore)
: templates_to_ignore(templates_to_ignore),
phosit:
static Status AddToTemplatesUnrestricted(const VfsPath& pathname, const CFileInfo& UNUSED(fileInfo), const uintptr_t cbData) static Status AddToTemplates(const VfsPath& pathname, const CFileInfo& UNUSED(fileInfo), const uintptr_t cbData)
{ {
std::vector<std::string>& templates = *(std::vector<std::string>*)cbData; ListTemplatesState* s = (ListTemplatesState*)cbData;
phositUnsubmitted
Done
Inline Actions
static Status AddToTemplates(const VfsPath& pathname, const CFileInfo& UNUSED(fileInfo), const uintptr_t cbData)
{
- ListTemplatesState* s = (ListTemplatesState*)cbData;
+ ListTemplatesState* s = reinterpret_cast<ListTemplatesState*>(cbData);
VfsPath pathstem = pathname.ChangeExtension(L"");

The c-style-cast is too powerfull in most situations.

phosit: The c-style-cast is too powerfull in most situations.
vladislavbelovUnsubmitted
Not Done
Inline Actions

state isn't a long word to use s instead.

vladislavbelov: `state` isn't a long word to use `s` instead.
VfsPath pathstem = pathname.ChangeExtension(L""); VfsPath pathstem = pathname.ChangeExtension(L"");
std::string name = pathstem.string8().substr(ARRAY_SIZE(TEMPLATE_ROOT)-1); std::string name = pathstem.string8().substr(ARRAY_SIZE(TEMPLATE_ROOT)-1);
// We want to ignore template_*.xml templates, since they may be incomplete. for (std::string template_to_ignore : s->templates_to_ignore)
if (name.substr(0, 9) == "template_") if (name.substr(0, template_to_ignore.size()) == template_to_ignore)
return INFO::OK; return INFO::OK;
StanUnsubmitted
Not Done
Inline Actions
std::string name = pathstem.string8().substr(ARRAY_SIZE(TEMPLATE_ROOT)-1);
- for (std::string template_to_ignore : s->templates_to_ignore)
+ for (const std::string& template_to_ignore : s->templates_to_ignore)
if (name.substr(0, template_to_ignore.size()) == template_to_ignore)
return INFO::OK;
s->templates.push_back(name);

Could also be https://en.cppreference.com/w/cpp/algorithm/find per @phosit
Bonus INFO::SKIPPED

Stan: Could also be https://en.cppreference.com/w/cpp/algorithm/find per @phosit Bonus `INFO…
phositUnsubmitted
Not Done
Inline Actions

Thanks for pointing it out. But in this case`std::any_of`

phosit: Thanks for pointing it out. But in this case`std::any_of`
phositUnsubmitted
Not Done
Inline Actions
std::string name = pathstem.string8().substr(ARRAY_SIZE(TEMPLATE_ROOT)-1);
- for (std::string_view template_to_ignore : s->templates_to_ignore)
- if (name.substr(0, template_to_ignore.size()) == template_to_ignore)
- return INFO::SKIPPED;
+ const auto matchesPathname = [&name](const std::string_view template_to_ignore)
+ {
+ return name.substr(0, template_to_ignore.size()) == template_to_ignore;
+ };
+ if (std::any_of(s->templates_to_ignore.begin(), s->templates_to_ignore.end(), matchesPathname))
+ return INFO::SKIPPED;
s->templates.push_back(name);
phosit:
vladislavbelovUnsubmitted
Not Done
Inline Actions

name is std::string so the name.substr call isn't desired here.

vladislavbelov: `name` is `std::string` so the `name.substr` call isn't desired here.
templates.push_back(name); s->templates.push_back(name);
return INFO::OK; return INFO::OK;
} }
static Status AddActorToTemplates(const VfsPath& pathname, const CFileInfo& UNUSED(fileInfo), const uintptr_t cbData) static Status AddActorToTemplates(const VfsPath& pathname, const CFileInfo& UNUSED(fileInfo), const uintptr_t cbData)
{ {
std::vector<std::string>& templates = *(std::vector<std::string>*)cbData; std::vector<std::string>& templates = *(std::vector<std::string>*)cbData;
// Strip the root from the path // Strip the root from the path
std::wstring name = pathname.string().substr(ARRAY_SIZE(ACTOR_ROOT)-1); std::wstring name = pathname.string().substr(ARRAY_SIZE(ACTOR_ROOT)-1);
templates.push_back("actor|" + std::string(name.begin(), name.end())); templates.push_back("actor|" + std::string(name.begin(), name.end()));
return INFO::OK; return INFO::OK;
} }
bool CTemplateLoader::TemplateExists(const std::string& templateName) const bool CTemplateLoader::TemplateExists(const std::string& templateName) const
{ {
size_t pos = templateName.rfind('|'); size_t pos = templateName.rfind('|');
std::string baseName(pos != std::string::npos ? templateName.substr(pos+1) : templateName); std::string baseName(pos != std::string::npos ? templateName.substr(pos+1) : templateName);
return VfsFileExists(VfsPath(TEMPLATE_ROOT) / wstring_from_utf8(baseName + ".xml")); return VfsFileExists(VfsPath(TEMPLATE_ROOT) / wstring_from_utf8(baseName + ".xml"));
} }
std::vector<std::string> CTemplateLoader::FindTemplates(const std::string& path, bool includeSubdirectories, ETemplatesType templatesType) const std::vector<std::string> CTemplateLoader::FindTemplates(const std::string& path, bool includeSubdirectories, ETemplatesType templatesType, const std::vector<std::string> ignoredTemplateStarts) const
phositUnsubmitted
Not Done
Inline Actions

spans are normaly not passed by reference since they are cheap to copy. There are many exceptions around references :)

phosit: `span`s are normaly not passed by reference since they are cheap to copy. There are many…
phositUnsubmitted
Not Done
Inline Actions
return VfsFileExists(VfsPath(TEMPLATE_ROOT) / wstring_from_utf8(baseName + ".xml"));
}
- std::vector<std::string> CTemplateLoader::FindTemplates(const std::string& path, bool includeSubdirectories, ETemplatesType templatesType, const PS::span<const std::string_view>& ignoredTemplateStarts) const
+ std::vector<std::string> CTemplateLoader::FindTemplates(const std::string& path, bool includeSubdirectories, ETemplatesType templatesType, const PS::span<const std::string_view> ignoredTemplateStarts) const
{
ListTemplatesState state(ignoredTemplateStarts);
phosit:
{ {
std::vector<std::string> templates; ListTemplatesState state(ignoredTemplateStarts);
if (templatesType != SIMULATION_TEMPLATES && templatesType != ACTOR_TEMPLATES && templatesType != ALL_TEMPLATES) if (templatesType != SIMULATION_TEMPLATES && templatesType != ACTOR_TEMPLATES && templatesType != ALL_TEMPLATES)
StanUnsubmitted
Done
Inline Actions
ListTemplatesState state(ignoredTemplateStarts);
- if (templatesType != SIMULATION_TEMPLATES && templatesType != ACTOR_TEMPLATES && templatesType != ALL_TEMPLATES)
+ if (templatesType == 0 || (templatesType & ALL_TEMPLATES) != templatesType)
{
LOGERROR("Undefined template type (valid: all, simulation, actor)");

Wonder if that'd be faster. Also, if it was an enum class, you might not need that check at all.
But you'd need to change

enum class ETemplatesType
{
	ACTOR_TEMPLATES = 1,
	SIMULATION_TEMPLATES = 2,
	ALL_TEMPLATES = 1 + 2
};
Stan: Wonder if that'd be faster. Also, if it was an enum class, you might not need that check at all.
{ {
LOGERROR("Undefined template type (valid: all, simulation, actor)"); LOGERROR("Undefined template type (valid: all, simulation, actor)");
return templates; return state.templates;
} }
size_t flags = includeSubdirectories ? vfs::DIR_RECURSIVE : 0; size_t flags = includeSubdirectories ? vfs::DIR_RECURSIVE : 0;
if (templatesType == SIMULATION_TEMPLATES || templatesType == ALL_TEMPLATES) if (templatesType == SIMULATION_TEMPLATES || templatesType == ALL_TEMPLATES)
WARN_IF_ERR(vfs::ForEachFile(g_VFS, VfsPath(TEMPLATE_ROOT) / path, AddToTemplates, (uintptr_t)&templates, L"*.xml", flags)); WARN_IF_ERR(vfs::ForEachFile(g_VFS, VfsPath(TEMPLATE_ROOT) / path, AddToTemplates, (uintptr_t)&state, L"*.xml", flags));
StanUnsubmitted
Not Done
Inline Actions

Wonder why .xml is hardcoded here.

Stan: Wonder why .xml is hardcoded here.
vladislavbelovUnsubmitted
Not Done
Inline Actions

reinterpret_cast<uintptr_t>(&state), the same below.

vladislavbelov: `reinterpret_cast<uintptr_t>(&state)`, the same below.
if (templatesType == ACTOR_TEMPLATES || templatesType == ALL_TEMPLATES) if (templatesType == ACTOR_TEMPLATES || templatesType == ALL_TEMPLATES)
WARN_IF_ERR(vfs::ForEachFile(g_VFS, VfsPath(ACTOR_ROOT) / path, AddActorToTemplates, (uintptr_t)&templates, L"*.xml", flags)); WARN_IF_ERR(vfs::ForEachFile(g_VFS, VfsPath(ACTOR_ROOT) / path, AddActorToTemplates, (uintptr_t)&state, L"*.xml", flags));
return templates;
}
std::vector<std::string> CTemplateLoader::FindTemplatesUnrestricted(const std::string& path, bool includeSubdirectories) const
{
std::vector<std::string> templates;
size_t flags = includeSubdirectories ? vfs::DIR_RECURSIVE : 0;
WARN_IF_ERR(vfs::ForEachFile(g_VFS, VfsPath(TEMPLATE_ROOT) / path, AddToTemplatesUnrestricted, (uintptr_t)&templates, L"*.xml", flags));
return templates; return state.templates;
FreagarachAuthorUnsubmitted
Done
Inline Actions
In D4751#206334, @phosit wrote:

Why do you introduce ListTemplatesState? ListTemplatesState.templates is never used.

Here? But you may mean something else? ^^'

Freagarach: >>! In D4751#206334, @phosit wrote: > Why do you introduce ListTemplatesState?
phositUnsubmitted
Not Done
Inline Actions

Well i mean excactly this XD

phosit: Well i mean excactly this XD
} }
const CParamNode& CTemplateLoader::GetTemplateFileData(const std::string& templateName) const CParamNode& CTemplateLoader::GetTemplateFileData(const std::string& templateName)
{ {
if (std::unordered_map<std::string, CParamNode>::const_iterator it = m_TemplateFileData.find(templateName); it != m_TemplateFileData.end()) if (std::unordered_map<std::string, CParamNode>::const_iterator it = m_TemplateFileData.find(templateName); it != m_TemplateFileData.end())
return it->second; return it->second;
CParamNode ret; CParamNode ret;
Show All 28 Lines
Wildfire Games · Phabricator