garbagemule
commented
3 years ago Good suggestion. I think leave-on-end is a perfectly valid idea, but I'm not sure how "correct" it can feasibly exist in the current code base. I haven't tried anything out yet, but I can see a couple of potential issues that we need to talk about.
Problem 1: Intra-process delays
Right off the bat, the proposed 5-second delay is very unlikely to happen before the Sessions Rework, simply because MobArena's internal and public API is completely synchronous, meaning everything that happens when the arena ends is expected to happen right then and there.
Changing this behavior in the current codebase is not realistic, because it pretty much requires rewrites across the project. It is, however, one of the core assumptions that I hope to abolish in a graceful manner in the rework, exactly because of the swath of limitations it incurs. If the API is asynchronous instead, we can do a whole lot more. For instance, we might be able to re-introduce force-restore, but spread its operation out over many ticks. This would result in snapshotting and restoration taking wall clock time to complete, but without lagging the server. I'm sure players can accept a 5-second start delay, for instance, and with a bit of clever design, those 5 seconds could be built into a grace period that players benefit from.
It should be noted that this is a breaking API change, which means it also affects all integrations with the plugin that rely on this assumption to be true (including MobArenaStats). Any event handler for ArenaEndEvent that expects the arena to actually end will need to change. The problem is a lot bigger than a first glance makes it seem.
As of right now, delays are not likely to make it in. We will have to do without them for now.
Problem 2: Dying and respawning
Ever since auto-respawn was removed (effectively disabling it for all setups), the death procedure has existed as two separate procedures: what happens when the player dies, and what happens when the player respawns. The processes are documented in my comment on issue #678.
The problem with "leave on end" functionality in this regard is that we can't actually kick players who are still dead. They don't become spectators until they respawn. "Forcing" them to be spectators doesn't help, because we can't "reset" a player who hasn't respawned yet, and auto-respawn was removed for a reason (it was extremely buggy).
What this all boils down to is that we have to somehow figure out what to do about players who respawn. Because spectators are decoupled from arena sessions, and because sessions don't exist explicitly (there's no session identifier or anything like that), we have to find a "correct" way to handle the respawning players. Normally, they are just dumped into the spectator area if spectate-on-death: true (which technically should be spectate-on-respawn).
Ideally, we want respawning players to leave if the session they were playing in has ended, but because we can't distinguish sessions, the closest we can feasibly get is probably something like "if an arena is running, spectate, otherwise leave". This means that if a new arena session begins before the last standing player of the previous session respawns, they will become spectators of the new session when they respawn.
If we can live with this quirky and technically incorrect behavior, and assuming there are no other issues with this approach, then this problem is moot.
Problem 3: Functionality placement
This isn't so much a problem as it is a question of "where does the piece fit in the puzzle?"
The proposed solution places the "leave on end" functionality slap down the middle of the end procedure, and I guess that's primarily to encourage a "realistic" or "smooth" experience if the delay came along for the ride. I don't think it matters where in the end procedure the "leave on end" functionality sits, but we'll probably have to try some different things out to be sure.
The one thing that I know for a fact will be affected by the placement is the the ARENA_END announcement, since this is sent to any current spectators (unless global announce is enabled, in which case everybody sees it regardless of their participation in the arena). If the "leave on end" functionality is implemented before the announcement, it basically cancels out that announcement, since it will be broadcast to a completely empty set of players.
As long as we place the functionality at the suggested place or later, I think it will be fine.
Conclusion
If we omit the suggested delay and accept the quirky and technically incorrect behavior of "spec if running, leave if not", I think this is perfectly doable in the current code base. In that case, I also think this could get the "good first issue" label, since it isn't actually very complicated.
Thoughts?
Feature request
Short description
Currently, spectators are left in the arena after a session has ended. This leaves most players confused, as a lot of them expect to be kicked once it's over, hence there's nothing left to spectate. It's also a little weird how if you're the last person in an arena, it puts you as spectator. This again, confuses a lot of players. The suggestion is quite simple; add a
leave-on-end, which kicks everyone spectating the arena whenever it ends. Ideally, there can be a small timer before before the kick actually happens (5 seconds would suffice) as to make sure the kick isn't too abrupt.Implementation details
Simply, we can just put
leave-on-end: truein any of the arenas, which serves to kick any spectators when the arena ends. Here's where we could put it in the arena end procedure:Additional info That's all!