feat(ltk_inibin): implement inibin crate with public api by Crauzer · Pull Request #122 · LeagueToolkit/league-toolkit

Crauzer · 2026-03-26T11:22:00Z

Summary

Add ltk_inibin crate for parsing, writing, and modifying
inibin/troybin binary configuration files
Add SDBM hash algorithm to ltk_hash::sdbm for computing
inibin key hashes
Re-export through league-toolkit umbrella behind the
inibin feature flag

Crauzer · 2026-03-26T11:22:43Z

@alanpq @FrogCsLoL I was making this earlier before you submitted your PR so I would like to merge this on top of the existing Pr possibly so we can merge something quickly

FrogCsLoL

.

Crauzer · 2026-03-26T11:32:13Z

We will merge this into your PR @FrogCsLoL just need to solve conflicts

FrogCsLoL · 2026-03-26T11:53:05Z

Feel free to merge #122 directly, I dont need the credit. I will then close my PR. @Crauzer

crates/ltk_hash/src/sdbm.rs

crates/ltk_inibin/src/section.rs

crates/ltk_inibin/src/value.rs

crates/ltk_inibin/src/value_kind.rs

- Updated `InibinSet` to `Section` and `InibinFlags` to `ValueFlags` for clarity. - Introduced unified accessors (`as_f32()`, `as_vec2()`, etc.) on `InibinValue` to handle both packed and non-packed variants. - Added `.keys()` and `.values()` methods to `Section` for idiomatic map-like access. - Adjusted SDBM hash functions to accept `AsRef<str>` for improved ergonomics. - Updated documentation and examples to reflect the new naming and API changes. - Ensured all existing tests pass and added new tests for the unified accessors and section methods.

… CLAUDE.md

Copilot

Pull request overview

Implements the new ltk_inibin crate (parse/write/modify inibin/troybin), adds SDBM hashing support in ltk_hash, and wires the crate into the league-toolkit umbrella (feature-gated) alongside extensive design/spec documentation.

Changes:

Added crates/ltk_inibin with public API, parsing/writing logic, examples, and round-trip/accessor tests.
Added ltk_hash::sdbm (SDBM hashing + hash_inibin_key) and exported it from ltk_hash.
Re-exported ltk_inibin from league-toolkit and updated repo docs/spec artifacts for the feature.

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
specs/001-inibin-crate/tasks.md	Task breakdown for PR review follow-ups and refactors.
specs/001-inibin-crate/spec.md	Feature specification for `ltk_inibin` behavior and requirements.
specs/001-inibin-crate/research.md	Research notes on format layout, hashing, and API decisions.
specs/001-inibin-crate/quickstart.md	Quickstart usage examples for hashing/reading/writing/modifying.
specs/001-inibin-crate/plan.md	Implementation plan tying requirements to concrete changes.
specs/001-inibin-crate/data-model.md	Data model and binary layout documentation.
specs/001-inibin-crate/contracts/public-api.md	Public API contract draft for `ltk_inibin`/`ltk_hash::sdbm`.
specs/001-inibin-crate/checklists/requirements.md	Spec quality checklist for the feature.
crates/ltk_inibin/tests/round_trip.rs	Integration tests for round-trip + accessors + bucket behavior.
crates/ltk_inibin/src/value_flags.rs	`ValueFlags` bitflags and non-string flag ordering constant.
crates/ltk_inibin/src/value.rs	`Value` enum, unified `as_*` accessors, and typed extraction trait/impls.
crates/ltk_inibin/src/section.rs	Section bucket type + set read/write (including BitList + StringList).
crates/ltk_inibin/src/lib.rs	Crate module wiring and public re-exports.
crates/ltk_inibin/src/file.rs	`Inibin` container + read/write entrypoints and key-based API.
crates/ltk_inibin/src/error.rs	`ltk_inibin` error type and `Result` alias.
crates/ltk_inibin/examples/round_trip.rs	Example CLI for verifying round-trip equality on a file.
crates/ltk_inibin/examples/read_inibin.rs	Example CLI to print entries grouped by section + decode packed values.
crates/ltk_inibin/examples/modify_inibin.rs	Example CLI for read-modify-write and safe access patterns.
crates/ltk_inibin/examples/inspect_inibin.rs	Example CLI for querying keys and summarizing sections/values.
crates/ltk_inibin/examples/create_inibin.rs	Example CLI for constructing and writing a new inibin from scratch.
crates/ltk_inibin/README.md	User-facing crate documentation and format overview.
crates/ltk_inibin/Cargo.toml	New crate manifest and workspace dependency usage.
crates/ltk_hash/src/sdbm.rs	New SDBM hashing implementation + tests + inibin convenience wrapper.
crates/ltk_hash/src/lib.rs	Re-export `sdbm` module from `ltk_hash`.
crates/league-toolkit/src/lib.rs	Re-export `ltk_inibin` under `league-toolkit`’s `inibin` feature.
crates/league-toolkit/Cargo.toml	Adds `inibin` feature + optional dep on `ltk_inibin` (and defaults update).
CLAUDE.md	Workspace guidance updates (hashing + I/O helper reuse notes).
.specify/memory/constitution.md	Adds/updates code style guidance and bumps constitution version metadata.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-26T14:30:45Z

crates/ltk_inibin/src/section.rs

+                    properties.insert(hash, Value::I64(reader.read_i64::<LE>()?));
+                }
+            }
+            _ => {}


read_non_string has a trailing _ => {} arm. If this function is ever called with an unsupported/incorrect kind, it will return an empty Section without consuming the value payload, leaving the reader misaligned for subsequent reads. Consider making the match exhaustive over the allowed non-string flags and treating any other flag as unreachable!() or returning an explicit error.

Suggested change

_ => {}

_ => unreachable!("unsupported non-string ValueFlags: {:?}", kind),

Copilot · 2026-03-26T14:30:46Z

crates/ltk_hash/src/sdbm.rs

+    #[test]
+    fn test_hash_lower_basic() {
+        // SDBM hash of "test" (all lowercase)
+        let h = hash_lower("test");
+        // Verify case-insensitivity
+        assert_eq!(h, hash_lower("TEST"));
+        assert_eq!(h, hash_lower("TeSt"));
+    }


ltk_hash::fnv1a has a golden-value test vector, but sdbm tests currently only assert relative properties (case-insensitivity, delimiter equivalence). Adding at least one fixed expected hash value (from a known reference implementation) would better protect against accidental algorithm changes/regressions.

Copilot · 2026-03-26T14:30:46Z

crates/ltk_inibin/src/section.rs

+        self.properties.get(&key)
+    }
+
+    pub fn insert(&mut self, key: u32, value: Value) {


Section::insert does not validate that value.flags() matches self.kind. Because Section is publicly mutable via Inibin::section_mut, callers can insert a mismatched Value and later produce corrupt output (e.g., STRING_LIST containing non-strings breaks write_string_list’s count/offset table invariants). Consider enforcing the invariant (e.g., return Result<()> on mismatch, or at least assert!/debug_assert! with a clear message).

Suggested change

pub fn insert(&mut self, key: u32, value: Value) {

pub fn insert(&mut self, key: u32, value: Value) {

debug_assert_eq!(

value.flags(),

self.kind,

"Section::insert: value.flags() ({:?}) does not match section kind ({:?})",

value.flags(),

self.kind,

);

Copilot · 2026-03-26T14:30:46Z

crates/ltk_inibin/src/section.rs

+        // Seek to each string offset, read null-terminated, then seek back
+        let mut properties = IndexMap::with_capacity(value_count);
+        for (i, hash) in hashes.into_iter().enumerate() {
+            let saved_pos = reader.stream_position()?;
+            reader.seek(SeekFrom::Start(string_data_offset + offsets[i] as u64))?;
+            let s = reader.read_str_until_nul()?;
+            reader.seek(SeekFrom::Start(saved_pos))?;
+            properties.insert(hash, Value::String(s));
+        }
+


read_string_list seeks around to read individual strings but always seeks back to the same saved_pos, so when it returns the reader is left at the start of the string data (right after the offset table), not after the string data. This is inconsistent with read_non_string consuming its full payload and can surprise callers that expect from_reader to advance the stream. Consider advancing to the end of the string data before returning (e.g., seek to string_data_offset + string_data_length).

Suggested change

// Seek to each string offset, read null-terminated, then seek back

let mut properties = IndexMap::with_capacity(value_count);

for (i, hash) in hashes.into_iter().enumerate() {

let saved_pos = reader.stream_position()?;

reader.seek(SeekFrom::Start(string_data_offset + offsets[i] as u64))?;

let s = reader.read_str_until_nul()?;

reader.seek(SeekFrom::Start(saved_pos))?;

properties.insert(hash, Value::String(s));

}

// Seek to each string offset, read null-terminated, then advance to

// the end of the string data region before returning.

let mut properties = IndexMap::with_capacity(value_count);

let mut max_end_pos = string_data_offset;

for (i, hash) in hashes.into_iter().enumerate() {

reader.seek(SeekFrom::Start(string_data_offset + offsets[i] as u64))?;

let s = reader.read_str_until_nul()?;

let end_pos = reader.stream_position()?;

if end_pos > max_end_pos {

max_end_pos = end_pos;

}

properties.insert(hash, Value::String(s));

}

// Leave the reader positioned just after the last string we read,

// so the entire string list payload is considered consumed.

reader.seek(SeekFrom::Start(max_end_pos))?;

Copilot · 2026-03-26T14:30:47Z

crates/ltk_inibin/src/section.rs

+                    writer.write_f32::<LE>(v.w)?;
+                }
+                Value::I64(v) => writer.write_i64::<LE>(*v)?,
+                Value::String(_) => {}


write_non_string silently ignores Value::String(_) (and would also happily write other mismatched variants if they made it into the section). If a section can be mutated externally (via section_mut), this can yield malformed files without any error. Once Section enforces kind/value invariants, this arm should likely be unreachable!() or return an error to avoid emitting corrupt output.

Suggested change

Value::String(_) => {}

Value::String(_) => unreachable!("String values are written via write_strings"),

Copilot · 2026-03-26T14:30:47Z

crates/ltk_inibin/src/section.rs

+        for (_, value) in &entries {
+            if let Value::String(s) = value {
+                offsets.push(string_data.len() as u16);
+                string_data.extend_from_slice(s.as_bytes());
+                string_data.push(0);
+            }
+        }
+
+        for offset in &offsets {
+            writer.write_u16::<LE>(*offset)?;
+        }
+
+        Ok(string_data)
+    }
+
+    pub(crate) fn string_data_length(&self) -> u16 {
+        let mut len: usize = 0;
+        for value in self.properties.values() {
+            if let Value::String(s) = value {
+                len += s.len() + 1;
+            }
+        }
+        len as u16
+    }


write_string_list truncates offsets to u16 via string_data.len() as u16 and string_data_length() truncates total length via len as u16. If string data exceeds u16::MAX (or if properties.len() exceeds u16::MAX), the output will wrap and become invalid. Consider validating sizes with u16::try_from(...) and returning a dedicated error when limits are exceeded.

Copilot · 2026-03-26T14:30:47Z

crates/ltk_inibin/README.md

+```rust
+pub enum Error {
+    UnsupportedVersion(u8),  // Version byte is not 1 or 2
+    U8FloatOverflow(f32),    // Fixed-point float outside 0.0-25.5 on write


The README’s Error example includes a U8FloatOverflow(f32) variant, but crates/ltk_inibin/src/error.rs does not define it (and the current public API stores packed floats as raw u8, so overflow on write doesn’t apply in the same way). Please update the README error section to match the actual error type, or add/restore the variant if you intend to validate float-to-u8 encoding during writes.

Suggested change

U8FloatOverflow(f32), // Fixed-point float outside 0.0-25.5 on write

Copilot · 2026-03-26T14:30:48Z

crates/league-toolkit/Cargo.toml

 default = [
    "anim",
    "file",
+    "inibin",
    "mesh",
    "meta",
    "primitives",


PR description says ltk_inibin is re-exported through league-toolkit behind the inibin feature flag, but this change also adds inibin to the default feature set. That makes the dependency enabled by default for all league-toolkit users. Either remove it from default (keeping it truly opt-in) or update the PR/docs to reflect that it’s now part of the default feature set.

Copilot · 2026-03-26T14:30:48Z

crates/ltk_inibin/src/value.rs

+/// Trait for extracting a typed value from an [`Value`] reference.
+///
+/// Enables the generic [`Inibin::get_as`](crate::Inibin::get_as) method:
+/// ```
+/// # use ltk_inibin::{Inibin, Value};
+/// let mut inibin = Inibin::new();
+/// inibin.insert(0x0001, 42i32);
+/// let v: Option<i32> = inibin.get_as(0x0001);
+/// assert_eq!(v, Some(42));
+/// ```
+pub trait FromValue<'a>: Sized {
+    /// Try to extract from an [`Value`] reference. Returns `None` on type mismatch.
+    fn from_inibin_value(value: &'a Value) -> Option<Self>;


Doc comments use the grammatically incorrect phrase “an [Value] reference” (should be “a [Value] reference”). Consider fixing wording in these new public docs to improve generated rustdoc readability.

feat(ltk_inibin): implement inibin crate with public api

2ebef5c

Crauzer requested review from FrogCsLoL and alanpq March 26, 2026 11:22

FrogCsLoL previously approved these changes Mar 26, 2026

View reviewed changes

alanpq requested changes Mar 26, 2026

View reviewed changes

Crauzer dismissed FrogCsLoL’s stale review via 6b7058c March 26, 2026 13:57

feat(ltk_inibin): reuse read_str_until_nul from ltk_io_ext and update…

a83b97f

… CLAUDE.md

Crauzer force-pushed the 001-inibin-crate branch from e7df811 to a83b97f Compare March 26, 2026 14:17

Crauzer self-assigned this Mar 26, 2026

Crauzer added area:reading File/format reading/parsing area:writing File/format writing/export area:api Public API design needs-research Requires reverse engineering/research priority:high High priority crate:ltk_inibin Inibin/troybin crate labels Mar 26, 2026

Crauzer requested a review from Copilot March 26, 2026 14:25

Copilot started reviewing on behalf of Crauzer March 26, 2026 14:25 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

	_ => {}
	_ => unreachable!("unsupported non-string ValueFlags: {:?}", kind),

-    pub fn insert(&mut self, key: u32, value: Value) {
+    pub fn insert(&mut self, key: u32, value: Value) {
+        debug_assert_eq!(
+            value.flags(),
+            self.kind,
+            "Section::insert: value.flags() ({:?}) does not match section kind ({:?})",
+            value.flags(),
+            self.kind,
+        );

	Value::String(_) => {}
	Value::String(_) => unreachable!("String values are written via write_strings"),

Conversation

Crauzer commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

Crauzer commented Mar 26, 2026

Uh oh!

FrogCsLoL left a comment

Choose a reason for hiding this comment

Uh oh!

Crauzer commented Mar 26, 2026

Uh oh!

FrogCsLoL commented Mar 26, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Crauzer commented Mar 26, 2026 •

edited

Loading