koobas.hobune.stream
The question is answered, right answer was accepted
(ArgumentException: GUILayout: Mismatched LayoutGroup.Repaint) when holding the Up/Down buttons of EditorGUILayout.BeginScrollView()!!
Currently the Unity's EditorGUILayout.BeginScrollView() is terrible in Performance for Lists and Arrays to display a huge amount of Contents into a Scrollview, even though the Scrollview only sees a certain amount of Contents only, so i decided to make it only Render/Display the current Contents that are only visible, it works fine if you move the ScrollView's Slider up and down, but if you hold the Up/Down buttons of the ScrollView, it will throw errors and warnings and some contents disappear for 1 frame and re-appear like normal, for that i made a small example script that you can test and see for your self:
using System.Collections.Generic;
using UnityEngine;
using UnityEditor;
public class TestCode : MonoBehaviour
{
public List<int> myList = new List<int>();
public Vector2 myScrollView = Vector2.zero;
}
[CustomEditor(typeof(TestCode))]
public class TestCodeEditor : Editor
{
private TestCode testCode {get {return (TestCode)target;}}
public override void OnInspectorGUI ()
{
serializedObject.Update();
MyVoid();
if(GUI.changed)
{
serializedObject.ApplyModifiedProperties();
EditorUtility.SetDirty(testCode);
}
}
private void MyVoid ()
{
int startIndex = 0;
if(testCode.myList.Count >= 5)
{
testCode.myScrollView = EditorGUILayout.BeginScrollView(testCode.myScrollView,GUILayout.Height(117));
startIndex = (int)testCode.myScrollView.y / 30;
}
if(testCode.myList.Count > 0)for(int a = 0; a < testCode.myList.Count; a++)
{
//Checks if the current index isn't in the view
if(a < startIndex || a > startIndex + 4)GUILayout.Space(30);
//Checks if the current index is in the view
if(a >= startIndex && a <= startIndex + 4)
{
EditorGUILayout.BeginHorizontal("Box");
{
//The Index Of The Property
GUILayout.Box(a.ToString("000"));
//The Integer Property
EditorGUILayout.PropertyField(serializedObject.FindProperty("myList").GetArrayElementAtIndex(a),GUIContent.none,true);
//These Buttons Will Swap/Remove Items From The List
GUI.enabled = a != 0;
if(GUILayout.Button("▲",GUILayout.Width(20),GUILayout.Height(20)))
{
int current = testCode.myList[a];
int previous = testCode.myList[a - 1];
testCode.myList[a] = previous;
testCode.myList[a - 1] = current;
break;
}
if(GUI.enabled != (a != testCode.myList.Count - 1))
GUI.enabled = a != testCode.myList.Count - 1;
if(GUILayout.Button("▼",GUILayout.Width(20),GUILayout.Height(20)))
{
int current = testCode.myList[a];
int next = testCode.myList[a + 1];
testCode.myList[a] = next;
testCode.myList[a + 1] = current;
break;
}
if(!GUI.enabled)GUI.enabled = true;
if(GUILayout.Button("-",GUILayout.Width(20),GUILayout.Height(20)))
{
testCode.myList.RemoveAt(a);
break;
}
}
EditorGUILayout.EndHorizontal();
}
}
if(testCode.myList.Count >= 5)EditorGUILayout.EndScrollView();
EditorGUILayout.BeginHorizontal();
if(GUILayout.Button("Add 10"))
{
int count = testCode.myList.Count;
while(testCode.myList.Count < count + 10)
testCode.myList.Add(0);
}
if(GUILayout.Button("Add 100"))
{
int count = testCode.myList.Count;
while(testCode.myList.Count < count + 100)
testCode.myList.Add(0);
}
EditorGUILayout.EndHorizontal();
}
}
The console throws these messages:
ArgumentException: GUILayout: Mismatched LayoutGroup.Repaint
Unexpected top level layout group! Missing GUILayout.EndScrollView/EndVertical/EndHorizontal?
GUI Error: You are pushing more GUIClips than you are popping. Make sure they are balanced)
I know what the middle warning message talks about, but the Console itself is wrong, not me or my code!
However if you replace the StartIndex with a number from the script, none of these happen, but the ScrollView will display a range of Contents only, i replaced it with "1" like:
//Checks if the current index isn't in the view
if(a < 1 || a > 1 + 4)GUILayout.Space(30);
//Checks if the current index is in the view
if(a >= 1 && a <= 1 + 4)
So is there a solution to fix that?
Thanks.
[EDIT] The error points at the line that Begins Horizontal with the "Box" GUIStyle, removing everything but keeping the EditorGUILayout.PropertyField() it will work fine, so there is something wrong with the EditorGUILayout.BeginHorizontal("Box); line.
Just a few comments on your code. First of all, do not call a method "a void". It's "a method". Void is just a special type which indicates "nothing". So while a method with the signature
int $$anonymous$$y$$anonymous$$ethod()
returns an integer a method with the signature
void $$anonymous$$y$$anonymous$$ethod2()
simply returns nothing. When you talk about a blue car you don't say "I have a blue" ^^
The If condition here is unnecessary and makes the line hard to read:
if(testCode.myList.Count > 0)for(int a = 0; a < testCode.myList.Count; a++)
{
// [body]
}
A for loop already has an if statement as you might know your for loop is equivalent to this construct:
{
int a = 0;
while (a < testCode.myList.Count)
{
// [body]
a++;
}
}
In case Count is 0 the loop body won't be entered at all since 0 is not smaller than 0
This:
if(GUI.enabled != (a != testCode.myList.Count - 1))
GUI.enabled = a != testCode.myList.Count - 1;
could be simplified to
GUI.enabled = a != testCode.myList.Count - 1;
or better to
GUI.enabled = a < testCode.myList.Count - 1;
this does already include all edge cases. When there is only a single element in the list the index will be 0 and "Count-1" will also be 0 so the button will be disabled.
Currently you mix direct object editing (using the target property) and the serializedObject. This is not recommended. It's difficult to suggest a specific solution as we don't know the exact purpose. Generally the serializedObject allows multi-object editor out-of-the-box while when using target / targets you have to handle it manually or do not have the feature at all.
SetDirty shouldn't be used for scene objects. You should use the Undo system. Note that RecordObject needs to be called before you do changes to the object.
Actually i was thinking of a name for that test script, and came up with "$$anonymous$$yVoid", so yeah.
And i always use:
if(list.Count > 0)for(int a = 0; a < list.Count; a++)
Because i think it won't create the Integer from the first place,like:
if(list.Count > 0)
{
//It won't pass here, because the condition is false at the first place!
int a = 0;
while(a < list.Count)
{
[Body]
a++;
}
}
And this line might increase a very little performance, because if the List.Count is one, the previous button gets disabled, then for the next button, it won't need to be disabled again if the GUI.enabled is false in the previous button!
if(GUI.enabled != (a != testCode.myList.Count - 1))
GUI.enabled = a != testCode.myList.Count - 1;
Also i already have Undo.RecordObject() in my full scripts, but i removed them in this example script. :)
No, that won't change anything. Local variables are not "created". When a method is entered it reserves a stackframe on the stack that includes all local variables which may be used or not. In the end your solution doesn't really improve anything as you just add another check which wouldn't be necessary.
GUI.enabled is just a state stored in an internal field. There is no overhead involved setting that state. It just makes the code harder to read and to understand ^^.
Note that if you have really a lot elements (say 10k or 100k) it would be better to combine all the out-of-view elements into a single GUILayout.Space. Since you have a fix height contant per item you can simply do GUILayout.Space(30 * startIndex) before the loop and another one with the remaining items after the loop. This should also significantly reduce the generated garbage as every layout element that is recorded during the layout event requires some memory.
The GUI layout system works in a two step procedure. Before any event is processed by OnGUI / OnInspectorGUI the system will issue a Layout event. Just to be clear here: Every event is paired with it's own layout event which is executed right before the actual event. So if a Repaint event should be processed you first get a Layout event and after that the Repaint event. Likewise when a MouseDown or MouseUp event should be processed there always will be a corresponding Layout event right before.
During the layout event Unity collects information about all the content that is there. This is necessary to be able to distribute the available space between the things it should draw. During the actual event Unity uses the information stored during the layout event to actually get the concrete rects for each element.
However that means you have to ensure that the layout event and the actual event "sees" and processes the same elements. Your fault here is that you just break out of your for loop when you press one of the buttons. That means you introduce several errors: First of all in line 38 you open a new horizontal layout group which you usually close at line 71. Though when you break out you do not close the group. That means at the end of OnGUI the layout stack has not been cleared. However you should already get an error at line 75 since you're still inside the horizontal layout group of the current loop item the layout stack doesn't expect opening a new group at this point.
Since there are many cases where you want to remove an element inplace it would be a pain to delay the removal of the element until the end of the OnGUI method. That's why Unity provides you a little helper tool GUIUtility.ExitGUI();. This is actually a sledgehammer solution to the problem ^^. ExitGUI simply does this:
public static void ExitGUI()
{
GUIUtility.guiIsExiting = true;
throw new ExitGUIException();
}
So it raises an exception which will cause the OnGUI method to terminate immediately. Unity will catch the ExitGUIException silently. This method is still undocumented but it's used in a lot of built-in editor code to exactly prevent such errors.
Where you also need to use ExitGUI is when you open a new EditorWindow from within inspector code. Opening a new EditorWindow causes a new layout stack to be created which makes it impossible to finish the current GUI procedure.
Note that the body of a GUI.Button will only be executed during a MouseUp event. So all you do is interrupting the MouseUp event processing. It will have no effect on the subsequent drawing as the repaint event is issued seperately with it's own layout event.
For more general information about the IMGUI system see my GUI crash course
Thanks for your answer, breaking out of the loop isn't the main problem, they work fine, but the problem is with the ScrollView's Up/Down buttons, because when you move the ScrollView's Slider to up or down, it works fine, but if you use the Up/Down buttons of the ScrollView, it will cause these errors!
I tested the GUIUtility.ExitGUI(); but i think it won't help, because i want it to draw the contents non stop, even for a frame, so the user won't notice something like that.
Or is there something that i'm missing? thanks.
Yes, that's why highly dynamic content changes are difficult to handle. Your problem is probably that you calculate "startIndex" in between based on the current y scroll position. This will result in a different behaviour between the layout and the actual event. There are essentially two solutions:
$$anonymous$$ake startIndex a member variable and set it based on the y position at the beginning only during the layout event.
$$anonymous$$ake startIndex a member variable and set it based on the y position at the end of your method only during an event that is not a layout event. This will ensure that you use the same startIndex for both, the layout event and the actual event.
Thanks, i will try that later and check if it works, now my PC is about to shutdown, 12% Battery, Thanks again.
