feat: add Open-Meteo provider and configurable weather cache#11
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for the Open-Meteo weather provider alongside Seniverse, with configurable caching and improved sun schedule syncing behavior.
Changes:
- Added Open-Meteo fetch + parsing paths for weather and sunrise/sunset.
- Made cache expiry configurable and introduced provider-aware cache keys.
- Added provider/lat/long config options and guarded sun sync from overlapping runs.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| RealTimeWeatherMod/Services/WeatherService.cs | Adds provider routing (OpenMeteo vs Seniverse), caching tweaks, Open-Meteo parsing, and LastFetchSucceeded. |
| RealTimeWeatherMod/Services/OpenMeteoWeatherMapper.cs | Maps Open-Meteo fields into existing WeatherInfo/condition semantics. |
| RealTimeWeatherMod/RealTimeWeather.csproj | Includes the new Open-Meteo mapper in compilation. |
| RealTimeWeatherMod/Core/AutoEnvRunner.cs | Uses provider-aware “usable key” check; prevents concurrent sun sync; gates sun sync on LastFetchSucceeded. |
| RealTimeWeatherMod/ChillEnvPlugin.cs | Adds config entries: provider selection, cache expiry, and Open-Meteo coordinates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public static IEnumerator FetchWeather(string apiKey, string location, bool force, Action<WeatherInfo> onComplete) | ||
| { | ||
| string normalizedLocation = NormalizeLocation(location); | ||
| LastFetchSucceeded = false; | ||
| string cacheKey = GetCacheKey(location); | ||
|
|
||
| if (!force && HasValidCacheNormalized(normalizedLocation)) | ||
| if (!force && HasValidCacheNormalized(cacheKey)) | ||
| { | ||
| onComplete?.Invoke(_cachedWeather); | ||
| yield break; | ||
| } |
| private static int GetCacheExpiryMinutes() | ||
| { | ||
| return ChillEnvPlugin.Cfg_CacheExpiryMinutes != null ? ChillEnvPlugin.Cfg_CacheExpiryMinutes.Value : 60; | ||
| } |
| private static bool HasValidCacheNormalized(string cacheKey) | ||
| { | ||
| return HasAnyCacheNormalized(cacheKey) && | ||
| DateTime.Now - _lastFetchTime < TimeSpan.FromMinutes(GetCacheExpiryMinutes()); |
| onComplete?.Invoke(fallback); | ||
| } | ||
| } | ||
| catch { onComplete?.Invoke(fallback); } |
| } | ||
| } | ||
| catch { onComplete?.Invoke(null); } | ||
| catch { onComplete?.Invoke(fallback); } |
| private static WeatherInfo ParseOpenMeteoWeatherJson(string json) | ||
| { | ||
| string current = ExtractObject(json, "\"current\":{"); | ||
| if (string.IsNullOrEmpty(current)) return null; |
| private static string ExtractObject(string json, string prefix) | ||
| { | ||
| int start = json.IndexOf(prefix, StringComparison.Ordinal); | ||
| if (start < 0) return null; | ||
| start += prefix.Length - 1; | ||
|
|
| private System.Collections.IEnumerator SyncSunScheduleRoutine(string targetDate) | ||
| { | ||
| _isSunSyncRunning = true; | ||
| int retryCount = 0; |
| if (success) | ||
| { | ||
| _isSunSyncRunning = false; | ||
| yield break; | ||
| } |
| ChillEnvPlugin.Log?.LogError("[SunSync] 达到最大重试次数，今日放弃同步"); | ||
| _isSunSyncRunning = false; |
|
/review |
please wait for a bit. I have fixed some parts. Will update now. Same Commit and PR. update: Done. Please check for new fix. Thanks! |
|
/review |
1 similar comment
|
/review |
|
Thanks for triggering It looks like the |
|
CI爆炸了，别慌，战术性调整 |
|
/review |
There was a problem hiding this comment.
🔍 OpenCodeReview found 8 issue(s).
- ✅ 7 inline comment(s)
- 📝 1 in summary
📄 RealTimeWeatherMod/Services/OpenMeteoWeatherMapper.cs
The hard-coded integer weather codes (21, 11, 14, 13, 26, 4, 1) used throughout as return values and comparisons reduce readability and make the mapping fragile to changes. Consider defining named constants (e.g., private const int SeniverseSnow = 21) or an enum aligned with the Seniverse code system. This would make the mapping intent self-documenting and easier to maintain alongside WeatherService.MapCodeToCondition.
💡 Suggested Change
// Seniverse codes: 1=Clear, 4=Cloudy, 11=Thunder, 13=LightRain, 14=HeavyRain, 21=Snow, 26=Fog
if (snowfall > 0d || IsSnow(weatherCode))
{
return 21;
}
if (IsThunder(weatherCode))
{
return 11;
}
if (IsHeavyRain(weatherCode) || precipitation >= 2.5d || rain + showers >= 2.5d)
{
return 14;
}
if (IsLightRain(weatherCode) || precipitation > 0d || rain > 0d || showers > 0d)
{
return 13;
}
if (weatherCode == 45 || weatherCode == 48)
{
return 26;
}
if ((weatherCode >= 1 && weatherCode <= 3) || cloudCover >= 65d)
{
return 4;
}
return 1;
Remove the cloudCover >= 65d override. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Reuse WeatherService.MapCodeToCondition(normalizedCode) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Change the fallback text from "OpenMeteo" to $"OpenMeteo({weatherCode})"
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
🔍 OpenCodeReview found 9 issue(s).
📄
|
|
😰bot发狂了 |
|
OK bro, I think the first advice copilot given is actually good and will apply. Others are just some AI trash :( Wish you a good sleep ^^ |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
| private static string ToWeatherText(int weatherCode, int normalizedCode) | ||
| { | ||
| if (normalizedCode == 21) return "Snow"; | ||
| if (normalizedCode == 11) return "ThunderRain"; | ||
| if (normalizedCode == 14) return "HeavyRain"; | ||
| if (normalizedCode == 13) return "LightRain"; | ||
| if (normalizedCode == 26) return "Fog"; | ||
| if (normalizedCode == 4) return "Cloudy"; | ||
| return weatherCode == 0 ? "Clear" : "Unknown"; |
There was a problem hiding this comment.
ToWeatherText uses the raw weatherCode in its fallback, which can produce "Unknown" when normalizedCode=1 but weatherCode≠0. Meanwhile, ToCondition(1) returns WeatherCondition.Clear via MapCodeToCondition, creating an inconsistency. Add an explicit case for normalizedCode=1 so the text is fully determined by the normalized code, and consider logging a warning for unrecognized weatherCode values.
Suggestion:
| private static string ToWeatherText(int weatherCode, int normalizedCode) | |
| { | |
| if (normalizedCode == 21) return "Snow"; | |
| if (normalizedCode == 11) return "ThunderRain"; | |
| if (normalizedCode == 14) return "HeavyRain"; | |
| if (normalizedCode == 13) return "LightRain"; | |
| if (normalizedCode == 26) return "Fog"; | |
| if (normalizedCode == 4) return "Cloudy"; | |
| return weatherCode == 0 ? "Clear" : "Unknown"; | |
| private static string ToWeatherText(int weatherCode, int normalizedCode) | |
| { | |
| if (normalizedCode == 21) return "Snow"; | |
| if (normalizedCode == 11) return "ThunderRain"; | |
| if (normalizedCode == 14) return "HeavyRain"; | |
| if (normalizedCode == 13) return "LightRain"; | |
| if (normalizedCode == 26) return "Fog"; | |
| if (normalizedCode == 4) return "Cloudy"; | |
| if (normalizedCode == 1) return "Clear"; | |
| return "Unknown"; |
| { | ||
| Code = normalizedCode, | ||
| Text = ToWeatherText(weatherCode, normalizedCode), | ||
| Temperature = (int)Math.Round(temperature), |
There was a problem hiding this comment.
Math.Round without a MidpointRounding argument defaults to banker's rounding (round-to-even). For example, 2.5°C rounds to 2 and 3.5°C rounds to 4, which can be counterintuitive for weather display. Consider using MidpointRounding.AwayFromZero for temperatures: Math.Round(temperature, MidpointRounding.AwayFromZero).
Suggestion:
| Temperature = (int)Math.Round(temperature), | |
| Temperature = (int)Math.Round(temperature, MidpointRounding.AwayFromZero), |
| return 4; | ||
| } | ||
|
|
||
| return 1; |
There was a problem hiding this comment.
When no conditions match, ToSeniverseCode silently returns 1 (Clear). Unrecognized weather codes are effectively hidden, making it harder to detect API changes or mapping gaps during development. Consider logging a warning via ChillEnvPlugin.Log?.LogWarning(...) when an unmapped code is encountered, or alternatively have the caller in WeatherService.ParseOpenMeteoWeatherJson validate the result.
Suggestion:
| return 1; | |
| // Fallback: treat as clear. Log a warning if the weatherCode was not explicitly 0. | |
| if (weatherCode != 0) | |
| { | |
| ChillEnvPlugin.Log?.LogWarning($"[OpenMeteo] Unmapped weather code {weatherCode}, falling back to Clear"); | |
| } | |
| return 1; |
| if (request.result != UnityWebRequest.Result.Success || string.IsNullOrEmpty(request.downloadHandler.text)) | ||
| { | ||
| ChillEnvPlugin.Log?.LogWarning($"[OpenMeteo] 请求失败: {request.error}"); | ||
| onComplete?.Invoke(fallback); | ||
| yield break; | ||
| } |
There was a problem hiding this comment.
Observability gap: no log when fallback cache is served. When a weather fetch fails (network error or parse failure), onComplete?.Invoke(fallback) is called with the previous cached data. The failure itself is logged ("[API] 请求失败" / "[OpenMeteo] 解析失败"), but there is no corresponding informational message indicating that stale cached data is being returned. In noisy logs, the failure warning can be overlooked, and the caller has no way to distinguish "fresh data" from "stale fallback" without comparing the returned WeatherInfo to CachedWeather. Consider adding a log like "[OpenMeteo] 使用缓存数据作为回退" to make this path observable.
Suggestion:
| if (request.result != UnityWebRequest.Result.Success || string.IsNullOrEmpty(request.downloadHandler.text)) | |
| { | |
| ChillEnvPlugin.Log?.LogWarning($"[OpenMeteo] 请求失败: {request.error}"); | |
| onComplete?.Invoke(fallback); | |
| yield break; | |
| } | |
| if (request.result != UnityWebRequest.Result.Success || string.IsNullOrEmpty(request.downloadHandler.text)) | |
| { | |
| ChillEnvPlugin.Log?.LogWarning($"[OpenMeteo] 请求失败: {request.error}"); | |
| if (fallback != null) ChillEnvPlugin.Log?.LogInfo("[OpenMeteo] 使用缓存数据作为回退"); | |
| onComplete?.Invoke(fallback); | |
| yield break; | |
| } |
|
🔍 OpenCodeReview found 11 issue(s).
📄
|
|
😇先别提交了，我现在没有电脑不好给这bot毙了。而且现有的ci完全不看历史评论的问题，明天我也一并解决掉。 |
|
Sounds good, I'll wait. I went through the bot's comments, fixed what actually needed fixing, and left the rest alone. Those are style opinions and design suggestions, nothing that affects correctness. One thing to flag: I never got to compile any of this. Missing the .NET Framework 4.7.2 targeting pack on my end, and the test project wants .NET 9 while I'm on 6.0. The changes were copilot-assisted too, and that thing needed more babysitting than I'd like to admit. With no build and no tests to catch me, I figured stopping here beats flying blind. Sorry about that. You'll want to give it a spin in-game yourself. Cache expiry, offline fallback, provider switching, the usual suspects. Oh and take your time with the CI fix tomorrow. Cheers. |
Changes
Notes
Validation