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.

Sign up for GitHub

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

Prefab safe set editor improvements #201

Merged
merged 4 commits into from
May 31, 2023
Merged

Prefab safe set editor improvements #201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b746e49
fix: error when we removed some property
anatawa12 May 31, 2023
33e0073
fix: error when we reverted whole PrefabSafeSet
anatawa12 May 31, 2023
f2061cc
docs(changelog): add Error when we removed some modification in Prefa…
anatawa12 May 31, 2023
5a41f23
docs(changelog): add Error when we reverted whole PrefabSafeSet with …
anatawa12 May 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG-PRERELEASE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ The format is based on [Keep a Changelog].
### Removed

### Fixed
- Error when we removed some modification in PrefabSafeSet `#201`
- There are several situations for this problem:
- When we removed value in original component
- When we removed new value in overrides
- When we reverted added twice in overrides
- When we reverted deletion in overrides
- When we reverted fake deletion in overrides
- Error when we reverted whole PrefabSafeSet with modifications `#201`

### Security

Expand Down
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ The format is based on [Keep a Changelog].
### Removed

### Fixed
- Error when we removed some modification in PrefabSafeSet `#201`
- There are several situations for this problem:
- When we removed value in original component
- When we removed new value in overrides
- When we reverted added twice in overrides
- When we reverted deletion in overrides
- When we reverted fake deletion in overrides
- Error when we reverted whole PrefabSafeSet with modifications `#201`

### Security

Expand Down
127 changes: 80 additions & 47 deletions Internal/PrefabSafeSet/Editor/EditorUtil.PrefabModification.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ private sealed class PrefabModification : EditorUtil<T>

// upstream change check
private ArraySizeCheck _mainSet;
private ArraySizeCheck _prefabLayersSize;
private readonly ArraySizeCheck[] _layerRemoves;
private readonly ArraySizeCheck[] _layerAdditions;

Expand All @@ -64,6 +65,7 @@ public PrefabModification([NotNull] SerializedProperty property, int nestCount,

// apply modifications until previous one
_prefabLayers = property.FindPropertyRelative(Names.PrefabLayers);
_prefabLayersSize = new ArraySizeCheck(_prefabLayers.FindPropertyRelative("Array.size"));
InitCurrentLayer();
DoInitializeUpstream();
DoInitialize();
Expand All @@ -87,6 +89,7 @@ private void InitCurrentLayer(bool force = false)
_currentAdditions = currentLayer.FindPropertyRelative(Names.Additions)
?? throw new InvalidOperationException("prefabLayers.additions not found");
}
_prefabLayersSize.Updated();
}

private void DoInitializeUpstream()
Expand Down Expand Up @@ -186,6 +189,9 @@ public override IReadOnlyList<IElement<T>> Elements
/// </summary>
public void Initialize()
{
if (_prefabLayersSize.Changed)
InitCurrentLayer();

if (_mainSet.Changed || _layerRemoves.Any(x => x.Changed) || _layerAdditions.Any(x => x.Changed))
{
DoInitializeUpstream();
Expand Down Expand Up @@ -279,7 +285,7 @@ private class ElementImpl : IElement<T>
{
public EditorUtil<T> Container => _container;
private readonly PrefabModification _container;
private int _indexInModifier;
internal int IndexInModifier;
internal readonly int SourceNestCount;
public T Value { get; }
public ElementStatus Status { get; private set; }
Expand All @@ -304,14 +310,14 @@ public bool Contains
}
}

public SerializedProperty ModifierProp { get; private set; }
public SerializedProperty ModifierProp { get; internal set; }

private ElementImpl(PrefabModification container, int indexInModifier, T value, ElementStatus status,
int sourceNestCount, SerializedProperty modifierProp)
{
if (value == null) throw new ArgumentNullException(nameof(value));
_container = container;
_indexInModifier = indexInModifier;
IndexInModifier = indexInModifier;
SourceNestCount = sourceNestCount;
Value = value;
Status = status;
Expand Down Expand Up @@ -345,25 +351,17 @@ public static ElementImpl NewSlot(PrefabModification container, T value) =>

private void DoAdd(bool forceAdd)
{
void AddToAdditions(ElementStatus status)
{
_container.InitCurrentLayer(true);
Debug.Assert(_container._currentAdditions != null, "_container._currentAdditions != null");
_indexInModifier = _container._currentAdditions.arraySize;
_container._setValue(ModifierProp = AddArrayElement(_container._currentAdditions), Value);
_container._currentAdditionsSize += 1;
Status = status;
}

switch (Status)
{
case ElementStatus.Natural:
if (forceAdd)
AddToAdditions(ElementStatus.AddedTwice);
{
(IndexInModifier, ModifierProp) = _container.AddToAdditions(Value);
Status = ElementStatus.AddedTwice;
}
break;
case ElementStatus.Removed:
_container._currentRemovesSize -= 1;
_container.RemoveArrayElementAt(_container._currentRemoves, _indexInModifier);
_container.RemoveRemovesAt(IndexInModifier);
Status = ElementStatus.Natural;
ModifierProp = null;
break;
Expand All @@ -372,56 +370,49 @@ void AddToAdditions(ElementStatus status)
// already added
break;
case ElementStatus.FakeRemoved:
_container._currentRemovesSize -= 1;
_container.RemoveArrayElementAt(_container._currentRemoves, _indexInModifier);
AddToAdditions(ElementStatus.NewElement);
_container.RemoveRemovesAt(IndexInModifier);
(IndexInModifier, ModifierProp) = _container.AddToAdditions(Value);
Status = ElementStatus.NewElement;
break;
case ElementStatus.NewSlot:
AddToAdditions(ElementStatus.NewElement);
(IndexInModifier, ModifierProp) = _container.AddToAdditions(Value);
Status = ElementStatus.NewElement;
_container._elements.Add(this);
break;
default:
throw new ArgumentOutOfRangeException();
}
}


private void DoRemove(bool forceRemove)
{
void AddToRemoves(ElementStatus status)
{
_container.InitCurrentLayer(true);
Debug.Assert(_container._currentRemoves != null, "_container._currentRemoves != null");
_indexInModifier = _container._currentRemoves.arraySize;
_container._setValue(ModifierProp = AddArrayElement(_container._currentRemoves), Value);
_container._currentRemovesSize += 1;
Status = status;
}

switch (Status)
{
case ElementStatus.Natural:
AddToRemoves(ElementStatus.Removed);
(IndexInModifier, ModifierProp) = _container.AddToRemoves(Value);
Status = ElementStatus.Removed;
break;
case ElementStatus.Removed:
case ElementStatus.FakeRemoved:
// already removed: nothing to do
break;
case ElementStatus.NewElement:
_container._currentAdditionsSize -= 1;
_container.RemoveArrayElementAt(_container._currentAdditions, _indexInModifier);
_container.RemoveAdditionsAt(IndexInModifier);
Status = ElementStatus.NewSlot;
_container._elements.Remove(this);
ModifierProp = null;
break;
case ElementStatus.AddedTwice:
_container._currentAdditionsSize -= 1;
_container.RemoveArrayElementAt(_container._currentAdditions, _indexInModifier);
AddToRemoves(ElementStatus.Removed);
_container.RemoveAdditionsAt(IndexInModifier);
(IndexInModifier, ModifierProp) = _container.AddToRemoves(Value);
Status = ElementStatus.Removed;
break;
case ElementStatus.NewSlot:
if (forceRemove)
{
AddToRemoves(ElementStatus.FakeRemoved);
(IndexInModifier, ModifierProp) = _container.AddToRemoves(Value);
Status = ElementStatus.FakeRemoved;
_container._elements.Add(this);
}
break;
Expand All @@ -437,27 +428,23 @@ internal void Revert()
case ElementStatus.Natural:
break; // nop
case ElementStatus.Removed:
_container._currentRemovesSize -= 1;
_container.RemoveArrayElementAt(_container._currentRemoves, _indexInModifier);
_container.RemoveRemovesAt(IndexInModifier);
Status = ElementStatus.Natural;
ModifierProp = null;
break;
case ElementStatus.NewElement:
_container._currentAdditionsSize -= 1;
_container.RemoveArrayElementAt(_container._currentAdditions, _indexInModifier);
_container.RemoveAdditionsAt(IndexInModifier);
Status = ElementStatus.NewSlot;
_container._elements.Remove(this);
ModifierProp = null;
break;
case ElementStatus.AddedTwice:
_container._currentAdditionsSize -= 1;
_container.RemoveArrayElementAt(_container._currentAdditions, _indexInModifier);
_container.RemoveAdditionsAt(IndexInModifier);
Status = ElementStatus.Natural;
ModifierProp = null;
break;
case ElementStatus.FakeRemoved:
_container._currentRemovesSize -= 1;
_container.RemoveArrayElementAt(_container._currentRemoves, _indexInModifier);
_container.RemoveRemovesAt(IndexInModifier);
Status = ElementStatus.NewSlot;
_container._elements.Remove(this);
ModifierProp = null;
Expand All @@ -474,15 +461,15 @@ internal void Revert()
public void MarkRemovedAt(int index)
{
Debug.Assert(_container._currentRemoves != null, "_container._currentRemoves != null");
_indexInModifier = index;
IndexInModifier = index;
ModifierProp = _container._currentRemoves.GetArrayElementAtIndex(index);
Status = ElementStatus.Removed;
}

public void MarkAddedTwiceAt(int index)
{
Debug.Assert(_container._currentAdditions != null, "_container._currentAdditions != null");
_indexInModifier = index;
IndexInModifier = index;
ModifierProp = _container._currentAdditions.GetArrayElementAtIndex(index);
Status = ElementStatus.AddedTwice;
}
Expand Down Expand Up @@ -539,6 +526,52 @@ public bool IsPrefabOverride()
}
}

private (int indexInModifier, SerializedProperty modifierProp) AddToAdditions(T value) =>
AddToModifications(value, ref _currentAdditionsSize, ref _currentAdditions);

private (int indexInModifier, SerializedProperty modifierProp) AddToRemoves(T value) =>
AddToModifications(value, ref _currentRemovesSize, ref _currentRemoves);

private (int indexInModifier, SerializedProperty modifierProp) AddToModifications(T value,
ref int currentModificationSize,
ref SerializedProperty currentModifications)
{
InitCurrentLayer(true);
Debug.Assert(currentModifications != null, "currentModifications != null");
var indexInModifier = currentModifications.arraySize;
var modifierProp = AddArrayElement(currentModifications);
_setValue(modifierProp, value);
currentModificationSize += 1;
return (indexInModifier, modifierProp);
}

private void RemoveAdditionsAt(int indexInModifier) =>
RemoveModificationsAt(indexInModifier, ref _currentAdditionsSize, _currentAdditions,
ElementStatus.NewElement, ElementStatus.AddedTwice);

private void RemoveRemovesAt(int indexInModifier) =>
RemoveModificationsAt(indexInModifier, ref _currentRemovesSize, _currentRemoves,
ElementStatus.Removed, ElementStatus.FakeRemoved);

private void RemoveModificationsAt(int indexInModifier,
ref int currentModificationSize,
SerializedProperty currentModifications,
ElementStatus userState1, ElementStatus userState2)
{
currentModificationSize -= 1;
RemoveArrayElementAt(currentModifications, indexInModifier);
foreach (var elementImpl in _elements)
{
if (elementImpl.Status == userState1 || elementImpl.Status == userState2)
if (elementImpl.IndexInModifier > indexInModifier)
{
elementImpl.IndexInModifier--;
elementImpl.ModifierProp =
currentModifications.GetArrayElementAtIndex(elementImpl.IndexInModifier);
}
}
}

public override void HandleApplyRevertMenuItems(IElement<T> element, GenericMenu genericMenu)
{
var elementImpl = (ElementImpl)element;
Expand Down
14 changes: 14 additions & 0 deletions Internal/PrefabSafeSet/Editor/EditorUtil.Root.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ public override IReadOnlyList<IElement<T>> Elements
}
}

private void ReIndexAll()
{
for (var i = 0; i < _list.Count; i++)
_list[i].UpdateIndex(i);
}

public override int ElementsCount => _mainSet.arraySize;

public override void Clear() => _mainSet.arraySize = 0;
Expand Down Expand Up @@ -81,6 +87,7 @@ public void Add()
_index = _container._mainSet.arraySize;
_container._setValue(ModifierProp = AddArrayElement(_container._mainSet), Value);
_container._list.Add(this);
//_container.ReIndexAll(); // appending does not change index so no reindex is required
}

public void EnsureRemoved() => Remove();
Expand All @@ -92,13 +99,20 @@ public void Remove()
_index = -1;
ModifierProp = null;
_container._list.Remove(this);
_container.ReIndexAll();
}

public void SetExistence(bool existence)
{
if (existence) Add();
else Remove();
}

public void UpdateIndex(int index)
{
_index = index;
ModifierProp = _container._mainSet.GetArrayElementAtIndex(index);
}
}
}
}
Expand Down