Changeset View
Standalone View
source/graphics/MapGenerator.cpp
Show All 24 Lines | |||||
#include "lib/external_libraries/libsdl.h" | #include "lib/external_libraries/libsdl.h" | ||||
#include "lib/status.h" | #include "lib/status.h" | ||||
#include "lib/timer.h" | #include "lib/timer.h" | ||||
#include "lib/file/vfs/vfs_path.h" | #include "lib/file/vfs/vfs_path.h" | ||||
#include "maths/MathUtil.h" | #include "maths/MathUtil.h" | ||||
#include "ps/CLogger.h" | #include "ps/CLogger.h" | ||||
#include "ps/FileIo.h" | #include "ps/FileIo.h" | ||||
#include "ps/Profile.h" | #include "ps/Profile.h" | ||||
#include "ps/TaskManager.h" | |||||
#include "ps/scripting/JSInterface_VFS.h" | #include "ps/scripting/JSInterface_VFS.h" | ||||
#include "ps/TemplateLoader.h" | |||||
#include "scriptinterface/FunctionWrapper.h" | #include "scriptinterface/FunctionWrapper.h" | ||||
#include "scriptinterface/ScriptContext.h" | #include "scriptinterface/ScriptContext.h" | ||||
#include "scriptinterface/ScriptConversions.h" | #include "scriptinterface/ScriptConversions.h" | ||||
#include "scriptinterface/ScriptInterface.h" | #include "scriptinterface/ScriptInterface.h" | ||||
#include "scriptinterface/JSON.h" | #include "scriptinterface/JSON.h" | ||||
#include "simulation2/helpers/MapEdgeTiles.h" | #include "simulation2/helpers/MapEdgeTiles.h" | ||||
#include <boost/random/linear_congruential.hpp> | |||||
#include <set> | |||||
#include <string> | #include <string> | ||||
#include <vector> | #include <vector> | ||||
// TODO: Maybe this should be optimized depending on the map size. | // TODO: Maybe this should be optimized depending on the map size. | ||||
constexpr int RMS_CONTEXT_SIZE = 96 * 1024 * 1024; | constexpr int RMS_CONTEXT_SIZE = 96 * 1024 * 1024; | ||||
extern bool IsQuitRequested(); | extern bool IsQuitRequested(); | ||||
static bool | namespace | ||||
MapGeneratorInterruptCallback(JSContext* UNUSED(cx)) | { | ||||
bool MapGeneratorInterruptCallback(JSContext* UNUSED(cx)) | |||||
{ | { | ||||
// This may not use SDL_IsQuitRequested(), because it runs in a thread separate to SDL, see SDL_PumpEvents | // This may not use SDL_IsQuitRequested(), because it runs in a thread separate to SDL, see SDL_PumpEvents | ||||
if (IsQuitRequested()) | if (IsQuitRequested()) | ||||
{ | { | ||||
LOGWARNING("Quit requested!"); | LOGWARNING("Quit requested!"); | ||||
return false; | return false; | ||||
} | } | ||||
return true; | return true; | ||||
} | } | ||||
CMapGeneratorWorker::CMapGeneratorWorker(ScriptInterface* scriptInterface) : | /** | ||||
m_ScriptInterface(scriptInterface) | * Provides callback's for the JavaScript. | ||||
{} | */ | ||||
struct CMapGeneratorCallbackData | |||||
vladislavbelov: It doesn't seem like a `struct`. | |||||
CMapGeneratorWorker::~CMapGeneratorWorker() | |||||
{ | { | ||||
// Cancel or wait for the task to end. | CMapGeneratorCallbackData(ScriptInterface& scriptInterface, std::atomic<int>& progress, | ||||
m_WorkerThread.CancelOrWait(); | const boost::rand48::result_type seed) : | ||||
} | m_ScriptInterface{scriptInterface}, | ||||
m_MapGenRNG{seed}, | |||||
void CMapGeneratorWorker::Initialize(const VfsPath& scriptFile, const std::string& settings) | m_Progress{progress} | ||||
{ | { | ||||
std::lock_guard<std::mutex> lock(m_WorkerMutex); | m_ScriptInterface.SetCallbackData(static_cast<void*>(this)); | ||||
// Set progress to positive value | |||||
m_Progress.store(1); | |||||
m_ScriptPath = scriptFile; | |||||
m_Settings = settings; | |||||
// Start generating the map asynchronously. | |||||
m_WorkerThread = Threading::TaskManager::Instance().PushTask([this]() { | |||||
PROFILE2("Map Generation"); | |||||
std::shared_ptr<ScriptContext> mapgenContext = ScriptContext::CreateContext(RMS_CONTEXT_SIZE); | |||||
// Enable the script to be aborted | // Enable the script to be aborted | ||||
JS_AddInterruptCallback(mapgenContext->GetGeneralJSContext(), MapGeneratorInterruptCallback); | JS_AddInterruptCallback(m_ScriptInterface.GetGeneralJSContext(), &MapGeneratorInterruptCallback); | ||||
m_ScriptInterface = new ScriptInterface("Engine", "MapGenerator", mapgenContext); | |||||
// Run map generation scripts | |||||
if (!Run() || m_Progress.load() > 0) | |||||
{ | |||||
// Don't leave progress in an unknown state, if generator failed, set it to -1 | |||||
m_Progress.store(-1); | |||||
} | |||||
SAFE_DELETE(m_ScriptInterface); | |||||
// At this point the random map scripts are done running, so the thread has no further purpose | |||||
// and can die. The data will be stored in m_MapData already if successful, or m_Progress | |||||
// will contain an error value on failure. | |||||
}); | |||||
} | |||||
bool CMapGeneratorWorker::Run() | |||||
{ | |||||
ScriptRequest rq(m_ScriptInterface); | |||||
// Parse settings | |||||
JS::RootedValue settingsVal(rq.cx); | |||||
if (!Script::ParseJSON(rq, m_Settings, &settingsVal) && settingsVal.isUndefined()) | |||||
{ | |||||
LOGERROR("CMapGeneratorWorker::Run: Failed to parse settings"); | |||||
return false; | |||||
} | |||||
// Prevent unintentional modifications to the settings object by random map scripts | |||||
if (!Script::FreezeObject(rq, settingsVal, true)) | |||||
{ | |||||
LOGERROR("CMapGeneratorWorker::Run: Failed to deepfreeze settings"); | |||||
return false; | |||||
} | |||||
// Init RNG seed | |||||
u32 seed = 0; | |||||
if (!Script::HasProperty(rq, settingsVal, "Seed") || | |||||
!Script::GetProperty(rq, settingsVal, "Seed", seed)) | |||||
LOGWARNING("CMapGeneratorWorker::Run: No seed value specified - using 0"); | |||||
InitScriptInterface(seed); | |||||
RegisterScriptFunctions_MapGenerator(); | |||||
// Copy settings to global variable | |||||
JS::RootedValue global(rq.cx, rq.globalValue()); | |||||
if (!Script::SetProperty(rq, global, "g_MapSettings", settingsVal, true, true)) | |||||
{ | |||||
LOGERROR("CMapGeneratorWorker::Run: Failed to define g_MapSettings"); | |||||
return false; | |||||
} | |||||
// Load RMS | |||||
LOGMESSAGE("Loading RMS '%s'", m_ScriptPath.string8()); | |||||
if (!m_ScriptInterface->LoadGlobalScriptFile(m_ScriptPath)) | |||||
{ | |||||
LOGERROR("CMapGeneratorWorker::Run: Failed to load RMS '%s'", m_ScriptPath.string8()); | |||||
return false; | |||||
} | |||||
return true; | |||||
} | |||||
#define REGISTER_MAPGEN_FUNC(func) \ | |||||
ScriptFunction::Register<&CMapGeneratorWorker::func, ScriptInterface::ObjectFromCBData<CMapGeneratorWorker>>(rq, #func); | |||||
#define REGISTER_MAPGEN_FUNC_NAME(func, name) \ | |||||
ScriptFunction::Register<&CMapGeneratorWorker::func, ScriptInterface::ObjectFromCBData<CMapGeneratorWorker>>(rq, name); | |||||
void CMapGeneratorWorker::InitScriptInterface(const u32 seed) | |||||
{ | |||||
m_ScriptInterface->SetCallbackData(static_cast<void*>(this)); | |||||
m_ScriptInterface->ReplaceNondeterministicRNG(m_MapGenRNG); | |||||
m_MapGenRNG.seed(seed); | |||||
// VFS | |||||
JSI_VFS::RegisterScriptFunctions_ReadOnlySimulationMaps(*m_ScriptInterface); | |||||
// Globalscripts may use VFS script functions | |||||
m_ScriptInterface->LoadGlobalScripts(); | |||||
// File loading | |||||
ScriptRequest rq(m_ScriptInterface); | |||||
REGISTER_MAPGEN_FUNC_NAME(LoadScripts, "LoadLibrary"); | |||||
REGISTER_MAPGEN_FUNC_NAME(LoadHeightmap, "LoadHeightmapImage"); | |||||
REGISTER_MAPGEN_FUNC(LoadMapTerrain); | |||||
// Engine constants | |||||
// Length of one tile of the terrain grid in metres. | |||||
// Useful to transform footprint sizes to the tilegrid coordinate system. | |||||
m_ScriptInterface->SetGlobal("TERRAIN_TILE_SIZE", static_cast<int>(TERRAIN_TILE_SIZE)); | |||||
// Number of impassable tiles at the map border | |||||
m_ScriptInterface->SetGlobal("MAP_BORDER_WIDTH", static_cast<int>(MAP_EDGE_TILES)); | |||||
} | |||||
void CMapGeneratorWorker::RegisterScriptFunctions_MapGenerator() | |||||
{ | |||||
ScriptRequest rq(m_ScriptInterface); | |||||
// Template functions | |||||
REGISTER_MAPGEN_FUNC(GetTemplate); | |||||
REGISTER_MAPGEN_FUNC(TemplateExists); | |||||
REGISTER_MAPGEN_FUNC(FindTemplates); | |||||
REGISTER_MAPGEN_FUNC(FindActorTemplates); | |||||
// Progression and profiling | |||||
REGISTER_MAPGEN_FUNC(SetProgress); | |||||
REGISTER_MAPGEN_FUNC(GetMicroseconds); | |||||
REGISTER_MAPGEN_FUNC(ExportMap); | |||||
} | |||||
#undef REGISTER_MAPGEN_FUNC | |||||
#undef REGISTER_MAPGEN_FUNC_NAME | |||||
int CMapGeneratorWorker::GetProgress() const | |||||
{ | |||||
return m_Progress.load(); | |||||
} | |||||
double CMapGeneratorWorker::GetMicroseconds() | |||||
{ | |||||
return JS_Now(); | |||||
} | |||||
Script::StructuredClone CMapGeneratorWorker::GetResults() | |||||
{ | |||||
std::lock_guard<std::mutex> lock(m_WorkerMutex); | |||||
return m_MapData; | |||||
} | |||||
void CMapGeneratorWorker::ExportMap(JS::HandleValue data) | |||||
{ | |||||
{ | |||||
// Copy results | |||||
std::lock_guard<std::mutex> lock(m_WorkerMutex); | |||||
m_MapData = Script::WriteStructuredClone(ScriptRequest(m_ScriptInterface), data); | |||||
} | |||||
m_Progress.store(0); | |||||
} | |||||
void CMapGeneratorWorker::SetProgress(int progress) | |||||
{ | |||||
// When the task is started, `m_Progress` is only mutated by this thread. | |||||
const int currentProgress = m_Progress.load(); | |||||
if (progress >= currentProgress) | |||||
m_Progress.store(progress); | |||||
else | |||||
LOGWARNING("The random map script tried to reduce the loading progress from %d to %d", | |||||
currentProgress, progress); | |||||
} | |||||
CParamNode CMapGeneratorWorker::GetTemplate(const std::string& templateName) | |||||
{ | |||||
const CParamNode& templateRoot = m_TemplateLoader.GetTemplateFileData(templateName).GetOnlyChild(); | |||||
if (!templateRoot.IsOk()) | |||||
LOGERROR("Invalid template found for '%s'", templateName.c_str()); | |||||
return templateRoot; | |||||
} | |||||
bool CMapGeneratorWorker::TemplateExists(const std::string& templateName) | |||||
{ | |||||
return m_TemplateLoader.TemplateExists(templateName); | |||||
} | } | ||||
Done Inline ActionsI was thinking and I'm not sure about the name. CallbackData sounds like we store only data for callbacks but we also provides functions and other data. Maybe CMapGeneratorCallbackContext? vladislavbelov: I was thinking and I'm not sure about the name. `CallbackData` sounds like we store only data… | |||||
Done Inline ActionsContext sounds js related. How about only CMapGeneratorCallback or CMapGeneratorCallbacks? phosit: `Context` sounds js related. How about only `CMapGeneratorCallback` or `CMapGeneratorCallbacks`? | |||||
Done Inline ActionsCMapGeneratorCallbacks seems fine. vladislavbelov: `CMapGeneratorCallbacks` seems fine. | |||||
std::vector<std::string> CMapGeneratorWorker::FindTemplates(const std::string& path, bool includeSubdirectories) | ~CMapGeneratorCallbackData() | ||||
{ | { | ||||
return m_TemplateLoader.FindTemplates(path, includeSubdirectories, SIMULATION_TEMPLATES); | JS_AddInterruptCallback(m_ScriptInterface.GetGeneralJSContext(), nullptr); | ||||
m_ScriptInterface.SetCallbackData(nullptr); | |||||
} | } | ||||
std::vector<std::string> CMapGeneratorWorker::FindActorTemplates(const std::string& path, bool includeSubdirectories) | /** | ||||
{ | * Load all scripts of the given library | ||||
return m_TemplateLoader.FindTemplates(path, includeSubdirectories, ACTOR_TEMPLATES); | * | ||||
} | * @param libraryName VfsPath specifying name of the library (subfolder of ../maps/random/) | ||||
* @return true if all scripts ran successfully, false if there's an error | |||||
bool CMapGeneratorWorker::LoadScripts(const VfsPath& libraryName) | */ | ||||
bool LoadLibrary(const VfsPath& libraryName) | |||||
{ | { | ||||
// Ignore libraries that are already loaded | // Ignore libraries that are already loaded | ||||
if (m_LoadedLibraries.find(libraryName) != m_LoadedLibraries.end()) | if (m_LoadedLibraries.find(libraryName) != m_LoadedLibraries.end()) | ||||
return true; | return true; | ||||
// Mark this as loaded, to prevent it recursively loading itself | // Mark this as loaded, to prevent it recursively loading itself | ||||
m_LoadedLibraries.insert(libraryName); | m_LoadedLibraries.insert(libraryName); | ||||
VfsPath path = VfsPath(L"maps/random/") / libraryName / VfsPath(); | VfsPath path = VfsPath(L"maps/random/") / libraryName / VfsPath(); | ||||
VfsPaths pathnames; | VfsPaths pathnames; | ||||
// Load all scripts in mapgen directory | // Load all scripts in mapgen directory | ||||
Status ret = vfs::GetPathnames(g_VFS, path, L"*.js", pathnames); | Status ret = vfs::GetPathnames(g_VFS, path, L"*.js", pathnames); | ||||
if (ret == INFO::OK) | if (ret == INFO::OK) | ||||
{ | { | ||||
for (const VfsPath& p : pathnames) | for (const VfsPath& p : pathnames) | ||||
{ | { | ||||
LOGMESSAGE("Loading map generator script '%s'", p.string8()); | LOGMESSAGE("Loading map generator script '%s'", p.string8()); | ||||
if (!m_ScriptInterface->LoadGlobalScriptFile(p)) | if (!m_ScriptInterface.LoadGlobalScriptFile(p)) | ||||
{ | { | ||||
LOGERROR("CMapGeneratorWorker::LoadScripts: Failed to load script '%s'", p.string8()); | LOGERROR("CMapGeneratorWorker::LoadScripts: Failed to load script '%s'", p.string8()); | ||||
return false; | return false; | ||||
} | } | ||||
} | } | ||||
} | } | ||||
else | else | ||||
{ | { | ||||
// Some error reading directory | // Some error reading directory | ||||
wchar_t error[200]; | wchar_t error[200]; | ||||
LOGERROR("CMapGeneratorWorker::LoadScripts: Error reading scripts in directory '%s': %s", path.string8(), utf8_from_wstring(StatusDescription(ret, error, ARRAY_SIZE(error)))); | LOGERROR("CMapGeneratorWorker::LoadScripts: Error reading scripts in directory '%s': %s", path.string8(), utf8_from_wstring(StatusDescription(ret, error, ARRAY_SIZE(error)))); | ||||
return false; | return false; | ||||
} | } | ||||
return true; | return true; | ||||
} | } | ||||
JS::Value CMapGeneratorWorker::LoadHeightmap(const VfsPath& filename) | /** | ||||
* Finalize map generation and pass results from the script to the engine. | |||||
* The `data` has to be according to this format: | |||||
* https://trac.wildfiregames.com/wiki/Random_Map_Generator_Internals#Dataformat | |||||
*/ | |||||
void ExportMap(JS::HandleValue data) | |||||
{ | |||||
// Copy results | |||||
m_MapData = Script::WriteStructuredClone(ScriptRequest(m_ScriptInterface), data); | |||||
m_Progress.store(0); | |||||
} | |||||
Done Inline ActionsMight want to add a comment what should be in data. vladislavbelov: Might want to add a comment what should be in `data`. | |||||
/** | |||||
* Load an image file and return it as a height array. | |||||
*/ | |||||
JS::Value LoadHeightmapImage(const VfsPath& filename) | |||||
{ | { | ||||
std::vector<u16> heightmap; | std::vector<u16> heightmap; | ||||
if (LoadHeightmapImageVfs(filename, heightmap) != INFO::OK) | if (LoadHeightmapImageVfs(filename, heightmap) != INFO::OK) | ||||
{ | { | ||||
LOGERROR("Could not load heightmap file '%s'", filename.string8()); | LOGERROR("Could not load heightmap file '%s'", filename.string8()); | ||||
return JS::UndefinedValue(); | return JS::UndefinedValue(); | ||||
} | } | ||||
ScriptRequest rq(m_ScriptInterface); | ScriptRequest rq(m_ScriptInterface); | ||||
JS::RootedValue returnValue(rq.cx); | JS::RootedValue returnValue(rq.cx); | ||||
Script::ToJSVal(rq, &returnValue, heightmap); | Script::ToJSVal(rq, &returnValue, heightmap); | ||||
return returnValue; | return returnValue; | ||||
} | } | ||||
// See CMapReader::UnpackTerrain, CMapReader::ParseTerrain for the reordering | /** | ||||
JS::Value CMapGeneratorWorker::LoadMapTerrain(const VfsPath& filename) | * Load an Atlas terrain file (PMP) returning textures and heightmap. | ||||
* | |||||
* See CMapReader::UnpackTerrain, CMapReader::ParseTerrain for the reordering | |||||
*/ | |||||
JS::Value LoadMapTerrain(const VfsPath& filename) | |||||
{ | { | ||||
ScriptRequest rq(m_ScriptInterface); | ScriptRequest rq(m_ScriptInterface); | ||||
if (!VfsFileExists(filename)) | if (!VfsFileExists(filename)) | ||||
{ | { | ||||
ScriptException::Raise(rq, "Terrain file \"%s\" does not exist!", filename.string8().c_str()); | ScriptException::Raise(rq, "Terrain file \"%s\" does not exist!", filename.string8().c_str()); | ||||
return JS::UndefinedValue(); | return JS::UndefinedValue(); | ||||
} | } | ||||
CFileUnpacker unpacker; | CFileUnpacker unpacker; | ||||
unpacker.Read(filename, "PSMP"); | unpacker.Read(filename, "PSMP"); | ||||
if (unpacker.GetVersion() < CMapIO::FILE_READ_VERSION) | if (unpacker.GetVersion() < CMapIO::FILE_READ_VERSION) | ||||
{ | { | ||||
ScriptException::Raise(rq, "Could not load terrain file \"%s\" too old version!", filename.string8().c_str()); | ScriptException::Raise(rq, "Could not load terrain file \"%s\" too old version!", filename.string8().c_str()); | ||||
return JS::UndefinedValue(); | return JS::UndefinedValue(); | ||||
} | } | ||||
// unpack size | // unpack size | ||||
ssize_t patchesPerSide = (ssize_t)unpacker.UnpackSize(); | ssize_t patchesPerSide = (ssize_t)unpacker.UnpackSize(); | ||||
size_t verticesPerSide = patchesPerSide * PATCH_SIZE + 1; | size_t verticesPerSide = patchesPerSide * PATCH_SIZE + 1; | ||||
// unpack heightmap | // unpack heightmap | ||||
std::vector<u16> heightmap; | std::vector<u16> heightmap; | ||||
heightmap.resize(SQR(verticesPerSide)); | heightmap.resize(SQR(verticesPerSide)); | ||||
unpacker.UnpackRaw(&heightmap[0], SQR(verticesPerSide) * sizeof(u16)); | unpacker.UnpackRaw(&heightmap[0], SQR(verticesPerSide) * sizeof(u16)); | ||||
// unpack texture names | // unpack texture names | ||||
size_t textureCount = unpacker.UnpackSize(); | size_t textureCount = unpacker.UnpackSize(); | ||||
std::vector<std::string> textureNames; | std::vector<std::string> textureNames; | ||||
textureNames.reserve(textureCount); | textureNames.reserve(textureCount); | ||||
for (size_t i = 0; i < textureCount; ++i) | for (size_t i = 0; i < textureCount; ++i) | ||||
{ | { | ||||
CStr texturename; | CStr texturename; | ||||
unpacker.UnpackString(texturename); | unpacker.UnpackString(texturename); | ||||
textureNames.push_back(texturename); | textureNames.push_back(texturename); | ||||
} | } | ||||
// unpack texture IDs per tile | // unpack texture IDs per tile | ||||
ssize_t tilesPerSide = patchesPerSide * PATCH_SIZE; | ssize_t tilesPerSide = patchesPerSide * PATCH_SIZE; | ||||
std::vector<CMapIO::STileDesc> tiles; | std::vector<CMapIO::STileDesc> tiles; | ||||
tiles.resize(size_t(SQR(tilesPerSide))); | tiles.resize(size_t(SQR(tilesPerSide))); | ||||
unpacker.UnpackRaw(&tiles[0], sizeof(CMapIO::STileDesc) * tiles.size()); | unpacker.UnpackRaw(&tiles[0], sizeof(CMapIO::STileDesc) * tiles.size()); | ||||
// reorder by patches and store and save texture IDs per tile | // reorder by patches and store and save texture IDs per tile | ||||
std::vector<u16> textureIDs; | std::vector<u16> textureIDs; | ||||
for (ssize_t x = 0; x < tilesPerSide; ++x) | for (ssize_t x = 0; x < tilesPerSide; ++x) | ||||
{ | { | ||||
size_t patchX = x / PATCH_SIZE; | size_t patchX = x / PATCH_SIZE; | ||||
size_t offX = x % PATCH_SIZE; | size_t offX = x % PATCH_SIZE; | ||||
for (ssize_t y = 0; y < tilesPerSide; ++y) | for (ssize_t y = 0; y < tilesPerSide; ++y) | ||||
{ | { | ||||
size_t patchY = y / PATCH_SIZE; | size_t patchY = y / PATCH_SIZE; | ||||
size_t offY = y % PATCH_SIZE; | size_t offY = y % PATCH_SIZE; | ||||
// m_Priority and m_Tex2Index unused | // m_Priority and m_Tex2Index unused | ||||
textureIDs.push_back(tiles[(patchY * patchesPerSide + patchX) * SQR(PATCH_SIZE) + (offY * PATCH_SIZE + offX)].m_Tex1Index); | textureIDs.push_back(tiles[(patchY * patchesPerSide + patchX) * SQR(PATCH_SIZE) + | ||||
(offY * PATCH_SIZE + offX)].m_Tex1Index); | |||||
} | } | ||||
} | } | ||||
JS::RootedValue returnValue(rq.cx); | JS::RootedValue returnValue(rq.cx); | ||||
Script::CreateObject( | Script::CreateObject( | ||||
rq, | rq, | ||||
&returnValue, | &returnValue, | ||||
"height", heightmap, | "height", heightmap, | ||||
"textureNames", textureNames, | "textureNames", textureNames, | ||||
"textureIDs", textureIDs); | "textureIDs", textureIDs); | ||||
return returnValue; | return returnValue; | ||||
} | } | ||||
////////////////////////////////////////////////////////////////////////////////// | /** | ||||
////////////////////////////////////////////////////////////////////////////////// | * Sets the map generation progress, which is one of multiple stages determining the loading screen progress. | ||||
*/ | |||||
void SetProgress(int progress) | |||||
{ | |||||
// When the task is started, `m_Progress` is only mutated by this thread. | |||||
const int currentProgress = m_Progress.load(); | |||||
if (progress >= currentProgress) | |||||
m_Progress.store(progress); | |||||
else | |||||
LOGWARNING("The random map script tried to reduce the loading progress from %d to %d", | |||||
currentProgress, progress); | |||||
} | |||||
/** | |||||
* Microseconds since the epoch. | |||||
*/ | |||||
double GetMicroseconds() const | |||||
Done Inline Actionsdouble GetMicroseconds() const? vladislavbelov: `double GetMicroseconds() const`? | |||||
{ | |||||
return JS_Now(); | |||||
} | |||||
/** | |||||
* Return the template data of the given template name. | |||||
*/ | |||||
CParamNode GetTemplate(const std::string& templateName) | |||||
Done Inline ActionsCParamNode GetTemplate(const std::string& templateName) const? vladislavbelov: `CParamNode GetTemplate(const std::string& templateName) const`? | |||||
Done Inline ActionsCan't be const CTemplateLoader::GetTemplateFileData isn't const. phosit: Can't be const `CTemplateLoader::GetTemplateFileData` isn't const. | |||||
{ | |||||
const CParamNode& templateRoot = | |||||
m_TemplateLoader.GetTemplateFileData(templateName).GetOnlyChild(); | |||||
if (!templateRoot.IsOk()) | |||||
LOGERROR("Invalid template found for '%s'", templateName.c_str()); | |||||
CMapGenerator::CMapGenerator() : m_Worker(new CMapGeneratorWorker(nullptr)) | return templateRoot; | ||||
} | |||||
/** | |||||
* Check whether the given template exists. | |||||
*/ | |||||
bool TemplateExists(const std::string& templateName) const | |||||
Done Inline Actionsbool TemplateExists(const std::string& templateName) const? vladislavbelov: `bool TemplateExists(const std::string& templateName) const`? | |||||
{ | { | ||||
return m_TemplateLoader.TemplateExists(templateName); | |||||
} | } | ||||
CMapGenerator::~CMapGenerator() | /** | ||||
* Returns all template names of simulation entity templates. | |||||
*/ | |||||
std::vector<std::string> FindTemplates(const std::string& path, bool includeSubdirectories) | |||||
{ | { | ||||
delete m_Worker; | return m_TemplateLoader.FindTemplates(path, includeSubdirectories, SIMULATION_TEMPLATES); | ||||
} | } | ||||
void CMapGenerator::GenerateMap(const VfsPath& scriptFile, const std::string& settings) | /** | ||||
* Returns all template names of actors. | |||||
*/ | |||||
std::vector<std::string> FindActorTemplates(const std::string& path, bool includeSubdirectories) | |||||
{ | { | ||||
m_Worker->Initialize(scriptFile, settings); | return m_TemplateLoader.FindTemplates(path, includeSubdirectories, ACTOR_TEMPLATES); | ||||
} | } | ||||
int CMapGenerator::GetProgress() const | /** | ||||
* Currently loaded script librarynames. | |||||
*/ | |||||
std::set<VfsPath> m_LoadedLibraries; | |||||
/** | |||||
* Result of the mapscript generation including terrain, entities and environment settings. | |||||
*/ | |||||
Script::StructuredClone m_MapData; | |||||
/** | |||||
* Deterministic random number generator. | |||||
*/ | |||||
boost::rand48 m_MapGenRNG; | |||||
/** | |||||
* Current map generation progress. | |||||
* Initialize to `-1`. If something happens before we start, that's a | |||||
* failure. | |||||
*/ | |||||
std::atomic<int>& m_Progress; | |||||
/** | |||||
* Provides the script context. | |||||
*/ | |||||
ScriptInterface& m_ScriptInterface; | |||||
/** | |||||
* Backend to loading template data. | |||||
*/ | |||||
CTemplateLoader m_TemplateLoader; | |||||
}; | |||||
#define REGISTER_MAPGEN_FUNC(func) \ | |||||
ScriptFunction::Register<&CMapGeneratorCallbackData::func, ScriptInterface::ObjectFromCBData<CMapGeneratorCallbackData>>(rq, #func); | |||||
/** | |||||
* Set initial seed, callback data. | |||||
* Expose functions, globals and classes defined in this class relevant to the map and test scripts. | |||||
*/ | |||||
void InitScriptInterface(CMapGeneratorCallbackData& callbackData) | |||||
Done Inline ActionsThis one really sounds like it should be in the anon namespace, and kinda same below wraitii: This one really sounds like it should be in the anon namespace, and kinda same below | |||||
Done Inline ActionsMeh, a unnamed namespace has the same efect as static phosit: Meh, a unnamed namespace has the same efect as `static` | |||||
{ | { | ||||
return m_Worker->GetProgress(); | callbackData.m_ScriptInterface.ReplaceNondeterministicRNG(callbackData.m_MapGenRNG); | ||||
// VFS | |||||
JSI_VFS::RegisterScriptFunctions_ReadOnlySimulationMaps(callbackData.m_ScriptInterface); | |||||
// Globalscripts may use VFS script functions | |||||
callbackData.m_ScriptInterface.LoadGlobalScripts(); | |||||
// File loading | |||||
ScriptRequest rq(callbackData.m_ScriptInterface); | |||||
Done Inline Actions_ might be removed to follow CC. vladislavbelov: `_` might be removed to follow CC. | |||||
Done Inline ActionsWhat do you think about removing the whole _MapGenerator suffix? This whole file is about map generation. phosit: What do you think about removing the whole `_MapGenerator` suffix? This whole file is about map… | |||||
Done Inline ActionsYeah, it's ok. vladislavbelov: Yeah, it's ok. | |||||
REGISTER_MAPGEN_FUNC(LoadLibrary); | |||||
Done Inline ActionsMapGenerator::INVALID_PROGRESS. vladislavbelov: `MapGenerator::INVALID_PROGRESS`. | |||||
REGISTER_MAPGEN_FUNC(LoadHeightmapImage); | |||||
REGISTER_MAPGEN_FUNC(LoadMapTerrain); | |||||
// Engine constants | |||||
// Length of one tile of the terrain grid in metres. | |||||
// Useful to transform footprint sizes to the tilegrid coordinate system. | |||||
callbackData.m_ScriptInterface.SetGlobal("TERRAIN_TILE_SIZE", static_cast<int>(TERRAIN_TILE_SIZE)); | |||||
// Number of impassable tiles at the map border | |||||
callbackData.m_ScriptInterface.SetGlobal("MAP_BORDER_WIDTH", static_cast<int>(MAP_EDGE_TILES)); | |||||
} | |||||
/** | |||||
* Expose functions defined in this class that are relevant to mapscripts but not the tests. | |||||
*/ | |||||
void RegisterScriptFunctions_MapGenerator(CMapGeneratorCallbackData& callbackData) | |||||
{ | |||||
Done Inline ActionsWhy it's not a method if it needs to access private members? vladislavbelov: Why it's not a method if it needs to access private members? | |||||
Done Inline ActionsThere isn't a big difference between public and private since everything has internal linkage. phosit: There isn't a big difference between `public` and `private` since everything has internal… | |||||
Done Inline ActionsThere is a semantic difference. It's not a POD and it has some logic when accessing members. So external modification is less safe. vladislavbelov: There is a semantic difference. It's not a POD and it has some logic when accessing members. So… | |||||
Done Inline ActionsYes it's not a POD. Does it have to be? phosit: Yes it's not a POD. Does it have to be?
The "scope" of `CMapGeneratorCallbackData` isn't that… | |||||
Done Inline ActionsIn that case it's not critical. Ideally I think we should use a) POD with possibility of an external modification or b) non-POD without external modification. vladislavbelov: In that case it's not critical.
Ideally I think we should use a) POD with possibility of an… | |||||
Done Inline ActionsWhy do you have such a strong oppinion about POD-idity? (It's deprecated in C++20 ;P) The coreguidline is even stricter then you... I'll make all member-variables private. phosit: Why do you have such a strong oppinion about POD-idity? (It's deprecated in C++20 ;P)
The… | |||||
ScriptRequest rq(callbackData.m_ScriptInterface); | |||||
// Template functions | |||||
REGISTER_MAPGEN_FUNC(GetTemplate); | |||||
REGISTER_MAPGEN_FUNC(TemplateExists); | |||||
REGISTER_MAPGEN_FUNC(FindTemplates); | |||||
REGISTER_MAPGEN_FUNC(FindActorTemplates); | |||||
// Progression and profiling | |||||
REGISTER_MAPGEN_FUNC(SetProgress); | |||||
REGISTER_MAPGEN_FUNC(GetMicroseconds); | |||||
REGISTER_MAPGEN_FUNC(ExportMap); | |||||
} | |||||
#undef REGISTER_MAPGEN_FUNC | |||||
} | |||||
Script::StructuredClone MapGenerator::GenerateMap(std::atomic<int>& progress, | |||||
ScriptInterface& scriptInterface, const VfsPath& script, const std::string& settings, | |||||
const bool isTest) | |||||
{ | |||||
ScriptRequest rq(scriptInterface); | |||||
// Parse settings | |||||
Done Inline ActionsWe don't use unnamed namespace comment, anonymous namespace or empty. vladislavbelov: We don't use `unnamed namespace` comment, `anonymous namespace` or empty. | |||||
Done Inline ActionsOutdated messages here and below. vladislavbelov: Outdated messages here and below. | |||||
JS::RootedValue settingsVal(rq.cx); | |||||
if (!Script::ParseJSON(rq, settings, &settingsVal) && settingsVal.isUndefined()) | |||||
{ | |||||
LOGERROR("CMapGeneratorWorker::Run: Failed to parse settings"); | |||||
progress.store(MapGenerator::errorConstant); | |||||
Done Inline ActionsToo many repeats of -1, might be a named constant. vladislavbelov: Too many repeats of `-1`, might be a named constant. | |||||
Done Inline ActionsIdealy errors are handled using exceptions. phosit: Idealy errors are handled using exceptions. | |||||
return nullptr; | |||||
} | |||||
// Prevent unintentional modifications to the settings object by random map scripts | |||||
if (!Script::FreezeObject(rq, settingsVal, true)) | |||||
{ | |||||
LOGERROR("CMapGeneratorWorker::Run: Failed to deepfreeze settings"); | |||||
progress.store(MapGenerator::errorConstant); | |||||
return nullptr; | |||||
} | |||||
// Init RNG seed | |||||
u32 seed = 0; | |||||
if (!Script::HasProperty(rq, settingsVal, "Seed") || | |||||
!Script::GetProperty(rq, settingsVal, "Seed", seed)) | |||||
LOGWARNING("CMapGeneratorWorker::Run: No seed value specified - using 0"); | |||||
CMapGeneratorCallbackData callbackData{scriptInterface, progress, seed}; | |||||
InitScriptInterface(callbackData); | |||||
if (!isTest) | |||||
{ | |||||
RegisterScriptFunctions_MapGenerator(callbackData); | |||||
// Copy settings to global variable | |||||
JS::RootedValue global(rq.cx, rq.globalValue()); | |||||
if (!Script::SetProperty(rq, global, "g_MapSettings", settingsVal, true, true)) | |||||
{ | |||||
LOGERROR("CMapGeneratorWorker::Run: Failed to define g_MapSettings"); | |||||
progress.store(-1); | |||||
Done Inline ActionsMapGenerator::INVALID_PROGRESS. vladislavbelov: `MapGenerator::INVALID_PROGRESS`. | |||||
return nullptr; | |||||
} | |||||
} | |||||
Done Inline ActionsWhy it can't work in tests? vladislavbelov: Why it can't work in tests? | |||||
Done Inline ActionsI'm not sure. The tests use mock functions and Engine.GetTemplate can't be overwritten. phosit: I'm not sure. The tests use mock functions and `Engine.GetTemplate` can't be overwritten. | |||||
Done Inline ActionsWhy it was working before the patch? vladislavbelov: Why it was working before the patch? | |||||
Done Inline ActionsBefore the patch the test also didn't executed that. phosit: Before the patch the test also didn't executed that. | |||||
// Load RMS | |||||
Done Inline ActionsWhy it should fail in tests? vladislavbelov: Why it should fail in tests? | |||||
Done Inline ActionsI don't understand this question: No it should not fail in tests... phosit: I don't understand this question: No it should not fail in tests...
The tests (js-scripts)… | |||||
Done Inline ActionsI'm worried about having production code under if section which depends on tests environment. vladislavbelov: I'm worried about having production code under `if` section which depends on tests environment. | |||||
Done Inline ActionsThe other approach would be to split up this function and expose multiple functions in the .h file. Which would make the interface more complicated again. (CMapGeneratorCallbacks would also have to be exposed.) A different design might be to pass a function (function-ptr or std::function) which could act on the rq to set mock-functions or the production-functions. This is very similar to the if solution but allows more customization. phosit: The other approach would be to split up this function and expose multiple functions in the .h… | |||||
LOGMESSAGE("Loading RMS '%s'", script.string8()); | |||||
Done Inline ActionsInitScriptInterface is called without isTest then RegisterScriptFunctions should be also called without it. vladislavbelov: `InitScriptInterface` is called without `isTest` then `RegisterScriptFunctions` should be also… | |||||
Done Inline ActionsI don't know excactly which functions the test-scripts use. I kept it as before. phosit: I don't know excactly which functions the test-scripts use. I kept it as before. | |||||
Not Done Inline Actions
It contradicts to your comment: // The test script don't call `ExportMap` so `GenerateMap` allways returns `nullptr`. vladislavbelov: > I don't know excactly which functions the test-scripts use.
It contradicts to your comment… | |||||
Done Inline ActionsExportMap is a function which is called in the test-scripts directly.
... Because they use some functions indirectly. The thing I know for sure is that some test-scripts don't call ExportMap. phosit: `ExportMap` is a function which is called in the test-scripts directly.
> I don't know exactly… | |||||
Not Done Inline ActionsWe can create g_MapSettings in tests (also rq might be passed as a parameter). vladislavbelov: We can create `g_MapSettings` in tests (also `rq` might be passed as a parameter). | |||||
Done Inline ActionsNo, the test scripts define g_MapSettings. Also g_MapSettings is "frozen". phosit: No, the test scripts define `g_MapSettings`. Also `g_MapSettings` is "frozen". | |||||
Not Done Inline ActionsI'd remove g_MapSettings from test_*.js and add the size parameter to the settings parameter: "{\"Seed\": 0, \"Size\": 512}", else the settings parameter is passed but unused by tests. vladislavbelov: I'd remove `g_MapSettings` from `test_*.js` and add the size parameter to the `settings`… | |||||
Done Inline Actionstest_DiskPlacer.js uses multiple different g_MapSettings. {"Seed": 0, "Size": 512, "CircularMap": true} should work for all tests. phosit: test_DiskPlacer.js uses multiple different `g_MapSettings`. `{"Seed": 0, "Size": 512… | |||||
Not Done Inline ActionsHmm. Maybe we need additional test environment. But it seems like a lot of work. For example test_*.js files don't have code in the global space but rather define test_* functions/test cases (like in C++ tests) where we can define possible sizes and other map properties. For example (not the cleanest one): function test_small_maps() { return { "test": () = { let map = new RandomMap(0, 0, "blackness"); // Does not error with floating point radius let area = createArea(map, new DiskPlacer(86.4, new Vector2D(160, 160))); // ... }, "cases": [{"Size": 320}, {"Size": 512, "CircularMap": false}], } } vladislavbelov: Hmm. Maybe we need additional test environment. But it seems like a lot of work. For example… | |||||
Done Inline ActionsYes that would certenly be cleaner. But it diverges more from the normal map generation scripts. phosit: Yes that would certenly be cleaner. But it diverges more from the normal map generation scripts. | |||||
if (!scriptInterface.LoadGlobalScriptFile(script)) | |||||
{ | |||||
LOGERROR("CMapGeneratorWorker::Run: Failed to load RMS '%s'", script.string8()); | |||||
Not Done Inline ActionsI'd add a comment here to describe which scoped is ended here: } // anonymous namespace. vladislavbelov: I'd add a comment here to describe which scoped is ended here: `} // anonymous namespace`. | |||||
progress.store(MapGenerator::errorConstant); | |||||
return nullptr; | |||||
} | |||||
return progress.load() == 0 ? std::move(callbackData.m_MapData) : nullptr; | |||||
Done Inline ActionsI think it's better to use {} instead of nullptr. vladislavbelov: I think it's better to use `{}` instead of `nullptr`. | |||||
Done Inline Actions{} can't be used in ternary expressions. Also nullptr is returned in other places on error. phosit: `{}` can't be used in ternary expressions.
Also `nullptr` is returned in other places on error. | |||||
Done Inline ActionsPure {} - can't, Script::StructuredClone{} can be used. But I don't insist here. vladislavbelov: Pure `{}` - can't, `Script::StructuredClone{}` can be used. But I don't insist here. | |||||
Done Inline ActionsI'd still use nullptr to keep it consistent with the other returns. phosit: I'd still use `nullptr` to keep it consistent with the other `returns`. | |||||
} | } | ||||
Script::StructuredClone CMapGenerator::GetResults() | Script::StructuredClone MapGenerator::GenerateMap(std::atomic<int>& progress, const VfsPath& script, | ||||
const std::string& settings) | |||||
{ | { | ||||
return m_Worker->GetResults(); | const std::shared_ptr<ScriptContext> mapgenContext = ScriptContext::CreateContext(RMS_CONTEXT_SIZE); | ||||
ScriptInterface scriptInterface{"Engine", "MapGenerator", mapgenContext}; | |||||
return MapGenerator::GenerateMap(progress, scriptInterface, script, settings, false); | |||||
} | } |
It doesn't seem like a struct.