- Removed `Server->getScheduler()`. All plugins now have their own scheduler which is accessible using `Plugin->getScheduler()`. Aside from being syntactically more concise and pleasant, this also allows much more effective management of tasks when plugins are disabled.
- Removed `PluginTask` class. Before this PR it was necessary for plugin tasks to descend from `PluginTask` to ensure that the server could clean them up correctly on plugin disable. This is no longer necessary, so the `PluginTask` class has been removed. Plugins may now utilize the `Task` class as a base if they like.
- Added `Server->getAsyncPool()`. Since the global scheduler does not exist any more, it does not manage the server's `AsyncPool` any more. Additionally, `ServerScheduler` was previously bloated by a lot of `AsyncTask` related methods, which are now not necessary because direct access to `AsyncPool` is granted instead.
- `ServerScheduler`:
- `ServerScheduler` has been renamed to `TaskScheduler` since it is now a general-purpose task scheduler which is non-dependent on the user. This allows much greater flexibility and also makes it possible to unit-test.
- All `AsyncTask`/`AsyncPool` related methods have been removed - the task scheduler does not manage the async pool anymore.
- Calls to `Server->getScheduler()->scheduleAsyncTask()` should be replaced with `Server->getAsyncPool()->submitTask()`.
- Calls to `Server->getScheduler()->scheduleAsyncTaskToWorker()` should be replaced with and `Server->getAsyncPool()->submitTaskToWorker()`.
## Backwards compatibility
This poses significant backwards compatibility breaks for any plugins utilizing Tasks or AsyncTasks. These breaks are described above, along with basic upgrade steps. The upgrade process is quite straightforward.
## Follow-up
A large part of the goal with this pull request is to modularize these parts of the code so that they can be reused and also unit-tested. I would like to remove the existing test set from TesterPlugin at some stage when the AsyncPool can operate without a Server.
Because of the above, I am considering making further backwards incompatible changes directly to `AsyncTask` to remove the `Server` parameters from `onCompletion()` and `onProgressUpdate()`. These shouldn't be too difficult to upgrade from and can be prepared for in advance.
This is a significant breaking change for anything utilizing Tiles as they now do not store their NBT at runtime anymore.
This is another step in the process of ridding PocketMine-MP of runtime NBT.
It can be noticed that all tiles are now using class fields to store properties instead of NBT, which is much faster, uses less memory and is also more concise when written in code.
Highlights:
- Tile->namedtag has been removed.
- Tile->saveNBT() now accepts a CompoundTag() parameter. Typically it's expected that this will be fed a newly-created CompoundTag (this part may be improved in the future).
- New internal methods Tile->readSaveData() and Tile->writeSaveData() have been added. Instead of overriding __construct() and saveNBT() to load and save properties from NBT, you should now implement these methods instead.
This is not final and will see further changes before it's done.
This commit contains quite a few breaking changes with respect to how AsyncTasks are handled. This is necessary to allow separation of the ServerScheduler and the AsyncPool, because in the future the ServerScheduler may be removed and instead there will be isolated per-plugin sync-task schedulers - but we cannot have every plugin with its own worker pool for memory usage reasons if nothing else.
The following things have changed:
- ServerScheduler: scheduleAsyncTask(), scheduleAsyncTaskToWorker(), getAsyncTaskPoolSize(), increaseAsyncTaskPoolSize() and similar methods have all been removed. Additionally the static \$WORKERS field has been removed.
- Server: added API method getAsyncPool(). This grants you direct access to the server's AsyncPool. Calls to getScheduler()->scheduleAsyncTask() and scheduleAsyncTaskToWorker() should be replaced with getAsyncPool()->submitTask() and submitTaskToWorker() respectively.
Once upon a time, these userland functions were faster than calling builtins, but not anymore. According to my test the Math functions are twice slower in PHP 7.2 with typehints and 50% slower without typehints.
Inlining is slightly faster than using builtins, but the difference is very small - not worth making the code look any more ugly than it does already.
This is the same bug that Entity->getBlocksAround() had, except this actually checks for BB intersections. What this means is that as a result of the bug, one extra layer of blocks is unnecessarily checked on the max sides of the BB.
For example:
Assume you have a BB with maxY -5.5
You're definitely colliding with block -6 (because you're inside it) and you want to check an extra block in case you hit something weird like a fence.
So you want to check _at most_ up to block -5 (inclusive).
Following this maths:
-5.5 + 1 = -4.5
ceil(-4.5) = -4
This causes us to check block -4 unnecessarily. This may be a performance waste - depending on the BB size it could be proportionally a lot of blocks getting unnecessarily checked. This has not been benchmarked.
this is only used in one place, where it's being given floats, and it's 10% faster to use int for this because it won't convert it.
It is also 25% faster to remove typehints and 60% faster to inline it. We really need a proper PHP preprocessor for inlining.
long overdue... this isn't quite as extensible as the original api3/blocks system was, but this is primarily intended to replace Item->useOn(). If plugins want to use it it can be extended later on.
this was causing unexpected behaviour particularly on burning trees, whereby fire would be unconditionally extinguished if the block below it was removed.
