Skip to content
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

fix(event-match): fix towers and barracks statuses parsing #3029

Merged
merged 1 commit into from
Oct 15, 2022

Conversation

Nadhum
Copy link
Contributor

@Nadhum Nadhum commented Oct 15, 2022

Fixes #2880

Description

https://wiki.teamfortress.com/wiki/WebAPI/GetMatchDetails states that Tower Status is a 16-bit unsigned integer and Barracks Status is a 8-bit unsigned integer.
It also states that the first 5 bits of the Tower status are unused and the same goes for the first 2 bits of Barracks Status.

The code meanwhile was assuming that these bits will always be unused

let bits = pad(match.tower_status_radiant.toString(2), 11);
bits += pad(match.barracks_status_radiant.toString(2), 6);
bits += pad(match.tower_status_dire.toString(2), 11);
bits += pad(match.barracks_status_dire.toString(2), 6);

In the case of this event match that was causing the issue the barracks_status_radiant was 11111111 (the full 8-bits), so when passed to the pad function which pads to 6, there was nothing to do and the pad function was returning the an 8 character string (as expected because 8 > 6).

What we end up with is bits.length = 38:
11 tower_status_radiant + 8 barracks_status_radiant + 11 tower_status_dire + 6 barracks_status_dire + 2 match_win

buildingData on the other hand is only 36 items long so this will obviously fail

for (let i = 0; i < bits.length; i += 1) {
let type = buildingData[i].id.slice(0, 1);

Changes

This PR fixes the issue by padding to full bit length specified by the docs 16 for Tower Status and 8 for Barracks Status and then remove the unused bits.

Note

#2882 missed the issue as there was no problem with the padding function, opened this PR instead as work seemed stale there.

@howardchung
Copy link
Member

Thanks! I wonder if the extra bits track some event specific building state.

@howardchung howardchung merged commit c780ca6 into odota:master Oct 15, 2022
This pull request was closed.
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.

Crash on event matches
2 participants