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

Improve Excel Functionality #102

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

WorkingRobot
Copy link
Contributor

This PR adds the following:

  • GetRowRef (and generic counterparts) extension methods for ExcelSheet
  • AsRowRef (and generic counterparts) extension methods for IExtendedExcelRow
  • AsRawRow extension methods for IExtendedExcelRow
  • Equality operators and IEquatable functionality for IExtendedExcelRow

IExtendedExcelRow is an additional interface that can be implemented for IExcelRow types to enable additional functionality. All that's required is an accessor for the page and offset fields given at construction time. This update is meant to make creation of Excel rows as easy as possible, while adding additional functionality for those who need it. Lumina.Excel will get an update implementing IExtendedExcelRow for all of its containing types if this gets approved.

Some questions:

  • Is the obsoleting of SubrowCollection.Sheet fine if replaced with RawSheet? This enables some additional optimizations related to RowRefs by preventing an additional GC object from being created on the heap when calling ExcelModule.GetSheet every time.
  • Should this be moved into a breaking change to prevent the confusion of having yet another interface? There shouldn't really be any reason for a type to not implement IExtendedExcelRow.

P.S. All of this is mirrored to subrow types as well.

@WorkingRobot
Copy link
Contributor Author

Based on discussion from the Dalamud server, the plan is to merge IExtendedExcelRow into IExcelRow when we can do breaking changes (Lumina 6). The only additional requirement to add support for IExtendedExcelRow, outside of the added interfaces, is to provide accessors to the page and offset constructor parameters.

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.

1 participant