Draft: Feature/wheel terrain#202
Conversation
… for checking friction type
…a ScriptableAsset
FilipAlg
left a comment
FilipAlg
left a comment
There was a problem hiding this comment.
Seems to work for the most part. The main thing that's missing is enable/disable support and test. Other than that and the crash that we've discussed, most of the comments is general cleanup.
On a higher level, I'm curious whether we should create default contact materials for all Wheel<>Terrain pairs in a scene automatically. Since the actual contact material configuration is minimal this should be pretty simple and could ease scene setup quite a bit. Not needed for this MR but I would be interested to hear your opinion on this.
| public float HeightThreshold { get; set; } = 0.01f; | ||
|
|
||
| [field: SerializeField] | ||
| [Range( 0.0f, 1.0f )] |
There was a problem hiding this comment.
Range attribute cannot be applied to properties in Unity 2022, prefer using FloatSliderInInspector which does the same thing
| public float FullStrengthHeightChange { get; set; } = 0.1f; | ||
|
|
||
| [field: SerializeField] | ||
| [Min( 0 )] |
There was a problem hiding this comment.
Min attribute does not work in Unity 2022, use ClampAboveZeroInInspector( true )
| [field: SerializeField] | ||
| [Min( 0 )] | ||
| [Tooltip( "Paint radius in heightmap samples around each modified terrain vertex." )] | ||
| public int BrushRadius { get; set; } = 0; |
There was a problem hiding this comment.
Actually, what is the purpose of having a brush radius here? Shouldn't we only paint the affected cells?
| /// NB: if using multiple shape materials on the terrain this is not reliable! | ||
| /// </summary> | ||
| [SerializeField] | ||
| [FormerlySerializedAs( "WarnIfNotUsingCorrectForceModel" )] |
There was a problem hiding this comment.
We dont need this since this script is new
| private bool m_warnIfNotUsingCorrectForceModel = false; | ||
|
|
||
| [Tooltip( "Helper that will output a warning if the contact material in use by the terrain wheel doesn't have the correct force model." )] | ||
| public bool WarnIfNotUsingCorrectForceModel |
There was a problem hiding this comment.
Prefer using default backing fields instead of explicit backing fields where possible
| return false; | ||
| } | ||
|
|
||
| var cylinders = RigidBody.GetComponentsInChildren<Collide.Cylinder>() |
There was a problem hiding this comment.
Should we use RigidBody.Shapes.Where(s => s is Shapes.Cylinder) instead?
| Native = new agxTerrain.TerrainWheel( cylinder ); | ||
|
|
||
| if ( Settings == null ) | ||
| Settings = CreateSettingsFromLegacyValues(); |
There was a problem hiding this comment.
Why are there legacy settings for a new feature?
| /// </summary> | ||
| [SerializeField] | ||
| [FormerlySerializedAs( "WarnIfNotUsingCorrectForceModel" )] | ||
| private bool m_warnIfNotUsingCorrectForceModel = false; |
There was a problem hiding this comment.
Should this be true by default? Does the component still work by default when the force model is not set correctly?
There was a problem hiding this comment.
I guess in that case we might wanna do the logging only once per run (and contact material?) as well
| base.OnEnable(); | ||
| } | ||
|
|
||
| protected override void OnDisable() |
There was a problem hiding this comment.
OnDisable & OnEnable need to be implemented
2 participants
Introduce GUI for DeformableTerrainWheel and related DeformableTerrainMaterial properties.