Unfortunately, the handshake attempt limit implemented in 4ebfcc4a21 for 0.7 and then ported over to 0.6 in a3a341fee6 does not appear to work properly, which is why gathering more information on it is important.
Additionally, delete DedicatedPlayers who disconnect.
Previously, all disconnected DedicatedPlayers were still kept in memory, but never used again. There was code that suggested they were meant to be reused upon reconnecting, but that reuse had never actually been implemented, and would probably not be that useful anyway.
Additionally, print player guids using their string representations for consistency.
The creation and updating of DedicatedPlayer references remains very inelegant, but this commit is the first step towards fixing that.
To fix warnings:
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/7.3.1/../../../../include/c++/7.3.1/bits/shared_ptr_base.h:588:8: warning: delete called on non-final 'NifOsg::ControllerFunction' that has virtual functions but non-virtual destructor
[-Wdelete-non-virtual-dtor]
delete __p;
^
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/7.3.1/../../../../include/c++/7.3.1/bits/shared_ptr_base.h:595:4: note: in instantiation of function template specialization
'std::__shared_count<__gnu_cxx::_S_atomic>::__shared_count<NifOsg::ControllerFunction *>' requested here
: __shared_count(__p)
^
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/7.3.1/../../../../include/c++/7.3.1/bits/shared_ptr_base.h:1079:17: note: in instantiation of function template specialization
'std::__shared_count<__gnu_cxx::_S_atomic>::__shared_count<NifOsg::ControllerFunction *>' requested here
: _M_ptr(__p), _M_refcount(__p, typename is_array<_Tp>::type())
^
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/7.3.1/../../../../include/c++/7.3.1/bits/shared_ptr.h:129:25: note: in instantiation of function template specialization 'std::__shared_ptr<NifOsg::ControllerFunction,
__gnu_cxx::_S_atomic>::__shared_ptr<NifOsg::ControllerFunction, void>' requested here
shared_ptr(_Yp* __p) : __shared_ptr<_Tp>(__p) { }
^
/home/elsid/dev/openmw/components/nifosg/nifloader.cpp:242:39: note: in instantiation of function template specialization 'std::shared_ptr<NifOsg::ControllerFunction>::shared_ptr<NifOsg::ControllerFunction, void>' requested here
callback->setFunction(std::shared_ptr<NifOsg::ControllerFunction>(new NifOsg::ControllerFunction(key)));
^
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/7.3.1/../../../../include/c++/7.3.1/bits/unique_ptr.h:78:2: warning: delete called on non-final 'MWGui::ResponseCallback' that has virtual functions but non-virtual destructor
[-Wdelete-non-virtual-dtor]
delete __ptr;
^
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/7.3.1/../../../../include/c++/7.3.1/bits/unique_ptr.h:268:4: note: in instantiation of member function 'std::default_delete<MWGui::ResponseCallback>::operator()' requested here
get_deleter()(__ptr);
^
/home/elsid/dev/openmw/apps/openmw/mwgui/dialogue.cpp:58:23: note: in instantiation of member function 'std::unique_ptr<MWGui::ResponseCallback, std::default_delete<MWGui::ResponseCallback> >::~unique_ptr' requested here
PersuasionDialog::PersuasionDialog(ResponseCallback* callback)
^
Previously, a Container packet with a REQUEST action always made the client respond with the contents of all the containers in that cell.
The previous behavior now only happens for requests that have no WorldObjects attached, while requests that have WorldObjects attached get a reply with the contents of those specific containers.
A main priority in TES3MP development is to avoid making major changes to OpenMW code, so as to avoid merge conflicts in the future. Whenever avoiding potential conflicts seems especially difficult for the proper implementation of a particular multiplayer feature, that multiplayer feature is often put off until later or partially implemented with the intent of being revisited in the future.
Container sync is the perfect example. Previously, the OpenMW code for container actions was kept exactly as it was, with clients unilaterally accepting their own container changes as per singleplayer-specific code, with only the addition that clients sent container packets every time they made a change in a container, packets which were then forwarded unquestioningly by the server to other players. This meant that two players clicking on the same item in a container at the same time both managed to take it, thus duplicating the item.
Immediately after the packets were already forwarded, server scripts were able to check for incorrect changes, such as the removal of more items than should have existed in a container, but they had to send their own packets that attempted to fix what had already been accepted on the initial client and then forwarded to all clients, which was quite onerous in some scenarios, such as when a player on a slow connection immediately dropped items in the world after taking them from a container (which is why the default TES3MP serverside scripts made no attempt at sending corrective packets at all, preferring to expect the matter to be solved in a later C++ implementation).
This commit fixes item duping in containers by preventing container actions from initially running on clients and by ending the automatic forwarding of container packets by the server. Instead, clients now send container packets that act as requests for container actions, and serverside scripts have to forward these requests themselves. In other words, without a matching Container event in the server's Lua scripts, players are completely unable to affect containers for themselves or for others.
To forward a received Container packet, the following line must be used in a Container event in the Lua scripts:
tes3mp.SendContainer(true, true)
When an invalid action count is used in a container request, the serverside scripts can amend it using the following new function:
tes3mp.SetReceivedContainerItemActionCount(objectIndex, itemIndex, actionCount)
Thus, the serverside scripts are able to allow only container actions that are correct based on their own recorded contents for that container.
The OpenMW code allowing unilateral container actions in mwgui/container.cpp is now prevented from executing. When a player's container request is returned to them, code in mwmp/WorldEvent.cpp simulates those container actions instead.
