We should probably exclude known admin accounts from expiring and being claimed. Otherwise, everything looks good.
Apologies to Chomp that those bugs were not addressed sooner. Many insightful critical bug reports were both filed and resolved by him, and I guess I thought that he had taken care of all of them. Personally I would have appreciated a more gentle reminder that these were active bugs.
Due to a queue system requiring more changes to the Player, I decided we do not need it right now. The Player needs to check if they are currently in the process of unregistering, but the process has not completed. There is a lot of overlap of different data stored on the Player related to logout/unregistering and should probably do a larger cleanup of it.
Beyond that, I think the DelayedEvent unregisterEvent
on the Player is what is causing the inactivity timer to always cause you to log out. There doesn't seem to be a way to stop the DelayedEvent unregisterEvent
event loop.
Bottom line: I think the entire unregister/logout system here needs a rewrite, so it's fine for this to get in as-is. We can add the queue later.
Kenix Whisperwind (4fb7a976) at 30 Jan 04:37
Changes the UnregisterRequest to hold the Player reference.
One more comment, I would keep the unregistering check and return from this function. In other words, I'd only remove line 1361 that does the unregister here. In even more different words, I would make this code block look like this:
if (player.isUnregistering() && player.isLoggedIn()) {
return;
}
This is largely how I would have addressed the issue at the high level.
One passing comment that I intend to investigate: When did we move logout processing to the movement step?? That seems super strange to me.
I agree with removing the echo of a Logout packet when the server receives a Logout packet.
The approach on the whole seems right. Essentially we need to ensure that saving is done last because that will be what removes the references from the script system to query the data.
The feedback I have on the actual design approach would be this: I like that we now have a "UnregisterRequest` class. I don't like that this is held on Player for three reasons, with the final being the most important:
executeUnregisterRequest
function to the UnregisterRequest
class so someone can't unwittingly call it on the Player, causing this bug to return. There are a lot of functions in Player with a similar name that someone not knowledgeable in the code could just "pick one," and if they pick the one that actually does the request the bug could be reintroduced. It also separates the concerns of the classes, which is what OOP is all about.My suggestion would be to follow the pattern we have added in other places and create a queue that we poll from. We could keep the UnregisterRequest
class, add a Player to it, add a queue of UnregisterRequest
to the Server, unregistering a player adds a UnregisterRequest
for itself to that list, and then .poll() them in a loop each tick.
Kenix Whisperwind (3472a734) at 30 Oct 04:10
Merge branch 'develop' of https://gitlab.com/open-runescape-classic...
... and 421 more commits
Kenix Whisperwind (9b71ab1f) at 03 Jun 03:43
Kenix Whisperwind (9b71ab1f) at 03 Jun 03:43
Re-adds check for pitch modification permissions
... and 3 more commits
Kenix Whisperwind (9b71ab1f) at 03 Jun 02:54
Re-adds check for pitch modification permissions
Kenix Whisperwind (6c0d08a8) at 03 Jun 01:47
Adds first person view to Open Runescape Classic
... and 324 more commits
Most of input protocol abstraction layer and including mud38 input protocol parsing. Also small fix on size of array for woodcutting and fishing since possible maximum level can be 142. Needs testing, particularly custom protocol features such as clans and parties, but in general cases where variable / conditional input length is sent out from client on same opcode
Work towards #2954
This should be sniffbatchfromcurrentthread
rather than sniffBatchFromCurrentThread
to be consistent to the rest of runescript calls. Also, Runescript calls are only allowed to return booleans or primitive types.