Several fixes for naked NPCs

[rfortier]

Original naked NPC fix worked around the "fighting over NPC ownership" issues in STR by checking periodically to see if NPCs were naked and redressing them if they were. It didn't really address root cause, and introduced some new bugs.

The changes here:

• Are constrained by the lack of solid reproducers. Simulations only get you so far.
• Address all the root-cause issues that could be found / inferred in reasonable time. Difficult to say if it gets all (or enough) of them without solid reproducers.
• Fixes the new bugs introduced by the first version.
• Keeps a last-resort timeout to redress NPCs in case this still doesn't fix all the root causes. If it ever happens, it will log at warning level and we will know there's more to hunt for.

Root cause:

• Common wisdom was the Ownership code was fighting over NPCs. Leader and Member jump into a cell together and each try to set up the NPCs, broadcasting conflicting changes to the other.
• This turned out to be directionally correct. First, when an Actor is constructed, shortly after creation the engine fires a bunch of EquipmentChangeEvents as the engine dresses the actor. Some to all of these these could be dropped because ServerIds for the actors or equipment aren't available yet. But, it is a real problem if some of them ARE broadcast once server IDs are allocated. Equipping an item the sender has but the receiver doesn't can end up unequipping. Even worse if inventory changes are sent.
• The new code blocks sending updates until WaitingFor3D ends (according to CK, accessing the default outfit before the Actor is fully constructed is undefined). And, it blocks sending updates while WaitingForAssignment. This is the KEY fix, since at the end of WaitingForAssignment, we know who should win, and the winner has sent a full Inventory and a full Equipment list. Applying that (by the non-owners) should fully dress the NPC. At least as long as the Owner has dressed the NPC.
• As a last resort/backstop, a "naked deadline" trips on the Actor. If this happens where the actor IsLocal(), and the actor is not dressed, then they will equip the default outfit (if there is one). These changes are synced.
• That last is sort of what the original fix did, but improved. This redressing is non-destructive.

Bugs fixed in Vanilla:

• Blocks the sending of potentially destructive equipment and possibly inventory changes while debating Ownership. This may or may not be a complete fix.

Bugs fixed in orignal Naked fix:

• The original code just tried to dress NPCs once every second. Note, that was not "after a second", but between 0 and 1 seconds after creation. I can't prove it, but this firing too soon, and in particular before WaitingFor3D leads to undefined behavior and is probably one of the reasons for lingering nakedness, and syncing broken equip states (and inventory?). This was replaced with a per-Actor timeout that works out to between 3 and 4 seconds. It is reset at the end of WaitingFor3D and again at the end of Ownership change.
• Replaces the forced-redress by fully resetting inventory with simply equipping the default outfit; no changes to inventory. This is critical for followers that may have accumulated a lot of loot, but applies to any NPC that has loot. The previous fix deleted all that loot.
• Does not try to recreate the default outfit inventory if you have removed it from inventory. This is probably what you wanted anyway.
• Only checks for redressing once per ownership change of the Actor, rather than rechecking every second. So you can change what the actor is wearing and it will stay that way as long as you stay in the cell (and the Leader doesn't enter and take over)

[otsffs]

NPCs appear naked because they are set as Remote before initialization completes. If the OnCharacterSpawn method is properly modified, the nakedness fix code can be completely removed.

[rfortier]

NPCs appear naked because they are set as Remote before initialization completes. If the OnCharacterSpawn method is properly modified, the nakedness fix code can be completely removed.

I'm going to give myself some time to attack this problem again today. The biggest challenge is a complete failure to reproduce the random occurrence to demonstrate that the issue is fixed (as opposed to simulating the issue which behaves differently). That includes using the method you / @otsffs suggested.

I'm going to load up something before 1.6.5 (the original "naked" fix) and try to reproduce there; if that works, I can prove or disprove that my fix code works, and if it does I can remove the "backstop" code (the code that trips "just in case" there's still something else causing random nakedness). I'll just #ifdef it out, because this problem has been very resilient defeating attempts to fix it, and maybe there still are multiple causes.

My fix code (not the backstop-of-last-resort part) works differently than what you suggest, it waits for the end of WaitingFor3D AND TakeOwnership() to apply the outfit rather than trying to fix it during the time multiple players are trying to simultaneously Spawn / Clothe (adds outfit items one at a time, triggering the next one after the slash) / Inventory Change sync (individual items) / UnequipAll (prep for applying a full outfit in TakeOwnership action) and applying the full inventory (which is still done one item at a time, so if other players are still tinkering you can get concurrency conflicts and nakedness). From what I see, serializing all those things so the last step takes place after the multithreaded thrash of all those other actions is the fix.

If there's a fix that involves just moving the ->SetRemote() in OnCharacterSpawn() a few lines, I don't see it in the face of all the other things that occur. Please provide the explicit change you are suggesting.