Made some oft requested tweaks #53

diceguyd30 · 2019-11-24T03:52:53Z

Current is not auto granted to the first level
Recent commands will log the users as online and supplements the Twitch API
Added a random command for when the queue gets to be unwieldy
Increased the default queue size to 30 as a long stream can really fill it up with offline levels. At some point we will want to create a system that will boot out the oldest offline level in favor of a recent online one (or something like that).

…is added as the submitter might not stick around and 'current' is immune to the online status check.

chat.cpp

nasamuffin · 2019-11-24T04:12:02Z

quesoqueue.h

    std::deque<Level> _levels;
    std::optional<Level> _current;
-    Twitch _twitch;  // query online state
+    Twitch* _twitch;  // query online state


Why switch to pointer? Is there a reason we want to manually manage the lifetime now?

I wanted to make sure that the same instance was being used by both quesoqueue and chat. I'll happily admit that I still don't largely understand how these things work so if switching to a const reference is a better idea, then I'm happy to do it. I just thought that pointers were how objects like this were shared.

It's the same instance. const reference is a pointer that is more idiotproof. It's still getting a pointer under the covers, but this way you can treat it like an object (. notation) and you don't have the ability to accidentally free it early and screw up somewhere else it's being used.

nasamuffin · 2019-11-24T04:12:36Z

twitch.cpp

+    auto user_snapshot = _recent_chatters;
+    _recent_chatters = {};
+    for(auto const& [user, last_heard_from] : user_snapshot) {
+        if (current_time - last_heard_from < std::chrono::minutes(5)) {


can you add a TODO that this should be runtime-configurable

nasamuffin · 2019-11-24T04:13:39Z

twitch.cpp

+    for(auto const& [user, last_heard_from] : user_snapshot) {
+        if (current_time - last_heard_from < std::chrono::minutes(5)) {
+            online_users.emplace(user);
+            _recent_chatters.emplace(user, last_heard_from);


Hm, it seems like it might be a little extra - do we really need to clean it up? Even a few hundred independent viewers per run isn't so bad in one map.

Ah, you can replace these two lines with a call to markAsOnline() too.

Good point, but this does use a different time than markAsOnline. Also, the first line can't be replaced, only the second.

nasamuffin · 2019-11-24T04:15:13Z

chat.cpp

+        _timer.Reset();
+        std::optional<Level> l = _qq.Random();
+        if (l) {
+            WriteMessage(std::string("/marker " + l->levelCode + ", submitted by "


Isn't this done with a method in Twitch right now? Better not duplicate the code, especially since we know this approach doesn't work.

There is a stub, but the actual code we've tried is here and in Next. I suppose we could leave it out.

nasamuffin · 2019-11-24T04:16:15Z

quesoqueue.cpp

+            return std::nullopt;
+        }
+    }
+    std::vector<Level> new_current = {};


Why is current in a vector?

I guess it's because you're trying to use std::sample. This seems weird, why not just generate an index and not use the extra vector? This leads to weird stuff like "new_current[0]"

Eh, what can I say? I like using these standard functions for things where I can accidentally insert bugs without realizing (https://stackoverflow.com/a/6943003).

nasamuffin · 2019-11-24T04:18:57Z

Hm. No, I didn't mean to approve the whole PR. I just tried to express that the one change didn't have a problem. Friggin Github.

… be added to a list of online users that will supplement Twitch's API. Any user that used a command within the last 5 minutes will be seen as online.

…ne ones if there are no online levels).

Make it so that the first level is not promoted to 'current' when it …

1d7ff5a

…is added as the submitter might not stick around and 'current' is immune to the online status check.

nasamuffin reviewed Nov 24, 2019

View reviewed changes

chat.cpp Show resolved Hide resolved

chat.cpp Show resolved Hide resolved

nasamuffin reviewed Nov 24, 2019

View reviewed changes

nasamuffin previously approved these changes Nov 24, 2019

View reviewed changes

diceguyd30 added 2 commits November 24, 2019 05:10

Adding a system to track users that recently used commands. They will…

3e82f00

… be added to a list of online users that will supplement Twitch's API. Any user that used a command within the last 5 minutes will be seen as online.

Add a 'random' command that will dip into the online levels (or offli…

61e2b07

…ne ones if there are no online levels).

diceguyd30 dismissed nasamuffin’s stale review via 61e2b07 November 24, 2019 05:12

diceguyd30 force-pushed the streamer_stash branch from 67f11fd to 61e2b07 Compare November 24, 2019 05:12

Made some oft requested tweaks #53

Are you sure you want to change the base?

Made some oft requested tweaks #53

Uh oh!

Conversation

diceguyd30 commented Nov 24, 2019

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nasamuffin commented Nov 24, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants