486c / rosu-memory

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

fix: manually set mode when beatmap isn't cloned #57

Closed Inconn closed 9 months ago

Inconn commented 9 months ago

fixes #55

found out that rosu-pp doesn't change the mode when converting to catch, so it just calculates the standard stars and pp. so you have to set it manually.

i'm unsure if this is the best way to do this, so i'm open to any better ideas

486c commented 9 months ago

Huge thanks for finding this ( i guess i'm too lazy to tests gamemodes properly :) ). After investigation found that in fact rosu-pp doesn't convert catch maps properly (look here) and it's a known issue by rosu-pp author. So even we acknowledge that we are in catch gamemode, convert_mode() is still gonna return us unconverted osu!std beatmap.

i'm unsure if this is the best way to do this, so i'm open to any better ideas

Best way to fix this was to put gamemode manually here

Quick walk thought what was happening (see comments)

    pub fn update_stars(&mut self) {
        let _span = tracy_client::span!("update stars and ss_pp");

        if let Some(beatmap) = &self.current_beatmap {
            self.stars = beatmap                      // At this point we thought that map is already is converted when in fact it's not
                .stars()                              // so our osu!catch was treated as osu!std map
                .calculate()
                .stars();                             // ^ because of this we got here a osu!std star rating not osu!catch

            let mods = {
                if self.state == GameState::Playing {
                    self.gameplay.mods
                } else {
                    self.menu_mods
                }
            };
            let attr = beatmap
                .pp()
                .mods(mods)                            // ^ same here
                .calculate();

            self.stars_mods = attr.stars();
            self.ss_pp = attr.pp();
        }
    }

Changing it to this fixed everythink

    pub fn update_stars_and_ss_pp(&mut self) {
        let _span = tracy_client::span!("update stars and ss_pp");

        if let Some(beatmap) = &self.current_beatmap {
            let mods = {
                if self.state == GameState::Playing {
                    self.gameplay.mods
                } else {
                    self.menu_mods
                }
            };

            let mode = {
                if self.state == GameState::Playing {
                    self.gameplay.gamemode()
                } else {
                    self.menu_gamemode()
                }
            };

            self.stars = beatmap
                .stars()
                .mode(mode)     // Catch convertions is 
                .calculate()    // broken so converting
                .stars();       // manually, read #57 & #55

            let attr = beatmap
                .pp()
                .mode(mode)    // ^
                .mods(mods)
                .calculate();

            self.stars_mods = attr.stars();
            self.ss_pp = attr.pp();
        }

Also ss_pp and fc_pp was affected by this, which is fixed now

Fixed in 1d8ff47be215bc458b146ab30f4a9b2025d4c53d and 0512cbb904a2b971da4869bc1c7e21da3cbaa16d

Sorry for fixing it by myself, did it along with other changes didn't wanna cause you pain with fixing huge merge conflicts 😃

Again huge thanks for finding it ❤️

Inconn commented 8 months ago

Huge thanks for finding this ( i guess i'm too lazy to tests gamemodes properly :) ). After investigation found that in fact rosu-pp doesn't convert catch maps properly (look here) and it's a known issue by rosu-pp author. So even we acknowledge that we are in catch gamemode, convert_mode() is still gonna return us unconverted osu!std beatmap.

i'm unsure if this is the best way to do this, so i'm open to any better ideas

Best way to fix this was to put gamemode manually here

Quick walk thought what was happening (see comments)

    pub fn update_stars(&mut self) {
        let _span = tracy_client::span!("update stars and ss_pp");

        if let Some(beatmap) = &self.current_beatmap {
            self.stars = beatmap                      // At this point we thought that map is already is converted when in fact it's not
                .stars()                              // so our osu!catch was treated as osu!std map
                .calculate()
                .stars();                             // ^ because of this we got here a osu!std star rating not osu!catch

            let mods = {
                if self.state == GameState::Playing {
                    self.gameplay.mods
                } else {
                    self.menu_mods
                }
            };
            let attr = beatmap
                .pp()
                .mods(mods)                            // ^ same here
                .calculate();

            self.stars_mods = attr.stars();
            self.ss_pp = attr.pp();
        }
    }

Changing it to this fixed everythink

    pub fn update_stars_and_ss_pp(&mut self) {
        let _span = tracy_client::span!("update stars and ss_pp");

        if let Some(beatmap) = &self.current_beatmap {
            let mods = {
                if self.state == GameState::Playing {
                    self.gameplay.mods
                } else {
                    self.menu_mods
                }
            };

            let mode = {
                if self.state == GameState::Playing {
                    self.gameplay.gamemode()
                } else {
                    self.menu_gamemode()
                }
            };

            self.stars = beatmap
                .stars()
                .mode(mode)     // Catch convertions is 
                .calculate()    // broken so converting
                .stars();       // manually, read #57 & #55

            let attr = beatmap
                .pp()
                .mode(mode)    // ^
                .mods(mods)
                .calculate();

            self.stars_mods = attr.stars();
            self.ss_pp = attr.pp();
        }

Also ss_pp and fc_pp was affected by this, which is fixed now

Fixed in 1d8ff47 and 0512cbb

Sorry for fixing it by myself, did it along with other changes didn't wanna cause you pain with fixing huge merge conflicts 😃

Again huge thanks for finding it ❤️

It's completely fine! I wasn't sure on my fix so you finding a better one is great. Also yeah merge conflicts would not be fun