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

Missed and delayed events when adding multiple track events #95

Closed
evanc577 opened this issue Sep 27, 2021 · 3 comments · Fixed by #96
Closed

Missed and delayed events when adding multiple track events #95

evanc577 opened this issue Sep 27, 2021 · 3 comments · Fixed by #96
Labels
bug Something isn't working events Relates to driver event handling/generation.

Comments

@evanc577
Copy link

Songbird version: 0.2.0

Rust version: rustc 1.55.0 (c8dfcfe04 2021-09-06)

Serenity version: 0.10.9

Description:

Setting multiple events on one track causes some strange behavior and missed events.

I am trying to add events that will skip certain parts of a track by adding a timed event, which when triggered will seek forward and then add another timed event to skip the next part. But I am running this issue where events are not being triggered. Is there a better way to do this?

Steps to reproduce:

Create an event that logs whenever it is triggered.

#[derive(Debug)]
struct EventTester {
    message: String,
}

#[async_trait]
impl VoiceEventHandler for EventTester {
    async fn act(&self, _ctx: &EventContext<'_>) -> Option<Event> {
        event!(Level::INFO, "EventHandler: {}", self.message);
        None
    }
}

Play a song and add some events.

let song = handler.play_source(source);

song.add_event(
    Event::Delayed(Duration::from_secs(1)),
    EventTester {
        message: "delayed 1".into(),
    },
)
.unwrap();
song.add_event(
    Event::Delayed(Duration::from_secs(2)),
    EventTester {
        message: "delayed 2".into(),
    },
)
.unwrap();
song.add_event(
    Event::Periodic(Duration::from_secs(1), Some(Duration::from_secs(0))),
    EventTester {
        message: "periodic 1".into(),
    },
)
.unwrap();
song.add_event(
    Event::Periodic(Duration::from_secs(2), Some(Duration::from_secs(0))),
    EventTester {
        message: "periodic 2".into(),
    },
)
.unwrap();

The 1 second delay event is only run after the 2 second delay event, and the 1 second periodic event is lost completely.

[2021-09-27T18:10:29Z INFO  discord_bot] EventHandler: delayed 2 
[2021-09-27T18:10:29Z INFO  discord_bot] EventHandler: delayed 1 
[2021-09-27T18:10:29Z INFO  discord_bot] EventHandler: periodic 2 
[2021-09-27T18:10:31Z INFO  discord_bot] EventHandler: periodic 2 
[2021-09-27T18:10:33Z INFO  discord_bot] EventHandler: periodic 2 
[2021-09-27T18:10:35Z INFO  discord_bot] EventHandler: periodic 2 
[2021-09-27T18:10:37Z INFO  discord_bot] EventHandler: periodic 2 
[2021-09-27T18:10:39Z INFO  discord_bot] EventHandler: periodic 2
@FelixMcFelix FelixMcFelix added bug Something isn't working events Relates to driver event handling/generation. labels Sep 28, 2021
@FelixMcFelix
Copy link
Member

I'll try and look into this later tonight -- this may be related to #92? I've been quite busy with a new job lately, so my time's a little constrained these days.

@FelixMcFelix
Copy link
Member

I realise now why this is happening! All events were being added, but only one was being cleared: the BinaryHeap of timed events is supposed to be a min-heap, but EventData's Ord wasn't reversed to account for this. Will have a fix up shortly.

FelixMcFelix added a commit to FelixMcFelix/songbird that referenced this issue Sep 28, 2021
Fixes an issue where the `EventData` were not stored in reverse order, meaning that only the last added TimedEvent would be serviced.

This reverses the `Ord` for `EventData`, which should only be internally compared, allowing all timed events to be processed correctly in order.

Fixes serenity-rs#95.
FelixMcFelix added a commit that referenced this issue Sep 29, 2021
Fixes an issue where the `EventData` were not stored in reverse order, meaning that only the last added TimedEvent would be serviced.

This reverses the `Ord` for `EventData`, which should only be internally compared, allowing all timed events to be processed correctly in order.

Fixes #95.
@evanc577
Copy link
Author

Thanks for the quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working events Relates to driver event handling/generation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants