This supersedes addChunkPacket() in most cases, and has a more clear name. It broadcasts the given packet to every player who has the target position within their chunk load radius.
Grass can cause issues here by requesting blocks randomly offset away from itself, which can cause silent chunk loading on chunk ticking. It also causes crashes if chunk autoloading is taken away, which is obviously undesired.
It was also noticed that player chunkloaders cause chunks to start getting ticked as soon as they load their first chunk, which is before the entity is visible to everyone else on the server. This is probably undesired behaviour.
This has the triple bonus effect of a) making a lot of code easier to read, b) reducing Server::getInstance() usages, and c) removing a whole bunch of Server dependencies.
The network and block namespaces are untouched by this commit due to potential for merge conflicts. These should be dealt with separately on master.
Previously any random error could occur during an AsyncTask preparing a chunk, and the Level would never know about it and thus never send the chunk.
I don't know how many invisible-chunk bug cases this fixes, but I expect it's quite a lot.
I don't care if this matches PC behaviour or not. bugs.mojang.com is full of bug reports about this. Just search for "minecraft spawn protection not working" and you'll see what I mean.
If you want to disable spawn protection, actually disable it. This behaviour is something that most users are not aware of and find astonishing when they discover it.
This behaviour was copied from Minecraft PC, and it's nearly as unexpected there as it is here.
This commit reverses the stupidity done in eb0525e892219508d0c0e4602e835d5ddbacaf45.
fixes#2295
There is still an issue in that the spawn point will not be offset if the chunk is not generated, but this is better than the spawn point being down at y=0. The other issue is a job for another time.
If there is an empty list of blocks in the changedBlocks array for a chunk, that means that blocks changed the normal way and then were later set the direct way in the same tick. This means that no action needs to be taken on these chunks.
This changes how the AsyncPool works so that it does not immediately always start all of the workers in the pool.
Instead, workers will be started only when an idle worker was not found.
This allows for significant memory footprint reductions while idle.
In effect the async-workers setting in pocketmine.yml now dictates a _maximum_ pool size, not a fixed pool size.
If the same chunk is set into multiple different places in the world, the chunk's position is no longer able to be relied on, because it will have the position of the last place it was set. This results in chunks not getting sent correctly when the same chunk is set in multiple places.
This avoids the bug by using known valid coordinates (using chunk hashes) to establish the real coordinates, and also adds an assert to notify developers should they unintentionally set a duplicate chunk by mistake.
- 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 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.
