Skip to content

feat: add ltk_rst crate for RST (Riot String Table) support#125

Open
DexalGT wants to merge 9 commits intomainfrom
feat/ltk-rst
Open

feat: add ltk_rst crate for RST (Riot String Table) support#125
DexalGT wants to merge 9 commits intomainfrom
feat/ltk-rst

Conversation

@DexalGT
Copy link
Copy Markdown
Contributor

@DexalGT DexalGT commented Mar 31, 2026

Implements reading and writing of League of Legends .stringtable / fontconfig files following the ltk API style (from_reader/to_writer, thiserror, byteorder, xxhash-rust from workspace deps).

Supports all four file versions (v2–v5), including the v2 font-config header, the deprecated mode byte present in v2–v4, and offset-based string deduplication on both read and write paths.

Registers the crate in the league-toolkit umbrella behind a rst feature flag.

DexalGT added 4 commits March 31, 2026 12:16
Implements reading and writing of League of Legends .stringtable / fontconfig
files following the ltk API style (from_reader/to_writer, thiserror, byteorder,
xxhash-rust from workspace deps).

Supports all four file versions (v2–v5), including the v2 font-config header,
the deprecated mode byte present in v2–v4, and offset-based string deduplication
on both read and write paths.

Registers the crate in the league-toolkit umbrella behind a `rst` feature flag.
@DexalGT DexalGT requested review from Crauzer and alanpq and removed request for alanpq April 1, 2026 13:11
@Crauzer
Copy link
Copy Markdown
Member

Crauzer commented Apr 1, 2026

I will take a deeper look at this sometime during the week, it needs some changes before we can merge it.

Comment thread crates/ltk_rst/src/hash.rs Outdated
@@ -0,0 +1,30 @@
use xxhash_rust::xxh64::xxh64;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use ltk_hash instead

Comment thread crates/ltk_rst/src/rst.rs Outdated
use crate::version::{RstMode, RstVersion};

/// Magic bytes at the start of every RST file: `"RST"`.
pub const MAGIC: [u8; 3] = [0x52, 0x53, 0x54];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
pub const MAGIC: [u8; 3] = [0x52, 0x53, 0x54];
pub const MAGIC: &[u8; 3] = b"RST";

Comment thread crates/ltk_rst/src/rst.rs Outdated
pub version: RstVersion,

/// Optional font-config string. Only present (and written) in v2 files.
pub config: Option<String>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

config and mode should be encoded alongside the version enum, since they are only valid for given versions

Comment thread crates/ltk_rst/src/rst.rs Outdated
pub fn to_writer(&self, writer: &mut impl Write) -> Result<(), RstError> {
let hash_type = self.version.hash_type();

let mut header: Vec<u8> = Vec::new();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why are we writing header bytes to an intermediate buffer?

Comment thread crates/ltk_rst/src/rst.rs Outdated
}
}

fn read_null_terminated(reader: &mut impl Read) -> Result<String, io::Error> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we not have a helper for this already?

DexalGT added 3 commits April 7, 2026 00:48
- Use ltk_hash instead of direct xxhash-rust dependency
- Use byte string literal for MAGIC constant
- Encode config/mode in RstVersion enum variants
- Write directly to writer instead of intermediate buffer
- Use ltk_io_ext::read_str_until_nul helper
Comment thread crates/ltk_rst/src/rst.rs Outdated
pub entries: HashMap<u64, String>,
}

impl RstFile {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think it makes sense to have an RstFile struct. We can rename it to Stringtable and have it be constructed through from_rst_reader fn

Comment thread crates/ltk_rst/src/rst.rs Outdated
Comment on lines +120 to +131
// Read mode byte for versions that have it
if version.has_mode_byte() {
let mode = RstMode::from_u8(reader.read_u8()?);
match &mut version {
RstVersion::V2 {
mode: ref mut m, ..
} => *m = mode,
RstVersion::V3 { mode: ref mut m } => *m = mode,
RstVersion::V4 { mode: ref mut m } => *m = mode,
_ => {}
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we do this differently so its actually readable code

/// - **V5** — simple (39-bit) hashing, no mode byte.
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum RstVersion {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think it makes sense to store any sort of data inside of this struct other than the actual version number. It just makes the code look confusing.

Comment thread crates/ltk_rst/src/version.rs Outdated
}

/// Returns the mode byte value, if applicable.
pub fn mode(&self) -> RstMode {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This shouldn't be here

Comment thread crates/ltk_rst/src/rst.rs Outdated
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct RstFile {
/// File version (encodes config and mode alongside version-specific data).
pub version: RstVersion,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't think we need to store the version for later use in the struct.
When saving/writing we can use the latest version.

@DexalGT DexalGT requested a review from Crauzer April 17, 2026 23:40
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.

3 participants