Differential D5001 Diff 22104 source/graphics/MapGenerator.h

Changeset View

Standalone View

source/graphics/MapGenerator.h

	Show All 13 Lines
	* You should have received a copy of the GNU General Public License			* You should have received a copy of the GNU General Public License
	* along with 0!A.D. If not, see <http://www.gnu.org/licenses/>.			* along with 0!A.D. If not, see <http://www.gnu.org/licenses/>.
	*/			*/

	#ifndef INCLUDED_MAPGENERATOR			#ifndef INCLUDED_MAPGENERATOR
	#define INCLUDED_MAPGENERATOR			#define INCLUDED_MAPGENERATOR

	#include "ps/FileIo.h"			#include "ps/FileIo.h"
	#include "ps/Future.h"
	#include "ps/TemplateLoader.h"
	#include "scriptinterface/StructuredClone.h"			#include "scriptinterface/StructuredClone.h"

	#include <atomic>			#include <atomic>
	#include <boost/random/linear_congruential.hpp>
	#include <mutex>
	#include <set>
	#include <string>			#include <string>

	class CMapGeneratorWorker;			class TestMapGenerator;

	/**
	* Random map generator interface. Initialized by CMapReader and then checked
	* periodically during loading, until it's finished (progress value is 0).
	*
	* The actual work is performed by CMapGeneratorWorker in a separate thread.
	*/
	class CMapGenerator			class CMapGenerator
				vladislavbelovUnsubmitted Done Inline Actions `CMapGenerator`. vladislavbelov: `CMapGenerator`.
	{			{
	NONCOPYABLE(CMapGenerator);			friend TestMapGenerator;
				vladislavbelovUnsubmitted Done Inline Actions That should be in the `private` section. vladislavbelov: That should be in the `private` section.
				phositAuthorUnsubmitted Done Inline Actions IDK phosit: IDK
				vladislavbelovUnsubmitted Done Inline Actions It's an implementation detail vladislavbelov: It's an implementation detail
				phositAuthorUnsubmitted Done Inline Actions It's already in the private section. Most (when not all) classes who have a `friend` declaration have it at the top. phosit: It's already in the private section. Most (when not all) classes who have a `friend`…
				vladislavbelovUnsubmitted Done Inline Actions The private friend should be after the public since it's an implementation/particular relationship detail. Definitely not all, I'd say about 50/50 or 40/60. vladislavbelov: The private friend should be after the public since it's an implementation/particular…

	public:			public:
	CMapGenerator();
	~CMapGenerator();

	/**
	* Start the map generator thread
	*
	* @param scriptFile The VFS path for the script, e.g. "maps/random/latium.js"
	* @param settings JSON string containing settings for the map generator
	*/
	void GenerateMap(const VfsPath& scriptFile, const std::string& settings);

	/**
	* Get status of the map generator thread
	*
	* @return Progress percentage 1-100 if active, 0 when finished, or -1 on error
	*/
	int GetProgress() const;

	/**			/**
	* Get random map data, according to this format:			* `progress` will take on this value if an error occurred.
	* http://trac.wildfiregames.com/wiki/Random_Map_Generator_Internals#Dataformat
	*
	* @return StructuredClone containing map data
	*/			*/
	Script::StructuredClone GetResults();			static constexpr int INVALID_PROGRESS{-1};

	private:
	CMapGeneratorWorker* m_Worker;

	};

				vladislavbelovUnsubmitted Done Inline Actions Private methods/fields should be after public ones. vladislavbelov: Private methods/fields should be after public ones.
	/**			/**
	* Random map generator worker thread.			* Generate the map. This does take a long time.
	* (This is run in a thread so that the GUI remains responsive while loading)
	*			*
	* Thread-safety:			* @param progress param progress Destination to write the function
				wraitiiUnsubmitted Done Inline Actions I'd call it TestGenerateMap wraitii: I'd call it TestGenerateMap
				phositAuthorUnsubmitted Done Inline Actions It's also (indirectly) called by non test code. phosit: It's also (indirectly) called by non test code.
				wraitiiUnsubmitted Done Inline Actions Right, wondered that. Then perhaps just change the comment to 'Intended for tests & inner usage' wraitii: Right, wondered that. Then perhaps just change the comment to 'Intended for tests & inner usage'
				wraitiiUnsubmitted Done Inline Actions `This takes a long time` maybe wraitii: `This takes a long time` maybe
				phositAuthorUnsubmitted Done Inline Actions I'll do that phosit: I'll do that
	* - Initialize and constructor/destructor must be called from the main thread.			* progress to. You must not write to it while GenerateMap is running.
	* - ScriptInterface created and destroyed by thread			* See Loader.h for the semantics.
	* - StructuredClone used to return JS map data - JS:Values can't be used across threads/contexts.			* @param script The VFS path for the script, e.g. "maps/random/latium.js"
				vladislavbelovUnsubmitted Done Inline Actions A subject is missing. vladislavbelov: A subject is missing.
				phositAuthorUnsubmitted Done Inline Actions Yes. Can you propose a better formulation? phosit: Yes. Can you propose a better formulation?
				vladislavbelovUnsubmitted Done Inline Actions I'd use `@param progress Destination to write the function progress to.`. or `@param progress The function writes its progress to the parameter.`. vladislavbelov: I'd use `@param progress Destination to write the function progress to.`. or `@param progress…
	*/
	class CMapGeneratorWorker
	{
	public:
	CMapGeneratorWorker(ScriptInterface* scriptInterface);
	~CMapGeneratorWorker();

	/**
	* Start the map generator thread
	*
	* @param scriptFile The VFS path for the script, e.g. "maps/random/latium.js"
	* @param settings JSON string containing settings for the map generator			* @param settings JSON string containing settings for the map generator
				* @return random map data, according to this format:
				* https://trac.wildfiregames.com/wiki/Random_Map_Generator_Internals#Dataformat
	*/			*/
	void Initialize(const VfsPath& scriptFile, const std::string& settings);			static Script::StructuredClone GenerateMap(std::atomic<int>& progress, const VfsPath& script,
				const std::string& settings);
				wraitiiUnsubmitted Done Inline Actions Feels like it'd be better to abstract the core functionality in a common path, and make the Test class a friend, so that we don't have any specific code for this. wraitii: Feels like it'd be better to abstract the core functionality in a common path, and make the…
				phositAuthorUnsubmitted Done Inline Actions I'll take a look at that. phosit: I'll take a look at that.
	/**
	* Get status of the map generator thread
	*
	* @return Progress percentage 1-100 if active, 0 when finished, or -1 on error
	*/
	int GetProgress() const;

	/**
	* Get random map data, according to this format:
	* http://trac.wildfiregames.com/wiki/Random_Map_Generator_Internals#Dataformat
	*
	* @return StructuredClone containing map data
	*/
	Script::StructuredClone GetResults();

	/**
	* Set initial seed, callback data.
	* Expose functions, globals and classes defined in this class relevant to the map and test scripts.
	*/
	void InitScriptInterface(const u32 seed);

	private:			private:

	/**			/**
				vladislavbelovUnsubmitted Done Inline Actions The style doesn't seem like a constant, I'd use `ERROR_CONSTANT` or at least `ErrorConstant`. Also `Constant` suffix seems redundant, we don't use such suffixes. vladislavbelov: The style doesn't seem like a constant, I'd use `ERROR_CONSTANT` or at least `ErrorConstant`.
				phositAuthorUnsubmitted Done Inline Actions You would use `ERROR_CONSTANT` but without the "constant" suffix. So only `ERROR`? phosit: You would use `ERROR_CONSTANT` but without the "constant" suffix. So only `ERROR`?
				vladislavbelovUnsubmitted Done Inline Actions `ERROR_CONSTANT` was an example of the style. I'd use `INVALID_PROGRESS` here. vladislavbelov: `ERROR_CONSTANT` was an example of the style. I'd use `INVALID_PROGRESS` here.
	* Expose functions defined in this class that are relevant to mapscripts but not the tests.			* This function is intended for tests and internal use.
	*/			*/
	void RegisterScriptFunctions_MapGenerator();			static Script::StructuredClone GenerateMap(std::atomic<int>& progress,
				ScriptInterface& scriptInterface, const VfsPath& script, const std::string& settings,
	/**			const bool isTest);
	* Load all scripts of the given library
	*
	* @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 LoadScripts(const VfsPath& libraryName);

	/**
	* Finalize map generation and pass results from the script to the engine.
	*/
	void ExportMap(JS::HandleValue data);

	/**
	* Load an image file and return it as a height array.
	*/
	JS::Value LoadHeightmap(const VfsPath& src);

	/**
	* Load an Atlas terrain file (PMP) returning textures and heightmap.
	*/
	JS::Value LoadMapTerrain(const VfsPath& filename);

	/**
	* Sets the map generation progress, which is one of multiple stages determining the loading screen progress.
	*/
	void SetProgress(int progress);

	/**
	* Microseconds since the epoch.
	*/
	double GetMicroseconds();

	/**
	* Return the template data of the given template name.
	*/
	CParamNode GetTemplate(const std::string& templateName);

	/**
	* Check whether the given template exists.
	*/
	bool TemplateExists(const std::string& templateName);

	/**
	* Returns all template names of simulation entity templates.
	*/
	std::vector<std::string> FindTemplates(const std::string& path, bool includeSubdirectories);

	/**
	* Returns all template names of actors.
	*/
	std::vector<std::string> FindActorTemplates(const std::string& path, bool includeSubdirectories);

	/**
	* Perform the map generation.
	*/
	bool Run();

	/**
	* 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{-1};

	/**
	* Provides the script context.
	*/
	ScriptInterface* m_ScriptInterface;

	/**
	* Map generation script to run.
	*/
	VfsPath m_ScriptPath;

	/**
	* Map and simulation settings chosen in the gamesetup stage.
	*/
	std::string m_Settings;

	/**
	* Backend to loading template data.
	*/
	CTemplateLoader m_TemplateLoader;

	/**
	* Holds the completion result of the asynchronous map generation.
	* TODO: this whole class could really be a future on its own.
	*/
	Future<void> m_WorkerThread;

	/**
	* Avoids thread synchronization issues.
	*/
	std::mutex m_WorkerMutex;
	};			};

				phositAuthorUnsubmitted Done Inline Actions I don't like the name of this function `RunMapGenerationScript` is more expressive. phosit: I don't like the name of this function `RunMapGenerationScript` is more expressive.

	#endif //INCLUDED_MAPGENERATOR			#endif //INCLUDED_MAPGENERATOR