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: update 18 as Instant Replay #384

Merged
merged 1 commit into from
Oct 10, 2023
Merged

Conversation

usharerose
Copy link
Contributor

Summary

correct ID 18 to Instant Replay, instead of UNKNOWN

Reference

Here is sample for a same event from playbyplayv2 and playbyplayv3 which GameID is 0042100212. According to the comparison, 18 should be Instant Replay

  • v2 (which eventmsgtype's index is 2)
[
    '0042100212',
    122,
    18,
    1,
    1,
    '7:19 PM',
    '2:46',
    None,
    'Instant Replay1st Period (7:19 PM EST)',
    None,
    '12 - 27',
    '15',
    0,
    0,
    None,
    None,
    None,
    None,
    None,
    0,
    0,
    None,
    None,
    None,
    None,
    None,
    0,
    0,
    None,
    None,
    None,
    None,
    None,
    0
]
  • v3
{
  'actionNumber': 122,
  'clock': 'PT02M46.00S',
  'period': 1,
  'teamId': 0,
  'teamTricode': '',
  'personId': 0,
  'playerName': '',
  'playerNameI': '',
  'xLegacy': 0,
  'yLegacy': 0,
  'shotDistance': 0,
  'shotResult': '',
  'isFieldGoal': 0,
  'scoreHome': '27',
  'scoreAway': '12',
  'pointsTotal': 39,
  'location': '',
  'description': 'Instant Replay1st Period (7:19 PM EST)',
  'actionType': 'Instant Replay',
  'subType': 'Overturn Ruling',
  'videoAvailable': 0,
  'actionId': 78
}

@rsforbes
Copy link
Collaborator

A huge thank you!!!

@rsforbes rsforbes merged commit 06230b0 into swar:master Oct 10, 2023
@rsforbes
Copy link
Collaborator

rsforbes commented Oct 10, 2023

@usharerose - After pulling this, I thought, "Uh...that's a constant and can be referenced." I have some unit tests here and will double-check for backward compatibility.

@usharerose
Copy link
Contributor Author

@usharerose - After pulling this, I thought, "Uh...that's a constant and can be referenced." I have some unit tests here and will double-check for backward compatibility.

@rsforbes Thanks. I have searched the usage in the whole repository and didn't find any cases about EventMsgType.UNKNOWN. Please feel free to let me know if something related need to update together.

@usharerose usharerose deleted the fix-eventmsgtype branch October 11, 2023 16:06
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.

2 participants