From 6526f9f6cb891036cfbbc0eda0102b46bef845aa Mon Sep 17 00:00:00 2001 From: Ted John Date: Thu, 19 Apr 2018 13:26:19 +0100 Subject: [PATCH 1/6] Fix scan-objects CLI action Ensure a context is available when objects are loaded. --- src/openrct2/cmdline/RootCommands.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/openrct2/cmdline/RootCommands.cpp b/src/openrct2/cmdline/RootCommands.cpp index 77d4f6f45c..080350ec54 100644 --- a/src/openrct2/cmdline/RootCommands.cpp +++ b/src/openrct2/cmdline/RootCommands.cpp @@ -15,12 +15,13 @@ #pragma endregion #include +#include #include "../config/Config.h" +#include "../Context.h" #include "../platform/Crash.h" #include "../platform/platform.h" #include "../localisation/Language.h" - #include "../core/Console.hpp" #include "../core/Memory.hpp" #include "../core/Path.hpp" @@ -394,12 +395,14 @@ static exitcode_t HandleCommandScanObjects(CommandLineArgEnumerator * enumerator return result; } - auto env = OpenRCT2::CreatePlatformEnvironment(); + gOpenRCT2Headless = true; // HACK: set gCurrentLanguage otherwise it be wrong for the index file gCurrentLanguage = gConfigGeneral.language; - auto objectRepository = CreateObjectRepository(env); + auto context = std::unique_ptr(OpenRCT2::CreateContext()); + auto env = context->GetPlatformEnvironment(); + auto objectRepository = std::unique_ptr(CreateObjectRepository(env)); objectRepository->Construct(); return EXITCODE_OK; } From 4edb450594d17e2416a987a75bccfbbc450aed03 Mon Sep 17 00:00:00 2001 From: Ted John Date: Thu, 19 Apr 2018 13:26:31 +0100 Subject: [PATCH 2/6] Run scan-objects before the tests --- openrct2.proj | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/openrct2.proj b/openrct2.proj index 6dc26809d6..df905159f8 100644 --- a/openrct2.proj +++ b/openrct2.proj @@ -185,7 +185,13 @@ - + + + + + + From edb9dc39ab3fc63490e203191acc3f8a48397984 Mon Sep 17 00:00:00 2001 From: Ted John Date: Thu, 19 Apr 2018 23:51:05 +0100 Subject: [PATCH 3/6] Pass object repository as a dependency when loading objects --- src/openrct2/object/BannerObject.cpp | 21 +++++++++------ src/openrct2/object/FootpathItemObject.cpp | 21 +++++++++------ src/openrct2/object/Object.h | 3 ++- src/openrct2/object/ObjectFactory.cpp | 26 +++++++++++------- src/openrct2/object/ObjectFactory.h | 7 ++--- src/openrct2/object/ObjectJsonHelpers.cpp | 2 +- src/openrct2/object/ObjectRepository.cpp | 31 ++++++++-------------- src/openrct2/rct1/S4Importer.cpp | 7 ++--- 8 files changed, 65 insertions(+), 53 deletions(-) diff --git a/src/openrct2/object/BannerObject.cpp b/src/openrct2/object/BannerObject.cpp index 749c919f6b..4fe37bafc5 100644 --- a/src/openrct2/object/BannerObject.cpp +++ b/src/openrct2/object/BannerObject.cpp @@ -18,6 +18,7 @@ #include "../drawing/Drawing.h" #include "../localisation/Language.h" #include "../object/Object.h" +#include "../object/ObjectRepository.h" #include "BannerObject.h" #include "ObjectJsonHelpers.h" #include "ObjectList.h" @@ -46,16 +47,20 @@ void BannerObject::ReadLegacy(IReadObjectContext * context, IStream * stream) // Add banners to 'Signs and items for footpaths' group, rather than lumping them in the Miscellaneous tab. // Since this is already done the other way round for original items, avoid adding those to prevent duplicates. - const std::string identifier = GetIdentifier(); - const rct_object_entry * objectEntry = object_list_find_by_name(identifier.c_str()); - static const rct_object_entry scgPathX = Object::GetScgPathXHeader(); + auto identifier = GetIdentifier(); - if (objectEntry != nullptr && - (object_entry_get_source_game(objectEntry) == OBJECT_SOURCE_WACKY_WORLDS || - object_entry_get_source_game(objectEntry) == OBJECT_SOURCE_TIME_TWISTER || - object_entry_get_source_game(objectEntry) == OBJECT_SOURCE_CUSTOM)) + auto& objectRepository = context->GetObjectRepository(); + auto item = objectRepository.FindObject(identifier); + if (item != nullptr) { - SetPrimarySceneryGroup(&scgPathX); + auto objectEntry = &item->ObjectEntry; + if (object_entry_get_source_game(objectEntry) == OBJECT_SOURCE_WACKY_WORLDS || + object_entry_get_source_game(objectEntry) == OBJECT_SOURCE_TIME_TWISTER || + object_entry_get_source_game(objectEntry) == OBJECT_SOURCE_CUSTOM) + { + auto scgPathX = Object::GetScgPathXHeader(); + SetPrimarySceneryGroup(&scgPathX); + } } } diff --git a/src/openrct2/object/FootpathItemObject.cpp b/src/openrct2/object/FootpathItemObject.cpp index ff4c6260fd..7d04eda219 100644 --- a/src/openrct2/object/FootpathItemObject.cpp +++ b/src/openrct2/object/FootpathItemObject.cpp @@ -20,6 +20,7 @@ #include "../interface/Cursors.h" #include "../localisation/Localisation.h" #include "../object/Object.h" +#include "../object/ObjectRepository.h" #include "ObjectList.h" #include "FootpathItemObject.h" #include "ObjectJsonHelpers.h" @@ -49,16 +50,20 @@ void FootpathItemObject::ReadLegacy(IReadObjectContext * context, IStream * stre // Add path bits to 'Signs and items for footpaths' group, rather than lumping them in the Miscellaneous tab. // Since this is already done the other way round for original items, avoid adding those to prevent duplicates. - const std::string identifier = GetIdentifier(); - const rct_object_entry * objectEntry = object_list_find_by_name(identifier.c_str()); - static const rct_object_entry scgPathX = Object::GetScgPathXHeader(); + auto identifier = GetIdentifier(); - if (objectEntry != nullptr && - (object_entry_get_source_game(objectEntry) == OBJECT_SOURCE_WACKY_WORLDS || - object_entry_get_source_game(objectEntry) == OBJECT_SOURCE_TIME_TWISTER || - object_entry_get_source_game(objectEntry) == OBJECT_SOURCE_CUSTOM)) + auto& objectRepository = context->GetObjectRepository(); + auto item = objectRepository.FindObject(identifier); + if (item != nullptr) { - SetPrimarySceneryGroup(&scgPathX); + auto sourceGame = object_entry_get_source_game(&item->ObjectEntry); + if (sourceGame == OBJECT_SOURCE_WACKY_WORLDS || + sourceGame == OBJECT_SOURCE_TIME_TWISTER || + sourceGame == OBJECT_SOURCE_CUSTOM) + { + auto scgPathX = Object::GetScgPathXHeader(); + SetPrimarySceneryGroup(&scgPathX); + } } } diff --git a/src/openrct2/object/Object.h b/src/openrct2/object/Object.h index e86a191ce7..43c78a6c16 100644 --- a/src/openrct2/object/Object.h +++ b/src/openrct2/object/Object.h @@ -111,6 +111,7 @@ struct rct_object_filters { assert_struct_size(rct_object_filters, 3); #pragma pack(pop) +interface IObjectRepository; interface IStream; struct ObjectRepositoryItem; struct rct_drawpixelinfo; @@ -119,6 +120,7 @@ interface IReadObjectContext { virtual ~IReadObjectContext() = default; + virtual IObjectRepository& GetObjectRepository() abstract; virtual bool ShouldLoadImages() abstract; virtual void LogWarning(uint32 code, const utf8 * text) abstract; @@ -206,7 +208,6 @@ sint32 object_calculate_checksum(const rct_object_entry * entry, const void * da bool find_object_in_entry_group(const rct_object_entry* entry, uint8* entry_type, uint8* entry_index); void object_create_identifier_name(char* string_buffer, size_t size, const rct_object_entry* object); -const rct_object_entry * object_list_find_by_name(const char *name); const rct_object_entry * object_list_find(rct_object_entry *entry); void object_entry_get_name_fixed(utf8 * buffer, size_t bufferSize, const rct_object_entry * entry); diff --git a/src/openrct2/object/ObjectFactory.cpp b/src/openrct2/object/ObjectFactory.cpp index 2956864330..8fcfdecf3d 100644 --- a/src/openrct2/object/ObjectFactory.cpp +++ b/src/openrct2/object/ObjectFactory.cpp @@ -41,6 +41,8 @@ class ReadObjectContext : public IReadObjectContext { private: + IObjectRepository& _objectRepository; + std::string _objectName; bool _loadImages; bool _wasWarning = false; @@ -50,12 +52,18 @@ public: bool WasWarning() const { return _wasWarning; } bool WasError() const { return _wasError; } - ReadObjectContext(const std::string &objectName, bool loadImages) - : _objectName(objectName), + ReadObjectContext(IObjectRepository& objectRepository, const std::string &objectName, bool loadImages) + : _objectRepository(objectRepository), + _objectName(objectName), _loadImages(loadImages) { } + IObjectRepository& GetObjectRepository() override + { + return _objectRepository; + } + bool ShouldLoadImages() override { return _loadImages; @@ -101,9 +109,9 @@ namespace ObjectFactory } } - Object * CreateObjectFromLegacyFile(const utf8 * path) + Object * CreateObjectFromLegacyFile(IObjectRepository& objectRepository, const utf8 * path) { - log_verbose("CreateObjectFromLegacyFile(\"%s\")", path); + log_verbose("CreateObjectFromLegacyFile(..., \"%s\")", path); Object * result = nullptr; try @@ -122,7 +130,7 @@ namespace ObjectFactory log_verbose(" size: %zu", chunk->GetLength()); auto chunkStream = MemoryStream(chunk->GetData(), chunk->GetLength()); - auto readContext = ReadObjectContext(objectName, !gOpenRCT2Headless); + auto readContext = ReadObjectContext(objectRepository, objectName, !gOpenRCT2Headless); ReadObjectLegacy(result, &readContext, &chunkStream); if (readContext.WasError()) { @@ -137,7 +145,7 @@ namespace ObjectFactory return result; } - Object * CreateObjectFromLegacyData(const rct_object_entry * entry, const void * data, size_t dataSize) + Object * CreateObjectFromLegacyData(IObjectRepository& objectRepository, const rct_object_entry * entry, const void * data, size_t dataSize) { Guard::ArgumentNotNull(entry, GUARD_LINE); Guard::ArgumentNotNull(data, GUARD_LINE); @@ -148,7 +156,7 @@ namespace ObjectFactory utf8 objectName[DAT_NAME_LENGTH + 1]; object_entry_get_name_fixed(objectName, sizeof(objectName), entry); - auto readContext = ReadObjectContext(objectName, !gOpenRCT2Headless); + auto readContext = ReadObjectContext(objectRepository, objectName, !gOpenRCT2Headless); auto chunkStream = MemoryStream(data, dataSize); ReadObjectLegacy(result, &readContext, &chunkStream); @@ -220,7 +228,7 @@ namespace ObjectFactory return 0xFF; } - Object * CreateObjectFromJsonFile(const std::string &path) + Object * CreateObjectFromJsonFile(IObjectRepository& objectRepository, const std::string &path) { log_verbose("CreateObjectFromJsonFile(\"%s\")", path.c_str()); @@ -249,7 +257,7 @@ namespace ObjectFactory memcpy(entry.name, originalName.c_str(), minLength); result = CreateObject(entry); - auto readContext = ReadObjectContext(id, !gOpenRCT2Headless); + auto readContext = ReadObjectContext(objectRepository, id, !gOpenRCT2Headless); result->ReadJson(&readContext, jRoot); if (readContext.WasError()) { diff --git a/src/openrct2/object/ObjectFactory.h b/src/openrct2/object/ObjectFactory.h index 4e88179058..4a05b87bef 100644 --- a/src/openrct2/object/ObjectFactory.h +++ b/src/openrct2/object/ObjectFactory.h @@ -18,14 +18,15 @@ #include "../common.h" +interface IObjectRepository; class Object; struct rct_object_entry; namespace ObjectFactory { - Object * CreateObjectFromLegacyFile(const utf8 * path); - Object * CreateObjectFromLegacyData(const rct_object_entry * entry, const void * data, size_t dataSize); + Object * CreateObjectFromLegacyFile(IObjectRepository& objectRepository, const utf8 * path); + Object * CreateObjectFromLegacyData(IObjectRepository& objectRepository, const rct_object_entry * entry, const void * data, size_t dataSize); Object * CreateObject(const rct_object_entry &entry); - Object * CreateObjectFromJsonFile(const std::string &path); + Object * CreateObjectFromJsonFile(IObjectRepository& objectRepository, const std::string &path); } diff --git a/src/openrct2/object/ObjectJsonHelpers.cpp b/src/openrct2/object/ObjectJsonHelpers.cpp index b80dd24e22..179245c6a3 100644 --- a/src/openrct2/object/ObjectJsonHelpers.cpp +++ b/src/openrct2/object/ObjectJsonHelpers.cpp @@ -234,7 +234,7 @@ namespace ObjectJsonHelpers { std::vector result; auto objectPath = FindLegacyObject(name); - auto obj = ObjectFactory::CreateObjectFromLegacyFile(objectPath.c_str()); + auto obj = ObjectFactory::CreateObjectFromLegacyFile(context->GetObjectRepository(), objectPath.c_str()); if (obj != nullptr) { auto &imgTable = static_cast(obj)->GetImageTable(); diff --git a/src/openrct2/object/ObjectRepository.cpp b/src/openrct2/object/ObjectRepository.cpp index 60785db976..42a5f18799 100644 --- a/src/openrct2/object/ObjectRepository.cpp +++ b/src/openrct2/object/ObjectRepository.cpp @@ -83,8 +83,10 @@ private: static constexpr uint16 VERSION = 17; static constexpr auto PATTERN = "*.dat;*.pob;*.json"; + IObjectRepository& _objectRepository; + public: - explicit ObjectFileIndex(IPlatformEnvironment * env) : + explicit ObjectFileIndex(IObjectRepository& objectRepository, IPlatformEnvironment * env) : FileIndex("object index", MAGIC_NUMBER, VERSION, @@ -92,7 +94,8 @@ public: std::string(PATTERN), std::vector({ env->GetDirectoryPath(DIRBASE::OPENRCT2, DIRID::OBJECT), - env->GetDirectoryPath(DIRBASE::USER, DIRID::OBJECT) })) + env->GetDirectoryPath(DIRBASE::USER, DIRID::OBJECT) })), + _objectRepository(objectRepository) { } @@ -102,7 +105,7 @@ public: auto extension = Path::GetExtension(path); if (String::Equals(extension, ".json", true)) { - auto object = ObjectFactory::CreateObjectFromJsonFile(path); + auto object = ObjectFactory::CreateObjectFromJsonFile(_objectRepository, path); if (object != nullptr) { ObjectRepositoryItem item = { 0 }; @@ -116,7 +119,7 @@ public: } else { - auto object = ObjectFactory::CreateObjectFromLegacyFile(path.c_str()); + auto object = ObjectFactory::CreateObjectFromLegacyFile(_objectRepository, path.c_str()); if (object != nullptr) { ObjectRepositoryItem item = { 0 }; @@ -213,7 +216,7 @@ class ObjectRepository final : public IObjectRepository public: explicit ObjectRepository(IPlatformEnvironment * env) : _env(env), - _fileIndex(env) + _fileIndex(*this, env) { } @@ -279,11 +282,11 @@ public: auto extension = Path::GetExtension(ori->Path); if (String::Equals(extension, ".json", true)) { - return ObjectFactory::CreateObjectFromJsonFile(ori->Path); + return ObjectFactory::CreateObjectFromJsonFile(*this, ori->Path); } else { - return ObjectFactory::CreateObjectFromLegacyFile(ori->Path); + return ObjectFactory::CreateObjectFromLegacyFile(*this, ori->Path); } } @@ -310,7 +313,7 @@ public: object_entry_get_name_fixed(objectName, sizeof(objectName), objectEntry); // Check that the object is loadable before writing it - Object * object = ObjectFactory::CreateObjectFromLegacyData(objectEntry, data, dataSize); + Object * object = ObjectFactory::CreateObjectFromLegacyData(*this, objectEntry, data, dataSize); if (object == nullptr) { Console::Error::WriteLine("[%s] Unable to export object.", objectName); @@ -659,18 +662,6 @@ const rct_object_entry * object_list_find(rct_object_entry * entry) return result; } -const rct_object_entry * object_list_find_by_name(const char * name) -{ - const rct_object_entry * result = nullptr; - auto objRepo = GetContext()->GetObjectRepository(); - auto item = objRepo->FindObject(name); - if (item != nullptr) - { - result = &item->ObjectEntry; - } - return result; -} - void object_list_load() { auto context = GetContext(); diff --git a/src/openrct2/rct1/S4Importer.cpp b/src/openrct2/rct1/S4Importer.cpp index d865c0dc14..b4bec07511 100644 --- a/src/openrct2/rct1/S4Importer.cpp +++ b/src/openrct2/rct1/S4Importer.cpp @@ -551,10 +551,11 @@ private: std::vector objects = RCT1::GetSceneryObjects(sceneryTheme); for (const char * objectName : objects) { - const rct_object_entry * foundEntry = object_list_find_by_name(objectName); - if (foundEntry != nullptr) + auto objectRepository = OpenRCT2::GetContext()->GetObjectRepository(); + auto foundObject = objectRepository->FindObject(objectName); + if (foundObject != nullptr) { - uint8 objectType = object_entry_get_type(foundEntry); + uint8 objectType = object_entry_get_type(&foundObject->ObjectEntry); switch (objectType) { case OBJECT_TYPE_SMALL_SCENERY: case OBJECT_TYPE_LARGE_SCENERY: From 4ce4101ac58ad4b9bb3f4cb72215d50a660be4de Mon Sep 17 00:00:00 2001 From: Ted John Date: Fri, 20 Apr 2018 00:12:40 +0100 Subject: [PATCH 4/6] Ensure correct language is used for scan-objects --- src/openrct2/cmdline/RootCommands.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/openrct2/cmdline/RootCommands.cpp b/src/openrct2/cmdline/RootCommands.cpp index 080350ec54..c42009b293 100644 --- a/src/openrct2/cmdline/RootCommands.cpp +++ b/src/openrct2/cmdline/RootCommands.cpp @@ -397,12 +397,13 @@ static exitcode_t HandleCommandScanObjects(CommandLineArgEnumerator * enumerator gOpenRCT2Headless = true; - // HACK: set gCurrentLanguage otherwise it be wrong for the index file - gCurrentLanguage = gConfigGeneral.language; - auto context = std::unique_ptr(OpenRCT2::CreateContext()); auto env = context->GetPlatformEnvironment(); auto objectRepository = std::unique_ptr(CreateObjectRepository(env)); + + // HACK: set gCurrentLanguage otherwise it be wrong for the index file + gCurrentLanguage = gConfigGeneral.language; + objectRepository->Construct(); return EXITCODE_OK; } From 24b3df58db0d6838e0e07a431244914f9c0db2b5 Mon Sep 17 00:00:00 2001 From: Ted John Date: Fri, 20 Apr 2018 00:23:07 +0100 Subject: [PATCH 5/6] Use low importance for scan-objects in msbuild proj --- openrct2.proj | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openrct2.proj b/openrct2.proj index df905159f8..b4a90b3d09 100644 --- a/openrct2.proj +++ b/openrct2.proj @@ -189,7 +189,8 @@ + WorkingDirectory="$(TargetDir)" + StandardOutputImportance="normal" /> Date: Sun, 22 Apr 2018 22:42:20 +0200 Subject: [PATCH 6/6] Execute scan-objects on Travis --- scripts/linux/build.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/linux/build.sh b/scripts/linux/build.sh index a46c6e8374..4e31e6bddc 100755 --- a/scripts/linux/build.sh +++ b/scripts/linux/build.sh @@ -28,15 +28,15 @@ pushd build if [[ $TARGET == "docker64" ]] then # CMAKE and MAKE opts from environment - docker run -v "$PARENT":"$PARENT" -w "$PARENT"/build -i -t openrct2/openrct2:64bit-only bash -c "cmake ../ -DWITH_TESTS=on $OPENRCT2_CMAKE_OPTS && ninja all install $OPENRCT_MAKE_OPTS && ctest --output-on-failure" + docker run -v "$PARENT":"$PARENT" -w "$PARENT"/build -i -t openrct2/openrct2:64bit-only bash -c "cmake ../ -DWITH_TESTS=on $OPENRCT2_CMAKE_OPTS && ninja all install $OPENRCT_MAKE_OPTS && ./openrct2-cli scan-objects && ctest --output-on-failure" elif [[ $TARGET == "ubuntu_i686" ]] then # CMAKE and MAKE opts from environment - docker run -v "$PARENT":"$PARENT" -w "$PARENT"/build -i -t openrct2/openrct2:ubuntu_i686 bash -c "cmake ../ -DWITH_TESTS=on $OPENRCT2_CMAKE_OPTS && ninja all testpaint install $OPENRCT_MAKE_OPTS && ctest --output-on-failure && ( ./testpaint --quiet || if [[ \$? -eq 1 ]] ; then echo Allowing failed tests to pass ; else echo here ; false; fi )" + docker run -v "$PARENT":"$PARENT" -w "$PARENT"/build -i -t openrct2/openrct2:ubuntu_i686 bash -c "cmake ../ -DWITH_TESTS=on $OPENRCT2_CMAKE_OPTS && ninja all testpaint install $OPENRCT_MAKE_OPTS && ./openrct2-cli scan-objects && ctest --output-on-failure && ( ./testpaint --quiet || if [[ \$? -eq 1 ]] ; then echo Allowing failed tests to pass ; else echo here ; false; fi )" elif [[ $TARGET == "ubuntu_amd64" ]] then # CMAKE and MAKE opts from environment - docker run -v "$PARENT":"$PARENT" -w "$PARENT"/build -i -t openrct2/openrct2:ubuntu_amd64 bash -c "cmake ../ -DWITH_TESTS=on $OPENRCT2_CMAKE_OPTS && ninja $OPENRCT_MAKE_OPTS install && ctest --output-on-failure" + docker run -v "$PARENT":"$PARENT" -w "$PARENT"/build -i -t openrct2/openrct2:ubuntu_amd64 bash -c "cmake ../ -DWITH_TESTS=on $OPENRCT2_CMAKE_OPTS && ninja $OPENRCT_MAKE_OPTS install && ./openrct2-cli scan-objects && ctest --output-on-failure" elif [[ $TARGET == "windows" ]] then # CMAKE and MAKE opts from environment