feat: implement weather system #6870
Conversation
dktapps
left a comment
There was a problem hiding this comment.
Also I don't know what editor you're using but it's made an enormous mess of your indentation. Indents should use tabs.
|
Overall this is looking better since the last time I reviewed, but there's still some work left to do. Thanks for sticking with it 🙂 |
dktapps
left a comment
There was a problem hiding this comment.
This looks mostly OK now, code-wise.
I think my main sticking points remaining are:
- There doesn't appear to be any way to disable the system, short of just setting the weather to clear for a very large duration. People aren't going to want it to rain in their lobbies.
- I don't like that we're constrained to clear/rainy/thunder.
- setRainLevel and setThunderLevel will not update the WeatherType, so that means plugins can make the
/weathercommand return wrong results anyway. - Instead of generally constraining it by the type, it would be better to have the
/weathercommand dynamically decide the weather type based on the rain and thunder amount (e.g. rain=0 + thunder=0 = clear, rain>0 + thunder=0 = rainy, thunder > 0 = thunder (whether or not it's raining)).
- setRainLevel and setThunderLevel will not update the WeatherType, so that means plugins can make the
|
@dktapps Would it be better if we add an on off argument in WeatherCommand? |
|
That might be a good idea. But I'm more thinking about an Normally weather cycle would be controlled by game rules, but we don't have a general system for those currently. But we should still save the appropriate NBT in Actually, I just noticed this doesn't save anything related to weather in level.dat. You'll need to fix that. |
|
My idea is, to save |
|
I don't want to put this in pocketmine.yml. pocketmine.yml is supposed to be the place for "don't touch, might break stuff" settings. Something this simple should be easily accessible. |
|
|
||
| $this->register(LightningBolt::class, function(World $world, CompoundTag $nbt) : LightningBolt{ | ||
| return new LightningBolt(Helper::parseLocation($nbt, $world), $nbt); | ||
| }, ['lightning_bolt', 'minecraft:lightning_bolt']); |
ShockedPlot7560
left a comment
There was a problem hiding this comment.
What I saw when I did a quick review
| if($sender instanceof Player){ | ||
| $world = $sender->getWorld(); | ||
| }else{ | ||
| $world = $sender->getServer()->getWorldManager()->getDefaultWorld(); | ||
| } |
There was a problem hiding this comment.
It could be better to have the ability to give world name or use the current/default as default. Even though, in my opinion, the name should be mandatory if it is not a player.
There was a problem hiding this comment.
These commands are typically meant to match vanilla, there's no option to specify world for other commands
| $type = match(strtolower($args[0])){ | ||
| "clear" => WeatherType::CLEAR, | ||
| "rain" => WeatherType::RAIN, | ||
| "thunder" => WeatherType::THUNDER, | ||
| default => throw new InvalidCommandSyntaxException(), | ||
| }; |
There was a problem hiding this comment.
As it's an enum, why not using WeatherType::from or tryFrom ?
There was a problem hiding this comment.
from() / tryFrom() can’t really be used here since the enum has no string values and the command input is lowercase. A manual mapping like this is still the simplest option.
| $rainInt = (int) ($this->rainLevel * 65535); | ||
| $thunderInt = (int) ($this->thunderLevel * 65535); |
There was a problem hiding this comment.
Will be clearer by using floor or round instead of casting to an int
There was a problem hiding this comment.
Since the packet requires an int, the final value needs to stay integer. The fields are floats, so casting is fine, but using round() before casting can make the conversion more explicit.
|
Okay, maybe I’ll continue this later, since I have other things to do. |
|
I don't know how to save to level.dat |
dktapps
left a comment
There was a problem hiding this comment.
On closer inspection there's a few features related to rain that need to be implemented:
- Fire should be extinguished when raining
- Rain should extinguish burning entities
- Farmland should hydrate when raining
Basically most stuff in this section needs to be implemented: https://minecraft.wiki/w/Rain#Effects
|
|
||
| public function getWeather() : WeatherType{ | ||
| return $this->weather; | ||
| } |
There was a problem hiding this comment.
As I said previously, "weather type" should be confined to commands only. It doesn't make sense anywhere else considering the dynamic rain & thunder strengths.
|
|
||
| public function setWeatherEnabled(bool $enabled) : void{ | ||
| $this->compoundTag->setByte(self::TAG_WEATHER_ENABLED, $enabled ? 1 : 0); | ||
| } |
There was a problem hiding this comment.
- These are not used anywhere
- The tag name likely doesn't match vanilla (check the wiki)
- It should be added to the
WorldDatainterface - Other world data types also need to implement something for this
| } | ||
|
|
||
| public function setWeatherEnabled(bool $enabled) : void{ | ||
| $this->compoundTag->setByte(self::TAG_WEATHER_ENABLED, $enabled ? 1 : 0); |
There was a problem hiding this comment.
Indentation is also not correct here
Note
The weather isn't as perfect as vanilla yet (can be continued later).
For every world, add a dynamic, automatic transitioning weather system including rain and thunderstorms.
Here, a simple weather manager is added which manages rain, thunder and lightning. It changes the state of the world, sends the appropriate packets to the players, and synchronizes the players’ visuals and sounds.
Related issues & PRs
Changes
API changes
Lightningworld/weatherLightningentityNew methods in
src/world/World.php:setWeather(),getWeather(), andtickWeather(),autoChange(),spawnLightning(),setRainLevel(),getRainLevel(),setThunderLevel(), andgetThunderLevel()Behavioural changes
Weather will transition back and forth between clear, rain, and thunder.
Thunderstorms will shoot lightning a random amount of times.
Clients will all see the rain effect and thunder sound, and the changes to the sky.
Backwards compatibility
No breaking changes.
Worlds without weather support behave the same.
Follow-up
Gradual brightness transition.
Adding rain variability.
Tests
Minecraft_2025-10-28-21-49-18.mp4
Manual tests show:
/weather rainand/weather thunderall work as intended.Thunder sounds and lightning visuals are synced.
No lag or crashing was able to be reproduced.