From 3222e788155e17931e3b0af82f35239722f6c847 Mon Sep 17 00:00:00 2001 From: David Markowitz <39972741+EmosewaMC@users.noreply.github.com> Date: Sun, 27 Nov 2022 16:48:46 -0800 Subject: [PATCH] Implement undo action for pre-built models (#830) Brick building as of right now does not implement the undo action properly. This commit addresses the issue with undoing button being non-functional server side and implements the GM needed for addressing further issues. Implement GameMessage UnUseModel which is called when a model in BrickBuilding is UnUsed. Important for UGC content down the line. Final code has been tested as follows: 1. Placed a model in brick build 2. saved placed a brick 3. repeat 2 and 3 twice more for 6 total models 4. Place a new model in brick mode and then edit all 7 models into one brick model instance 5. Pressing undo returns the converted model to the inventory and properly discards the other 6 without crashing. Intended live behavior is to store this in the inventory instead however behind the scenes work is needed to implement UGC models properly. Implement enum Implement the BlueprintSaveResponseType enum so there are less magic numbers sent via packets. Correct int sizes from unsigned int to uint32_t Add deserialize test Add a test for de-serializing a GM that is sent to the client. Assertions verify the data is in the correct order and has no extra information. --- dCommon/dEnums/dCommonVars.h | 1 + dCommon/dEnums/dMessageIdentifiers.h | 1 + dCommon/eBlueprintSaveResponseType.h | 26 +++++++ dGame/dComponents/InventoryComponent.cpp | 13 ++-- .../PropertyManagementComponent.cpp | 2 +- dGame/dGameMessages/GameMessageHandler.cpp | 4 ++ dGame/dGameMessages/GameMessages.cpp | 70 ++++++++++++++++--- dGame/dGameMessages/GameMessages.h | 11 +++ dGame/dInventory/Inventory.h | 2 +- dMasterServer/ObjectIDManager.cpp | 4 +- dWorldServer/WorldServer.cpp | 7 +- tests/dGameTests/CMakeLists.txt | 3 + tests/dGameTests/GameDependencies.h | 9 ++- .../dGameMessagesTests/CMakeLists.txt | 9 +++ .../dGameMessagesTests/GameMessageTests.cpp | 52 ++++++++++++++ 15 files changed, 188 insertions(+), 26 deletions(-) create mode 100644 dCommon/eBlueprintSaveResponseType.h create mode 100644 tests/dGameTests/dGameMessagesTests/CMakeLists.txt create mode 100644 tests/dGameTests/dGameMessagesTests/GameMessageTests.cpp diff --git a/dCommon/dEnums/dCommonVars.h b/dCommon/dEnums/dCommonVars.h index 005d7205..74d83b11 100644 --- a/dCommon/dEnums/dCommonVars.h +++ b/dCommon/dEnums/dCommonVars.h @@ -434,6 +434,7 @@ enum eInventoryType : uint32_t { ITEMS = 0, VAULT_ITEMS, BRICKS, + MODELS_IN_BBB, TEMP_ITEMS = 4, MODELS, TEMP_MODELS, diff --git a/dCommon/dEnums/dMessageIdentifiers.h b/dCommon/dEnums/dMessageIdentifiers.h index 5f7e9128..5a5c9088 100644 --- a/dCommon/dEnums/dMessageIdentifiers.h +++ b/dCommon/dEnums/dMessageIdentifiers.h @@ -433,6 +433,7 @@ enum GAME_MSG : unsigned short { GAME_MSG_ORIENT_TO_POSITION = 906, GAME_MSG_ORIENT_TO_ANGLE = 907, GAME_MSG_BOUNCER_ACTIVE_STATUS = 942, + GAME_MSG_UN_USE_BBB_MODEL = 999, GAME_MSG_BBB_LOAD_ITEM_REQUEST = 1000, GAME_MSG_BBB_SAVE_REQUEST = 1001, GAME_MSG_BBB_SAVE_RESPONSE = 1006, diff --git a/dCommon/eBlueprintSaveResponseType.h b/dCommon/eBlueprintSaveResponseType.h new file mode 100644 index 00000000..29d15695 --- /dev/null +++ b/dCommon/eBlueprintSaveResponseType.h @@ -0,0 +1,26 @@ +#pragma once + +#ifndef __EBLUEPRINTSAVERESPONSETYPE__H__ +#define __EBLUEPRINTSAVERESPONSETYPE__H__ + +#include + +enum class eBlueprintSaveResponseType : uint32_t { + EverythingWorked = 0, + SaveCancelled, + CantBeginTransaction, + SaveBlueprintFailed, + SaveUgobjectFailed, + CantEndTransaction, + SaveFilesFailed, + BadInput, + NotEnoughBricks, + InventoryFull, + ModelGenerationFailed, + PlacementFailed, + GmLevelInsufficient, + WaitForPreviousSave, + FindMatchesFailed +}; + +#endif //!__EBLUEPRINTSAVERESPONSETYPE__H__ diff --git a/dGame/dComponents/InventoryComponent.cpp b/dGame/dComponents/InventoryComponent.cpp index 9f53cfa3..77f3f035 100644 --- a/dGame/dComponents/InventoryComponent.cpp +++ b/dGame/dComponents/InventoryComponent.cpp @@ -606,16 +606,17 @@ void InventoryComponent::UpdateXml(tinyxml2::XMLDocument* document) { return; } - std::vector inventories; + std::vector inventoriesToSave; + // Need to prevent some transfer inventories from being saved for (const auto& pair : this->m_Inventories) { auto* inventory = pair.second; - if (inventory->GetType() == VENDOR_BUYBACK) { + if (inventory->GetType() == VENDOR_BUYBACK || inventory->GetType() == eInventoryType::MODELS_IN_BBB) { continue; } - inventories.push_back(inventory); + inventoriesToSave.push_back(inventory); } inventoryElement->SetAttribute("csl", m_Consumable); @@ -630,7 +631,7 @@ void InventoryComponent::UpdateXml(tinyxml2::XMLDocument* document) { bags->DeleteChildren(); - for (const auto* inventory : inventories) { + for (const auto* inventory : inventoriesToSave) { auto* bag = document->NewElement("b"); bag->SetAttribute("t", inventory->GetType()); @@ -649,7 +650,7 @@ void InventoryComponent::UpdateXml(tinyxml2::XMLDocument* document) { items->DeleteChildren(); - for (auto* inventory : inventories) { + for (auto* inventory : inventoriesToSave) { if (inventory->GetSize() == 0) { continue; } @@ -1260,7 +1261,7 @@ BehaviorSlot InventoryComponent::FindBehaviorSlot(const eItemType type) { } bool InventoryComponent::IsTransferInventory(eInventoryType type) { - return type == VENDOR_BUYBACK || type == VAULT_ITEMS || type == VAULT_MODELS || type == TEMP_ITEMS || type == TEMP_MODELS; + return type == VENDOR_BUYBACK || type == VAULT_ITEMS || type == VAULT_MODELS || type == TEMP_ITEMS || type == TEMP_MODELS || type == MODELS_IN_BBB; } uint32_t InventoryComponent::FindSkill(const LOT lot) { diff --git a/dGame/dComponents/PropertyManagementComponent.cpp b/dGame/dComponents/PropertyManagementComponent.cpp index d196e935..eaade8be 100644 --- a/dGame/dComponents/PropertyManagementComponent.cpp +++ b/dGame/dComponents/PropertyManagementComponent.cpp @@ -474,7 +474,7 @@ void PropertyManagementComponent::DeleteModel(const LWOOBJID id, const int delet settings.push_back(propertyObjectID); settings.push_back(modelType); - inventoryComponent->AddItem(6662, 1, eLootSourceType::LOOT_SOURCE_DELETION, eInventoryType::HIDDEN, settings, LWOOBJID_EMPTY, false, false, spawnerId); + inventoryComponent->AddItem(6662, 1, eLootSourceType::LOOT_SOURCE_DELETION, eInventoryType::MODELS_IN_BBB, settings, LWOOBJID_EMPTY, false, false, spawnerId); auto* item = inventoryComponent->FindItemBySubKey(spawnerId); if (item == nullptr) { diff --git a/dGame/dGameMessages/GameMessageHandler.cpp b/dGame/dGameMessages/GameMessageHandler.cpp index 2ce79966..b7687c19 100644 --- a/dGame/dGameMessages/GameMessageHandler.cpp +++ b/dGame/dGameMessages/GameMessageHandler.cpp @@ -46,6 +46,10 @@ void GameMessageHandler::HandleMessage(RakNet::BitStream* inStream, const System switch (messageID) { + case GAME_MSG_UN_USE_BBB_MODEL: { + GameMessages::HandleUnUseModel(inStream, entity, sysAddr); + break; + } case GAME_MSG_PLAY_EMOTE: { GameMessages::HandlePlayEmote(inStream, entity); break; diff --git a/dGame/dGameMessages/GameMessages.cpp b/dGame/dGameMessages/GameMessages.cpp index 896dfa0c..6639197c 100644 --- a/dGame/dGameMessages/GameMessages.cpp +++ b/dGame/dGameMessages/GameMessages.cpp @@ -70,6 +70,7 @@ #include "TradingManager.h" #include "ControlBehaviors.h" #include "AMFDeserialize.h" +#include "eBlueprintSaveResponseType.h" void GameMessages::SendFireEventClientSide(const LWOOBJID& objectID, const SystemAddress& sysAddr, std::u16string args, const LWOOBJID& object, int64_t param1, int param2, const LWOOBJID& sender) { CBITSTREAM; @@ -2139,6 +2140,32 @@ void GameMessages::HandleSetPropertyAccess(RakNet::BitStream* inStream, Entity* PropertyManagementComponent::Instance()->SetPrivacyOption(static_cast(accessType)); } +void GameMessages::HandleUnUseModel(RakNet::BitStream* inStream, Entity* entity, const SystemAddress& sysAddr) { + bool unknown{}; + LWOOBJID objIdToAddToInventory{}; + inStream->Read(unknown); + inStream->Read(objIdToAddToInventory); + auto* inventoryComponent = entity->GetComponent(); + if (inventoryComponent) { + auto* inventory = inventoryComponent->GetInventory(eInventoryType::MODELS_IN_BBB); + auto* item = inventory->FindItemById(objIdToAddToInventory); + if (item) { + inventoryComponent->MoveItemToInventory(item, eInventoryType::MODELS, 1); + } else { + Game::logger->Log("GameMessages", "item id %llu not found in MODELS_IN_BBB inventory, likely because it does not exist", objIdToAddToInventory); + } + } + + if (unknown) { + CBITSTREAM; + PacketUtils::WriteHeader(bitStream, CLIENT, MSG_CLIENT_BLUEPRINT_SAVE_RESPONSE); + bitStream.Write(LWOOBJID_EMPTY); //always zero so that a check on the client passes + bitStream.Write(eBlueprintSaveResponseType::PlacementFailed); // Sending a non-zero error code here prevents the client from deleting its in progress build for some reason? + bitStream.Write(0); + SEND_PACKET; + } +} + void GameMessages::HandleUpdatePropertyOrModelForFilterCheck(RakNet::BitStream* inStream, Entity* entity, const SystemAddress& sysAddr) { bool isProperty{}; LWOOBJID objectId{}; @@ -2353,16 +2380,40 @@ void GameMessages::HandleDeletePropertyModel(RakNet::BitStream* inStream, Entity } void GameMessages::HandleBBBLoadItemRequest(RakNet::BitStream* inStream, Entity* entity, const SystemAddress& sysAddr) { - LWOOBJID itemID = LWOOBJID_EMPTY; - inStream->Read(itemID); + LWOOBJID previousItemID = LWOOBJID_EMPTY; + inStream->Read(previousItemID); - Game::logger->Log("BBB", "Load item request for: %lld", itemID); + Game::logger->Log("BBB", "Load item request for: %lld", previousItemID); + LWOOBJID newId = previousItemID; + auto* inventoryComponent = entity->GetComponent(); + if (inventoryComponent) { + auto* inventory = inventoryComponent->GetInventory(eInventoryType::MODELS); + auto* itemToMove = inventory->FindItemById(previousItemID); + if (itemToMove) { + LOT previousLot = itemToMove->GetLot(); + inventoryComponent->MoveItemToInventory(itemToMove, eInventoryType::MODELS_IN_BBB, 1, false); + + auto* destinationInventory = inventoryComponent->GetInventory(eInventoryType::MODELS_IN_BBB); + if (destinationInventory) { + auto* movedItem = destinationInventory->FindItemByLot(previousLot); + if (movedItem) newId = movedItem->GetId(); + } + } else { + Game::logger->Log("GameMessages", "item id %llu not found in MODELS inventory, likely because it does not exist", previousItemID); + } + } + + // Second argument always true (successful) for now + SendBlueprintLoadItemResponse(sysAddr, true, previousItemID, newId); +} + +void GameMessages::SendBlueprintLoadItemResponse(const SystemAddress& sysAddr, bool success, LWOOBJID oldItemId, LWOOBJID newItemId) { CBITSTREAM; PacketUtils::WriteHeader(bitStream, CLIENT, MSG_CLIENT_BLUEPRINT_LOAD_RESPONSE_ITEMID); - bitStream.Write(static_cast(1)); - bitStream.Write(itemID); - bitStream.Write(itemID); + bitStream.Write(static_cast(success)); + bitStream.Write(oldItemId); + bitStream.Write(newItemId); SEND_PACKET; } @@ -2413,7 +2464,7 @@ void GameMessages::HandleControlBehaviors(RakNet::BitStream* inStream, Entity* e } auto owner = PropertyManagementComponent::Instance()->GetOwner(); - if (!owner) return; + if (!owner) return; ControlBehaviors::ProcessCommand(entity, sysAddr, static_cast(amfArguments.get()), command, owner); } @@ -2609,8 +2660,8 @@ void GameMessages::HandleBBBSaveRequest(RakNet::BitStream* inStream, Entity* ent CBITSTREAM; PacketUtils::WriteHeader(bitStream, CLIENT, CLIENT::MSG_CLIENT_BLUEPRINT_SAVE_RESPONSE); bitStream.Write(localId); - bitStream.Write(0); - bitStream.Write(1); + bitStream.Write(eBlueprintSaveResponseType::EverythingWorked); + bitStream.Write(1); bitStream.Write(blueprintID); bitStream.Write(sd0Size); @@ -2620,7 +2671,6 @@ void GameMessages::HandleBBBSaveRequest(RakNet::BitStream* inStream, Entity* ent } SEND_PACKET; - // PacketUtils::SavePacket("MSG_CLIENT_BLUEPRINT_SAVE_RESPONSE.bin", (char*)bitStream.GetData(), bitStream.GetNumberOfBytesUsed()); //Now we have to construct this object: diff --git a/dGame/dGameMessages/GameMessages.h b/dGame/dGameMessages/GameMessages.h index 3594a86a..f9968d14 100644 --- a/dGame/dGameMessages/GameMessages.h +++ b/dGame/dGameMessages/GameMessages.h @@ -121,6 +121,7 @@ namespace GameMessages { void SendMatchResponse(Entity* entity, const SystemAddress& sysAddr, int response); void SendMatchUpdate(Entity* entity, const SystemAddress& sysAddr, std::string data, int type); + void HandleUnUseModel(RakNet::BitStream* inStream, Entity* entity, const SystemAddress& sysAddr); void SendStartCelebrationEffect(Entity* entity, const SystemAddress& sysAddr, int celebrationID); /** @@ -197,6 +198,16 @@ namespace GameMessages { void SendDownloadPropertyData(LWOOBJID objectId, const PropertyDataMessage& data, const SystemAddress& sysAddr); + /** + * @brief Send an updated item id to the client when they load a blueprint in brick build mode + * + * @param sysAddr SystemAddress to respond to + * @param oldItemId The item ID that was requested to be loaded + * @param newItemId The new item ID of the loaded item + * + */ + void SendBlueprintLoadItemResponse(const SystemAddress& sysAddr, bool success, LWOOBJID oldItemId, LWOOBJID newItemId); + void SendPropertyRentalResponse(LWOOBJID objectId, LWOCLONEID cloneId, uint32_t code, LWOOBJID propertyId, int64_t rentDue, const SystemAddress& sysAddr); void SendLockNodeRotation(Entity* entity, std::string nodeName); diff --git a/dGame/dInventory/Inventory.h b/dGame/dInventory/Inventory.h index 30f753da..6c6a4306 100644 --- a/dGame/dInventory/Inventory.h +++ b/dGame/dInventory/Inventory.h @@ -95,7 +95,7 @@ public: * @param lot the lot to find items for * @param ignoreEquipped ignores equipped items * @param ignoreBound ignores bound items - * @return item in the inventory for the provided LOT + * @return item with the lowest stack count in the inventory for the provided LOT */ Item* FindItemByLot(LOT lot, bool ignoreEquipped = false, bool ignoreBound = false) const; diff --git a/dMasterServer/ObjectIDManager.cpp b/dMasterServer/ObjectIDManager.cpp index 0f3b98c9..83dde8dd 100644 --- a/dMasterServer/ObjectIDManager.cpp +++ b/dMasterServer/ObjectIDManager.cpp @@ -51,13 +51,13 @@ uint32_t ObjectIDManager::GeneratePersistentID(void) { uint32_t toReturn = ++this->currentPersistentID; // So we peroidically save our ObjID to the database: - if (toReturn % 25 == 0) { // TEMP: DISABLED FOR DEBUG / DEVELOPMENT! + // if (toReturn % 25 == 0) { // TEMP: DISABLED FOR DEBUG / DEVELOPMENT! sql::PreparedStatement* stmt = Database::CreatePreppedStmt( "UPDATE object_id_tracker SET last_object_id=?"); stmt->setUInt(1, toReturn); stmt->execute(); delete stmt; - } + // } return toReturn; } diff --git a/dWorldServer/WorldServer.cpp b/dWorldServer/WorldServer.cpp index b7d7cc14..16e0b1c1 100644 --- a/dWorldServer/WorldServer.cpp +++ b/dWorldServer/WorldServer.cpp @@ -57,6 +57,7 @@ #include "Player.h" #include "PropertyManagementComponent.h" #include "AssetManager.h" +#include "eBlueprintSaveResponseType.h" #include "ZCompression.h" @@ -1082,9 +1083,9 @@ void HandlePacket(Packet* packet) { CBITSTREAM; PacketUtils::WriteHeader(bitStream, CLIENT, MSG_CLIENT_BLUEPRINT_SAVE_RESPONSE); - bitStream.Write(0); //always zero so that a check on the client passes - bitStream.Write(0); - bitStream.Write(1); + bitStream.Write(LWOOBJID_EMPTY); //always zero so that a check on the client passes + bitStream.Write(eBlueprintSaveResponseType::EverythingWorked); + bitStream.Write(1); bitStream.Write(blueprintID); bitStream.Write(lxfmlSize); diff --git a/tests/dGameTests/CMakeLists.txt b/tests/dGameTests/CMakeLists.txt index 68192b3f..ba7d4d1c 100644 --- a/tests/dGameTests/CMakeLists.txt +++ b/tests/dGameTests/CMakeLists.txt @@ -5,6 +5,9 @@ set(DGAMETEST_SOURCES add_subdirectory(dComponentsTests) list(APPEND DGAMETEST_SOURCES ${DCOMPONENTS_TESTS}) +add_subdirectory(dGameMessagesTests) +list(APPEND DGAMETEST_SOURCES ${DGAMEMESSAGES_TESTS}) + # Add the executable. Remember to add all tests above this! add_executable(dGameTests ${DGAMETEST_SOURCES}) diff --git a/tests/dGameTests/GameDependencies.h b/tests/dGameTests/GameDependencies.h index 593ec0fc..ee52dec6 100644 --- a/tests/dGameTests/GameDependencies.h +++ b/tests/dGameTests/GameDependencies.h @@ -5,15 +5,18 @@ #include "dLogger.h" #include "dServer.h" #include "EntityManager.h" -class dZoneManager; -class AssetManager; #include +class dZoneManager; +class AssetManager; + class dServerMock : public dServer { + RakNet::BitStream* sentBitStream = nullptr; public: dServerMock() {}; ~dServerMock() {}; - void Send(RakNet::BitStream* bitStream, const SystemAddress& sysAddr, bool broadcast) override {}; + RakNet::BitStream* GetMostRecentBitStream() { return sentBitStream; }; + void Send(RakNet::BitStream* bitStream, const SystemAddress& sysAddr, bool broadcast) override { sentBitStream = bitStream; }; }; class GameDependenciesTest : public ::testing::Test { diff --git a/tests/dGameTests/dGameMessagesTests/CMakeLists.txt b/tests/dGameTests/dGameMessagesTests/CMakeLists.txt new file mode 100644 index 00000000..2417d29c --- /dev/null +++ b/tests/dGameTests/dGameMessagesTests/CMakeLists.txt @@ -0,0 +1,9 @@ +SET(DGAMEMESSAGES_TESTS + "GameMessageTests.cpp") + +# Get the folder name and prepend it to the files above +get_filename_component(thisFolderName ${CMAKE_CURRENT_SOURCE_DIR} NAME) +list(TRANSFORM DGAMEMESSAGES_TESTS PREPEND "${thisFolderName}/") + +# Export to parent scope +set(DGAMEMESSAGES_TESTS ${DGAMEMESSAGES_TESTS} PARENT_SCOPE) diff --git a/tests/dGameTests/dGameMessagesTests/GameMessageTests.cpp b/tests/dGameTests/dGameMessagesTests/GameMessageTests.cpp new file mode 100644 index 00000000..3d8b2d04 --- /dev/null +++ b/tests/dGameTests/dGameMessagesTests/GameMessageTests.cpp @@ -0,0 +1,52 @@ +#include "GameMessages.h" +#include "GameDependencies.h" +#include + +class GameMessageTests : public GameDependenciesTest { + protected: + void SetUp() override { + SetUpDependencies(); + } + void TearDown() override { + TearDownDependencies(); + } +}; + +/** + * @brief Tests that the serialization struct BlueprintLoadItemResponse is serialized correctly + * + */ +TEST_F(GameMessageTests, SendBlueprintLoadItemResponse) { + GameMessages::SendBlueprintLoadItemResponse(UNASSIGNED_SYSTEM_ADDRESS, true, 515, 990); + auto* bitStream = static_cast(Game::server)->GetMostRecentBitStream(); + ASSERT_NE(bitStream, nullptr); + ASSERT_EQ(bitStream->GetNumberOfUnreadBits(), 200); + // First read in the packets' header + uint8_t rakNetPacketId{}; + uint16_t remoteConnectionType{}; + uint32_t packetId{}; + uint8_t always0{}; + + bitStream->Read(rakNetPacketId); + bitStream->Read(remoteConnectionType); + bitStream->Read(packetId); + bitStream->Read(always0); + ASSERT_EQ(rakNetPacketId, 0x53); + ASSERT_EQ(remoteConnectionType, 0x05); + ASSERT_EQ(packetId, 0x17); + ASSERT_EQ(always0, 0x00); + + // Next read in packet data + + uint8_t bSuccess{}; // unsigned bool + LWOOBJID previousId{}; + LWOOBJID newId{}; + bitStream->Read(bSuccess); + bitStream->Read(previousId); + bitStream->Read(newId); + ASSERT_EQ(bSuccess, static_cast(true)); + ASSERT_EQ(previousId, 515); + ASSERT_EQ(newId, 990); + + ASSERT_EQ(bitStream->GetNumberOfUnreadBits(), 0); +}