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

Minor tweak to MxList template #1083

Merged
merged 2 commits into from
Aug 11, 2024
Merged

Conversation

disinvite
Copy link
Contributor

This is a very small code change bundled with a huge number of beta addresses for MxList and related template classes.

Prior to this change, these functions call DeleteAll on an MxList1:

  • LegoWorld::Destroy
  • MxDSMultiAction::CopyFrom
  • MxDSSelectAction::CopyFrom
  • MxDSSelectAction::Deserialize
  • MxRegion::Reset
  • MxCompositePresenter::EndAction

MxCompositePresenter::EndAction is a special case; it is the only one of these that skips destroying the values for each entry in the list. The optional parameter has a default of TRUE, but we use FALSE explicitly here.

The beta shows that these are two different functions and no parameter is passed. It doesn't seem to be a case where the destroy-or-not option is part of the template, because MxCompositePresenter calls the same functions on MxList<MxDSAction *> as other functions that use this list. I changed DeleteAll() and the new function Empty() so the for-loop matches the beta.

The functions that call DeleteAll() or Empty() appear unaffected, but this change generated the usual noise to other functions. Setting the m_first and m_last members to null in one line matched the beta but generated no diff in retail.

Other stuff:

The beta supports a change to the Next and Prev functions of MxListCursor where the if-block is reversed, but this drops accuracy across the board so I left it alone. i.e.:

// Retail (how it is now)
if (!m_match) {
    m_match = m_list->m_first;
}
else {
    m_match = m_match->GetNext();
}

// Beta
if (m_match) {
    m_match = m_match->GetNext();
}
else {
    m_match = m_list->m_first;
}

There's a note in mxregionlist.h about the MxRegionTopBottomListCursor constructor. I couldn't find any clear evidence that this should be MxRegionTopBottomList versus MxPtrList<MxRegionTopBottom>*, but the addrs are there so we can take another look.

Footnotes

  1. Each MxList destructor also calls DeleteAll.

@foxtacles foxtacles merged commit 9ab3954 into isledecomp:master Aug 11, 2024
16 checks passed
@disinvite disinvite deleted the pr-mxlist2 branch August 11, 2024 20:58
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.

2 participants