Skip to content

Commit

Permalink
fix bugs for formlist filters, add unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@Demorome
Demorome committed Jul 20, 2022
1 parent a2ef4e3 commit 1501a0a
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 49 deletions.
73 changes: 50 additions & 23 deletions nvse/nvse/EventManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1244,43 +1244,70 @@ void ClearFlushOnLoadEvents()
}
}

bool DoesFormMatchFilter(TESForm* inputForm, TESForm* filter, bool expectReference, const UInt32 recursionLevel)
bool DoesFormMatchFilter(TESForm* inputForm, TESForm* filterForm, bool expectReference, const UInt32 recursionLevel)
{
if (filter == inputForm) //filter and form could both be null, and that's okay.
if (filterForm == inputForm) //filter and form could both be null, and that's okay.
return true;
if (!filter || !inputForm)
if (!filterForm || !inputForm)
return false;
if (recursionLevel > 100) [[unlikely]]
return false;
if (IS_ID(filter, BGSListForm))
if (IS_ID(filterForm, BGSListForm))
{
const auto* listFilter = static_cast<BGSListForm*>(filter);
const auto* filterList = static_cast<BGSListForm*>(filterForm);
if (IS_ID(inputForm, BGSListForm))
{
const auto* inputList = static_cast<BGSListForm*>(inputForm);

// Compare the contents of two lists (which could be recursive).
const auto* listArg = static_cast<BGSListForm*>(inputForm);
auto listArgForm = listArg->list.Begin();
for (auto* listFilterForm : listFilter->list)
// The order of the contents does not matter.
// Everything in the inputList must be matched with a form in the filterList.
for (auto* inputListForm : inputList->list)
{
if (!DoesFormMatchFilter(listArgForm.Get(), listFilterForm, expectReference, recursionLevel + 1))
bool foundMatch = false;
for (auto* filterListForm : filterList->list)
{
if (DoesFormMatchFilter(inputListForm, filterListForm, expectReference, recursionLevel + 1))
{
foundMatch = true;
break;
}
}
if (!foundMatch)
return false;
++listArgForm;
}
return true;
}
//inputForm is not a formlist.

// try matching the inputForm with a Form from the filter list
for (auto* listFilterForm : listFilter->list)
for (auto* filterListForm : filterList->list)
{
if (DoesFormMatchFilter(inputForm, listFilterForm, expectReference, recursionLevel + 1))
if (DoesFormMatchFilter(inputForm, filterListForm, expectReference, recursionLevel + 1))
return true;
}

}
// If input form is a reference, then try matching its baseForm to the filter.
else if (expectReference && inputForm->GetIsReference())
else //filterForm is not a formlist.
{
if (filter == GetPermanentBaseForm(static_cast<TESObjectREFR*>(inputForm)))
// Allow matching a single form with a formlist if the formlist effectively only contains that form.
if (IS_ID(inputForm, BGSListForm))
{
auto const inputList = static_cast<BGSListForm*>(inputForm);
for (auto const inputListForm : inputList->list)
{
if (!DoesFormMatchFilter(inputListForm, filterForm, expectReference))
return false;
}
return true;
}
//inputForm is not a formlist.

// If input form is a reference, then try matching its baseForm to the filter.
if (expectReference && inputForm->GetIsReference())
{
if (filterForm == GetPermanentBaseForm(static_cast<TESObjectREFR*>(inputForm)))
return true;
}
}
return false;
}
Expand Down Expand Up @@ -1483,7 +1510,7 @@ bool EventCallback::ShouldRemoveCallback(const EventCallback& toCheck, const Eve
{
// Check if arrays are exactly equal.
// Covers case #1
if (DoesFilterMatch<true, true>(toRemoveFilter, existingFilter.GetAsVoidArg(), paramType))
if (DoesFilterMatch<true>(toRemoveFilter, existingFilter.GetAsVoidArg(), paramType))
continue;
}

Expand All @@ -1496,7 +1523,7 @@ bool EventCallback::ShouldRemoveCallback(const EventCallback& toCheck, const Eve
bool filtersMatch = true;
for (auto iter = existingFilters->Begin(); !iter.End(); ++iter)
{
if (!DoesParamMatchFiltersInArray<true, true>(toRemoveFilter, paramType,
if (!DoesParamMatchFiltersInArray<true>(toRemoveFilter, paramType,
iter.second()->GetAsVoidArg(), index))
{
filtersMatch = false;
Expand All @@ -1515,7 +1542,7 @@ bool EventCallback::ShouldRemoveCallback(const EventCallback& toCheck, const Eve
filtersMatch = true;
for (auto iter = existingFilters->Begin(); !iter.End(); ++iter)
{
if (!DoesFilterMatch<true, true>(toRemoveFilter, iter.second()->GetAsVoidArg(), paramType))
if (!DoesFilterMatch<true>(toRemoveFilter, iter.second()->GetAsVoidArg(), paramType))
{
filtersMatch = false;
break;
Expand All @@ -1526,13 +1553,13 @@ bool EventCallback::ShouldRemoveCallback(const EventCallback& toCheck, const Eve

// Cover case #4
// ToRemoveFilter array could hold many array filters, so try matching existingFilter array to any of those arrays.
if (DoesParamMatchFiltersInArray<true, true>(toRemoveFilter, paramType, existingFilter.GetAsVoidArg(), index))
if (DoesParamMatchFiltersInArray<true>(toRemoveFilter, paramType, existingFilter.GetAsVoidArg(), index))
continue;

return false;
}
// else, must be a simple filter
if (!DoesFilterMatch<true, true>(toRemoveFilter, existingFilter.GetAsVoidArg(), paramType))
if (!DoesFilterMatch<true>(toRemoveFilter, existingFilter.GetAsVoidArg(), paramType))
{
return false;
}
Expand All @@ -1544,7 +1571,7 @@ bool EventCallback::ShouldRemoveCallback(const EventCallback& toCheck, const Eve
else if (toRemoveFilter.DataType() == kDataType_Array)
{
// Case #1: check if any of the elements in the array match existingFilter.
if (!DoesParamMatchFiltersInArray<true, true>(toRemoveFilter, paramType, existingFilter.GetAsVoidArg(), index))
if (!DoesParamMatchFiltersInArray<true>(toRemoveFilter, paramType, existingFilter.GetAsVoidArg(), index))
return false;
}
else if (existingFilter.DataType() == kDataType_Array)
Expand All @@ -1560,7 +1587,7 @@ bool EventCallback::ShouldRemoveCallback(const EventCallback& toCheck, const Eve
for (auto iter = existingFilters->GetRawContainer()->begin();
!iter.End(); ++iter)
{
if (!DoesFilterMatch<true, true>(toRemoveFilter, iter.second()->GetAsVoidArg(), paramType))
if (!DoesFilterMatch<true>(toRemoveFilter, iter.second()->GetAsVoidArg(), paramType))
return false;
}
}
Expand Down
36 changes: 10 additions & 26 deletions nvse/nvse/EventManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ namespace EventManager
const EventInfo& eventInfo,
ExpressionEvaluator* eval) const;

template<bool ExtractIntTypeAsFloat, bool IsParamExistingFilter>
template<bool ExtractIntTypeAsFloat>
bool DoesParamMatchFiltersInArray(const Filter& arrayFilter,
EventArgType paramType, void* param, int index) const;

Expand Down Expand Up @@ -395,10 +395,10 @@ namespace EventManager

// == Template definitions

bool DoesFormMatchFilter(TESForm* inputForm, TESForm* filter, bool expectReference, const UInt32 recursionLevel = 0);
bool DoesFormMatchFilter(TESForm* inputForm, TESForm* filterForm, bool expectReference, const UInt32 recursionLevel = 0);

// eParamType_Anything is treated as "use default param type" (usually for a User-Defined Event).
template<bool ExtractIntTypeAsFloat, bool IsParamExistingFilter>
template<bool ExtractIntTypeAsFloat>
bool DoesFilterMatch(const ArrayElement& sourceFilter, void* arg, EventArgType argType)
{
switch (sourceFilter.DataType()) {
Expand Down Expand Up @@ -441,24 +441,8 @@ namespace EventManager
auto* filterForm = LookupFormByID(filterFormId);
bool const expectReference = argType != EventArgType::eParamType_BaseForm;

if constexpr (IsParamExistingFilter)
{
if (inputForm && IS_ID(inputForm, BGSListForm))
{
// Multiple form filters
auto const existingFilters = static_cast<BGSListForm*>(inputForm);
for (auto const nthExistingFilterForm : existingFilters->list)
{
if (!DoesFormMatchFilter(nthExistingFilterForm, filterForm, expectReference))
return false;
}
return true;
}
}
if (!DoesFormMatchFilter(inputForm, filterForm, expectReference))
{
return false;
}
if (!DoesFormMatchFilter(inputForm, filterForm, expectReference))
return false;
break;
}
case kDataType_String:
Expand Down Expand Up @@ -493,7 +477,7 @@ namespace EventManager
return true;
}

template<bool ExtractIntTypeAsFloat, bool IsParamExistingFilter>
template<bool ExtractIntTypeAsFloat>
bool EventCallback::DoesParamMatchFiltersInArray(const Filter& arrayFilter, EventArgType paramType, void* param, int index) const
{
ArrayID arrayFiltersId{};
Expand All @@ -513,7 +497,7 @@ namespace EventManager
{
continue;
}
if (DoesFilterMatch<ExtractIntTypeAsFloat, IsParamExistingFilter>(elem, param, paramType))
if (DoesFilterMatch<ExtractIntTypeAsFloat>(elem, param, paramType))
return true;
}
return false;
Expand Down Expand Up @@ -569,19 +553,19 @@ namespace EventManager
}

// elements of array are filters
if (!DoesParamMatchFiltersInArray<ExtractIntTypeAsFloat, false>(filter, argType, arg, index))
if (!DoesParamMatchFiltersInArray<ExtractIntTypeAsFloat>(filter, argType, arg, index))
return false;
continue;
}

// Try directly comparing them.
if (DoesFilterMatch<ExtractIntTypeAsFloat, false>(filter, arg, argType))
if (DoesFilterMatch<ExtractIntTypeAsFloat>(filter, arg, argType))
continue;

// If both are arrays, then maybe the filter array contains multiple arrays to match.
if (filterVarType == Script::eVarType_Array)
{
if (DoesParamMatchFiltersInArray<ExtractIntTypeAsFloat, false>(filter, argType, arg, index))
if (DoesParamMatchFiltersInArray<ExtractIntTypeAsFloat>(filter, argType, arg, index))
continue;
}

Expand Down
31 changes: 31 additions & 0 deletions nvse/nvse/unit_tests/event_handler_functions.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,37 @@ begin Function { }
assert (RemoveEventHandler "nvseTestEvent" rTestEventUDF 5::rFormList) == 0
assert (RemoveEventHandler "nvseTestEvent" rTestEventUDF 5::rFormList2)

ref rFormList3 = CreateFormList "" (Ar_List SunnyREF, EasyPeteREF, PlayerREF)
ref rFormList4 = CreateFormList "" (Ar_List SunnyREF, EasyPeteREF)
ref rFormList5 = CreateFormList "" (Ar_List EasyPeteREF, SunnyREF, GSChetREF, PlayerREF)
ref rFormList6 = CreateFormList "" (Ar_List PlayerREF, SunnyREF, EasyPeteREF)
ref rFormList7 = CreateFormList "" (Ar_List PlayerREF, SunnyREF, GSChetREF)

assert (SetEventHandlerAlt "nvseTestEvent" rTestEventUDF 5::rFormList3)
assert (RemoveEventHandler "nvseTestEvent" rTestEventUDF 5::rFormList4) == 0
assert (RemoveEventHandler "nvseTestEvent" rTestEventUDF 5::rFormList3)

assert (SetEventHandlerAlt "nvseTestEvent" rTestEventUDF 5::rFormList3)
assert (RemoveEventHandler "nvseTestEvent" rTestEventUDF 5::rFormList6) ; same contents, but different order, which we ignore.

assert (SetEventHandlerAlt "nvseTestEvent" rTestEventUDF 5::rFormList3)
assert (RemoveEventHandler "nvseTestEvent" rTestEventUDF 5::rFormList7) == 0 ; different contents
assert (RemoveEventHandler "nvseTestEvent" rTestEventUDF 5::rFormList3)

assert (SetEventHandlerAlt "nvseTestEvent" rTestEventUDF 5::rFormList3)
; rFormList5 contains everything rFormList3 has and more, so it encompasses 3,
; so it should be able to remove the handler with that filter.
assert (RemoveEventHandler "nvseTestEvent" rTestEventUDF 5::rFormList5)

ref rFormList8 = CreateFormList "" (Ar_List GSChetREF, GSChetREF, GSChetREF, GSChetREF, GSChetREF)

assert (SetEventHandlerAlt "nvseTestEvent" rTestEventUDF 5::rFormList8)
; We allow removing a formlist filter by a single form if the formlist effectively just contains that form.
assert (RemoveEventHandler "nvseTestEvent" rTestEventUDF 5::GSChetREF)

assert (SetEventHandlerAlt "nvseTestEvent" rTestEventUDF 5::GSChetREF)
assert (RemoveEventHandler "nvseTestEvent" rTestEventUDF 5::rFormList8)


; == Test array-of-filters filter.

Expand Down

0 comments on commit 1501a0a

Please sign in to comment.