-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unity weather parameters, weather HUD, and a visual effect for snow #2909
Conversation
Looks like the serialization of these scenes is behind on some updates. Saving with no changes means that when I later make intentional changes they'll show up by themselves.
Looks like the serialization of this prefab is behind on some updates. Saving with no changes means that when I later make intentional changes they'll show up by themselves.
This is designed to be a Unity implementation of weather analogous to AirSim's Unreal weather system. Currently only the snow weather type is implemented. * Weather class stores global weather settings and applies them to instances of weather effects in the scene. * AirSimGlobal singleton provides access to global Weather object. * WeatherFX implements weather visuals surrounding a vehicle. * HUD contains weather menu for enabling weather and setting snow quantity. Triggered by F10. * Snow particle system.
FYI, this is work from the Microsoft Hackathon 2020. |
I am unfamiliar with this build system. Can anyone help me understand why the CI build failed on Ubuntu 16.04? Perhaps related to this?
|
It's just a rate limit on Travis which happens sometimes
You could close and reopen the PR or rebase against master, or if anyone has permissions, they can restart the Travis build |
Hopefully this will tickle the CI build to rerun and fix a rate limit issue.
Thanks @rajat2004! I made an update and the checks passed this time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested this locally and it works for me, however there's a few compile error and warnings we'd like to see addressed before merging this.
Unity/UnityDemo/Assets/AirSimAssets/Scripts/Weather/WeatherFX.cs
Outdated
Show resolved
Hide resolved
Unity/UnityDemo/Assets/AirSimAssets/Scripts/HUD/WeatherHUDSlider.cs
Outdated
Show resolved
Hide resolved
Unity/UnityDemo/Assets/AirSimAssets/Scripts/HUD/WeatherHUDSlider.cs
Outdated
Show resolved
Hide resolved
Unity/UnityDemo/Assets/AirSimAssets/Scripts/HUD/WeatherHUDSlider.cs
Outdated
Show resolved
Hide resolved
Unity/UnityDemo/Assets/AirSimAssets/Scripts/Weather/WeatherFX.cs
Outdated
Show resolved
Hide resolved
See https://forum.unity.com/threads/warning-cs0649-not-suppressed-properly-when-field-is-marked-as-serializefield.556009/ for discussion of this warning and how to avoid it.
Thanks for the review, @zimmy87. I made changes that I think are straightforward and resolve these issues. However, I wasn't able to test running them. It has been a long time since I worked on this and, currently, Unity crashes whenever I play the DroneDemo scene in the editor. It doesn't seem to be related to my changes. I crash with the original version of my PR branch, with these updates, or with the latest master branch. I'm not sure what has gone wrong and don't have time to investigate. Would you be willing to test it locally again? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested this locally again and the latest version is working for me. There's one remaining item that I think is a a typo, once that's addressed, I think this will be ready for merging.
Thanks for fixing my typo, testing, and merging @zimmy87!! |
This is a Unity implementation of weather analogous to AirSim's Unreal weather system. Currently only the snow weather type is implemented.
This PR is a step towards #2395, but is not yet a complete implementation of weather in Unity. I haven't contributed to AirSim before, so please let me know if there's something that doesn't make sense or isn't following conventions.