Differential D3676 Diff 16436 source/graphics/MapGenerator.cpp

Changeset View

Standalone View

source/graphics/MapGenerator.cpp

Show First 20 Lines • Show All 96 Lines • ▼ Show 20 Lines	void CMapGeneratorWorker::RunThread(CMapGeneratorWorker* self)
JS_AddInterruptCallback(mapgenContext->GetGeneralJSContext(), MapGeneratorInterruptCallback);		JS_AddInterruptCallback(mapgenContext->GetGeneralJSContext(), MapGeneratorInterruptCallback);

self->m_ScriptInterface = new ScriptInterface("Engine", "MapGenerator", mapgenContext);		self->m_ScriptInterface = new ScriptInterface("Engine", "MapGenerator", mapgenContext);

// Run map generation scripts		// Run map generation scripts
if (!self->Run() \|\| self->m_Progress > 0)		if (!self->Run() \|\| self->m_Progress > 0)
{		{
// Don't leave progress in an unknown state, if generator failed, set it to -1		// Don't leave progress in an unknown state, if generator failed, set it to -1
std::lock_guard<std::mutex> lock(self->m_WorkerMutex);
self->m_Progress = -1;		self->m_Progress = -1;
}		}

SAFE_DELETE(self->m_ScriptInterface);		SAFE_DELETE(self->m_ScriptInterface);

// At this point the random map scripts are done running, so the thread has no further purpose		// 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		// and can die. The data will be stored in m_MapData already if successful, or m_Progress
// will contain an error value on failure.		// will contain an error value on failure.
▲ Show 20 Lines • Show All 95 Lines • ▼ Show 20 Lines	void CMapGeneratorWorker::RegisterScriptFunctions_MapGenerator()
REGISTER_MAPGEN_FUNC(SetProgress);		REGISTER_MAPGEN_FUNC(SetProgress);
REGISTER_MAPGEN_FUNC(GetMicroseconds);		REGISTER_MAPGEN_FUNC(GetMicroseconds);
REGISTER_MAPGEN_FUNC(ExportMap);		REGISTER_MAPGEN_FUNC(ExportMap);
}		}

#undef REGISTER_MAPGEN_FUNC		#undef REGISTER_MAPGEN_FUNC
#undef REGISTER_MAPGEN_FUNC_NAME		#undef REGISTER_MAPGEN_FUNC_NAME

int CMapGeneratorWorker::GetProgress()		int CMapGeneratorWorker::GetProgress() const
{		{
std::lock_guard<std::mutex> lock(m_WorkerMutex);
return m_Progress;		return m_Progress;
}		}

double CMapGeneratorWorker::GetMicroseconds()		double CMapGeneratorWorker::GetMicroseconds()
{		{
return JS_Now();		return JS_Now();
}		}

ScriptInterface::StructuredClone CMapGeneratorWorker::GetResults()		ScriptInterface::StructuredClone CMapGeneratorWorker::GetResults()
{		{
std::lock_guard<std::mutex> lock(m_WorkerMutex);		std::lock_guard<std::mutex> lock(m_WorkerMutex);
return m_MapData;		return m_MapData;
}		}

void CMapGeneratorWorker::ExportMap(JS::HandleValue data)		void CMapGeneratorWorker::ExportMap(JS::HandleValue data)
{		{
// Copy results		// Copy results
std::lock_guard<std::mutex> lock(m_WorkerMutex);		std::lock_guard<std::mutex> lock(m_WorkerMutex);
m_MapData = m_ScriptInterface->WriteStructuredClone(data);		m_MapData = m_ScriptInterface->WriteStructuredClone(data);
m_Progress = 0;		m_Progress = 0;
}		}
		vladislavbelovUnsubmitted Not Done Inline Actions That might be compiled differently on non-x86 hardware but shouldn't break something. vladislavbelov: That might be compiled differently on non-x86 hardware but shouldn't break something.
		phositAuthorUnsubmitted Done Inline Actions Why only on non-x86? phosit: Why only on non-x86?
		vladislavbelovUnsubmitted Not Done Inline Actions Because x86 is simpler in terms of atomic operations. vladislavbelov: Because x86 is simpler in terms of atomic operations.
		phositAuthorUnsubmitted Done Inline Actions In the new code the compiler is allowed to reorder some instructions from after the store to before it. That doesn't depend on the architecture. phosit: In the new code the compiler is allowed to reorder some instructions from after the store to…
		StanUnsubmitted Done Inline Actions Yeah but maybe int assignation is different than std::atomic<int> on some platforms I suppose? Stan: Yeah but maybe int assignation is different than std::atomic<int> on some platforms I suppose?
		vladislavbelovUnsubmitted Not Done Inline Actions It's still not allowed to put the store before the lock. Also it depends on architecture. In the first comment I meant that a locked store and a store after unlock might generate different assembly for some platforms. vladislavbelov: It's still not allowed to put the store before the lock. Also it depends on architecture. In…
		phositAuthorUnsubmitted Done Inline Actions The discussion continued on IRC but didn't come to a conclusion. phosit: The discussion continued on [[ https://irclogs.wildfiregames.com/%230ad-dev/2023-06-14-QuakeNet…

void CMapGeneratorWorker::SetProgress(int progress)		void CMapGeneratorWorker::SetProgress(int progress)
{		{
// Copy data		if (progress < m_Progress)
std::lock_guard<std::mutex> lock(m_WorkerMutex);		{
		LOGWARNING("The random map script tried to reduce the loading progress from %d to %d", previousProgress, progress);
		return;
		}

if (progress >= m_Progress)		const int previousProgress = m_Progress.exchange(progress);
m_Progress = progress;		ENSURE(previousProgress <= progress);
		wraitiiUnsubmitted Not Done Inline Actions Isn't there technically a possibility that another thread would modify the value such that this ENSURE is wrong? If you're relying on "single map generator thread" anyways, this ensure seems rather redundant. wraitii: Isn't there technically a possibility that another thread would modify the value such that this…
		vladislavbelovUnsubmitted Done Inline Actions Yeah, it relies on a single threaded generator. vladislavbelov: Yeah, it relies on a single threaded generator.
		phositAuthorUnsubmitted Not Done Inline Actions So this function could stay(as before the diff) only remove the lock_guard? It would be good when there is a coment `After the worker is started(Initialize is called) m_Progress is only mutated by that task.` phosit: So this function could stay(as before the diff) only remove the lock_guard? It would be good…
else
LOGWARNING("The random map script tried to reduce the loading progress from %d to %d", m_Progress, progress);
}		}

CParamNode CMapGeneratorWorker::GetTemplate(const std::string& templateName)		CParamNode CMapGeneratorWorker::GetTemplate(const std::string& templateName)
{		{
const CParamNode& templateRoot = m_TemplateLoader.GetTemplateFileData(templateName).GetChild("Entity");		const CParamNode& templateRoot = m_TemplateLoader.GetTemplateFileData(templateName).GetChild("Entity");
if (!templateRoot.IsOk())		if (!templateRoot.IsOk())
LOGERROR("Invalid template found for '%s'", templateName.c_str());		LOGERROR("Invalid template found for '%s'", templateName.c_str());

▲ Show 20 Lines • Show All 153 Lines • ▼ Show 20 Lines	CMapGenerator::~CMapGenerator()
delete m_Worker;		delete m_Worker;
}		}

void CMapGenerator::GenerateMap(const VfsPath& scriptFile, const std::string& settings)		void CMapGenerator::GenerateMap(const VfsPath& scriptFile, const std::string& settings)
{		{
m_Worker->Initialize(scriptFile, settings);		m_Worker->Initialize(scriptFile, settings);
}		}

int CMapGenerator::GetProgress()		int CMapGenerator::GetProgress() const
{		{
return m_Worker->GetProgress();		return m_Worker->GetProgress();
}		}

ScriptInterface::StructuredClone CMapGenerator::GetResults()		ScriptInterface::StructuredClone CMapGenerator::GetResults()
{		{
return m_Worker->GetResults();		return m_Worker->GetResults();
}		}