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
Ability to embed twitch badges and bit emotes #437
Conversation
public string prefix { get; set; } | ||
public Dictionary<int, EmbedEmoteData> tierList { get; set; } | ||
} | ||
|
||
public class Emotes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe create new classes for embedded badges and bits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doing that means no dealing with "upgrading" old chat files because the TryGetProperty
would return false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought on it a bit more. Keeping the single class could be nice IF we rename it to something more appropriate like "embeddedData". Still we do not need to deal with "upgrading" old jsons because we can TryGetProperty("embeddedData")
else TryGetProperty("emotes")
.
We can also change --embed-emotes
to --embed-data
and add a simple find and replace to ConvertFromOldSyntax
for backwards compatibility.
Well that wasn't what I had in mind when you said updating chat files. I still stand by my second suggestion as it would make developing less of a headache. I will make a PR to your PR with my suggestion since I am asking for it |
…tes' to '--embed-images', update README, complete some TODOs, typos
PR is live goldbattle#1 |
Oh looks like I messed up the merge, never resolved a conflict on Github before... |
Yeah I've messed up every merge I've done on github. I think I found the issue though and left a comment |
Fixed the function: public static async Task<ChatRoot> ParseJsonStatic(string inputJson)
{
ChatRoot chatRoot = new ChatRoot();
using FileStream fs = new FileStream(inputJson, FileMode.Open, FileAccess.Read);
using var jsonDocument = JsonDocument.Parse(fs);
if (jsonDocument.RootElement.TryGetProperty("streamer", out JsonElement streamerJson))
{
chatRoot.streamer = streamerJson.Deserialize<Streamer>();
}
if (jsonDocument.RootElement.TryGetProperty("video", out JsonElement videoJson))
{
if (videoJson.TryGetProperty("start", out _) && videoJson.TryGetProperty("end", out _))
{
chatRoot.video = videoJson.Deserialize<VideoTime>();
}
}
if (jsonDocument.RootElement.TryGetProperty("embeddedData", out JsonElement embedDataJson))
{
chatRoot.embeddedData = embedDataJson.Deserialize<EmbeddedData>();
}
else if (jsonDocument.RootElement.TryGetProperty("emotes", out JsonElement emotesJson))
{
chatRoot.embeddedData = emotesJson.Deserialize<EmbeddedData>();
}
if (jsonDocument.RootElement.TryGetProperty("comments", out JsonElement commentsJson))
{
chatRoot.comments = commentsJson.Deserialize<List<Comment>>();
}
return chatRoot;
} |
This will address issue #359 with the goal of the whole renderer being able to render without internet access.
I am not sure what else is required to do the render, but we have this
Fixes #359
TODOS:
--offline
option for CLI which doesn't do any http requests