luxaritas / pylus Goto Github PK

• Instead of using strings for event names, class references should be used instead
• These classes should encode type information about their payload so that the handler function can be typed accordingly

I had good intentions here, but I don't think it is ultimately beneficial. We should probably just have a folder full of packet definitions and other serialization/deserialization code.

The blocking point for upgrading is that we use DB calls as synchronous operations, whereas in v3 we basically have to treat them asynchronously since we execute within an asynchronous context (given that our communications run on top of asyncio). Some work is required in order to make that work - specifically, we probably need to convert a bunch of functions to be async (including the core event handling mechanism)

I really should have made an alt account to raise this issue... But it was annoying my OCD! :P

Ultimately, I think handling things this way will lead to issues with both performance and add complication without really providing useful flexibility for mods that I had intended. This functionality should instead be moved into core plugins

In addition to the priority system, it may necessary to be able to fully disable a plugin. In particular, while handle_until_value and handle_until_return allow you to insert a plugin at a higher priority and "intercept" another action, simply using handle accumulates results. Either:

• Plugins should be able to disable other plugins by referencing their class name
• Plugins should be able to be disabled in the config by eg the module name

This should just be a component of a zone server (specifying a dedicated chat server doesn't work)

Instead of using the actions function, it would be great to have a function decorator that specifies what event is being listened for for a function within a plugin

Currently, we load plugins by folder depending on what type of server we boot. Instead, we should have a class parameter or decorator that specifies which types of server it should be loaded for. This could be done using a syntax like:

• * (always loaded)
• auth (auth only)
• master (master only)
• world:* (any world server)
• world:char (char only)
• world:<zoneid> (only world servers of a zone with the given numeric ID - perhaps we add an enum with common world names)

For type safety and autocomplete goodness. Suggested by @alentoghostflame

• Instead of hardcoding spawnpoints, luz/lvl paths, etc we should use the cdclient
• Instead of shipping the client_assets folder, we should have a config parameter to point to the res folder of an unpacked client

It would be great to set up a github actions CI running pylint, mypy, and a unittest-based test runner

We're using the database to store sessions, but wouldn't it be a better idea to use a cache like redis to store them?

Currently, a new process is started for every entry in the config file. This is not ideal for a handful of reasons. Instead, we should introduce a proper instancing mechanism. That is:

• There should be a new server instance type called a master server. This can still run on pyraknet (and even use the same handshake mechanism with a different ID, etc), but it will only connect to other servers rather than clients.
• The address of the master server and auth server should be statically configured (as clients need to know where to find auth, and all servers need to know where to find master)
• The master server should be the only instance directly created in the bootstrap script. Master should automatically instantiate auth and char (ie, world with no zone/zone 0). Servers can then request the address of a different zone server to redirect a client to. If the master server doesn't already have a record to a server for that zone, the master server will then initialize a new server (by default, just by starting a new process).
• The master server should keep track of the number of users in a given zone. The cdclient ZoneTable defines both a population_soft_cap and population_hard_cap. When the soft cap is reached, additional players are only sent to the zone if explicitly requested (eg, to join a friend or teammate), whereas once the hard cap is reached, players are sent to a new zone regardless.
• All other servers (auth and all zones) should continually ping the master server so that it can ensure that they are still healthy. If it does not receive a heartbeat within k*ping_interval period of time (where ping_interval is a configurable amount of time describing how often servers ping the master, and k being some configurable value k > 2 to account for potential unreliability or intermittent latency).
• When a zone has not had any users for some period of time, it should be closed. Either a zone should be responsible for closing itself, or instructed by the master server, though I'm not sure which. In the case of closing itself, it would presumably need some way for the master server to ensure that it stays alive if for some reason you want to keep an instance "warm". If the master server is responsible, it would also need consistent heartbeats from the master server to ensure that it hasn't been permanently disconnected with no way of being notified that it should be shut down - however, you probably don't want to shut down if you still have players on it.

This functionality should be open to plugins extending it. For example:

• Instead of starting a new process directly, a plugin might find space among a series of cloud server instances and notify a bridge server listening on that host to start a new instance, or it might even request a new VPS to be spun up.
• It might be desirable to have instances provisioned before they are strictly needed, ie to avoid the cold boot time.
• It might be desirable to implement a more advanced metric to determine if an instance is healthy.