The skill widgets were configured in two places: initial creation, and
updates. The former was redundant, and duplicated the functionality of
the latter.
To fix this redundancy, replace the duplicate code with a call to
setValue.
Previously, when recharging or repairing an item, the client sent a PlayerInventory packet to the server with the old version of the item that was supposed to be removed and then it sent a PlayerInventory packet with the new version of the item that was supposed to be added.
Unfortunately, the current CoreScripts make it so custom items using generated IDs have their records deleted when they are completely removed from the world, however briefly, even if they are added back immediately afterwards. In practice, this meant that – before this commit – recharging or repairing a custom item led to its removal from the player inventory stored on the server, followed by the deletion of its record, followed by its readdition to the inventory (but with the record staying deleted). Logging out and logging back in immediately prevented the player from receiving the item anymore because of its now non-existent record.
When the CopyFramebufferToTextureCallback callback is called, in its operator() it resets setInitialDrawCallback by providing a NULL pointer.
However, this causes the callback to get deleted. In turn, the "this" pointer is invalidated.
When execution returns to DrawCallback::run, it accesses a _nestedCallback member of deleted "this" which is UB.
1. Do not update physics and animations for actors outside processing range (bug #4647)
2. Do not render such actors
3. Add transparency to actors near processing border, so they will not pop up suddenly
Previously, client mods adding packet-sending scripts to the spawn area made clients send the associated packets as soon as they inputted their character name when joining a server using those mods. This made the clients either get disconnected for not replying to a handshake first, or it made them get kicked for sending object packets that are disallowed for players who are not logged in.
To fix this, LocalPlayer's hasFinishedCharGen() has been replaced with isLoggedIn(), because the former was already returning true when players inputted their names.
Whenever an item is added to or removed from the player's ContainerStore, that player sends a PlayerInventory packet with just that addition or removal.
This eliminates all the unnecessary packet spam related to oversized PlayerInventory packets that had existed in one form or another since the initial implementation of inventory sync in 1b259e2d33
Additionally, move booleans from BasePlayer to LocalPlayer when they are only needed on the client, and make the usage of the isReceivingQuickKeys boolean consistent with the new isReceivingInventory boolean by having them both in the processors of their associated packets.
Resolve merge issues related to CHANGELOG.md
* master:
Optimize skinning (task #4605)
Update changelog
Update some comments
Set the OpenAL source offset after setting the buffer
Make Move and MoveWorld console commands move actors standing on moving object (bug #2274)
Adding Changelog entry
Allow messageboxes arguments to have newline characters (bug #3836)
Check for impact immediately when launch a projectile (bug #3059)
Fix gold count calculation in pickupObject (bug #4604)
Correct special case soundgen comparisons
Move "land" check earlier
Fixes#3681
Play landing sound manually and ignore land soundgen textkeys (bug #2256)
Make some more optimizations to actor processing loops
Fix freeze in getActorsSidingWith
Addiong missing "to" word
Adding common problems that were previous on the site FAQ
Treat <> and << operators as < and >< and >> as > in scripts
stage1: priorities for event music and other minor improvements to the music system
# Conflicts:
# CHANGELOG.md
For a long time, whenever a PlayerInventory packet was sent, it contained all the items in the player's inventory, because that's how Koncord originally implemented it and I always had too many other priorities to go back and rework it.
From now on, clients only send PlayerInventory packet with the one item added or removed, with the single exception being trading with merchants, where the entire inventory is still sent for the time being.
Players can no longer unilaterally use items on themselves in their inventory. When they try to use an item, they send a PlayerItemUse packet to the server with the item's details. A serverside script can then check the item and either send the packet back to make the item use go through or drop it.
Spell, potion, enchantment, creature, NPC, armor, book, clothing, miscellaneous and weapon record data can now be sent in a RecordDynamic packet. Additionally, the packets include data related to associated magical effects (for spells, potions and enchantments), data related to default inventory contents (for creatures and NPCs) and data related to body parts affected (for armor and clothing).
The server now has associated script functions for setting most of the details of the above, with the main exception being individual creature and NPC stats.
Records can either be created entirely from scratch or can use an existing record (set via the baseId variable) as a starting point for their values. In the latter case, only the values that are specifically set override the starting values. Creature and NPC records also have an inventoryBaseId that can be used on top of the baseId to base their inventories on another existing record.
The client's RecordHelper class has been heavily expanded to allow for the above mentioned functionality.
When players create spells, potions and enchantments as part of regular gameplay, they send RecordDynamic packets that provide the server with the complete details of the records that should be created. When they create enchantments, they also provide the server with armor, book, clothing and weapon records corresponding to the items they've enchanted.
This functionality added by this packet was originally supposed to be exclusive to the rewrite, but I've gone ahead and tried to provide it for the pre-rewrite in a way that can mostly be reused for the rewrite.
Prevents compiler warnings such as this:
```
/Users/Will/CLionProjects/OpenMW/apps/openmw/mwgui/windowbase.hpp:65:22: warning: 'onOpen' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
virtual void onOpen();
^
/Users/Will/CLionProjects/OpenMW/apps/openmw/mwgui/windowbase.hpp:38:22: note: overridden virtual function is here
virtual void onOpen() {}
^
```
added a MWWorld::ContainerStore to hold item copies which are then used to find
real items with findReplacement().
(storing the RefId could be a better solution but would probably leave tooltips broken...)
Previously, after finishing the TES3MP chargen once, mCreationStage was set to 4 in OpenMW, which in turn made it impossible to go through only specific chargen menus again as the result of the relevant TES3MP script function (tes3mp.SetCharGenStage(pid, startStage, endStage) in 0.6.3, player:setCharGenStages(startStage, endStage) in 0.7). In other words, trying to allow a player to just choose their class again made it so the player started at that menu and went through all the other subsequent menus as well, i.e. the player went through the class, birthsign and review menus.
cppcheck:
[apps/esmtool/record.cpp:697]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[apps/esmtool/record.cpp:1126]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[apps/esmtool/record.cpp:1138]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[apps/niftest/niftest.cpp:36]: (performance) Function parameter 'filename' should be passed by reference.
[apps/niftest/niftest.cpp:41]: (performance) Function parameter 'filename' should be passed by reference.
[apps/opencs/model/prefs/boolsetting.cpp:25]: (warning) Possible leak in public function. The pointer 'mWidget' is not deallocated before it is allocated.
[apps/opencs/model/prefs/shortcuteventhandler.cpp:52]: (warning) Return value of std::remove() ignored. Elements remain in container.
[apps/openmw/mwstate/quicksavemanager.cpp:5]: (performance) Variable 'mSaveName' is assigned in constructor body. Consider performing initialization in initialization list.
PVS-Studio:
apps/opencs/model/filter/parser.cpp 582 warn V560 A part of conditional expression is always true: allowPredefined.
apps/opencs/view/world/referencecreator.cpp 67 warn V547 Expression '!errors.empty()' is always false.
apps/opencs/view/world/referencecreator.cpp 74 warn V547 Expression '!errors.empty()' is always false.
apps/opencs/view/doc/loader.cpp 170 warn V560 A part of conditional expression is always true: !completed.
apps/opencs/view/doc/loader.cpp 170 warn V560 A part of conditional expression is always true: !error.empty().
apps/opencs/model/tools/pathgridcheck.cpp 32 err V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 32, 34.
apps/opencs/model/world/refidadapterimp.cpp 1376 err V547 Expression 'subColIndex < 3' is always true.
apps/openmw/mwgui/widgets.hpp 318 warn V703 It is odd that the 'mEnableRepeat' field in derived class 'MWScrollBar' overwrites field in base class 'ScrollBar'. Check lines: widgets.hpp:318, MyGUI_ScrollBar.h:179.
apps/openmw/mwgui/widgets.hpp 319 warn V703 It is odd that the 'mRepeatTriggerTime' field in derived class 'MWScrollBar' overwrites field in base class 'ScrollBar'. Check lines: widgets.hpp:319, MyGUI_ScrollBar.h:180.
apps/openmw/mwgui/widgets.hpp 320 warn V703 It is odd that the 'mRepeatStepTime' field in derived class 'MWScrollBar' overwrites field in base class 'ScrollBar'. Check lines: widgets.hpp:320, MyGUI_ScrollBar.h:181
apps/openmw/mwmechanics/actors.cpp 1425 warn V547 Expression '!detected' is always true.
apps/openmw/mwmechanics/character.cpp 2155 err V547 Expression 'mode == 0' is always true.
apps/openmw/mwmechanics/character.cpp 1192 warn V592 The expression was enclosed by parentheses twice: ((expression)). One pair of parentheses is unnecessary or misprint is present.
apps/openmw/mwmechanics/character.cpp 521 warn V560 A part of conditional expression is always true: (idle == mIdleState).
apps/openmw/mwmechanics/pathfinding.cpp 317 err V547 Expression 'mPath.size() >= 2' is always true.
apps/openmw/mwscript/interpretercontext.cpp 409 warn V560 A part of conditional expression is always false: rank > 9.
apps/openmw/mwgui/windowbase.cpp 28 warn V560 A part of conditional expression is always true: !visible.
apps/openmw/mwgui/journalwindow.cpp 561 warn V547 Expression '!mAllQuests' is always false.
apps/openmw/mwgui/referenceinterface.cpp 18 warn V571 Recurring check. The '!mPtr.isEmpty()' condition was already verified in line 16.
apps/openmw/mwworld/scene.cpp 463 warn V547 Expression 'adjustPlayerPos' is always true.
apps/openmw/mwworld/worldimp.cpp 409 err V766 An item with the same key '"sCompanionShare"' has already been added.
apps/openmw/mwworld/cellstore.cpp 691 warn V519 The 'state.mWaterLevel' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 689, 691.
apps/openmw/mwworld/weather.cpp 1125 warn V519 The 'mResult.mParticleEffect' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1123, 1125.
apps/openmw/mwworld/weather.cpp 1137 warn V519 The 'mResult.mParticleEffect' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1135, 1137.
apps/wizard/unshield/unshieldworker.cpp 475 warn V728 An excessive check can be simplified. The '(A && B) || (!A && !B)' expression is equivalent to the 'bool(A) == bool(B)' expression.
apps/wizard/installationpage.cpp 163 warn V735 Possibly an incorrect HTML. The "</p" closing tag was encountered, while the "</span" tag was expected.
components/fontloader/fontloader.cpp 427 err V547 Expression 'i == 1' is always true.
components/nifosg/nifloader.cpp 282 warn V519 The 'created' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 278, 282.
components/esm/loadregn.cpp 119 err V586 The 'clear' function is called twice for deallocation of the same resource. Check lines: 112, 119.
components/esm/cellref.cpp 178 warn V581 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 175, 178.
components/esmterrain/storage.cpp 235 warn V560 A part of conditional expression is always true: colStart == 0.
components/esmterrain/storage.cpp 237 warn V560 A part of conditional expression is always true: rowStart == 0.
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.
James Stephens
Andrei Kortunov
David Cernat
ArgonianAccount
capostrophic
Artem Nykolenko
Evil Eye
Alexei Dobrohotov
Phillip A
Frederic Chardon
Ilya Zhuravlev
Bret Curtis
bzzt
uramer
Vladimir Panteleev
AnyOldName3
Marc Zinnschlag
Sergey Fukanchik
Sophie Kirschner
terrabyte25
Thunderforge
Finbar Crago
Miloslav Číž
Capostrophic
scrawl
tri4ng1e
elsid