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

Update to Lumina 5 (new Excel parsing) #2022

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

WorkingRobot
Copy link
Contributor

New Features:

  • Transparent RSV resolution using a hook to LayoutWorld.AddRsvString.
  • Removal of ExcelResolver. Lumina.Excel.RowRef works as a better and more featured alternative now that lazy construction is obsolete/useless.
  • IDataManager service now accepts a string? name optional parameter for explicit sheet names.
  • IDataManager.GetSheet is now split into IDataManager.GetSubrowSheet as well.
  • IDataManager.Get(Subrow)Sheet functions now throw. Not sure if this is 100% a good idea, but I don't like quiet failures for stuff that should be constant and never fail (except for updates and sheet changes). If needed, Lumina.GameData.Get(Subrow)Sheet is an alternative to Lumina.Excel.ExcelModule.Get(Subrow)Sheet that throws only when the schema/struct type is invalid.

@WorkingRobot WorkingRobot requested a review from a team as a code owner August 19, 2024 20:11
@WorkingRobot WorkingRobot marked this pull request as draft August 19, 2024 20:11
@WorkingRobot
Copy link
Contributor Author

Until Lumina is updated to v5 and ExdSheets is upstreamed to Lumina.Excel, you'll need to resolve the dll paths by compiling WorkingRobot/ExdGenerator on the new-lumina branch.

@KazWolfe KazWolfe added core Dalamud core needs api bump Held for the next API bump dependencies Pull requests that update a dependency file labels Aug 19, 2024
Copy link
Member

@KazWolfe KazWolfe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the CS bump. If you need it, let us know so that we can upstream the proper changes. Completed, thank you.

As noted as well, this'll be blocked until Lumina updates itself and/or we decide that we want to maintain our own repo/project separately.

@KazWolfe
Copy link
Member

Also, still somewhere on the todo list:

  • Write migration guides for changing concepts.
    • Special callout to sheet extensions/additions/calculated methods.
    • Any relevant pitfalls that may occur as part of this new mechanism.
  • Figure out how to rapidly update EXDSheets to get changes upstreamed in a timely manner.
  • Deal with any other strange dev use-cases that might come up.

@WorkingRobot
Copy link
Contributor Author

WorkingRobot commented Aug 30, 2024

  • Figure out how to rapidly update EXDSheets to get changes upstreamed in a timely manner.

Changed this PR to use Lumina.Excel instead. (Use the lumina2 branch)
As long as EXDSchema gets updated, Lumina.Excel can be easily built + tagged + pushed to NuGet.

I'm considering writing some sort of generator to allow users to easily create IExcel(Sub)Row types, which will be made available on NuGet. Some features may include inheritance support and automatic generic RowRef generation (it has an additional hash that should be computed at compile time for a 2x speed increase). If there are any suggestions for this future generator, let me know, and I'll work on including them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Dalamud core dependencies Pull requests that update a dependency file needs api bump Held for the next API bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants