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

Add Channel Points Automatic Reward Redemption Add classes #28

Conversation

djinnet
Copy link

@djinnet djinnet commented May 11, 2024

The names and scheme comes from this: https://dev.twitch.tv/docs/eventsub/eventsub-reference/#channel-points-automatic-reward-redemption-add-event

I noticed this subcription doesnt exist in the eventsub of TwitchLib, so I am doing an attempt to add the automatic reward redemption models to core, which then can be used in TwitchLib.EventSub.Websocket.

As it is my first pull request inside of here; let me know if there are any words that should be changed or anything that I forgot to add to the models.

Copy link

@AoshiW AoshiW left a comment

Choose a reason for hiding this comment

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

looks good but there are a few things that need to be changed for consistency between files

Comment on lines 3 to 5
using TwitchLib.EventSub.Core.Models.Chat;
namespace TwitchLib.EventSub.Core.SubscriptionTypes.Channel;
/// <summary>
Copy link

Choose a reason for hiding this comment

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

missing new line

Suggested change
using TwitchLib.EventSub.Core.Models.Chat;
namespace TwitchLib.EventSub.Core.SubscriptionTypes.Channel;
/// <summary>
using TwitchLib.EventSub.Core.Models.Chat;
namespace TwitchLib.EventSub.Core.SubscriptionTypes.Channel;
/// <summary>

Comment on lines 1 to 3
using System;
namespace TwitchLib.EventSub.Core.Models.Chat;
/// <summary>
Copy link

Choose a reason for hiding this comment

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

missing new line

Suggested change
using System;
namespace TwitchLib.EventSub.Core.Models.Chat;
/// <summary>
using System;
namespace TwitchLib.EventSub.Core.Models.Chat;
/// <summary>

/// <summary>
/// An array that includes the emote ID and start and end positions for where the emote appears in the text.
/// </summary>
public ChatMessageEmoteFragment[] Emotes { get; set; } = Array.Empty<ChatMessageEmoteFragment>();
Copy link

Choose a reason for hiding this comment

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

Suggested change
public ChatMessageEmoteFragment[] Emotes { get; set; } = Array.Empty<ChatMessageEmoteFragment>();
public ChatMessageEmoteFragment[] Emotes { get; set; } = [];

Comment on lines 1 to 2
namespace TwitchLib.EventSub.Core.Models.Chat;
/// <summary>
Copy link

Choose a reason for hiding this comment

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

missing new line

Suggested change
namespace TwitchLib.EventSub.Core.Models.Chat;
/// <summary>
namespace TwitchLib.EventSub.Core.Models.Chat;

/// <summary>

Comment on lines 1 to 2
namespace TwitchLib.EventSub.Core.Models.ChannelPoints;
/// <summary>
Copy link

Choose a reason for hiding this comment

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

missing new line

Suggested change
namespace TwitchLib.EventSub.Core.Models.ChannelPoints;
/// <summary>
namespace TwitchLib.EventSub.Core.Models.ChannelPoints;

/// <summary>

Comment on lines 1 to 2
namespace TwitchLib.EventSub.Core.Models.ChannelPoints;
/// <summary>
Copy link

Choose a reason for hiding this comment

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

missing new line

Suggested change
namespace TwitchLib.EventSub.Core.Models.ChannelPoints;
/// <summary>
namespace TwitchLib.EventSub.Core.Models.ChannelPoints;

/// <summary>

/// <summary>
/// Optional. Emote that was unlocked.
/// </summary>
public UnlockedEmote? UnlockedEmote { get; set; } = new();
Copy link

Choose a reason for hiding this comment

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

if it can be null, the default value is not set

Suggested change
public UnlockedEmote? UnlockedEmote { get; set; } = new();
public UnlockedEmote? UnlockedEmote { get; set; }

/// The reward cost.
/// </summary>
public int Cost { get; set; }

Copy link

Choose a reason for hiding this comment

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

unnecessary new line

Suggested change

/// </summary>
public sealed class AutomaticRedemptionReward
{

Copy link

Choose a reason for hiding this comment

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

unnecessary new line

Suggested change

/// The text of the chat message.
/// </summary>
public string Text { get; set; } = string.Empty;

Copy link

Choose a reason for hiding this comment

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

unnecessary new line

Suggested change

@djinnet
Copy link
Author

djinnet commented May 12, 2024

@AoshiW Let me know if it looks good?

Copy link
Member

@swiftyspiffy swiftyspiffy left a comment

Choose a reason for hiding this comment

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

lgtm thanks!

@swiftyspiffy
Copy link
Member

Actually, can we target dev branch for this change instead of main?

@djinnet
Copy link
Author

djinnet commented May 15, 2024

Actually, can we target dev branch for this change instead of main?

Sure @swiftyspiffy , but I dont know how I can swap the target to the dev branch in a pull request. But I can try to mess around and see if I can do something about it.

@djinnet djinnet changed the base branch from main to dev May 15, 2024 05:32
@djinnet
Copy link
Author

djinnet commented May 15, 2024

Let me know if this was the correct way to swap the target. It should targeted the dev branch now.

Copy link
Member

@swiftyspiffy swiftyspiffy left a comment

Choose a reason for hiding this comment

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

yup looks good to me!

@swiftyspiffy swiftyspiffy merged commit 5c56f4a into TwitchLib:dev May 15, 2024
1 check passed
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.

4 participants