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

Migrate to 7TV v3 api #412

Merged
merged 7 commits into from Nov 12, 2022
Merged

Migrate to 7TV v3 api #412

merged 7 commits into from Nov 12, 2022

Conversation

ScrubN
Copy link
Collaborator

@ScrubN ScrubN commented Nov 6, 2022

Fixes #410

@sunkhaskasis
Copy link
Contributor

sunkhaskasis commented Nov 6, 2022

I just tried it on my end and it didn't work, throws the ERROR: Error reading JArray from JsonReader. Current JsonReader item is not an array: StartObject. Path '', line 1, position 1. error, though I'm not sure if that's what causes it.

@ScrubN ScrubN marked this pull request as draft November 7, 2022 03:38
@ScrubN
Copy link
Collaborator Author

ScrubN commented Nov 7, 2022

The one time I don't test...

Working on it. Thank you for testing it

@ScrubN
Copy link
Collaborator Author

ScrubN commented Nov 7, 2022

Found the problem! The old api was structured:

[
    {
    },
    {
    },
]

where each {} is an emote. The new api is structured:

{
    ...data...
    "emotes": [
    {
    },
    {
    }
    ]
    ...data...
}

where each {} is an emote.

The key difference is that the new API isn't wrapped in [] making it not an array, and thus not a valid input to the JArray Parser. Thanks 7tv!

@ScrubN
Copy link
Collaborator Author

ScrubN commented Nov 7, 2022

The old and new APIs looked close enough to me when I initially checked their outputs, but I should've looked further. If the JArray exception hadn't happened then it would've failed in other ways. They completely restructured the API, it wasn't just a renaming like last time.

@goldbattle
Copy link
Collaborator

goldbattle commented Nov 7, 2022 via email

@ScrubN
Copy link
Collaborator Author

ScrubN commented Nov 7, 2022

It should still be valid (its an array of json objects). Should be able to first create a JSON object and get its value for the "emotes" which is then the json array you expect.

Yes, that was more or less the plan. But many properties were renamed or moved, for example the mime field is gone and has been replaced by "animated" : <bool> and an array of valid formats in a variety of sizes.

@goldbattle
Copy link
Collaborator

goldbattle commented Nov 7, 2022

To get the file you need to get it from the "host"

You might be able to just assume there is always there is a webp there (I would put a check that goes through "files" and ensures there is a webp). For example chatterino only uses webp versions currently.

{
  "id": "6185d97a8d50b5f26ee802fb",
  "name": "PartyParrot",
  "flags": 0,
  "timestamp": 1657657262073,
  "actor_id": null,
  "data": {
    "id": "6185d97a8d50b5f26ee802fb",
    "name": "PartyParrot",
    "flags": 0,
    "lifecycle": 3,
    "listed": true,
    "animated": true,
    "host": {
      "url": "//cdn.7tv.app/emote/6185d97a8d50b5f26ee802fb",
      "files": [
        {
          "name": "1x.webp",
          "static_name": "1x",
          "width": 32,
          "height": 32,
          "size": 7540,
          "format": "WEBP"
        },
        ....
      ]
    }
  }
}

@goldbattle
Copy link
Collaborator

Here are the definitions:
https://github.com/SevenTV/API/blob/dev/data/model/emote.model.go#L60-L71
https://github.com/Chatterino/chatterino2/blob/dd6cb80ab945a4f0a40da9da8de83eea2de1ce08/src/providers/seventv/SeventvEmotes.hpp#L27-L47

There are actually a few extra checks that need to be made that is not currently checked.

  1. Emote is not an unlisted one (these shouldn't be seen by users), can be enforced by checking "listed": true, in json
  2. Emote has not been disallowed on twitch, this would need to be checked in the flags 1 << 24

If you want good examples see sodapoppin's channel:

@ScrubN
Copy link
Collaborator Author

ScrubN commented Nov 7, 2022

My adaptation is complete. If you test it for yourself, odds are you'll end up with a slightly smaller output json because the streamer has unlisted emotes. I used Mizkif and he has an unlisted emote that the v2 parser did not ignore.

Please let me know if you have any critiques of my code, this is my first time properly working with Newtonsoft.Json

@ScrubN ScrubN marked this pull request as ready for review November 7, 2022 06:54
@sunkhaskasis
Copy link
Contributor

sunkhaskasis commented Nov 7, 2022 via email

@ScrubN
Copy link
Collaborator Author

ScrubN commented Nov 7, 2022

Does SkiaSharp not support AVIF? IMO, it'd be better if it was preferred instead of WEBP, as it features better compression and weighs less.

Looks like SkiaSharp does support AVIF as of 2.88.1. I just figured may as well prefer webp since Chatterino does

@ScrubN
Copy link
Collaborator Author

ScrubN commented Nov 7, 2022

Also currently master is on SkiaSharp 2.80.2. In #407 I updated the SkiaSharp packages to 2.88.3, so that would need to be merged first before prioritizing AVIF.

@lay295 lay295 merged commit 944748f into lay295:master Nov 12, 2022
@ScrubN ScrubN deleted the migrate-7tv-api branch November 13, 2022 04:25
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.

Migrate 7TV API endpoints
4 participants