From 72463cfdb6197fb682644e8fea884e0b58e46a05 Mon Sep 17 00:00:00 2001 From: David Cernat Date: Tue, 10 Apr 2018 12:49:11 +0300 Subject: [PATCH 01/11] [Client] Refresh equipment for DedicatedPlayers when setting base info Additionally, move default fatigue value to DedicatedPlayer initialization. --- apps/openmw/mwmp/DedicatedPlayer.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/apps/openmw/mwmp/DedicatedPlayer.cpp b/apps/openmw/mwmp/DedicatedPlayer.cpp index 9b6533dbc..78a068a7d 100644 --- a/apps/openmw/mwmp/DedicatedPlayer.cpp +++ b/apps/openmw/mwmp/DedicatedPlayer.cpp @@ -43,7 +43,12 @@ DedicatedPlayer::DedicatedPlayer(RakNet::RakNetGUID guid) : BasePlayer(guid) { reference = 0; attack.pressed = 0; + creatureStats.mDead = false; + // Give this new character a temporary high fatigue so it doesn't spawn on + // the ground + creatureStats.mDynamic[2].mBase = 1000; + movementFlags = 0; attack.instant = false; @@ -142,9 +147,7 @@ void DedicatedPlayer::setBaseInfo() std::string recId = getNpcRecordId(); createReference(recId); - // Give this new character a temporary high fatigue of at least 1 so it doesn't - // spawn on the ground - creatureStats.mDynamic[2].mBase = 1000; + setEquipment(); } void DedicatedPlayer::setShapeshift() From 969759585743edc7d4b6c773c8a893e9c1aa29fb Mon Sep 17 00:00:00 2001 From: David Cernat Date: Tue, 10 Apr 2018 18:22:27 +0300 Subject: [PATCH 02/11] [Client] Don't equip already equipped items in local setEquipment() This avoids the following error when receiving repeated PlayerBaseInfo packets: "Error in frame: Invalid slot, make sure you are not calling RefData::setCount for a container object" Additionally, only re-equip items as the result of a PlayerBaseInfo packet if resetStats is true (because of its side effect of auto-equipping items for the player). --- apps/openmw/mwmp/LocalPlayer.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/apps/openmw/mwmp/LocalPlayer.cpp b/apps/openmw/mwmp/LocalPlayer.cpp index 214821eda..46ea85915 100644 --- a/apps/openmw/mwmp/LocalPlayer.cpp +++ b/apps/openmw/mwmp/LocalPlayer.cpp @@ -841,7 +841,10 @@ void LocalPlayer::setCharacter() MWBase::Environment::get().getWorld()->getPlayer().setBirthSign(birthsign); if (resetStats) + { MWBase::Environment::get().getMechanicsManager()->setPlayerRace(npc.mRace, npc.isMale(), npc.mHead, npc.mHair); + setEquipment(); + } else { ESM::NPC player = *world->getPlayerPtr().get()->mBase; @@ -855,8 +858,6 @@ void LocalPlayer::setCharacter() MWBase::Environment::get().getMechanicsManager()->playerLoaded(); } - setEquipment(); - MWBase::Environment::get().getWindowManager()->getInventoryWindow()->rebuildAvatar(); } else @@ -1103,7 +1104,11 @@ void LocalPlayer::setEquipment() } } else - ptrInventory.equip(slot, it, ptrPlayer); + { + // Don't try to equip an item that is already equipped + if (!ptrInventory.getSlot(slot).isEqual(it)) + ptrInventory.equip(slot, it, ptrPlayer); + } } else ptrInventory.unequipSlot(slot, ptrPlayer); From 9d27f5f154e5ee960dcbb4ed06a6970ca68b9659 Mon Sep 17 00:00:00 2001 From: David Cernat Date: Thu, 12 Apr 2018 14:18:19 +0300 Subject: [PATCH 03/11] [Client] Create RecordHelper class with initial NPC and creature methods --- apps/openmw/CMakeLists.txt | 2 +- apps/openmw/mwmp/RecordHelper.cpp | 48 +++++++++++++++++++++++++++++++ apps/openmw/mwmp/RecordHelper.hpp | 19 ++++++++++++ 3 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 apps/openmw/mwmp/RecordHelper.cpp create mode 100644 apps/openmw/mwmp/RecordHelper.hpp diff --git a/apps/openmw/CMakeLists.txt b/apps/openmw/CMakeLists.txt index b1af68eb5..a4e932eb5 100644 --- a/apps/openmw/CMakeLists.txt +++ b/apps/openmw/CMakeLists.txt @@ -98,7 +98,7 @@ add_openmw_dir (mwbase ) add_openmw_dir (mwmp Main Networking LocalPlayer DedicatedPlayer PlayerList LocalActor DedicatedActor ActorList WorldEvent - Cell CellController MechanicsHelper GUIController + Cell CellController MechanicsHelper RecordHelper GUIController ) add_openmw_dir (mwmp/GUI GUIChat GUILogin PlayerMarkerCollection GUIDialogList TextInputDialog diff --git a/apps/openmw/mwmp/RecordHelper.cpp b/apps/openmw/mwmp/RecordHelper.cpp new file mode 100644 index 000000000..032afcb0a --- /dev/null +++ b/apps/openmw/mwmp/RecordHelper.cpp @@ -0,0 +1,48 @@ +#include + +#include "../mwbase/environment.hpp" + +#include "../mwworld/worldimp.hpp" + +#include "RecordHelper.hpp" + +bool RecordHelper::doesCreatureExist(const std::string& refId) +{ + MWBase::World *world = MWBase::Environment::get().getWorld(); + + return world->getStore().get().search(refId); +} + +std::string RecordHelper::createCreatureRecord(const ESM::Creature& creature) +{ + MWBase::World *world = MWBase::Environment::get().getWorld(); + + return world->createRecord(creature)->mId; +} + +std::string RecordHelper::createNpcRecord(const ESM::NPC& npc) +{ + MWBase::World *world = MWBase::Environment::get().getWorld(); + + return world->createRecord(npc)->mId; +} + +void RecordHelper::updateCreatureRecord(const ESM::Creature& creature) +{ + MWBase::World *world = MWBase::Environment::get().getWorld(); + + MWWorld::ESMStore *esmStore = const_cast(&world->getStore()); + MWWorld::Store *creatureStore = const_cast *> (&esmStore->get()); + + creatureStore->insert(creature); +} + +void RecordHelper::updateNpcRecord(const ESM::NPC& npc) +{ + MWBase::World *world = MWBase::Environment::get().getWorld(); + + MWWorld::ESMStore *esmStore = const_cast(&world->getStore()); + MWWorld::Store *npcStore = const_cast *> (&esmStore->get()); + + npcStore->insert(npc); +} diff --git a/apps/openmw/mwmp/RecordHelper.hpp b/apps/openmw/mwmp/RecordHelper.hpp new file mode 100644 index 000000000..b66580e37 --- /dev/null +++ b/apps/openmw/mwmp/RecordHelper.hpp @@ -0,0 +1,19 @@ +#ifndef OPENMW_RECORDHELPER_HPP +#define OPENMW_RECORDHELPER_HPP + +#include +#include + +namespace RecordHelper +{ + bool doesCreatureExist(const std::string& refId); + + std::string createCreatureRecord(const ESM::Creature& creature); + std::string createNpcRecord(const ESM::NPC& npc); + + void updateCreatureRecord(const ESM::Creature& creature); + void updateNpcRecord(const ESM::NPC& npc); +} + + +#endif //OPENMW_RECORDHELPER_HPP From 49e94725aa2c8c2cb81ddab6d8b068b0a58857dd Mon Sep 17 00:00:00 2001 From: David Cernat Date: Thu, 12 Apr 2018 17:18:06 +0300 Subject: [PATCH 04/11] [Client] Prevent MechanicsManager::playerLoaded() from enabling AI --- apps/openmw/mwmechanics/mechanicsmanagerimp.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp b/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp index 0fb40ff0e..b39334c5c 100644 --- a/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp +++ b/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp @@ -870,7 +870,16 @@ namespace MWMechanics mUpdatePlayer = true; mClassSelected = true; mRaceSelected = true; - mAI = true; + + /* + Start of tes3mp change (major) + + Avoid enabling AI in multiplayer + */ + mAI = false; + /* + End of tes3mp change (major) + */ } bool MechanicsManager::isBoundItem(const MWWorld::Ptr& item) From a70e14e2824bf97f61bf010350af7770ff520ea9 Mon Sep 17 00:00:00 2001 From: scrawl <720642+scrawl@users.noreply.github.com> Date: Thu, 12 Apr 2018 15:16:52 +0000 Subject: [PATCH 05/11] Add guidelines for pull request reviews --- CONTRIBUTING.md | 63 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 52 insertions(+), 11 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b5543d11e..5218c2ed9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -8,10 +8,10 @@ https://bugs.openmw.org/ Currently, we are focused on completing the MW game experience and general polishing. Features out of this scope may be approved in some cases, but you should probably start a discussion first. Note: -- Tasks set to 'openmw-future' are usually out of the current scope of the project and can't be started yet. -- Bugs that are not 'Confirmed' should be confirmed first. -- Larger Features should have a discussion before you start implementing. -- In many cases, it's best to have a discussion about possible solutions before you jump into coding. +* Tasks set to 'openmw-future' are usually out of the current scope of the project and can't be started yet. +* Bugs that are not 'Confirmed' should be confirmed first. +* Larger Features should have a discussion before you start implementing. +* In many cases, it's best to have a discussion about possible solutions before you jump into coding. Aside from coding, you can also help by triaging the issues list. Check for bugs that are 'Unconfirmed' and try to confirm them on your end, working out any details that may be necessary. Check for bugs that do not conform to [Bug reporting guidelines](https://wiki.openmw.org/index.php?title=Bug_Reporting_Guidelines) and improve them to do so! @@ -20,7 +20,7 @@ There are various [Tools](https://wiki.openmw.org/index.php?title=Tools) to faci Pull Request Guidelines ======================= -Thought of a change? Great! To facilitate the review process, your pull request description should include the following (if applicable): +To facilitate the review process, your pull request description should include the following, if applicable: * A link back to the bug report or forum discussion that prompted the change * Summary of the changes made @@ -29,11 +29,12 @@ Thought of a change? Great! To facilitate the review process, your pull request Furthermore, we advise to: -* Separate your work into multiple pull requests whenever possible. As a rule of thumb, each feature and each bugfix should go into a separate PR, unless they are closely related or dependent upon each other. Small pull requests are easier to review, and are less likely to require further changes before we can merge them. A "mega" pull request with lots of unrelated commits in it is likely to get held up in review for a long time. +* Avoid stuffing unrelated commits into one pull request. As a rule of thumb, each feature and each bugfix should go into a separate PR, unless they are closely related or dependent upon each other. Small pull requests are easier to review, and are less likely to require further changes before we can merge them. A "mega" pull request with lots of unrelated commits in it is likely to get held up in review for a long time. * Feel free to submit incomplete pull requests. Even if the work can not be merged yet, pull requests are a great place to collect early feedback. Just make sure to mark it as *[Incomplete]* or *[Do not merge yet]* in the title. * If you plan on contributing often, please read the [Developer Reference](https://wiki.openmw.org/index.php?title=Developer_Reference) on our wiki, especially the [Policies and Standards](https://wiki.openmw.org/index.php?title=Policies_and_Standards). * Make sure each of your changes has a clear objective. Unnecessary changes may lead to merge conflicts, clutter the commit history and slow down review. Code formatting 'fixes' should be avoided, unless you were already changing that particular line anyway. * Reference the bug / feature ticket(s) in your commit message (e.g. 'Bug #123') to make it easier to keep track of what we changed for what reason. Our bugtracker will show those commits next to the ticket. If your commit message includes 'Fixes #123', that bug/feature will automatically be set to 'Closed' when your commit is merged. +* When pulling changes from master, prefer rebase over merge. Consider using a merge if there are conflicts or for long-running PRs. Guidelines for original engine "fixes" ================================= @@ -62,10 +63,50 @@ We get it, you have waited so long for feature XYZ to be available in Morrowind Unfortunately, since maintaining features comes at a cost and our resources are limited, we have to be a little selective in what features we allow into the main repository. Generally: -- Features should be as generic and non-redundant as possible. -- Any feature that is also possible with modding should be done as a mod instead. -- In the future, OpenMW plans to expand the scope of what is possible with modding, e.g. by moving certain game logic into editable scripts. -- Currently, modders can edit OpenMW's GUI skins and layout XML files, although there are still a few missing hooks (e.g. scripting support) in order to make this into a powerful way of modding. -- If a feature introduces new game UI strings, that reduces its chance of being accepted because we do not currently have any way of localizing these to the user's Morrowind installation language. +* Features should be as generic and non-redundant as possible. +* Any feature that is also possible with modding should be done as a mod instead. +* In the future, OpenMW plans to expand the scope of what is possible with modding, e.g. by moving certain game logic into editable scripts. +* Currently, modders can edit OpenMW's GUI skins and layout XML files, although there are still a few missing hooks (e.g. scripting support) in order to make this into a powerful way of modding. +* If a feature introduces new game UI strings, that reduces its chance of being accepted because we do not currently have any way of localizing these to the user's Morrowind installation language. If you are in doubt of your feature being within our scope, it is probably best to start a forum discussion first. See the [settings documentation](https://openmw.readthedocs.io/en/stable/reference/modding/settings/index.html) and [Features list](https://wiki.openmw.org/index.php?title=Features) for some examples of features that were deemed acceptable. + +Reviewing pull requests +======================= + +We welcome any help in reviewing open PRs. You don't need to be a developer to comment on new features. We also encourage ["junior" developers to review senior's work](https://pagefault.blog/2018/04/08/why-junior-devs-should-review-seniors-commits/). + +This review process is divided into two sections because complaining about code or style issues hardly makes sense until the functionality of the PR is deemed OK. Anyone can help with the Functionality Review while most parts of the Code Review require you to have programming experience. + +In addition to the checklist below, make sure to check that the Pull Request Guidelines (first half of this document) were followed. + +First review +============ + +1. Ask for missing information or clarifications. Compare against the project's design goals and roadmap. +2. Check if the automated tests are passing. If they are not, make the PR author aware of the issue and potentially link to the error line on Travis CI or Appveyor. If the error appears unrelated to the PR and/or the master branch is failing with the same error, our CI has broken and needs to be fixed independently of any open PRs. Raise this issue on the forums, bug tracker or with the relevant maintainer. The PR can be merged in this case as long as you've built it yourself to make sure it does build. +3. Make sure that the new code has been tested thoroughly, either by asking the author or, preferably, testing yourself. In a complex project like OpenMW, it is easy to make mistakes, typos, etc. Therefore, prefer testing all code changes, no matter how trivial they look. When you have tested a PR that no one has tested so far, post a comment letting us know. +4. On long running PRs, request the author to update its description with the current state or a checklist of things left to do. + +Code Review +=========== + +1. Carefully review each line for issues the author may not have thought of, paying special attention to 'special' cases. Often, people build their code with a particular mindset and forget about other configurations or unexpected interactions. +2. If any changes are workarounds for an issue in an upstream library, make sure the issue was reported upstream so we can eventually drop the workaround when the issue is fixed and the new version of that library is a build dependency. +3. Make sure PRs do not turn into arguments about hardly related issues. If the PR author disagrees with an established part of the project (e.g. supported build environments), they should open a forum discussion or bug report and in the meantime adjust the PR to adhere to the established way, rather than leaving the PR hanging on a dispute. +4. Check if the code matches our style guidelines. +5. Check to make sure the commit history is clean. Squashing should be considered if the review process has made the commit history particularly long. Commits that don't build should be avoided because they are a nuisance for ```git bisect```. + +Merging +======= + +To be able to merge PRs, commit priviledges are required. If you do not have the priviledges, just ping someone that does have them with a short comment like "Looks good to me @user". + +The person to merge the PR may either use github's Merge button or if using the command line, use the ```--no-ff``` flag and include the pull request number in the commit description. + + +Other resources +=============== + +[GitHub blog - how to write the perfect pull request](https://blog.github.com/2015-01-21-how-to-write-the-perfect-pull-request/) + From 70f9cb535eed5ad99fdeb895b6d6b83a3e2db6d8 Mon Sep 17 00:00:00 2001 From: David Cernat Date: Fri, 13 Apr 2018 08:37:06 +0300 Subject: [PATCH 06/11] [General] Use RecordHelper methods to create and update DedicatedPlayers --- apps/openmw/mwmp/DedicatedPlayer.cpp | 138 +++++++++--------- apps/openmw/mwmp/DedicatedPlayer.hpp | 7 +- apps/openmw/mwmp/RecordHelper.cpp | 7 + apps/openmw/mwmp/RecordHelper.hpp | 1 + .../player/ProcessorPlayerBaseInfo.hpp | 2 +- .../player/ProcessorPlayerShapeshift.hpp | 4 + components/openmw-mp/Base/BasePlayer.hpp | 2 + 7 files changed, 90 insertions(+), 71 deletions(-) diff --git a/apps/openmw/mwmp/DedicatedPlayer.cpp b/apps/openmw/mwmp/DedicatedPlayer.cpp index 78a068a7d..80f6216ac 100644 --- a/apps/openmw/mwmp/DedicatedPlayer.cpp +++ b/apps/openmw/mwmp/DedicatedPlayer.cpp @@ -34,6 +34,7 @@ #include "GUIController.hpp" #include "CellController.hpp" #include "MechanicsHelper.hpp" +#include "RecordHelper.hpp" using namespace mwmp; @@ -55,6 +56,10 @@ DedicatedPlayer::DedicatedPlayer(RakNet::RakNetGUID guid) : BasePlayer(guid) cell.blank(); position.pos[0] = position.pos[1] = Main::get().getCellController()->getCellSize() / 2; position.pos[2] = 0; + + MWBase::World *world = MWBase::Environment::get().getWorld(); + npc = *world->getPlayerPtr().get()->mBase; + npc.mId = "Dedicated Player"; } DedicatedPlayer::~DedicatedPlayer() { @@ -137,45 +142,85 @@ void DedicatedPlayer::move(float dt) void DedicatedPlayer::setBaseInfo() { - MWBase::World *world = MWBase::Environment::get().getWorld(); + static std::string previousRace; - if (reference) + // Use the previous race if the new one doesn't exist + if (!RecordHelper::doesRaceExist(npc.mRace)) + npc.mRace = previousRace; + + if (!reference) { - deleteReference(); + npc.mId = RecordHelper::createNpcRecord(npc); + createReference(npc.mId); + } + else + { + RecordHelper::updateNpcRecord(npc); + reloadPtr(); } - - std::string recId = getNpcRecordId(); - createReference(recId); setEquipment(); + + previousRace = npc.mRace; } void DedicatedPlayer::setShapeshift() { MWBase::World *world = MWBase::Environment::get().getWorld(); - if (reference) - { - deleteReference(); - } + bool isNpc = false; - std::string recId; + if (reference) + isNpc = ptr.getTypeName() == typeid(ESM::NPC).name(); - if (!creatureRefId.empty()) + if (!creatureRefId.empty() && RecordHelper::doesCreatureExist(creatureRefId)) { + if (isNpc) + { + deleteReference(); + } + const ESM::Creature *tmpCreature = world->getStore().get().search(creatureRefId); - if (tmpCreature != 0) + creature = *tmpCreature; + creature.mScript = ""; + if (!displayCreatureName) + creature.mName = npc.mName; + LOG_APPEND(Log::LOG_INFO, "- %s is disguised as %s", npc.mName.c_str(), creatureRefId.c_str()); + + // Is this our first time creating a creature record id for this player? If so, keep it around + // and reuse it + if (creatureRecordId.empty()) { - recId = getCreatureRecordId(); + creature.mId = "Dedicated Player"; + creatureRecordId = RecordHelper::createCreatureRecord(creature); + LOG_APPEND(Log::LOG_INFO, "- Creating new creature record %s", creatureRecordId.c_str()); } - } - if (recId.empty()) - { - recId = getNpcRecordId(); + creature.mId = creatureRecordId; + + if (!reference) + { + createReference(creature.mId); + } + else + { + RecordHelper::updateCreatureRecord(creature); + reloadPtr(); + } } + // This player was already a creature, but the new creature refId was empty or + // invalid, so we'll turn this player into their NPC self again as a result + else if (!isNpc) + { + if (reference) + { + deleteReference(); + } - createReference(recId); + RecordHelper::updateNpcRecord(npc); + createReference(npc.mId); + reloadPtr(); + } if (ptr.getTypeName() == typeid(ESM::NPC).name()) { @@ -353,54 +398,6 @@ void DedicatedPlayer::playSpeech() winMgr->messageBox(MWBase::Environment::get().getDialogueManager()->getVoiceCaption(sound), MWGui::ShowInDialogueMode_Never); } -std::string DedicatedPlayer::getNpcRecordId() -{ - MWBase::World *world = MWBase::Environment::get().getWorld(); - - MWWorld::Ptr player = world->getPlayerPtr(); - - ESM::NPC newNpc = *player.get()->mBase; - - // To avoid freezes caused by invalid races, only set race if we find it - // on our client - if (world->getStore().get().search(npc.mRace) != 0) - newNpc.mRace = npc.mRace; - - newNpc.mHead = npc.mHead; - newNpc.mHair = npc.mHair; - newNpc.mClass = npc.mClass; - newNpc.mName = npc.mName; - newNpc.mFlags = npc.mFlags; - - LOG_APPEND(Log::LOG_INFO, "- Creating new NPC record"); - newNpc.mId = "Dedicated Player"; - std::string recId = world->createRecord(newNpc)->mId; - - return recId; -} - -std::string DedicatedPlayer::getCreatureRecordId() -{ - MWBase::World *world = MWBase::Environment::get().getWorld(); - - ESM::Creature creature; - - const ESM::Creature *tmpCreature = world->getStore().get().search(creatureRefId); - - creature = *tmpCreature; - creature.mScript = ""; - if (!displayCreatureName) - creature.mName = npc.mName; - LOG_APPEND(Log::LOG_INFO, "Player %s looks like %s", npc.mName.c_str(), creatureRefId.c_str()); - - LOG_APPEND(Log::LOG_INFO, "- Creating new NPC record"); - creature.mId = "Dedicated Player"; - - std::string recId = world->createRecord(creature)->mId; - - return recId; -} - void DedicatedPlayer::createReference(const std::string& recId) { MWBase::World *world = MWBase::Environment::get().getWorld(); @@ -440,3 +437,10 @@ void DedicatedPlayer::setPtr(const MWWorld::Ptr& newPtr) { ptr = newPtr; } + +void DedicatedPlayer::reloadPtr() +{ + MWBase::World *world = MWBase::Environment::get().getWorld(); + world->disable(ptr); + world->enable(ptr); +} diff --git a/apps/openmw/mwmp/DedicatedPlayer.hpp b/apps/openmw/mwmp/DedicatedPlayer.hpp index bb3718a0a..aefa08b76 100644 --- a/apps/openmw/mwmp/DedicatedPlayer.hpp +++ b/apps/openmw/mwmp/DedicatedPlayer.hpp @@ -50,9 +50,6 @@ namespace mwmp void playAnimation(); void playSpeech(); - std::string getNpcRecordId(); - std::string getCreatureRecordId(); - void createReference(const std::string& recId); void deleteReference(); @@ -60,6 +57,7 @@ namespace mwmp MWWorld::ManualRef* getRef(); void setPtr(const MWWorld::Ptr& newPtr); + void reloadPtr(); private: @@ -72,6 +70,9 @@ namespace mwmp ESM::CustomMarker marker; bool markerEnabled; + + std::string npcRecordId; + std::string creatureRecordId; }; } #endif //OPENMW_DEDICATEDPLAYER_HPP diff --git a/apps/openmw/mwmp/RecordHelper.cpp b/apps/openmw/mwmp/RecordHelper.cpp index 032afcb0a..186558d18 100644 --- a/apps/openmw/mwmp/RecordHelper.cpp +++ b/apps/openmw/mwmp/RecordHelper.cpp @@ -13,6 +13,13 @@ bool RecordHelper::doesCreatureExist(const std::string& refId) return world->getStore().get().search(refId); } +bool RecordHelper::doesRaceExist(const std::string& raceId) +{ + MWBase::World *world = MWBase::Environment::get().getWorld(); + + return world->getStore().get().search(raceId); +} + std::string RecordHelper::createCreatureRecord(const ESM::Creature& creature) { MWBase::World *world = MWBase::Environment::get().getWorld(); diff --git a/apps/openmw/mwmp/RecordHelper.hpp b/apps/openmw/mwmp/RecordHelper.hpp index b66580e37..314438332 100644 --- a/apps/openmw/mwmp/RecordHelper.hpp +++ b/apps/openmw/mwmp/RecordHelper.hpp @@ -7,6 +7,7 @@ namespace RecordHelper { bool doesCreatureExist(const std::string& refId); + bool doesRaceExist(const std::string& raceId); std::string createCreatureRecord(const ESM::Creature& creature); std::string createNpcRecord(const ESM::NPC& npc); diff --git a/apps/openmw/mwmp/processors/player/ProcessorPlayerBaseInfo.hpp b/apps/openmw/mwmp/processors/player/ProcessorPlayerBaseInfo.hpp index eab9e8182..35e3669f4 100644 --- a/apps/openmw/mwmp/processors/player/ProcessorPlayerBaseInfo.hpp +++ b/apps/openmw/mwmp/processors/player/ProcessorPlayerBaseInfo.hpp @@ -23,7 +23,7 @@ namespace mwmp if (isLocal()) { - LOG_APPEND(Log::LOG_INFO, "- Packet was about my id"); + LOG_APPEND(Log::LOG_INFO, "- Packet was about LocalPlayer"); if (isRequest()) { diff --git a/apps/openmw/mwmp/processors/player/ProcessorPlayerShapeshift.hpp b/apps/openmw/mwmp/processors/player/ProcessorPlayerShapeshift.hpp index 70479221f..448cab9cb 100644 --- a/apps/openmw/mwmp/processors/player/ProcessorPlayerShapeshift.hpp +++ b/apps/openmw/mwmp/processors/player/ProcessorPlayerShapeshift.hpp @@ -15,8 +15,12 @@ namespace mwmp virtual void Do(PlayerPacket &packet, BasePlayer *player) { + LOG_MESSAGE_SIMPLE(Log::LOG_INFO, "Received ID_PLAYER_SHAPESHIFT from server"); + if (isLocal()) { + LOG_APPEND(Log::LOG_INFO, "- Packet was about LocalPlayer"); + static_cast(player)->setShapeshift(); } else if (player != 0) diff --git a/components/openmw-mp/Base/BasePlayer.hpp b/components/openmw-mp/Base/BasePlayer.hpp index 9186a383c..119db0eca 100644 --- a/components/openmw-mp/Base/BasePlayer.hpp +++ b/components/openmw-mp/Base/BasePlayer.hpp @@ -6,6 +6,7 @@ #define OPENMW_BASEPLAYER_HPP #include +#include #include #include #include @@ -274,6 +275,7 @@ namespace mwmp ESM::Cell cell; ESM::NPC npc; ESM::NpcStats npcStats; + ESM::Creature creature; ESM::CreatureStats creatureStats; ESM::Class charClass; Item equipedItems[19]; From acb1335d78bc73b20c25b0bc1460f05c16ba24b3 Mon Sep 17 00:00:00 2001 From: David Cernat Date: Fri, 13 Apr 2018 09:50:13 +0300 Subject: [PATCH 07/11] [Client] Make creature disguises update correctly --- apps/openmw/mwmp/DedicatedPlayer.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/apps/openmw/mwmp/DedicatedPlayer.cpp b/apps/openmw/mwmp/DedicatedPlayer.cpp index 80f6216ac..5b26e63ab 100644 --- a/apps/openmw/mwmp/DedicatedPlayer.cpp +++ b/apps/openmw/mwmp/DedicatedPlayer.cpp @@ -159,7 +159,9 @@ void DedicatedPlayer::setBaseInfo() reloadPtr(); } - setEquipment(); + // Only set equipment if the player isn't disguised as a creature + if (ptr.getTypeName() == typeid(ESM::NPC).name()) + setEquipment(); previousRace = npc.mRace; } @@ -192,19 +194,22 @@ void DedicatedPlayer::setShapeshift() if (creatureRecordId.empty()) { creature.mId = "Dedicated Player"; - creatureRecordId = RecordHelper::createCreatureRecord(creature); + creature.mId = creatureRecordId = RecordHelper::createCreatureRecord(creature); LOG_APPEND(Log::LOG_INFO, "- Creating new creature record %s", creatureRecordId.c_str()); } - - creature.mId = creatureRecordId; + else + { + creature.mId = creatureRecordId; + RecordHelper::updateCreatureRecord(creature); + } if (!reference) { + LOG_APPEND(Log::LOG_INFO, "- Creating reference for %s", creature.mId.c_str()); createReference(creature.mId); } else { - RecordHelper::updateCreatureRecord(creature); reloadPtr(); } } From 68ee64902dfbcc600643b22ff96b1507c5900ff5 Mon Sep 17 00:00:00 2001 From: David Cernat Date: Fri, 13 Apr 2018 16:16:43 +0300 Subject: [PATCH 08/11] [Client] Track & use previous race & creatureRefId for DedicatedPlayers --- apps/openmw/mwmp/DedicatedPlayer.cpp | 88 +++++++++++++++------------- apps/openmw/mwmp/DedicatedPlayer.hpp | 4 +- 2 files changed, 49 insertions(+), 43 deletions(-) diff --git a/apps/openmw/mwmp/DedicatedPlayer.cpp b/apps/openmw/mwmp/DedicatedPlayer.cpp index 5b26e63ab..10c3f8770 100644 --- a/apps/openmw/mwmp/DedicatedPlayer.cpp +++ b/apps/openmw/mwmp/DedicatedPlayer.cpp @@ -60,6 +60,7 @@ DedicatedPlayer::DedicatedPlayer(RakNet::RakNetGUID guid) : BasePlayer(guid) MWBase::World *world = MWBase::Environment::get().getWorld(); npc = *world->getPlayerPtr().get()->mBase; npc.mId = "Dedicated Player"; + previousRace = npc.mRace; } DedicatedPlayer::~DedicatedPlayer() { @@ -142,8 +143,6 @@ void DedicatedPlayer::move(float dt) void DedicatedPlayer::setBaseInfo() { - static std::string previousRace; - // Use the previous race if the new one doesn't exist if (!RecordHelper::doesRaceExist(npc.mRace)) npc.mRace = previousRace; @@ -175,56 +174,61 @@ void DedicatedPlayer::setShapeshift() if (reference) isNpc = ptr.getTypeName() == typeid(ESM::NPC).name(); - if (!creatureRefId.empty() && RecordHelper::doesCreatureExist(creatureRefId)) + if (creatureRefId != previousCreatureRefId) { - if (isNpc) + if (!creatureRefId.empty() && RecordHelper::doesCreatureExist(creatureRefId)) { - deleteReference(); - } + if (isNpc) + { + deleteReference(); + } - const ESM::Creature *tmpCreature = world->getStore().get().search(creatureRefId); - creature = *tmpCreature; - creature.mScript = ""; - if (!displayCreatureName) - creature.mName = npc.mName; - LOG_APPEND(Log::LOG_INFO, "- %s is disguised as %s", npc.mName.c_str(), creatureRefId.c_str()); + const ESM::Creature *tmpCreature = world->getStore().get().search(creatureRefId); + creature = *tmpCreature; + creature.mScript = ""; + if (!displayCreatureName) + creature.mName = npc.mName; + LOG_APPEND(Log::LOG_INFO, "- %s is disguised as %s", npc.mName.c_str(), creatureRefId.c_str()); - // Is this our first time creating a creature record id for this player? If so, keep it around - // and reuse it - if (creatureRecordId.empty()) - { - creature.mId = "Dedicated Player"; - creature.mId = creatureRecordId = RecordHelper::createCreatureRecord(creature); - LOG_APPEND(Log::LOG_INFO, "- Creating new creature record %s", creatureRecordId.c_str()); - } - else - { - creature.mId = creatureRecordId; - RecordHelper::updateCreatureRecord(creature); - } + // Is this our first time creating a creature record id for this player? If so, keep it around + // and reuse it + if (creatureRecordId.empty()) + { + creature.mId = "Dedicated Player"; + creature.mId = creatureRecordId = RecordHelper::createCreatureRecord(creature); + LOG_APPEND(Log::LOG_INFO, "- Creating new creature record %s", creatureRecordId.c_str()); + } + else + { + creature.mId = creatureRecordId; + RecordHelper::updateCreatureRecord(creature); + } - if (!reference) - { - LOG_APPEND(Log::LOG_INFO, "- Creating reference for %s", creature.mId.c_str()); - createReference(creature.mId); + if (!reference) + { + LOG_APPEND(Log::LOG_INFO, "- Creating reference for %s", creature.mId.c_str()); + createReference(creature.mId); + } + else + { + reloadPtr(); + } } - else + // This player was already a creature, but the new creature refId was empty or + // invalid, so we'll turn this player into their NPC self again as a result + else if (!isNpc) { + if (reference) + { + deleteReference(); + } + + RecordHelper::updateNpcRecord(npc); + createReference(npc.mId); reloadPtr(); } - } - // This player was already a creature, but the new creature refId was empty or - // invalid, so we'll turn this player into their NPC self again as a result - else if (!isNpc) - { - if (reference) - { - deleteReference(); - } - RecordHelper::updateNpcRecord(npc); - createReference(npc.mId); - reloadPtr(); + previousCreatureRefId = creatureRefId; } if (ptr.getTypeName() == typeid(ESM::NPC).name()) diff --git a/apps/openmw/mwmp/DedicatedPlayer.hpp b/apps/openmw/mwmp/DedicatedPlayer.hpp index aefa08b76..f77585477 100644 --- a/apps/openmw/mwmp/DedicatedPlayer.hpp +++ b/apps/openmw/mwmp/DedicatedPlayer.hpp @@ -71,7 +71,9 @@ namespace mwmp ESM::CustomMarker marker; bool markerEnabled; - std::string npcRecordId; + std::string previousRace; + std::string previousCreatureRefId; + std::string creatureRecordId; }; } From 716809f2dbd320ac08400dfbcbc0d4189fd15eee Mon Sep 17 00:00:00 2001 From: David Cernat Date: Fri, 13 Apr 2018 16:23:42 +0300 Subject: [PATCH 09/11] [Client] Prevent errors from NPC-only packets for DedicatedPlayers --- apps/openmw/mwmp/DedicatedPlayer.cpp | 3 +++ apps/openmw/mwmp/processors/player/ProcessorPlayerSkill.hpp | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/apps/openmw/mwmp/DedicatedPlayer.cpp b/apps/openmw/mwmp/DedicatedPlayer.cpp index 10c3f8770..5ce364450 100644 --- a/apps/openmw/mwmp/DedicatedPlayer.cpp +++ b/apps/openmw/mwmp/DedicatedPlayer.cpp @@ -276,6 +276,9 @@ void DedicatedPlayer::setAnimFlags() void DedicatedPlayer::setEquipment() { + // Go no further if the player is disguised as a creature + if (!ptr.getClass().hasInventoryStore(ptr)) return; + MWWorld::InventoryStore& invStore = ptr.getClass().getInventoryStore(ptr); for (int slot = 0; slot < MWWorld::InventoryStore::Slots; ++slot) { diff --git a/apps/openmw/mwmp/processors/player/ProcessorPlayerSkill.hpp b/apps/openmw/mwmp/processors/player/ProcessorPlayerSkill.hpp index 4b61b0a0a..eeee67a80 100644 --- a/apps/openmw/mwmp/processors/player/ProcessorPlayerSkill.hpp +++ b/apps/openmw/mwmp/processors/player/ProcessorPlayerSkill.hpp @@ -30,6 +30,10 @@ namespace mwmp else if (player != 0) { MWWorld::Ptr ptrPlayer = static_cast(player)->getPtr(); + + // Go no further if the player is disguised as a creature + if (ptrPlayer.getTypeName() != typeid(ESM::NPC).name()) return; + MWMechanics::NpcStats *ptrNpcStats = &ptrPlayer.getClass().getNpcStats(ptrPlayer); MWMechanics::SkillValue skillValue; From a01fc577f12e428fe7b31684b48544c284fbb155 Mon Sep 17 00:00:00 2001 From: David Cernat Date: Fri, 13 Apr 2018 16:59:48 +0300 Subject: [PATCH 10/11] [Client] Add setAttributes() and setSkills() methods to DedicatedPlayer --- apps/openmw/mwmp/DedicatedPlayer.cpp | 27 +++++++++++++++++++ apps/openmw/mwmp/DedicatedPlayer.hpp | 2 ++ .../player/ProcessorPlayerAttribute.hpp | 10 +------ .../player/ProcessorPlayerSkill.hpp | 14 +--------- 4 files changed, 31 insertions(+), 22 deletions(-) diff --git a/apps/openmw/mwmp/DedicatedPlayer.cpp b/apps/openmw/mwmp/DedicatedPlayer.cpp index 5ce364450..dc96c71fd 100644 --- a/apps/openmw/mwmp/DedicatedPlayer.cpp +++ b/apps/openmw/mwmp/DedicatedPlayer.cpp @@ -274,6 +274,33 @@ void DedicatedPlayer::setAnimFlags() ptrCreatureStats->setMovementFlag(CreatureStats::Flag_ForceMoveJump, (movementFlags & CreatureStats::Flag_ForceMoveJump) != 0); } +void DedicatedPlayer::setAttributes() +{ + MWMechanics::CreatureStats *ptrCreatureStats = &ptr.getClass().getCreatureStats(ptr); + MWMechanics::AttributeValue attributeValue; + + for (int i = 0; i < 8; ++i) + { + attributeValue.readState(creatureStats.mAttributes[i]); + ptrCreatureStats->setAttribute(i, attributeValue); + } +} + +void DedicatedPlayer::setSkills() +{ + // Go no further if the player is disguised as a creature + if (ptr.getTypeName() != typeid(ESM::NPC).name()) return; + + MWMechanics::NpcStats *ptrNpcStats = &ptr.getClass().getNpcStats(ptr); + MWMechanics::SkillValue skillValue; + + for (int i = 0; i < 27; ++i) + { + skillValue.readState(npcStats.mSkills[i]); + ptrNpcStats->setSkill(i, skillValue); + } +} + void DedicatedPlayer::setEquipment() { // Go no further if the player is disguised as a creature diff --git a/apps/openmw/mwmp/DedicatedPlayer.hpp b/apps/openmw/mwmp/DedicatedPlayer.hpp index f77585477..b9ead74e1 100644 --- a/apps/openmw/mwmp/DedicatedPlayer.hpp +++ b/apps/openmw/mwmp/DedicatedPlayer.hpp @@ -40,6 +40,8 @@ namespace mwmp void setBaseInfo(); void setShapeshift(); void setAnimFlags(); + void setAttributes(); + void setSkills(); void setEquipment(); void setCell(); diff --git a/apps/openmw/mwmp/processors/player/ProcessorPlayerAttribute.hpp b/apps/openmw/mwmp/processors/player/ProcessorPlayerAttribute.hpp index c49c065a7..2bf358e65 100644 --- a/apps/openmw/mwmp/processors/player/ProcessorPlayerAttribute.hpp +++ b/apps/openmw/mwmp/processors/player/ProcessorPlayerAttribute.hpp @@ -31,15 +31,7 @@ namespace mwmp } else if (player != 0) { - MWWorld::Ptr ptrPlayer = static_cast(player)->getPtr(); - MWMechanics::CreatureStats *ptrCreatureStats = &ptrPlayer.getClass().getCreatureStats(ptrPlayer); - MWMechanics::AttributeValue attributeValue; - - for (int i = 0; i < 8; ++i) - { - attributeValue.readState(player->creatureStats.mAttributes[i]); - ptrCreatureStats->setAttribute(i, attributeValue); - } + static_cast(player)->setAttributes(); } } }; diff --git a/apps/openmw/mwmp/processors/player/ProcessorPlayerSkill.hpp b/apps/openmw/mwmp/processors/player/ProcessorPlayerSkill.hpp index eeee67a80..180c2ab49 100644 --- a/apps/openmw/mwmp/processors/player/ProcessorPlayerSkill.hpp +++ b/apps/openmw/mwmp/processors/player/ProcessorPlayerSkill.hpp @@ -29,19 +29,7 @@ namespace mwmp } else if (player != 0) { - MWWorld::Ptr ptrPlayer = static_cast(player)->getPtr(); - - // Go no further if the player is disguised as a creature - if (ptrPlayer.getTypeName() != typeid(ESM::NPC).name()) return; - - MWMechanics::NpcStats *ptrNpcStats = &ptrPlayer.getClass().getNpcStats(ptrPlayer); - MWMechanics::SkillValue skillValue; - - for (int i = 0; i < 27; ++i) - { - skillValue.readState(player->npcStats.mSkills[i]); - ptrNpcStats->setSkill(i, skillValue); - } + static_cast(player)->setSkills(); } } }; From db41704e52b4674613e0f589c956cea9285834d7 Mon Sep 17 00:00:00 2001 From: David Cernat Date: Fri, 13 Apr 2018 17:28:28 +0300 Subject: [PATCH 11/11] [Server] Use clearer function name for checking creature name display --- apps/openmw-mp/Script/Functions/Mechanics.cpp | 2 +- apps/openmw-mp/Script/Functions/Mechanics.hpp | 48 +++++++++---------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/apps/openmw-mp/Script/Functions/Mechanics.cpp b/apps/openmw-mp/Script/Functions/Mechanics.cpp index 2635748bc..57790932a 100644 --- a/apps/openmw-mp/Script/Functions/Mechanics.cpp +++ b/apps/openmw-mp/Script/Functions/Mechanics.cpp @@ -100,7 +100,7 @@ const char *MechanicsFunctions::GetCreatureRefId(unsigned short pid) noexcept return player->creatureRefId.c_str(); } -bool MechanicsFunctions::DisplaysCreatureName(unsigned short pid) noexcept +bool MechanicsFunctions::GetCreatureNameDisplayState(unsigned short pid) noexcept { Player *player; GET_PLAYER(pid, player, 0); diff --git a/apps/openmw-mp/Script/Functions/Mechanics.hpp b/apps/openmw-mp/Script/Functions/Mechanics.hpp index 24b91344c..31e379401 100644 --- a/apps/openmw-mp/Script/Functions/Mechanics.hpp +++ b/apps/openmw-mp/Script/Functions/Mechanics.hpp @@ -6,34 +6,34 @@ #define MECHANICSAPI \ {"GetMiscellaneousChangeType", MechanicsFunctions::GetMiscellaneousChangeType},\ \ - {"GetMarkCell", MechanicsFunctions::GetMarkCell},\ - {"GetMarkPosX", MechanicsFunctions::GetMarkPosX},\ - {"GetMarkPosY", MechanicsFunctions::GetMarkPosY},\ - {"GetMarkPosZ", MechanicsFunctions::GetMarkPosZ},\ - {"GetMarkRotX", MechanicsFunctions::GetMarkRotX},\ - {"GetMarkRotZ", MechanicsFunctions::GetMarkRotZ},\ - {"GetSelectedSpellId", MechanicsFunctions::GetSelectedSpellId},\ + {"GetMarkCell", MechanicsFunctions::GetMarkCell},\ + {"GetMarkPosX", MechanicsFunctions::GetMarkPosX},\ + {"GetMarkPosY", MechanicsFunctions::GetMarkPosY},\ + {"GetMarkPosZ", MechanicsFunctions::GetMarkPosZ},\ + {"GetMarkRotX", MechanicsFunctions::GetMarkRotX},\ + {"GetMarkRotZ", MechanicsFunctions::GetMarkRotZ},\ + {"GetSelectedSpellId", MechanicsFunctions::GetSelectedSpellId},\ \ - {"GetScale", MechanicsFunctions::GetScale},\ - {"IsWerewolf", MechanicsFunctions::IsWerewolf},\ - {"GetCreatureRefId", MechanicsFunctions::GetCreatureRefId},\ - {"DisplaysCreatureName", MechanicsFunctions::DisplaysCreatureName},\ + {"GetScale", MechanicsFunctions::GetScale},\ + {"IsWerewolf", MechanicsFunctions::IsWerewolf},\ + {"GetCreatureRefId", MechanicsFunctions::GetCreatureRefId},\ + {"GetCreatureNameDisplayState", MechanicsFunctions::GetCreatureNameDisplayState},\ \ - {"SetMarkCell", MechanicsFunctions::SetMarkCell},\ - {"SetMarkPos", MechanicsFunctions::SetMarkPos},\ - {"SetMarkRot", MechanicsFunctions::SetMarkRot},\ - {"SetSelectedSpellId", MechanicsFunctions::SetSelectedSpellId},\ + {"SetMarkCell", MechanicsFunctions::SetMarkCell},\ + {"SetMarkPos", MechanicsFunctions::SetMarkPos},\ + {"SetMarkRot", MechanicsFunctions::SetMarkRot},\ + {"SetSelectedSpellId", MechanicsFunctions::SetSelectedSpellId},\ \ - {"SetScale", MechanicsFunctions::SetScale},\ - {"SetWerewolfState", MechanicsFunctions::SetWerewolfState},\ - {"SetCreatureRefId", MechanicsFunctions::SetCreatureRefId},\ + {"SetScale", MechanicsFunctions::SetScale},\ + {"SetWerewolfState", MechanicsFunctions::SetWerewolfState},\ + {"SetCreatureRefId", MechanicsFunctions::SetCreatureRefId},\ \ - {"SendMarkLocation", MechanicsFunctions::SendMarkLocation},\ - {"SendSelectedSpell", MechanicsFunctions::SendSelectedSpell},\ - {"SendShapeshift", MechanicsFunctions::SendShapeshift},\ + {"SendMarkLocation", MechanicsFunctions::SendMarkLocation},\ + {"SendSelectedSpell", MechanicsFunctions::SendSelectedSpell},\ + {"SendShapeshift", MechanicsFunctions::SendShapeshift},\ \ - {"Jail", MechanicsFunctions::Jail},\ - {"Resurrect", MechanicsFunctions::Resurrect} + {"Jail", MechanicsFunctions::Jail},\ + {"Resurrect", MechanicsFunctions::Resurrect} class MechanicsFunctions { @@ -138,7 +138,7 @@ public: * \param pid The player ID. * \return The creature name display state. */ - static bool DisplaysCreatureName(unsigned short pid) noexcept; + static bool GetCreatureNameDisplayState(unsigned short pid) noexcept; /** * \brief Set the Mark cell of a player.