The test was relying on immediate updates from the prefab system which are now scheduled for a later tick - this reworks the tests to wait for deferred updates before validating state
Signed-off-by: nvsickle <nvsickle@amazon.com>
The EntityOutlinerListModel was violating the QAbstractItemModel contract in a few cases, as reported by `QAbstractItemModelTester`. The important ones causing issues were:
- Entry order was not guaranteed, leading to model indices pointing at invalid data
- Parent/child relationships could be temporarily invalid due to a change I made in EditorEntityModel::RemoveEntity to try to avoid an unnecessary reparent operation - as it turned out, the parent/child data was being cached even for recreated entities and not clearing child data could cause issues
- `EntityOutlinerListModel::ProcessEntityUpdates` was emitting data changed between two indices that didn't necessarily share a parent, which is [undefined behavior](https://doc.qt.io/qt-5/qabstractitemmodel.html#dataChanged)
The other reported issues (that weren't really causing issues with `QTreeView`) were:
- The root index had flags other than `Qt::ItemIsDropEnabled`
- `rowCount` showed all columns as having children
- `parent` showed indices as being parented to a non-0 column
This change introduces fixes for the above issues, namely:
- Reverts my change to `EditorEntityModel::RemoveEntity` to ensure we don't have invalid parent/child references sitting in the cache
- Ensures `EditorEntityModelEntry` child ordering is guaranteed sorted by EntityId, to prevent the `EntityOutlinerListModel` from having indices pointed at invalid data*.
- Fixes various model sanity issues, such as `rowCount` being 0 for indices with a non-0 column
Two unit tests were added to reproduce the invalid behavior and validate the fix: TestCreateFlatHierarchyUndoAndRedoWorks and TestCreateNestedHierarchyUndoAndRedoWorks
This change focuses on correctness over performance. My subjective in-Editor outliner experience is about the same, but it may be worthwhile to expand the test coverage with a benchmarking suite to look into areas for optimization.
*As a rough illustration of the previous child ordering behavior, consider the following entity hierarchy:
```
Root (EID 9999)
|_ Child1 (EID 2)
|_ Child2 (EID 3)
|_ Child3 (EID 4)
```
With an representations like the following pseudocode:
```
// EditorEntityModel representation
EditorEntityModelEntry root;
root.children[0] = 2;
root.children[1] = 3;
root.children[2] = 4;
// EditorOutlinerListModel representation
// row, column, user data (64 bit uint)
child1 = QModelIndex(0, 0, 2)
child2 = QModelIndex(1, 0, 3)
child3 = QModelIndex(2, 0, 4)
```
When removing a child, the `EditorEntityModel` used to do roughly the following:
```
// Swap and pop the last child
int indexToRemove = 0;
swap(root.children[indexToRemove], root.children[root.children.size() - 1]);
root.children.resize(root.children.size() - 1);
model.notifyRemoved(root, indexToRemove); // model removes the row indicated
// Leading to this EditorEntityModel state
root.children[0] = 4;
root.children[1] = 3;
// And this EntityOutlinerListModel state, note that the row indices are swapped from the indices in the backing storage
child2 = QModelIndex(0, 0, 3)
child3 = QModelIndex(1, 0, 4)
```
A QModelIndex having a row that doesn't match its underlying data is undefined behavior, and was the source of an intermittent crash in our `QSortFilterProxyModel` as subsequent updates to the wrong row led to an invalid proxy state.
Signed-off-by: nvsickle <nvsickle@amazon.com>
* Final update copyright headers to reference license files at the repo root
Signed-off-by: spham <spham@amazon.com>
* Fix copyright validator unit tests to support the stale O3DE header scenario
Signed-off-by: spham <spham@amazon.com>
Nicholas Van Sickle
Steve Pham
Esteban Papp
greerdv
alexpete