Skip to content

Display rank information on ranked CSS #391

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 22 commits into
base: slippi
Choose a base branch
from

Conversation

walz0
Copy link

@walz0 walz0 commented Jul 27, 2023

Paired with slippi-ssbm-c project-slippi/slippi-ssbm-c#8

DAT File Changes:

Adds a new JOBJDesc to slpCSS.dat that contains a TextureAnimation of all of the rank icons (similar to how the game displays stock icons). I'm using RGB5A3 as the image format because CMP was leaving compression artifacts on the images and RGBA8 was producing a very large file (ran out of memory). The current file is 551kb which is small enough that the game runs without any memory issues but it may be a problem in the future if any new files are added to the CSS. Also worth mentioning that using the CMP image format reduces the file size to 239kb.

New slpCSS.dat structure:

  • slpCSS (root)
    • JOBJDesc (Chat input)
    • JOBJDesc (Chat bubble)
    • MatAnimJoint (Gamemode text)
    • JOBJ (Direct mode UI)
    • JOBJDesc (Rank icons)

Overview:

The game sends an EXI command to dolphin to get the rank info through a curl request. This could be changed to something more efficient that doesnt require a new request each time the user goes to the CSS. Fizzi suggested that the matchmaking service return an expected ELO change which would significantly reduce the number of requests to the server: https://discord.com/channels/328261477372919811/733158076827303997/1060705864878657606
If this is the optimal route it may also be worth returning the opponents rank when a match is found so that it can be displayed in the GamePrep scene (suggested by altf4) https://discord.com/channels/328261477372919811/733158076827303997/1060387305501769809

Currently the only rank information that is displayed is the icon and the rating ordinal. If a user hasnt completed their placement matches for the season (ratingUpdateCount < 5) the question mark icon will be shown with a message telling them how many matches they need to play to get a rank.

User without a rank:

image

User with a rank:

image

Additional Notes

There have been instances where the game has crashed when executing the curl request on the CSS (maybe something to do with running LoadMatchState at the same time?). This hasn't been happening to me recently and I've had a few people testing the build and it doesn't seem to be crashing at all for them. Hopefully someone more knowledgeable can provide some insight and this can be debugged.

@walz0 walz0 changed the title Feature/rank display Display rank information on ranked CSS Jul 27, 2023
@rapito
Copy link
Contributor

rapito commented Jul 27, 2023

😯🙌🏻 looking good!

I know this is a draft but some notes:

The rank thresholds should not be coded in dolphin but instead come directly from the server. If those values change then you will be showing an incorrect badge. (I understand this is not public info, but Fizzi might shed some more light to it)

Furthermore, regarding your crash, I havent seen the asm/c code yet, but you might want to load this up using a proc instead so it is a faux-asynchronous process instead. This might prevent you from getting crashes.

@likevin97
Copy link

Curious why this hasn't been merged? This is an awesome feature that seems to have been sitting for a few months @JLaferri

@JLaferri
Copy link
Member

JLaferri commented Jan 5, 2024

Curious why this hasn't been merged? This is an awesome feature that seems to have been sitting for a few months @JLaferri

I have a few problems with the implementation of this feature and I think there would need to be additional considerations. I discussed one of my issues with Walz here: https://discord.com/channels/328261477372919811/733158076827303997/1139032666818216069

But here are my full list of concerns:

  1. We would likely need a way to turn it off for people that don't want to see their rank changes (ladder anxiety)
  2. If the report is a bit slow (which I actually think will be quite common), this method of fetching the new rating would not work and people would think that their report did not go through.
  3. Additional network requests

The second issue is the bigger problem. One way to fix it is to have the backend tell you before a match the player's next rating both on a win and loss. Then upon a win/loss being detected, you can adjust the rating to the expected value. This method has a downside too though. Considering that p2p reporting is pretty troublesome there are times when reports fail to adjust rating. In this case the client would show that a result happened but the backend would not agree and people would similarly be confused about what happened.

I should also note that the ratings at the start of the match may not necessarily match the ratings at the end of the match. This is because ratings can change mid-set in the case of a late report because of cheating detection. This would cause the rating displayed to be incorrect.

return RANK_DIAMOND_3;
if (ratingOrdinal >= 2191.75 && globalPlacing && regionalPlacing)
return RANK_GRANDMASTER;
if (ratingOrdinal > 2191.75 && ratingOrdinal <= 2274.99)
Copy link

Choose a reason for hiding this comment

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

What if ratingOrdinal == 2191.75 but !(globalPlacing && regionalPlacing)?

}

auto r = json::parse(resp);
auto rankedObject = r["data"]["getConnectCode"]["user"]["rankedNetplayProfile"];
Copy link

Choose a reason for hiding this comment

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

should probably make sure json::parse(resp) returned something sane before you start dereferencing everything.

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.

5 participants