Westwud1 / QuickStack

10 stars 5 forks source link

Multiplayer check for chests #2

Closed billbo99 closed 2 years ago

billbo99 commented 2 years ago

Not saying this is a working solution or anything ... but these mod dev's are using a method "IsUserAccessing()" to check whether a chest is open or not.

https://github.com/aedenthorn/7D2DMods/blob/2fa0d9c8593888e263d822ed02c13d9261084034/CraftFromContainers/CraftFromContainers.cs#L255

https://github.com/OCB7D2D/ClaimAutoRepair/blob/d22a246a078fa9991853ca7f799dd5e48865d553/Harmony/TileEntityClaimAutoRepair.cs#L353

Westwud1 commented 2 years ago

Hi, I also have that method. I think it's only used to check if the current player is accessing a chest (so it would only work on singleplayer). Do you have any idea if these mods work on multiplayer?

jonathan-robertson commented 2 years ago

Looking through the code to track this down.

Context

It appears that GameManager.TELockServer(int, Vector3i, int, int, string) is the method called both to handle local client requests to interact with a storage box and handle client net requests to get access and 'lock' the box ('lock' is different from locking a secured storage).

When the client is done with the container, it uses GameManager.TEUnlockServer(int, Vector3i, int) to send a net request to the server, indicating the Tile Entity (storage container) is now being 'unlocked', meaning it should be free for use by other players. This same method is both used by the client to send this net request and appears to also be used by the server to handle the received net request.

I think this is what we're looking for

In TELockServer, the server-side operation that makes several checks to validate if the player is able to 'unlock' (apply a write lock) to this Tile Entity on the server can be found here and is being used to exit the happy path early if the player is not identified as being allowed.

Dump of A20.5's GameManager.TEUnlockServer(int, Vector3i, int) method for reference

public void TELockServer(int _clrIdx, Vector3i _blockPos, int _lootEntityId, int _entityIdThatOpenedIt, string _customUi = null)
{
    if (!SingletonMonoBehaviour<ConnectionManager>.Instance.IsServer)
    {
        SingletonMonoBehaviour<ConnectionManager>.Instance.SendToServer(NetPackageManager.GetPackage<NetPackageTELock>().Setup(NetPackageTELock.TELockType.LockServer, _clrIdx, _blockPos, _lootEntityId, _entityIdThatOpenedIt, _customUi), false);
        return;
    }
    foreach (KeyValuePair<TileEntity, int> keyValuePair in this.lockedTileEntities)
    {
        if (_entityIdThatOpenedIt == keyValuePair.Value)
        {
            return;
        }
    }
    TileEntity tileEntity;
    if (_lootEntityId == -1)
    {
        tileEntity = this.m_World.GetTileEntity(_clrIdx, _blockPos);
    }
    else
    {
        tileEntity = this.m_World.GetTileEntity(_lootEntityId);
    }
    if (tileEntity == null)
    {
        return;
    }
    if (!this.OpenTileEntityAllowed(_entityIdThatOpenedIt, tileEntity, _customUi))
    {
        return;
    }
    EntityAlive entityAlive;
    if (this.lockedTileEntities.ContainsKey(tileEntity) && (entityAlive = (EntityAlive)this.m_World.GetEntity(this.lockedTileEntities[tileEntity])) != null && !entityAlive.IsDead())
    {
        if (this.m_World.GetEntity(_entityIdThatOpenedIt) as EntityPlayerLocal == null)
        {
            SingletonMonoBehaviour<ConnectionManager>.Instance.SendPackage(NetPackageManager.GetPackage<NetPackageTELock>().Setup(NetPackageTELock.TELockType.DeniedAccess, _clrIdx, _blockPos, _lootEntityId, _entityIdThatOpenedIt, _customUi), false, _entityIdThatOpenedIt, -1, -1, -1);
            return;
        }
        this.TEDeniedAccessClient(_clrIdx, _blockPos, _lootEntityId, _entityIdThatOpenedIt);
        return;
    }
    else
    {
        this.lockedTileEntities[tileEntity] = _entityIdThatOpenedIt;
        if (tileEntity == null)
        {
            return;
        }
        this.OpenTileEntityUi(_entityIdThatOpenedIt, tileEntity, _customUi);
        SingletonMonoBehaviour<ConnectionManager>.Instance.SendPackage(NetPackageManager.GetPackage<NetPackageTELock>().Setup(NetPackageTELock.TELockType.AccessClient, _clrIdx, _blockPos, _lootEntityId, _entityIdThatOpenedIt, _customUi), true, -1, -1, -1, -1);
    }
}

Several pieces of this method are interesting:

Possible Solution

I'm sorry to say that a solution to this isn't very clear. It looks like this mod is 99% client-side currently, so updating it to also support server-side operations could be very tricky.

In short, adding a new Network Package would probably be the best way to approach it, but I also haven't had to do that before.

This is something I'd be motivated to build a completely server-side solution for (so EAC can be enabled and the client doesn't need to download any mods), but this would also mean not having keyboard shortcuts to trigger this operation. Triggering such an operation would probably be done via a chat command or something in that case... not sure that would feel right to the player.

A combo server-side/client-side mod would probably provide the best experience. I'll put my thoughts down here and hopefully they will help. If not, this might be a little over my head. I do think Harmony is the right way to go with this (and I'm seeing some Harmony patches in the project); I haven't used Harmony yet, so I might end up suggesting something here that isn't possible in Harmony - sorry in advance if I do:

Broad strokes, this "should" allow you to safely interact with boxes, lock them while your code is running to restock/resupply, and allow you to unlock any boxes you were adjusting when it's safe for others to use them.

I say "should" because I haven't even begun to test this out... and don't currently have the skills with Harmony necessary to tinker with this idea. But I hope this helps!

jonathan-robertson commented 2 years ago

I would also like to quickly say that the mod you created is fascinating! Thank you for sharing it with all of us :)

Westwud1 commented 2 years ago

Hi, thank you for the thorough response :). I've come to the same conclusion mostly.

Like you said, the code is 99% client-side, because all of the multiplayer inventory/storage interaction is done client side, and when the player closes the inventory, it updates the whole container. This is why we can't have multiple players accessing one inventory, since the last player to close it will overwrite anything the other players have modified (locally) in the meantime.

The lockedTileEntities in GameManager is populated server side only unfortunately.

I've also been thinking about something similar to your suggestion. However say you have to go through couple of containes, you would have to send a TELockServer request for each one (then wait for the response), and depending on your latency, it would freeze the game for a few seconds. You could maybe multithread this which may come with its own item duplication bugs, and you wouldn't be able to access a container until it's done (which some players may find confusing).

Another idea was to send the lockedTileEntities as a Map<Vector3, bool> (positions of containers and a flag if they are locked) to the client before using quickstack/restock, so the client can check every position without sending too much requests. I think I would have to create a new NetPackage for this, but it may not be as easy to implement.

I'm a total beginner when in comes to harmony as this is my first mod. The basics are that you can only modify the output of functions or not run them/redirect them to your own. You can not create properties or new methods in an existing class, you would have to subclass it, add the new property/method and patch it everywhere that you need it (from what I've read, probably the whole codebase which is a nightmare). More info on harmony here.

In the past I directly modified the .dll with dnSpy, however since they've added support for harmony in A19, I've switched to that since most people like to play with other mods as well.

jonathan-robertson commented 2 years ago

I've also been thinking about something similar to your suggestion. However say you have to go through couple of containes, you would have to send a TELockServer request for each one (then wait for the response), and depending on your latency, it would freeze the game for a few seconds. You could maybe multithread this which may come with its own item duplication bugs, and you wouldn't be able to access a container until it's done (which some players may find confusing).

Async could be handled pretty easily with a Coroutine. I've done this on the server's end a few times and for REST API Calls and couldn't say for sure how well it'd function on the client's end. It seems to work pretty well, but definitely can slow the game down if done incorrectly.

I suppose that part of the danger here would be... you can fit a LOT of boxes into a 15x15 area lol Even async calls might not go unnoticed with enough crates.

Another idea was to send the lockedTileEntities as a Map<Vector3, bool> (positions of containers and a flag if they are locked) to the client before using quickstack/restock, so the client can check every position without sending too much requests. I think I would have to create a new NetPackage for this, but it may not be as easy to implement.

Great idea! Perhaps the client could send a lock request for the entire collection of containers it's planning to interact with, then the server could lock the ones that aren't currently locked and reply with the list of containers the client has been granted access to. After inventory adjustments, the client could then respond to indicate it's ok to unlock those same containers it received locks for... sounds like a solid approach for very minimal networking.

But your explanation of Harmony does make this seem more difficult than I'd originally thought. I also haven't tried adding custom Net calls, but have heard it's possible. Will let you know if I find anything related to this or come up with an example.

misterperson commented 2 years ago

Instead of sending many request to the server for each container, the client should instead send a single request that it wishes to quick stack / quick restock. Let the server handle the rest, including getting the containers in range and finding unlocked ones. Should minimize packets sent by the client and the time containers are locked by the quick stack.

jonathan-robertson commented 2 years ago

Instead of sending many request to the server for each container, the client should instead send a single request that it wishes to quick stack / quick restock. Let the server handle the rest, including getting the containers in range and finding unlocked ones. Should minimize packets sent by the client and the time containers are locked by the quick stack.

I would think that this is the right approach, yeah.

I'm assuming that the host in a p2p game represents the "server" in the code. So taking this perspective would mean that the host would be responsible for restocking/resupplying items for the requesting player. I think in this case, the host would have the 'lock/unlock' state of all boxes.

So this should (I hope) cover all 3 scenarios: local solo game (as it does now), p2p, and dedicated server.

misterperson commented 2 years ago

I'm hosting a small server for some friends, and quick stacking into nearby containers is a QoL feature I wanted to add.

I am off-and-on debugging a dedicated server modlet that triggers a quick stack / restock via a chat command. I'm mainly working around not having a local player and poking around at different classes. So far, I've gotten items to occasionally be duplicated instead of stacking, which is progress. I was happy at all to get a project to build and debug correctly.

Once I get quick stacking to work as expected with one player, I will try adding in the more complex lock check and test with a friend. It would be very bare bones without the slot locking and user preferences, but sufficient for me, and a good starting point for making a client + server or client + host mod that has the current features of the local mod.

misterperson commented 2 years ago

It doesn't look like there's a safe way to remove items from a player's inventory server-side, so I think a client + server mod with a NetPackage is required

misterperson commented 2 years ago

The pull request is up #3