[Compat] Ambient sound 1.20.1 - Rain drop sound
Closed this issue · 38 comments
mod: Ambient Sound
url: https://www.curseforge.com/minecraft/mc-mods/ambientsounds
version: AmbientSounds_FORGE_v6.1.11_mc1.20.1
description：
Ambient Sound triggers a rain drop sound when its raining, but with the solar weather, the rain sound may occur during clear weather for example.
Also, how does react "WeatherManager.getRainOrSnow(level, MapChecker.getSurfaceBiome(level, pos).value(), pos)" if solar weather is OFF ?
This function is not recommended for use, as there is actually an API alternative available. For any functionality, we only recommend using what is provided in EclipticSeasonsApi.
You might have seen this in EclipticUtil, but EclipticUtil is part of the internal implementation of EclipticSeasonsApi and an early intermediate result.
Additionally, this function is internal and considered experimental. Due to the fact that some mods do not support frequent changes in precipitation—or may even crash (e.g., Simple Cloud)—we eventually introduced a more stable getPrecipitationAt method. This newer function is now used for quickly querying the precipitation status of a biome.
It will not activate when the Level does not have Solar Weather enabled.
If you're using the API, it will automatically check for this condition.
Rain sounds might still occur during clear weather — this could be because the rainLevel exceeds the threshold defined by AmbientSounds.
I'm not sure what that exact threshold value is.
As for compatibility, there’s not much I can do on my end. I believe it would be better if they are willing to handle it themselves.
Because once we attempt to do it, we’d have to mixin into their condition checks, which isn't ideal.
When Solar Weather is enabled, Ecliptic Seasons performs a buffered calculation on the client side and at biome boundaries to provide a transitional rainLevel value.
This might be one of the reasons.
Actually, you can adjust the WeatherBufferDistance in the client settings — setting it to 4 might help improve the situation.
The default value is 16, which is meant to handle basic scenarios.
By the way, getRainOrSnow doesn't get triggered because we never call that function in that case. So if you try to call it while Solar Weather is disabled (there’s a function in EclipticUtil that can check this, but it’s not part of the official API since manual checks are usually unnecessary), it will most likely result in an error.
I'll move the code to EclipticSeasonsApi
Could you add there also the EclipticUtil.getTimeInSolarTerm(level) and EclipticUtil.getNowSolarDay(level) in it ?
Instead of mixin their code, maybe mixin the isRainingAt method of Minecraft ?
just need to set it to a lower value
These two functions are planned to be added in the next version and are not expected to change significantly in the foreseeable future.
what does the weather buffer do ?
you do some kind of calculation every X tick on how much rain level increase, and when it reaches a certain value, you trigger the rain ?
But if the threshold is per biome, then atsome point, level.isRaining will return true is not in sync with the threshold
"Weather buffer distance" is used on the client side to perceive the weather state around the player, so that transitions during biome changes are smoother. However, this requires a suitable distance.
level.isRaining is only applicable on the client side, since the server side does not have a position parameter.
Please not repeat asking about this — it seems you haven’t really read what I said. I’ve already provided a solution. If you’re interested in technical details that you don’t actually need to use, you’re welcome to debug or playtest it yourself.
I don’t have that much free time lately to spend half an hour now and then discussing a mod.
rainLevel and isRaining are part of Minecraft’s original settings.
Solar Weather is a biome-based local weather system, but it's largely constrained by the current rendering system's settings.
I haven’t made many adjustments to that low-level foundation. Therefore, on the client side, it is currently set up to update the weather state when the player is about to change biomes — to accommodate shaders and existing client-side mods.
If certain mods rigidly rely on isRaining, issues can of course occur at times. This is also possible in vanilla.
That’s why shaders, for example, generally use rainLevel instead of isRaining.
On the server side, this kind of handling is not performed — because there is no position parameter, and isRainingAt is generally used instead.
Moreover, the server typically doesn’t require such smooth transitions in the first place.
The purpose of this system is to define different rainfall frequencies for each biome, rather than simply representing overcast weather.
I believe what I've said so far is sufficient, and this discussion has gone beyond the scope of this issue.
Sorry, I just realized the API hasn't been updated.
Since it's usually automated — but this time I forgot to change the version number.
Thanks
Let me know when Solar Weather will be more polished, in the meantime we'll simply disable it
Overall, when it comes to Solar Weather, it's a system full of compromises.
One of the main considerations is compatibility with DH, since certain systems there don't load chunks in the same way. Additionally, rain particles in Minecraft can only render around the player, which introduces further limitations.
A known issue is when the player is near the edge of a biome — this can indeed cause noticeable problems. One significant drawback is that players cannot verify the weather in biomes they are not physically in, which is actually why the weather buffer distance exists. A common visual issue may occur when a player sees zombies not burning in the distance.
If a player simply walks through a biome, rainfall will change gradually to give a scene-transition-like effect. Also, regarding level.isRaining, it’s directly tied to rainLevel, so there’s no issue of desynchronized thresholds.
If you find this feature unsuitable for your gameplay or use case, I recommend disabling Solar Weather in the settings.
By default, it remains enabled because it also brings considerable benefits: it's compatible with shaders, works well with DH, and allows for biome-specific weather differentiation. This helps make biome humidity feel more realistic, and adds meaningful variation to biome visuals (e.g., making greenery look different for a reason). As long as players don’t settle right next to biome borders, everything works quite well — and even if they do, it usually doesn’t cause serious issues.
There’s probably not much more we can do with Solar Weather at this stage.
If you have more demanding or specific needs, you might consider using it together with Simple Cloud.
(However, this may cause issues in snowy environments, as we currently do not support snow cover information storage on a per-chunk basis. This approach is clearly incompatible with DH and would introduce additional data synchronization complexity.)
Of course, if you enable IceAndSnow in the settings, it will revert to vanilla behavior for snow coverage.
(Be sure to also enable IceAndSnowMelt at the same time.)
This is also the core purpose of the Solar Weather option.
Biome-based weather is a system that might be interesting — but it also comes with imperfections that some players may find unacceptable. Due to the presence of shaders, many things become difficult or undesirable to modify, which adds to the challenge.
Another notable drawback is compatibility issues.
Many mods rely heavily on methods like level.isRaining — and while it’s not always a problem, it can cause trouble in some cases.
Without a targeted Mixin implementation or explicit compatibility from the other mod, things can become quite tricky to handle.
Another change — where falling snow no longer turns into snow blocks — may also introduce some minor compatibility issues, such as with footprints or snow block detection. However, since there aren't many mods that rely on this behavior, it's generally not a major concern.
That’s basically everything. @sfiomn
And I’d still recommend describing your specific needs directly if you run into any issues — that way I can better explain what’s possible or what limitations might exist.
Lately I haven’t had much time to go in-depth about mod behavior or design principles, and these discussions can be quite time-consuming.
Another source of incompatibility comes from this aspect.
If a biome starts snowing, the ambient sound may have handling issues.
It seems that ambient sounds are determined in combination with temperature, but we no longer modify biome temperatures (because doing so can easily cause some world generation problems).
Maybe Ambient sounds could make a compat with ecliptic then
having the rain sound while being on clear weather is a major concern for us, so better waiting this compat to work then
It seems that Ambient Sounds uses isRainAt, which creates a bit of a conflict.
This means its threshold isn’t based on rainLevel > 0.2f. Since for isRainAt, our handling is basically server-side and only returns true or false depending on whether it’s raining in the biome, this might be the reason why certain locations are incorrectly detected.
In this case, you could try setting WeatherBufferDistance to the minimum value first — it should be 0 or 1.
btw, it looks like my guess about it using temperature was correct.
You may have seen isRainAt in ClientWeatherChecker, but that function is actually deprecated
In the level, the final check for whether it's raining at a certain position has actually been changed to WeatherManager.isRainingUnderSky (although it doesn't take edge buffering into account at the moment, due to performance considerations).
Ambient sound doesn't maintain 1.20.1 anymore, so they won't add a compat on their end sadly
Adding compatibility at this point is the same for us — it basically requires a mixin. Currently, we already have too many mixins, and I’d like to reduce them as much as possible. Moreover, the seasons mod is, in a sense, more like an API. Additionally, Ambient Sounds is simply too large and not very build-friendly. Our current plan is to avoid adding any new compatibility that would require a mixin before the official release.
Another issue is that we have our own season ambient system — it's relatively simple, but it's also part of our development plan. I think our sounds are relatively quiet, making them more suitable as subtle background noise.
Ambient Sounds doesn't specifically target seasons, even though it does take temperature into account.
I’ve prepared a dedicated repository for storing small compatibility patches that are not suitable to be included in the core mod. Some mods may only require a few simple mixins to work properly without causing major issues, and including such mods in the core mod wouldn't be appropriate.
https://legacy.curseforge.com/minecraft/mc-mods/ecliptic-seasons-multimod-patch
However, unfortunately, it hasn't passed the CurseForge review yet.