Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear kicked/banned players from device slots automatically #714

Open
ClaudiaMia opened this issue Apr 18, 2024 · 6 comments · Fixed by #722
Open

Clear kicked/banned players from device slots automatically #714

ClaudiaMia opened this issue Apr 18, 2024 · 6 comments · Fixed by #722
Labels
Component: Directory server Changes to the Directory server Priority: Low Type: Improvement Improvement of existing feature

Comments

@ClaudiaMia
Copy link
Contributor

ClaudiaMia commented Apr 18, 2024

Update: The solution #722 contained an uncaught bug and had to be reverted, as described in #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
Copy link
Contributor Author

#722 contained an uncaught bug and had to be reverted, as described in #736. So this issue is still open and the remaining issue in the solution needs a fix.

@XenonRope
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member

Sekkmer commented May 1, 2024

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
Copy link
Contributor

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

@XenonRope XenonRope removed their assignment May 5, 2024
@ClaudiaMia
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Directory server Changes to the Directory server Priority: Low Type: Improvement Improvement of existing feature
Development

Successfully merging a pull request may close this issue.

4 participants