I've stuck to only doing this in the places where I'm sure we should never get false back. Other places I'm less sure of (and I found more bugs along the way).
this fixes many cases of corruption during disk-full situations - file_put_contents() would write an empty file, destroying the original data.
fixes#3152
this is not as good as phpstan/phpstan-src#769 (e.g. array_key_first()/array_key_last() aren't covered by this, nor is array_rand()) but it does eliminate the most infuriating cases where this usually crops up.
This reverts commit a5418a019dc2a83210084632130db8ce06f529ea.
The more I assessed this, the more I realized that this implementation
doesn't actually offer any value. Since modcounters don't persist after
chunk unload + reload, they can't be reliably used to detect changes in
chunks without additional event subscriptions.
For the purpose I actually intended to use them for (population task
cancellation) there's a) another solution, and b) modcounts are
unreliable for that too, because of the aforementioned potential for
chunks to get unloaded and reloaded.
For the case of detecting dirty chunks within PopulationTask itself,
they are also unnecessary, since the dirty flags are sufficient within
there, since FastChunkSerializer doesn't copy dirty flags.
In conclusion, this was a misbegotten addition with little real value,
but does impact performance in hot paths.
setPopulated() sets dirty flags on the chunk, causing the autosave sweep
to think they've been changed when they haven't. We now pass
terrainPopulated to the constructor to avoid this ambiguity recurring in
the future.
this allows entities to exist outside of generated chunks, with one caveat: they won't be saved in such cases.
Obviously, for player entities, this doesn't matter.
fixes#3947
instead, just ungate this and allow the provider to decide what to do.
Any chunk that contains entities or tiles is already always considered dirty, so the only thing the flags are good for is flagging chunks that previously had tiles and/or entities but no longer do.
In those cases, it's just removing keys from LevelDB anyway, so it's already very cheap.
Avoiding these redundant deletions is not worth the extra complexity and fragility of relying on flags to track this stuff.
WorldProviders now have the following requirements removed:
- __construct() is no longer required to have a specific signature
- static isValid() no longer needs to be implemented (you will still need it for registering, but it can be declared anywhere now)
- static generate() no longer needs to be implemented
This paves the way for more interesting types of world providers that use something other than local disk to store chunks (e.g. a mysql database).
WorldProviderManager no longer accepts class-string<WorldProvider>. Instead, WorldProviderManagerEntry is required, with 2 or 3 callbacks:
- ReadOnlyWorldProviderManager must provide a callback for isValid, and a callback for fromPath
- WritableWorldProviderManagerEntry must provide the same, and also a generate() callback
In practice, this requires zero changes to the WorldProviders themselves, since a WorldProviderManagerEntry can be created like this:
`new WritableWorldProviderManagerEntry(\Closure::fromCallable([LevelDB::class, 'isValid']), fn(string ) => new LevelDB(), \Closure::fromCallable([LevelDB::class, 'generate']))`
This provides identical functionality to before for the provider itself; only registration is changed.
this can fail if the backups directory points to a different drive than the original worlds location. In this case, we have to copy and delete the files instead, which is much slower, but works.
I REALLY advise against putting backups on a different mount point than worlds if you plan to convert large worlds.
