486c / rosu-memory

Lightweight, cross-platform memory reader for osu! but in rust
21 stars 7 forks source link

Strings stuff #23

Closed uzervlad closed 8 months ago

uzervlad commented 8 months ago

Simplifies strings reading by moving the string pointer read to read_string()

486c commented 8 months ago

Whole idea of reading pointer inside read_string() is actually very good. But i don't get why should we return String's len with string itself, when we can simply do our_string.len()

486c commented 8 months ago

Also you returning length of UTF16 String, not resulting string (which is UTF8)

uzervlad commented 8 months ago

Also you returning length of UTF16 String, not resulting string (which is UTF8)

That was done without much thought, just to match the test

486c commented 8 months ago

But old test has already been testing if length is correct

    let read_len = p.read_u32(0x4).unwrap();
    assert_eq!(read_len, len, "random_string={random_string:?}");

   let s = p.read_string(0).unwrap();
   assert_eq!(s, random_string);

If you really want test the length you can do assert_eq!(readed_string.len(), random_string.len()) But i'm still don't get it since assert_eq!(s, random_string); already was doing that, if length is incorrect this line gonna fail anyway.

I guess you could just return resulting string without length. Only after that change i'm gonna merge. Or explain in more details why should we return length with string and not just string

486c commented 8 months ago

thanks