ackwell
Path to 1.0
With my focus on boilmaster over the last while, the public API surface - and indeed, general structure - of ironworks has stagnated quite a lot. While I feel that the library performs well-enough, and is sufficiently flexible for typical use cases, it's quite clunky for anything non-trivial. The public API suffers somewhat from my comparative inexperience at the time it was authored.
With boilmaster now stable and in production (not finished, nothing ever is, but hey), I think it's time to come back to ironworks and do a bit of a deep clean, and get it into a position I'd be happy to mark as a stable release. (a stable release? in a rust project? impossible!).
This issue will be used to track progress towards this goal.
Code changes
- https://github.com/ackwell/ironworks/pull/31
Expected / considered changes
Top-level trait
Currently, the Ironworks struct does little more than act as indirection to the underlying resources. It was originally intended to allow for "layering" multiple resources together - i.e. a flat-file (penumbra-esque) subset of files could be preferenced, falling back to a sqpack install - however this use-case is far from common, and I no longer consider it vital to include as a first-class concept.
Instead, I currently intend to remove the Ironworks struct, and lift the current ironworks::Resource trait up to be the top-level interface of the library (currently, renamed to Filesystem). With this, rather than passing sqpack to ironworks, and ironworks to excel, you would just pass sqpack directly to excel.
Consumers who do want layered behavior akin to the old ironworks struct can fairly trivially implement a Filesystem that fans out to multiple others - this may be included in the library directly.
At this point in time, I don't expect to flatten the sqpack::Resource trait in the same way - the rest of the sqpack:: logic does quite a lot of work on both sides of the trait's behavior.
Intermediary File values
Currently, the .file method is used to read a structure from the game files. This is a little generically named, and leaves little room for further features associated with files.
While I personally have no immediate plans for writing to sqpack (or zipatch, fuuuuuck that), I see no reason that ironworks as a library should not have the capacity to support writing (esp. for flat file purposes).
To this end, I'm planning to make top-level .file return an intermediary value, that then can support further behavior. In combination with the aforementioned top-level trait, the file representation would be an associated type of the trait, and consumers of these files would be able to gate impls based on what traits the type implements - i.e. logic that needs to read files would gate where FS: Filesystem, FS::File: Read + Seek, etc.
Split error types
This one is a bit of a no-brainer. A single top-level error enum has served poorly, and lead to a lot of "eh, good enough" error handling. I'll be splitting up errors a bit more, and reconsidering what is communicated with / within errors.
Logging / tracing
Debugging ironworks in larger codebases has been a bit of a thorn recently, and I've found myself doing a lot of ad-hoc logging. I'd like to formalise some of that logging with actual logging in the library.
At current, I'm considering implementing this with tracing, with the log emitting fallback enabled. As much as I'd like to use log instead, I'm fairly keen on including structured data in logs, and log's kv feature is completely ignored by tracing's tracing-log adapter. Thoughts welcome here.
Remove logic from file::
file:: was originally intended to be the raw structs / DTOs, with all actual logic contained elsewhere - however this quickly failed to be the case, in part because of early concerns around public API stability.
I've seen a few forks over the years that effectively just pub everything, which is a significant sign that this balance is well out of alignment.
To this end, I'm in the process of removing all non-read/write logic from file::, and making all fields public.
Given these are representations of file formats we do not control, nor fully understand, there will need to be a semver carve out for them, details undecided as yet. Alternative would be to split the library up a lot so that individual files could be semver'd - while this would work, I'm not sure that I'm keen on the approach. cargo package namespaces when 🥺
cc @anna-is-cute, @WorkingRobot as (i believe?) recent consumers with more involved setups.
These all sound like good changes. I can look at my code tomorrow and see if I had any ergonomics issues or other qualms, but I don't recall any.
Any thoughts on structs for excel? Feels weird getting to stable without some sort of codegen for that (or an updated version of the existing branch).
Given these are representations of file formats we do not control, nor fully understand, there will need to be a semver carve out for them, details undecided as yet.
I've only recently started looking at this project so I might not have the full context here, but this problem seems similar to -sys crates. I'm not sure if moving the file definitions & r/w logic into a similar crate is feasible, but even if not, the versioning scheme could be useful. sys crates often handle this by tracking the version of the external library as a suffix to their own library version <semver>+<external-version> (e.g. for zstd-sys it would be 2.0.15+zstd.1.5.7, i.e. zstd-sys has a version of 2.0.15 which supports/tracks the upstream zstd version 1.5.7).
Version changes would need to be at least as big as the upstream change, but otherwise this still allows normal development. As a rudimentary example, 1.0.1+ffxiv7.2 can either be bumped to 1.1.0+ffxiv7.2 if there's a minor change that's only on the Rust side, or 1.0.2+ffxiv7.2hf1 if there's a ffxiv-side change that's equivalent to a semver patch.
I have a fork holding my own personal changes with ironworks. I had to implement quite a few changes to get exactly what I want. For EXDViewer, I had to implement my own excel parsing code (on top of file::exd) and make significant changes to make it work effectively for my use case (especially for wasm). It's all up to you on what you wish to upstream, but my biggest gripe with the library as it currently stands is the lack of easy access to internal file:: fields.
@anna-is-cute: Any thoughts on structs for excel? Feels weird getting to stable without some sort of codegen for that (or an updated version of the existing branch).
A... lot of thoughts. Many conflicting. I agree that it feels weird, and I'm aiming to update the generator to work with the updates so it's at least functional.
The biggest (mental) blocker on this front is that I can readily automate struct generation - but either the CI/automation needs game file access (for column types), or the structs need to be type-agnostic (unpleasant to use, imo). Neither is something I'm comfortable committing to.
At this stage, I think the best option will be ship a CLI bin with or alongside the library that makes it trivial to generate structs yourself, and/or maybe a derive macro as a stretch goal? I've seen a few GraphQL libraries that take this approach, iirc.
@FreezyLemon: I've only recently started looking at this project so I might not have the full context here, but this problem seems similar to -sys crates.
Yeah, something akin to this would be the go if I split them out. I'm not ruling out doing so, but i'm a little hesitant to move this from one library to multiple. Can't quite place why at the moment, brain be like. If I do split the library, the main namespace ironworks package would act akin to tokio's barrel package.
@WorkingRobot: I have a fork holding my own personal changes with ironworks. I had to implement quite a few changes to get exactly what I want. For EXDViewer, I had to implement my own excel parsing code (on top of file::exd) and make significant changes to make it work effectively for my use case (especially for wasm). It's all up to you on what you wish to upstream [...]
I'm admittedly not really in a mood for looking through diffs at the moment - but if there's any additional things you'd like to bring attention to / point to in particular, please do throw some line/range links and I'll 👀.
@WorkingRobot: but my biggest gripe with the library as it currently stands is the lack of easy access to internal file:: fields.
Yeah, this is absolutely something i'm tackling with this. The first pass of this was actually implemented for exd and exh as part of the subrows with strings work @ e532fbdd65df0d85906dbd5d663e7eef0f63057c, so if you / others have any thoughts on the updated structures here, that'll be good input before they're finalised.
Rescinding this mostly. Will be splitting header structs out of the main file struct, but keeping the main file in-place.
Previous comment
In part due to zipatch, I'm also somewhat tempted to remove buffers from `file::` - i.e. rather than `exd` having a `data: VecTheoretically, this allows for more targeted reads and reduces the impl gap between most files (relatively small most of the time) and patches (over a gig lmao).
Materially, this would mean existing direct consumers of file:: would need to either lazily read the buffers, or wrap the header(s) with a struct that eagerly does so.
consistency in file:: isn't be-all-end-all but it'd certainly be nice to improve.