Skip to content
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

Fix collection drawer render order bug #65

Conversation

jeffcampbellmakesgames
Copy link
Contributor

@jeffcampbellmakesgames jeffcampbellmakesgames commented Apr 19, 2019

Summary

I've fixed a bug with the way ReorderableList elements were being drawn in CollectionEditor/GenericPropertyDrawer that should result in each item in the collection being rendered properly in the ReorderableList itself inline with their indexed position rather than directly below the list and in between the developer description. The chief thing to note here is that usage of ReorderableList is akin to a property drawer in that it does not respect GUILayout elements.

Before
image

image

After
image

image

Testing

I've created some test collection variables in an additional test commit to inspect and if there any custom types that would normally span a single line in height they should also work with this change. One of the results of this fix is that each element should be sized properly for the height of the property drawer rendering it.

Changes

Fixed issue with reorderable list elements

  • Renamed the existing method DrawPropertyDrawer to DrawPropertyDrawerLayout to indicate it used the native Unity auto-layout controls and created a new method using the older method name that uses the non-layout controls. Added method documentation to both explaining their usage.
  • Changed BaseVariableEditor to use the new DrawPropertyDrawerLayout method name.
  • Cleaned up CollectionEditor and removed several redundant calls to modify the rect, redundant callbacks, etc..
  • Fixed a bug in CollectionEditor where the ReorderableList elements were being drawn by EditorGUILayout methods. These elements act similarly to PropertyDrawers in that they do not respect the auto-layout system methods and need to drawn using the non-layout methods that accept a Rect. I have added usage of callback elementHeightCallback so that the height of the element rect can vary depending on the generic T type element's property drawer and changed the element drawing logic to use a non-layout version of the same method from GenericPropertyDrawer. The end result is that these elements are now drawn inline with the reorderable list element instead of below the entire list.

* Renamed the existing method DrawPropertyDrawer to DrawPropertyDrawerLayout to indicate it used the native Unity auto-layout controls and created a new method using the older method name that uses the non-layout controls. Added method documentation to both explaining their usage.
* Changed BaseVariableEditor to use the new DrawPropertyDrawerLayout method name.
* Cleaned up CollectionEditor and removed several redundant calls to modify the rect, redundant callbacks, etc..
* Fixed a bug in CollectionEditor where the ReorderableList elements were being drawn by EditorGUILayout methods. These elements act similarly to PropertyDrawers in that they do not respect the auto-layout system methods and need to drawn using the non-layout methods that accept a Rect. I have added usage of callback `elementHeightCallback` so that the height of the element rect can vary depending on the generic T type element's property drawer and changed the element drawing logic to use a non-layout version of the same method from GenericPropertyDrawer. The end result is that these elements are now drawn inline with the reorderable list element instead of below the entire list.
* This is a PR content commit that can be discarded after review/approval.
@DanielEverland
Copy link
Owner

Merged

@jeffcampbellmakesgames jeffcampbellmakesgames deleted the fix/collection_drawer branch April 21, 2019 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants