Project-Pandora-Game / pandora

https://project-pandora.com
Other
11 stars 3 forks source link

Clear kicked/banned players from device slots automatically #714

Open ClaudiaMia opened 4 months ago

ClaudiaMia commented 4 months ago

Update: The solution https://github.com/Project-Pandora-Game/pandora/pull/722 contained an uncaught bug and had to be reverted, as described in https://github.com/Project-Pandora-Game/pandora/pull/736. So this issue is still open and the remaining issue in the solution needs a fix.


Original description:

Currently, the "clear occupancy of the slot" is a way for admins to clean up after any sync issues, such as someone being kicked from the space. If they just kick and forget to clear the slot, then no one one else that is not admin can clear it and use it again.

I just tested it, and if the one kicked joins again, then even the admin can no longer clear the slot and they have to undeploy and deploy again. It is a bit of an inconvenient that this is a manual step that can easily be forgotten.

This issue is about investigating if admin action could have an automated clean-up step that clears the target from all slots in all devices of the space (by cycling through them).

Note: When Pandora's Space Service kicks some ghost out of a room device, it should also clear the slot.

ClaudiaMia commented 4 months ago

https://github.com/Project-Pandora-Game/pandora/pull/722 contained an uncaught bug and had to be reverted, as described in https://github.com/Project-Pandora-Game/pandora/pull/736. So this issue is still open and the remaining issue in the solution needs a fix.

XenonRope commented 4 months ago

I wrongly assumed that a character is removed from a space only when the character leaves the space or is kicked/banned. I reproduced the bug locally and I'm going to fix it.

Jomshir98 commented 4 months ago

Yes... All good about that bug. I myself literally wrote the code in question, reviewed your PR, tested it locally before merging it; and still found out about the issue only after deploying it to production 😅. We really are in need for tests for asset framework, which would catch problems like this that were once solved, but the existing solution is relatively fragile. (This exact problem of characters getting removed from room devices has happened few months back during development, but I forgot about it since; it also is the main reason for the current implementation) I think the easiest way to go about this would be having an explicit cleanup method that would have to be called explicitly by Shard. But there is a problem on Shard that it currently doesn't differentiate between character moving from the space into another (possibly on a different shard) and between character being unloaded right before space unload (there is a valid invariant that space the character is in is always loaded when the character is, so space gets loaded before character, but unloaded only after character unload)

Sekkmer commented 4 months ago

couldn't we just simply introduce a flag on unload that would disable this cleanup, so while the space unload or loads it doesn't clean up room devices only if the space is fully loaded

XenonRope commented 4 months ago

Unfortunately, I won't have time to continue this issue so I will unassign myself.

ClaudiaMia commented 4 months ago

Unfortunately, I won't have time to continue this issue so I will unassign myself.

Thanks for letting us know. Someone will for sure find a solution with the last remaining problem and bring your contribution into Pandora. You will be credited of course.